fix: runAct treats nonzero-exit JSON as a failed act, like runDecide and runProve - #13
Merged
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.
![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 still report success
When runAct returns ok: false, the demo marks the action passed and never raises its exit code. A successful proof then reports the failed action as a successful run.
Prompt for agents
Update the act handling in bin/aas.mjs runDemo to consume runAct's new ok/status contract. A result with ok: false must record the act stage as failed, preserve raw and stderr for the bundle and report, set the overall exit code to failure, and avoid presenting the action as passed. Define whether proof still runs for failed actions based on the structured outcome, while ensuring a successful proof cannot erase the action failure. Add coverage for nonzero act JSON through runDemo, including the persisted artifact, stage status, and process exit code.
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:
--fault duplicatefailure 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.