fix(decision): derive the exit code and the merge verdict from the one decision engine

[pengfei-threemoonslab]

Summary

Closes #1029.

Exit code. exit_code_for_report dropped every suppressed finding before matching fail_on. build_release_decision keeps a suppressed mandatory current-surface control as a blocker that no suppression can waive. So in advisory mode with fail_on, checks.ignore on samples/support_refund_agent gave Decision: blocked, would_fail_ci=false and exit 0. Without the ignores, the same config exits 20. The same thing happened through verify and the Action: a PR that adds the ignores turned its own gate green.

• ci/exit_policy.py: exit_code_for_report takes the blockers that build_release_decision computed, as a required keyword. It exits 20 when any blocker's severity is in the resolved fail_on, in either mode. That list includes verify's base ci.fail_on floor. The rule reads the engine's blockers and does not restate which suppressions count. The stale comment ("strict mode intentionally passes when every critical finding has an explicit suppression reason") is replaced.
• Strict-mode exit codes cannot move: a non-empty blockers[] is a blocked decision, and strict mode already exits 20 on one.
• core/semantic_consistency.py: refuses to write a report whose blocker in fail_on (or strict blocked decision) has would_fail_ci=false. This sits next to the existing strict-insufficient_evidence invariant, so a future exit rule that stops reading the decision fails closed and cannot ship a green gate.

Merge verdict. The decision-to-merge-verdict table in schemas/verifier.py (_DECISION_TO_VERDICT / map_merge_verdict / merge_verdict_for) had copies in three places:

• schemas/verify_run.py VerifyRunOutcome._control_projects_outcome now calls merge_verdict_for and applicability_for. The applicability table it held was a copy of applicability_for.
• schemas/verification_identity.py VerificationReceipt calls map_merge_verdict. The import sits inside the validator because schemas.verifier imports this module (CONTENT_ID_PATTERN).
• scripts/run_safety_qualification.py EXPECTED_MERGE_VERDICT is now derived from map_merge_verdict (same name, since tests import it).

Not annotated with MergeVerdict (a deliberate deviation from the issue's proposed fix):

• VerifyRunOutcome.merge_verdict: str changes the SHA-frozen docs/verify-run-schema.v5.json. I tried it: generate_schemas.py --check adds an enum, and FROZEN_CONTROL_SCHEMAS pins that file. A narrowed field needs a successor schema, so the field stays str. The validator enforces the projection.
• VerificationReceipt.merge_verdict is already a Literal identical to MergeVerdict, members and order alike. It cannot import MergeVerdict at class definition (import cycle). A new test pins it to MergeVerdict and its decision literal to ReleaseDecisionStatus. Published receipt v1 schema bytes are unchanged.

Docs.

• STABILITY.md: the truth-table preface and the "Why exit code 20 depends on --baseline-mode" paragraph (STABILITY.md:4716 when the issue was filed) said suppressed findings are excluded. They now name the mandatory-control exception, and the table gains its row. A new paragraph states that the exit code reads the decision's blockers. The verify: read an in-repo --diff-from and ci.mode from the base, never from the PR head #1024 floor paragraph is corrected to match.
• CHANGELOG.md: ## Unreleased entry.

Type

• Check or risk-model change
• Input adapter change
• CLI or GitHub Action behavior
• Report, schema, or SARIF output
• Documentation only

Authorship

• A coding agent wrote some or all of this change. Agent: Claude Code (Claude Opus 5.5)
• A person read the whole diff before opening this pull request.

Acceptance criteria

• Test: support_refund_agent with checks.ignore on its mandatory controls, advisory mode, fail_on: [critical]: fail_policy.exit_code and the process exit are 20.
  • Evidence: tests/test_scan.py::test_checks_ignore_cannot_turn_an_advisory_fail_on_gate_green[ignored0-fail_on0-20] uses the issue's three ignores, through the CLI (CliRunner). It asserts process exit 20, fail_policy.exit_code == 20, would_fail_ci is True and decision blocked. It also asserts that the two waivable controls left the blockers and SHIP-ACTION-FINANCIAL-WRITE-CONTROL-MISSING stayed. Control rows: without the ignores the exit is 20; with the ignores and no fail_on it is 0.
  • Through verify: tests/test_verify_pr_controlled_inputs.py::test_ignoring_a_control_no_suppression_can_waive_still_fails_ci. A PR ignores a refund tool's SHIP-POLICY-APPROVAL-MISSING and SHIP-ACTION-DESTRUCTIVE-ROLLBACK-MISSING. The test asserts that report.json fail_policy, verifier.json release_decision.fail_policy and the process exit all agree on 20.
  • Unit matrix: tests/test_release_decision.py::test_advisory_fail_on_fails_on_a_blocker_no_suppression_can_waive (6 rows) and test_the_base_fail_on_floor_reads_the_same_blockers.
  • Fails without the fix (src reverted to origin/main): the CLI test fails with exit 0 vs 20, the verify test with {'exit_code': 0, 'would_fail_ci': False}, and the unit rows with assert 0 == 20.
• Test: strict-mode exit codes are unchanged across the existing exit-policy tests.
  • Evidence: the existing exit-policy tests are unchanged apart from passing blockers= to the now-required keyword, and they pass. tests/test_release_decision.py::test_strict_exit_codes_are_unchanged adds 13 strict rows: suppressed mandatory, critical and high, matched with new-findings-only; suppressed waivable, with and without fail_on; matched and unmatched criticals; fail_on: [high]; policy blockers matched and unmatched. The rows go through build_release_decision only, and all 13 also pass against origin/main's source.
• No dict literal mapping decision to merge verdict exists outside schemas/verifier.py (unit test or grep guard).
  • Evidence: tests/test_verdict_contract.py::test_no_dict_literal_outside_the_verifier_maps_a_decision_to_a_merge_verdict is an AST guard over src/agents_shipgate/ and scripts/. It flags a dict literal with a decision-status key whose value is a merge verdict, or a tuple or list holding one (the receipt's ("mergeable", True) shape). It also asserts it still finds the authority itself, so it cannot go blind, and test_the_copy_guard_recognises_both_shapes_of_copy covers its detector. Against origin/main it reports verify_run.py:221, verification_identity.py:363 and run_safety_qualification.py:86. harness/adoption/ is excluded on purpose, with the reason in the test: its scorer is an independent oracle that never reads the agents-shipgate schema.
• test_verdict_contract runs every decision status through all three validators.
  • Evidence: test_every_decision_status_runs_through_every_validator, 4 statuses × 3 validators (VerifierArtifact, VerifyRunOutcome, VerificationReceipt). Each case accepts map_merge_verdict(status) and refuses every other MergeVerdict with that validator's projection error. test_a_changed_projection_moves_every_validator_with_it stages the issue's scenario: insufficient_evidence re-mapped to human_review_required in _DECISION_TO_VERDICT. Against origin/main, the verify-run.json and verification-receipt.json cases fail.

Verification

Branch rebased onto origin/main at #1088 (4e1f047438, pydantic 2.14.1). Everything below ran on the rebased tree, in a scratch venv built from that constraints/dev.txt with --require-hashes, plus constraints/build-backend.txt, pip install -e . --no-deps and pip check. agents_shipgate.__file__ resolved to this worktree. The suite also gave the same result on the pre-rebase base under pydantic 2.13.5.

Command	Exit
python -m pytest -n auto -m "not perf" --ignore=tests/test_adapter_static_only.py	1, from two known local-only failures, and every other test passed. The failures are test_check_unmodelled_host_config_keys.py::…[local_settings_enabled_plugins-worktree|git_range]: this machine's global ~/.config/git/ignore holds **/.claude/settings.local.json, so the fixture's git commit has nothing to commit. This PR does not touch that file.
python -m pytest tests/test_adapter_static_only.py	0
python -m pytest tests/test_latency_budget.py -m perf	0
python -m pytest tests/test_distribution_surface_parity.py tests/test_install_hooks.py tests/test_cold_start_replay.py tests/test_semantic_cold_start.py tests/test_cold_reader_order.py	0
python -m ruff check .	0
python scripts/regenerate_goldens.py --check	0 ("Checked 24 sample artifacts; 0 changed.")
python scripts/generate_schemas.py --check	0

Sample goldens did not move: no sample combines a suppressed mandatory control with fail_on, so no report.json content changed.

End-to-end before/after, run with ./shipgate:

Run	origin/main	this branch
scan, support_refund_agent, advisory, fail_on: [critical], 3 ignores	blocked, exit 0, would_fail_ci=false	blocked, exit 20, would_fail_ci=true
same, no fail_on	blocked, exit 0	blocked, exit 0
same, strict	blocked, exit 20	blocked, exit 20
verify --base HEAD~1, PR adds the 3 ignores	blocked, exit 0; report.json and verifier.json exit_code: 0	blocked, exit 20; both exit_code: 20

Out of scope

• Exit-code tables in docs: contract: one exit-code table and one command table, rendered and tested against CLI #1045. These still describe exit 20 as needing an "unsuppressed" finding: the STABILITY exit-code table row, docs/checks.md:7, docs/integrations.md:152 and docs/quickstart.md:1113. Strict mode already contradicted them before this PR.
• Moving the decision engine to core/decision: architecture: move the decision engine to core/decision; enforce import layers #1053.

Release-readiness notes

• No user-code import added to default scan paths
• No network access added to default scan paths
• New or changed check IDs are documented in docs/checks.md (none changed)
• Report/schema changes are additive or documented in STABILITY.md (no schema change; exit-code behavior documented)

🤖 Generated with Claude Code

[pengfei-threemoonslab]

🤖 Engineering review — implementer self-review

I read git diff origin/main...HEAD adversarially against the four questions in the brief. I reverted the source to origin/main to prove each new acceptance test fails there for the stated reason, then restored it.

1. Can any combination still produce Decision: blocked with exit 0 where policy says 20?

Where fail_on names a blocker's severity, no. Every blocker now reaches the exit code: unsuppressed, baseline-unmatched, or a suppressed mandatory control, through scan, verify and the Action. The base ci.fail_on floor applies too, because the blocker rule uses the same resolved fail_on, floor included.

Two cases still exit 0 on blocked, both by documented advisory policy:

• advisory with no fail_on;
• advisory where the blocker's severity is not in fail_on, for example a high blocks_release blocker under fail_on: [critical]. Unsuppressed, the same finding also exits 0 there, so suppression no longer changes the answer.

The new emission invariant in validate_semantic_consistency makes any regression fail closed. A blocker in fail_on, or a strict blocked decision, with would_fail_ci=false is refused when the report is written. test_a_blocker_in_fail_on_that_would_not_fail_ci_is_refused_at_emission monkeypatches the exit rule to ignore blockers and proves the refusal. I also ran it with only the invariant reverted, and it fails with DID NOT RAISE.

2. Can any combination now exit 20 where it previously correctly exited 0 (over-blocking)?

The new rule only fires on a blocker that is not already in the finding check's active set, which means a suppressed mandatory current-surface control. Strict mode cannot move: a non-empty blockers[] is a blocked decision. test_strict_exit_codes_are_unchanged pins 13 strict rows, and all 13 also pass against origin/main.

One case for the owner to rule on. It is the engine's behavior, not something this PR introduces. build_release_decision checks suppressed before baseline_status. So a mandatory control that is baseline-matched and also suppressed is a blocker, while the same control baseline-matched and not suppressed is accepted debt in review_items. Adding a suppression makes the decision stricter. Under --baseline-mode new-findings:

• strict mode already exited 20 on that combination;
• advisory with a matching fail_on now exits 20 too, where it was 0.

The exit code follows the decision, which is what #1029 asks for. test_advisory_fail_on_fails_on_a_blocker_no_suppression_can_waive[fail_on5-True-matched-20] pins this row with a comment. If you would rather a baseline acceptance be able to accept a mandatory control whether or not it is also suppressed, the fix belongs in the engine's suppressed branch: fall through to the normal classification instead of appending a blocker. That changes release_decision, so I left it out of this PR.

3. Do baseline-accepted findings, waivers and checks.ignore on non-mandatory checks still behave as documented?

• checks.ignore on a non-mandatory finding: still waived in both modes. Covered by test_a_suppression_still_waives_a_finding_that_is_not_a_mandatory_control[advisory|strict] and by the CLI test, where SHIP-POLICY-APPROVAL-MISSING and SHIP-SIDEFX-IDEMPOTENCY-MISSING leave the blockers.
• Baseline-matched, unsuppressed findings: unchanged. The finding check still uses baseline_filtered_active, and the strict pins cover matched findings with and without new_findings_only.
• --no-heuristics: it marks filtered findings suppressed. It cannot reach a mandatory control: action-surface findings are static_declaration, which the filter never excludes.
• Waivers: they go through the same suppression path, so the same rule applies.

4. STABILITY and CHANGELOG

Corrected in STABILITY.md:

• the truth-table preface, which said suppressed findings are always excluded;
• a new table row for the suppressed mandatory control;
• the "Why exit code 20 depends on --baseline-mode" paragraph, which said blockers exclude suppressed findings (STABILITY.md:4716 when the issue was filed);
• the verify: read an in-repo --diff-from and ci.mode from the base, never from the PR head #1024 floor paragraph;
• a new paragraph on the exit code reading the decision's blockers.

CHANGELOG placement differs from the brief. The brief asked for Fixed, but this CHANGELOG has never used a ### Fixed heading. Fixes go under ### Changes, and ## Unreleased already lists #1070/#1074 there. I followed the file's convention. Say so if you want a ### Fixed heading started.

Found and fixed during self-review

• A third copy of the table. scripts/run_safety_qualification.py EXPECTED_MERGE_VERDICT was a third copy, which the issue did not list. The AST guard found it. It is now derived from map_merge_verdict and keeps its name, which tests import.
• Another copied table. VerifyRunOutcome also held a copy of applicability_for's table. It now calls the authority. The behavior is identical; I checked every execution value.
• Annotation dropped for verify-run. Annotating VerifyRunOutcome.merge_verdict with MergeVerdict, as the issue proposed, drifts the SHA-frozen docs/verify-run-schema.v5.json. I confirmed this with generate_schemas.py --check. The field stays str, and the validator enforces the projection.
• Annotation dropped for the receipt. The receipt cannot import MergeVerdict at class definition because of an import cycle, so its literal is pinned to MergeVerdict by test instead.
• blockers is a required keyword on exit_code_for_report. A future caller cannot silently compute an exit code beside the decision.
• The verify path. I added a verify-level test (test_ignoring_a_control_no_suppression_can_waive_still_fails_ci). The Action runs verify, and before this fix a PR that ignored its own refund tool's rollback control exited 0 with verifier.json exit_code: 0.

Left for the owner

• The baseline-matched plus suppressed engine question in §2.
• The CHANGELOG heading in §4.
• Out of scope, owned by contract: one exit-code table and one command table, rendered and tested against CLI #1045: the exit-code table in STABILITY.md ("≥ 1 unsuppressed finding hit fail_on"), docs/checks.md:7, docs/integrations.md:152 and docs/quickstart.md:1113 still describe exit 20 as needing an unsuppressed finding. Strict mode already contradicted them before this PR.

[pengfei-threemoonslab]

🤖 Engineering review — independent review, cycle 1

Reviewed head: 2f85ad7ab4. No P0–P2 findings.

What I checked

• Exit code vs. decision. exit_code_for_report now reads the blockers build_release_decision computed. The new branch sits after strict mode's early returns, so it cannot move a strict exit, and after if not fail_on_resolved: return 0, so advisory without fail_on still exits 0. The exit set (exit_fail_on, which carries the base floor) is always a superset of fail_policy.fail_on (fail_on_resolved). So the new semantic_consistency rule, which requires 20 for a blocker in fail_policy.fail_on, can never be raised by a valid report.
• Verdict projections. merge_verdict_for and applicability_for reproduce the removed tables case for case: decision present maps through map_merge_verdict; with no decision, skipped is mergeable/not_applicable, failed is unknown/failed, and anything else is unknown/not_evaluated. The receipt's can_merge_without_human == (decision == "passed") matches the removed table.
• End-to-end repro of decision: derive the exit code and the merge verdict from the one decision engine #1029. On a copy of samples/support_refund_agent with ci.mode: advisory, fail_on: [critical] and checks.ignore on the three controls:
  • main @ 4e1f0474: decision=blocked, exit_code=0, would_fail_ci=false, process exit 0.
  • This PR: decision=blocked, exit_code=20, would_fail_ci=true, process exit 20. The blockers are the same on both sides.

On the open questions

• Baseline-matched and suppressed mandatory control, exiting 20 in advisory with a matching fail_on. This follows the decision engine's rule that no suppression waives such a control, and strict mode already behaved this way. Accepted as is.
• CHANGELOG under ### Changes. That is the file's convention. Fine.
• Not annotating VerifyRunOutcome.merge_verdict with MergeVerdict. Doing so would move the SHA-frozen verify-run-schema.v5.json. The pin test is the right substitute.