fix: GUI returns 400 (not 403) when a POST omits the Origin header - #14
Merged
Conversation
…and runProve
Bug review of EauDoon/agent-action-stack found:
- MEDIUM: runAct (bin/aas.mjs) short-circuits to childProcessError when result.status !== 0, before parseStageJson is reached. Any JSON crctl wrote to stdout is dropped. The throw also bypasses failedStderr, but the bigger loss is the stdout payload: persistRunBundle only writes a stage artifact when current.raw !== undefined, and stageErrorFields never sets raw, so a `--fault duplicate` failure that surfaces as JSON on stdout is not persisted, does not appear in report.stages.act, and printHuman shows only act_code / act_reason / act_stderr. runDecide and runProve both parse the JSON first and return {ok: false, raw, status, stderr} for nonzero exits; runAct was the asymmetric outlier.
Fix: parse the JSON first, then if status is nonzero return the same {ok: false, raw, status, ...failedStderr(result)} shape that runDecide and runProve use. Throw childProcessError only when the spawn itself failed (result.error) or when the child exited nonzero with no parseable JSON.
Verified: node --test test/stack.test.mjs runs 40 tests, all pass.
Bug review of EauDoon/agent-action-stack found:
- LOW: bin/aas-gui.mjs requestBoundaryFailure returns the same "origin" code whether the caller is missing the Origin header entirely or sent a wrong-origin header. The handler at line 100 returns 403 Forbidden for both, with the body { error: "Forbidden" }. A programmatic local client that forgets to send Origin (curl, a CI step, a non-browser agent) sees a generic 403 and has no way to tell that the only fix is to add the header. A wrong-origin POST is genuinely forbidden; a missing-Origin POST is a misconfiguration that should be 400.
Fix: add a "missing-origin" branch in requestBoundaryFailure and a 400 response with a clear "Origin header required for POST" message in the handler. Wrong-origin POSTs still get 403. Update test/gui.test.mjs to assert the 400 and the new error message.
Verified: node --test test/gui.test.mjs runs 2 tests, all pass. node --test test/stack.test.mjs still runs 40 tests, all pass.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Comment on lines
+525
to
+530
| if (result.status !== 0) { | ||
| // A nonzero exit with parseable JSON is an unsuccessful act (the CLI | ||
| // surfaces structured errors as JSON on stdout), not a child-process | ||
| // error. Mirror runProve so persistRunBundle and printHuman see the | ||
| // structured failure and the GUI can render the stage artifact. | ||
| return { ok: false, raw: payload, status: result.status, ...failedStderr(result) }; |
There was a problem hiding this comment.
🔴 Failed actions become successful runs
When the action child exits nonzero with JSON, runAct returns ok: false, but the orchestrator ignores it. The run records success and can exit zero.
Prompt for agents
Update bin/aas.mjs so runDemo handles a false runAct result as a failed action, preserving raw output and stderr in the stage artifact and report, setting exitCode to 1, and preventing a later successful proof from converting the overall run to success. Add stack tests using the real runAct path with a nonzero child result containing parseable JSON, and assert failed action status, nonzero run exit, preserved artifact, and appropriate proof handling.
Was this helpful? React with 👍 or 👎 to provide feedback.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug review of EauDoon/agent-action-stack found:
Fix: add a "missing-origin" branch in requestBoundaryFailure and a 400 response with a clear "Origin header required for POST" message in the handler. Wrong-origin POSTs still get 403. Update test/gui.test.mjs to assert the 400 and the new error message.
Verified: node --test test/gui.test.mjs runs 2 tests, all pass. node --test test/stack.test.mjs still runs 40 tests, all pass.