Skip to content

fix: gate act stage on act.ok (mirror prove-side failure handling) - #42

Merged
EauDoon merged 1 commit into
mainfrom
fix/act-stage-ok-false-gate
Sep 13, 2026
Merged

EauDoon merged 1 commit into
mainfrom
fix/act-stage-ok-false-gate

Conversation

@EauDoon

@EauDoon EauDoon commented Sep 13, 2026

Copy link
Copy Markdown
Owner

What

The orchestrator in bin/aas.mjs was marking stages.act.status as "passed" on every successful runAct return, even when runAct returned { ok: false, raw: payload, status: <non-zero> } — i.e., a structured act failure with parseable JSON on stdout. The corresponding runProve path correctly gates on prove.ok; the act path was the asymmetric one.

Concrete impact: a structured act failure (e.g. consequence-rail CLI exiting non-zero with parseable error JSON) gets persisted as act: passed, exitCode stays 0, and the prove stage runs anyway. End result: a fully failed act can produce a passed-act + passed-prove + exit 0 run bundle — durable state mislabeled.

Fix

In bin/aas.mjs:1599-1622, mirror the prove-side pattern:

  • gate stages.act.status on act.ok
  • set exitCode = 1 on !act.ok
  • mark stages.prove as skipped with reason "act_failed"
  • update report.flow to "decide -> act -> stop (act failed)"

Test

Added a regression test in test/stack.test.mjs paralleling the existing "nonzero prove JSON is recorded as a failed proof" test (line 568). The new test forces runActFn to return ok:false and asserts:

  • result.exitCode === 1
  • result.manifest.stages.act.status === "failed"
  • result.report.stages.act.status === "failed"
  • result.manifest.stages.prove.status === "skipped"
  • result.report.stages.prove.reason === "act_failed"
  • result.report.flow === "decide -> act -> stop (act failed)"

Severity

HIGH (sensitive 4, asset bump +1). Routing per audit bar: PR + maintainer review, NOT auto-merge. The tier consideration was HIGH vs CRITICAL — calling HIGH because the raw payload is preserved in the bundle (data not lost, only mislabeled); CRITICAL was a strict reading of the bar's "mis-represent durable state" clause.

Verification

  • node --test test/stack.test.mjs → 94/94 pass (was 93, +1 new regression test)
  • node --test test/gui.test.mjs → 45/45 pass
  • Reproducer (an ok:false runActFn injection) flips all six label/exit-code fields correctly. See cycle-2-status report for before/after table.

Out of scope

No public API change. The runActFn injection point is unchanged; the runAct wrapper contract is unchanged. Only the orchestrator's interpretation of { ok: false, ... } flips to mirror the prove side.

Refs: FINDING-20260911-01 in the audit ledger.


Devin Review

…failure handling

The orchestrator was marking stages.act.status as 'passed' on every
successful runAct return, even when runAct returned ok:false with
parseable JSON on stdout (a structured act failure). The prove path
already gates on prove.ok; the act path was the asymmetric one.

A structured act failure now: marks act as 'failed', sets exitCode=1,
records prove as 'skipped' with reason 'act_failed', and updates the
flow string to 'decide -> act -> stop (act failed)'.

Adds a regression test paralleling the existing 'nonzero prove JSON'
test (test/stack.test.mjs:568).

Refs: eaudoon-audit/findings/agent-action-stack.md FINDING-20260911-01
@EauDoon
EauDoon merged commit 80b2a1e into main Sep 13, 2026
9 of 10 checks passed

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

Devin Review

Comment thread bin/aas.mjs
});
const outcome = act.raw?.outcome ?? null;
stages.act = stageRecord("passed", { raw: act.raw });
stages.act = stageRecord(act.ok ? "passed" : "failed", { raw: act.raw });

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Act failure diagnostics are discarded

When runAct returns ok: false, its stderr is omitted from both the report and manifest. Human output then shows no child diagnostic for the failed act.

Learn more

Structured child failures return stderr separately from their JSON payload through runAct. The prove path clips that field and stores it in both the stage record and report, making it available to the manifest and human output. The new act failure path changes the stage to failed but stores only raw, so persistRunBundle receives no stderr and printHuman has nothing to display. The regression test supplies stderr: "rail: failure", but no assertion checks its persistence.

Example: An act child exits with status 1, JSON { "error": { "code": "RECEIPT_FAILED" } }, and stderr rail: failure. The bundle records the failed act and its JSON artifact, but manifest.stages.act.stderr is null and CLI output omits act_stderr.

Recommended fix: Mirror the prove path: compute clipChildStderr(act.stderr), spread it into stages.act, and include it in report.stages.act. Extend the regression test to assert the report and manifest values.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant