fix(verify): read in-repo --diff-from and ci policy from the base

[pengfei-threemoonslab]

Summary

• Closes verify: read an in-repo --diff-from and ci.mode from the base, never from the PR head #1024
• In-repository --diff-from is read from the base. When a base resolves (--base, or a detected one from verify: fail closed with no base or a deleted manifest; anchor gate paths at repo root #1023's ladder), a --diff-from path inside the repository is read as the base commit holds it (git ls-tree + cat-file of a regular-file blob, into the run-private directory the base cache already uses). The checkout/head copy is never opened. A PR that regenerates ci/base-report.json from its own head therefore compares against the base's report: the issue's scenario now lands on insufficient_evidence (was mergeable). SHIP-VERIFY-POLICY-WEAKENED's base snapshot comes from the same base copy.
• If the base holds no regular file at that path, verify falls back to the normal scan of the base and says so in base_notes. A path outside the repository is the operator's and keeps today's behaviour, as does any path with --no-base.
• Configured inputs are trust roots. An in-repository --diff-from, --baseline or --policy-pack file the change edits now gets SHIP-VERIFY-TRUST-ROOT-TOUCHED. The class is shipgate_state for a reference or baseline and policy for a pack, and matched_glob is the configured path. The paths are carried on VerificationContext.configured_trust_root_inputs, which is internal and never serialized. A path the glob table already classifies keeps its class and glob, so its fingerprint does not move.
• Exit-policy floor. With a base and without --ci-mode, ci.mode resolves to the stricter of the base manifest (read from the base commit) and the head manifest. A head that flips strict to advisory and adds a blocking tool now exits 20 (was 0). Without --fail-on, the base's declared ci.fail_on is a floor for the exit code only, with or without --ci-mode. It is threaded from verify through run_scan into build_release_decision, so fail_policy.exit_code and would_fail_ci match the process exit. The blocker severities, decision, merge_verdict and fail_policy.fail_on keep the head's list. base_notes names each knob the base held, and --ci-mode / --fail-on remain the operator's overrides.
• Help text for --ci-mode, --fail-on and --diff-from states the precedence. docs/baseline.md, STABILITY (§ Exit codes, the release decision truth table, § Verify Orchestrator, and a verify: read an in-repo --diff-from and ci.mode from the base, never from the PR head #1024 migration note) and CHANGELOG are updated.

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), engineering-review implementation loop
• A person read the whole diff before opening this pull request.

No person has read this diff before merge; it merges only if the independent review cycles and merge gates recorded in the PR comments pass.

Verification

CI is authoritative for python -m ruff check ., python -m compileall -q src tests, and python -m pytest.

Additional local checks run (Python 3.14, macOS):

• python -m ruff check .: all checks passed
• python -m compileall -q src tests: ok
• python scripts/generate_schemas.py --check: exit 0, no drift
• python scripts/regenerate_goldens.py --check: Checked 24 sample artifacts; 0 changed.
• python scripts/verify_dependency_lock.py: OK
• python -m pytest tests/test_verify_pr_controlled_inputs.py: 25 passed (head 5b9aa3c2). On the first head, 12 end-to-end tests ran against an origin/main export: 9 failed and 3 passed. The 3 that passed pin behaviour that stays the same: an out-of-repo reference, an explicit --ci-mode advisory override, and a head that adopts strict. Since then the file gained the detected-base variants. It also gained the exit-floor cases: a loosened fail_on exits 20 with the verdict still human_review_required, both with and without --ci-mode advisory, and an explicit --fail-on lifts the floor. It covers dropping fail_on beside a blocking tool too, plus unit tests of the ref reader and the floor helper.
• Targeted verify, current-control, trust-root, first-adoption and scan suites (35 files, about 1,490 tests): exit 0, no failures. Afterwards, the policy-related subset (test_verify_weakening, test_policy_reason_code_split, test_first_adoption, test_verify_base_and_gate_removal) was rerun after the final fail_on refinement: exit 0. tests/test_shard_partition.py passes with the new weight.
• tests/test_governance_benchmark.py, tests/test_governance_benchmark_baseline.py, tests/test_adapter_static_only.py, tests/test_release_decision.py, tests/test_scan.py and tests/test_cli.py: pass on 5b9aa3c2. The live benchmark result is byte-identical to the committed baseline.v0.2.json, which this PR does not change; the benchmark records no exit code.
• Full local suite: stopped when origin/main was merged, so there is no local result. CI's six shards are authoritative.
• ./shipgate verify --workspace . --config shipgate.yaml --base $(git merge-base origin/main HEAD) --head HEAD: control.state=complete, all permissions true, release_decision.decision=passed, merge_verdict=mergeable

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
• Report/schema changes are additive or documented in STABILITY.md

Decisions taken

• Base-tree copy, not refusal, when --diff-from (in-repo) and a base are both given. The issue allowed either. Refusing with exit 2 would break every Action that sets both diff_from and diff_base: target. Reading the base copy keeps those workflows working and never reads the head.
• The ref read is the base ref, not the merge base. The fallback base scan and the base report already scan the base ref, so the reference and the floor describe the same commit the change merges into. The issue says "merge-base tree", but its pointer (_map_optional_tree_path) is how baselines and policy packs reach that same base-ref tree.
• A base without a regular file at the path falls back to the base scan. This covers an absent file, a symlink, a directory or an oversize blob, and it is not a refusal. That case has no base copy to read, and the base scan is the trusted default comparison. base_notes names what happened. (A base tree that holds a symlink cannot be archived at all, so that variant fails closed as archive_failed, exactly as it does without --diff-from.)
• ci.mode: "stricter of base and head" rather than "base only". The mode is strict if either side declares it, so a PR that adopts strict still takes effect in the same PR, and a head that keeps its base's mode behaves exactly as before. The mode decides only the exit code. Blockers are {critical} ∪ fail_on, and neither mode's default fail_on adds anything beyond critical, so no verdict moves.
• ci.fail_on is floored for the exit code only (review cycle 2, C2-1). The first head floored fail_on as a whole. That changed the governance benchmark's ci-advisory-from-strict case: base mode: strict, fail_on: [critical, high], head mode: advisory with no fail_on. decision went from review_required to blocked, and merge_verdict from human_review_required to blocked. The cause is that STABILITY's truth table defines blocker_severities = {critical} ∪ fail_policy.fail_on, so the PR's own SHIP-VERIFY-POLICY-WEAKENED (high) blocked under the list it loosens. The second head dropped the fail_on floor entirely, but that left the head's fail_on setting the exit code: in the Action, which passes --fail-on only when set, dropping a severity still turned exit 20 into 0. Now the base's declared ci.fail_on reaches build_release_decision as exit_fail_on_floor and is used only for exit_code_for_report. Blockers, the decision, merge_verdict and fail_policy.fail_on keep the head's list, and the truth-table equation is unchanged. A PR that loosens fail_on therefore fails CI (exit 20) until a human handles it, while its verdict stays human review. SHIP-VERIFY-POLICY-WEAKENED is not exempt from the floor. The benchmark baseline is unchanged, and ci/exit_policy.py is unchanged.
• Floor computed in the verify orchestrator. The mode is passed as ci_mode to run_scan. The fail_on floor goes through a new optional exit_fail_on_floor keyword on run_scan → _build_final_report → build_report → build_release_decision; it defaults to None, so scan (which has no base) is unchanged. declared_ci still records the head's own block, so SHIP-VERIFY-POLICY-WEAKENED still compares the two declarations.
• Unreadable base manifest leaves no floor. An absent base manifest is first adoption, so the head's declaration applies. One that cannot be read or validated also leaves no floor and is named in base_notes. Neither case is PR-controlled.
• Trust-root classes reuse existing vocabulary. The reference and baseline use shipgate_state ("the recorded baselines/waivers", already band-excluded) and a pack uses policy, so no new trust_root_class value appears. A configured input is never cleared by the instruction-structure or host-settings-narrowed shortcuts.
• No schema, contract, base_status value or check ID changes, so no schema regeneration was needed.

Protected paths touched

none

Follow-up candidates

• Three out-of-scope items found while testing (two verify-gate trust gaps adjacent to this issue, one robustness bug) are reported to the maintainer with the implementation report. They are not in this PR.

[pengfei-threemoonslab]

🤖 Engineering review — independent review, cycle 1
Reviewed head: 02b067c

No P0–P2 findings.

Severity	File:line	What	Why	Suggested fix
P3	tests/test_verify_pr_controlled_inputs.py	No end-to-end case for an in-repo --diff-from with an auto-detected base (no --base)	diff_from_at_base keys on base, which detection also sets. I checked it by hand: the base copy is read and the path is a touched trust root. No test pins it.	Add one parametrized case without --base
P3	src/agents_shipgate/cli/verify/command.py:198	The help text says "read as the base commits it"	The wording is ambiguous	"read as committed at the base"

Merge note (not a finding): the head no longer merges cleanly with current origin/main (c6bb59ce). git merge-tree reports a content conflict in STABILITY.md. Rebase before merging.

Adversarial checks of the judgement points

1. Steering the verdict through --diff-from. Every variant fails closed or reads the base:

   • Absolute in-repo path: base copy.
   • ci/../ci/./base-report.json: normalized, base copy.
   • Auto-detected base: base copy.
   • ci/ replaced in the head by a symlink to a forged dir: exit 2, symlink component rejected.
   • Workspace reached through a symlinked alias: exit 2, rejected.
   • Case variant CI/Base-Report.json: exit 2, exact spelling required.
   • Symlink at the path in the base: archive_failed.
   • Path absent in the base: base scan, base_status: succeeded, not mergeable, trust root touched.
   • Unresolvable --base: exit 2 ref_missing (verify: fail closed with no base or a deleted manifest; anchor gate paths at repo root #1023), head copy not used.

   --no-base reads the head copy, as documented. It is mergeable both with and without --diff-from, so the reference does not change the outcome. The base ref is operator- or workflow-supplied in every route.

2. Exit-policy floor:

   • Strict→advisory flip: exit 20 with --head and in worktree mode.
   • Dropped base fail_on: still floored with and without the Action's --ci-mode advisory.
   • Tightening to strict still applies.
   • A head declaring strict with fail_on: [] or [low] over a strict base without fail_on stays exit 20. blocker_severities always includes critical, so a base default list adds nothing.
   • Explicit --ci-mode and --fail-on each override only their own knob.

3. Trust roots: policies/p.yaml edited with and without --policy-pack policies/p.yaml gives the same policy / **/policies/** finding and the same fingerprint (fp_f0a599ed69177e5d) on the merge base 9a3317a0 and on this head. Goldens are unchanged.

4. A manifest-declared checks.policy_packs entry outside policies/** is not a trust root. This is pre-existing (the same on 9a3317a0) and tracked separately.

Commands run (detached worktree at the head; Python 3.14)

• Private reproductions:
  • r7 (regenerated in-repo reference): merge base mergeable, can_merge_without_human=true, trust_root_touched=false → head insufficient_evidence, false, true.
  • r6 (strict→advisory): merge base exit 0 → head exit 20.
• pytest tests/test_verify_pr_controlled_inputs.py: 22 passed
• Targeted suites (test_verify_pr_controlled_inputs, test_trust_root, test_verify_weakening, test_policy_reason_code_split, test_first_adoption, test_verify_base_and_gate_removal, test_verify_auto_base, test_verify_config_binding, test_verify_capability_scope, test_verify_orchestrator, test_verify, test_verify_run, test_baseline_integrity, test_current_control_input_origins, test_current_control_auxiliary_inputs, test_distribution_surface_parity, test_shard_partition): exit 0, about 800 passed, 1 skipped
• ruff check .: all checks passed
• compileall -q src tests: ok
• scripts/generate_schemas.py --check: exit 0
• scripts/regenerate_goldens.py --check: 24 checked, 0 changed
• scripts/verify_dependency_lock.py: OK
• Contract: CHANGELOG entry and STABILITY migration note plus § Verify Orchestrator precedence are present and match the behaviour. docs/baseline.md is updated. No schema, check ID or stable field changes. configured_trust_root_inputs is internal. No commit trailers and no text addressed to an AI.

[pengfei-threemoonslab]

🤖 Engineering review — implementer, addressed cycle 1

Head is now f4034109. It contains merge 7f790507 of origin/main at c6bb59ce (#1072) plus the review fixes in f4034109.

Item	Disposition
Merge origin/main (STABILITY.md conflict)	Fixed in 7f79050. Both migration notes are kept, each separated by --- as in the earlier merges: #1024, then #1025, then #1023 follows unchanged. CHANGELOG auto-merged with both entries present. git merge-base --is-ancestor origin/main HEAD succeeds.
C1-1 (P3): in-repo --diff-from with a detected base	Fixed in f403410. test_an_in_repo_reference_is_never_read_from_the_change_under_review now also runs with no --base (detected-base), in both the --head and worktree variants. Each case asserts base_ref == "main", base_status == "diff_from_provided", the "as committed at 'main'" note, base bytes in verification-base-report.json, and ci/base-report.json as a shipgate_state touched trust root. The shard weight for the file was re-measured and updated in place (24 cases, 21.3 s).
C1-2 (P3): --diff-from help wording	Fixed in f403410. The help (cli/verify/command.py) now reads "read as committed at the base", and the matching orchestrator comment was changed too. The docs never used that phrase; they say "as the base commit holds it", which is left as is.

Checks re-run on the merged tree:

• ruff, compileall, generate_schemas.py --check and verify_dependency_lock.py: pass.
• regenerate_goldens.py --check: 24 checked, 0 changed.
• Targeted pytest: the new file, test_shard_partition, verify base/weakening/orchestrator, test_trust_root, test_policy_reason_code_split, plus the merged-in test_n8n, test_mcp_manifest and test_adopter_vocabulary. Exit 0.
• ./shipgate verify --base $(git merge-base origin/main HEAD) --head HEAD: complete, every permission true, passed, mergeable.

[pengfei-threemoonslab]

🤖 Engineering review — implementer, CI fix

Head is now b1ca815e. Both causes came from this PR, not from the merge of main. CI run 38464318571 on the first head 02b067c1 already failed the same tests:

• test: four test_adapter_static_only.py failures
• suite (2): test_full_executable_governance_benchmark_passes
• suite (5): test_fresh_run_reproduces_committed_baseline

1. tests/test_adapter_static_only.py (test job): fixed in b1ca815.
regular_file_at_ref adds 61 lines to cli/verify/git.py. That moves the two allowlisted spawns from 3380 to 3441 (subprocess.Popen) and from 3789 to 3850 (subprocess.run). Only those two line= values changed. Locally the file now passes (545 tests).

2. Governance benchmark (suite (2) and suite (5)): fixed in code, baseline kept.

• What diverged. Running scripts/run_governance_benchmark.py on f4034109 and diffing against baseline.v0.2.json gives exactly one case, ci-advisory-from-strict. Its decision went from review_required to blocked and its merge_verdict from human_review_required to blocked. passed_cases went from 10 to 9. capability_diff was unchanged, and no metric total other than pass/fail moved.
• Fixture. The base declares mode: strict with fail_on: [critical, high]. The head declares mode: advisory with no fail_on. The benchmark calls run_verify with ci_mode=None, fail_on=None.
• Cause. It is the ci.fail_on half of the stricter-of-base-and-head resolution, not the mode half, the base-ref --diff-from read or the new trust-root classes. With the floor, fail_on became [critical, high]. The STABILITY truth table defines blocker_severities = {critical} ∪ fail_policy.fail_on, so the change's own SHIP-VERIFY-POLICY-WEAKENED (high) became a blocker under the very list it loosens. The catalog's hand-authored ground truth for this case is human review, so the change was not intended. I fixed the code and did not regenerate the baseline.
• Fix. Only ci.mode is floored at the base now (_ci_mode_with_base_floor). The mode decides only the exit code: neither mode's default fail_on adds anything beyond critical. ci.fail_on stays the head's (or --fail-on). A loosened list is still reported by SHIP-VERIFY-POLICY-WEAKENED, which keeps the PR out of can_merge_without_human.
• Result. With the mode-only floor, the live benchmark output is byte-identical to the committed baseline (exit 0). The issue's acceptance cases still hold:
  • strict→advisory with a blocking tool exits 20
  • an explicit --ci-mode advisory exits 0
• Tests. I removed the fail_on-floor tests. A new test, test_a_loosened_fail_on_is_routed_to_review_not_floored, reproduces the benchmark shape end to end: human_review_required, policy_weakening_proven, fail_policy {ci_mode: strict, fail_on: [critical]}, exit 0.
• Docs. CHANGELOG, the STABILITY migration note and § Verify Orchestrator precedence, the --fail-on help text (reverted to the original) and the PR body's Decisions taken are updated.

Checks on b1ca815:

• ruff, compileall, generate_schemas.py --check and verify_dependency_lock.py: pass.
• regenerate_goldens.py --check: 24 checked, 0 changed.
• pytest exits 0 across: both governance-benchmark tests, test_adapter_static_only, test_shard_partition (weight re-measured: 21 cases, 17.5 s), the new file, and the verify, weakening, policy-split, first-adoption and trust-root tests.
• ./shipgate verify against the merge base: complete, passed, mergeable.
• git merge-base --is-ancestor origin/main HEAD succeeds.

[pengfei-threemoonslab]

🤖 Engineering review — independent review, cycle 2
Reviewed head: b1ca815

Findings

Severity	File:line	What is wrong	Why	Suggested fix
P1	src/agents_shipgate/cli/verify/orchestrator.py:1587 (fail_on=fail_on into run_scan), :1313; STABILITY.md:391, :5510	Without --fail-on, the PR head's ci.fail_on still sets verify's exit code. A PR can drop or lower it to turn exit 20 into exit 0 while it adds the findings that list would have caught.	Issue #1024 (finding #19) names both knobs: "ci.mode and fail_on come from the PR's manifest". Its proposed fix is "Take ci.mode and fail_on from the base manifest". The Action always passes --ci-mode and passes --fail-on only when its fail_on input is set. That makes ci.fail_on the one manifest knob a PR can still use to set its exit code in the Action. STABILITY.md:5510 still says "The change under review never supplies its own comparison or exit policy", which is no longer true. The new test test_a_loosened_fail_on_is_routed_to_review_not_floored (tests/…:513) asserts exit 0 for exactly this loosening, so it locks the gap in place.	Apply the base fail_on floor to the exit code only, the same way the ci.mode floor already works. Leave blocker_severities and fail_policy.fail_on computed from the head's list (or --fail-on), so the benchmark keeps routing ci-advisory-from-strict to human review. See the design note below.

Reproductions on this head. Each uses samples/agent_weakens_gate with verify --base main --head HEAD and no --ci-mode or --fail-on:

Base ci	PR change	Exit	Decision	fail_policy.fail_on
advisory, fail_on: [high]	adds bound stripe.create_refund (2 critical + 2 high)	20	blocked	[high]
advisory, fail_on: [high]	same tool and drops fail_on	0	blocked	[]
advisory, fail_on: [high]	adds bound docs.update (high SHIP-AUTH-SCOPE-COVERAGE-MISSING)	20	blocked	[high]
advisory, fail_on: [high]	same tool and fail_on: [critical]	0	review_required	[critical]
advisory, fail_on: [high]	only drops fail_on	0	review_required	[]

Row 2 has the same shape as the issue's r6: exit 20 becomes 0 and the verdict stays blocked. The only signal left is SHIP-VERIFY-POLICY-WEAKENED in the verdict. Nothing on the exit-code path reads it.

Design note (part b): floor fail_on for the exit code only. In ci/release_decision.py build_release_decision, blocker_severities = {"critical", *fail_on_resolved} (line 205) and the exit_code_for_report(..., fail_on=fail_on, ...) call (line 432) take the same fail_on. The change is to add an optional exit_fail_on_floor input there, threaded from verify's base_ci.fail_on through run_scan/prepare. Pass effective_fail_on(ci_mode, fail_on) ∪ floor to exit_code_for_report only. Leave blocker_severities (and therefore the decision, merge_verdict and fail_policy.fail_on) on the head's list. ci/exit_policy.py does not need to change, since exit_code_for_report already takes fail_on as an argument. Doing it inside build_release_decision also keeps fail_policy.exit_code/would_fail_ci in step with the process exit. A recompute in the orchestrator after the scan would leave report.json saying 0 while verify exits 20.

• Governance benchmark: run_governance_benchmark.py records decision, merge_verdict and next_actor, not the exit code. So ci-advisory-from-strict stays review_required/human_review_required and the baseline stays byte-identical. In that case the exit becomes 20, because its own SHIP-VERIFY-POLICY-WEAKENED (high) hits the base's [critical, high]. Strict mode already produces exit 20 alongside review_required (semantic gaps), so this is not a new pairing. If the project wants a fail_on-loosening PR to stay green like a mode-loosening one, the floor can skip SHIP-VERIFY-POLICY-WEAKENED itself, because that finding already routes the loosening to review. The rows above still exit 20 either way, since they come from the added tools' findings.
• STABILITY: the truth table's blocker definition (blocker_severities = {critical} ∪ fail_policy.fail_on) does not change. What changes is the exit-code wording: the § Exit codes row for 20, the truth table's "strict-mode exit" column (a review_required row can exit 20 when its severity is in the base floor), the § Verify Orchestrator precedence paragraph (:5510), and the verify: read an in-repo --diff-from and ci.mode from the base, never from the PR head #1024 migration note (:391). base_notes should name the floored severities, as the cycle-1 implementation did.

No P2 or P3 findings on lines changed since 02b067c1.

Verified (part c)

• Governance benchmark: scripts/run_governance_benchmark.py --out … → rc 0. The output is byte-identical to benchmark/agent-pr-governance/results/baseline.v0.2.json (cmp).
• Acceptance: base strict with the PR flipping to advisory and adding the refund tool exits 20 (base_notes names the floor). The same run with explicit --ci-mode advisory exits 0. A forged in-repo --diff-from ci/base-report.json gives insufficient_evidence with can_merge_without_human: false, and ci/base-report.json is a touched trust root (shipgate_state).
• Allowlist edits are line shifts only. The PR inserts one 61-line hunk at git.py:1110, before both sites, and origin/main's 3380/3789 become 3441/3850. The snippets at the new lines are unchanged.
• No commit trailers or attribution footer. The PR body's agent-disclosure checkbox is template content.

Commands

• pytest tests/test_verify_pr_controlled_inputs.py tests/test_governance_benchmark.py tests/test_governance_benchmark_baseline.py tests/test_adapter_static_only.py → 578 passed
• ruff check . → All checks passed
• compileall -q src tests → ok
• scripts/generate_schemas.py --check → rc 0
• scripts/regenerate_goldens.py --check → 24 artifacts, 0 changed
• scripts/verify_dependency_lock.py → OK
• Full suite: not run (the change since cycle 1 is narrow)

Earlier findings

• C1-1 (no end-to-end test for an in-repo --diff-from with an auto-detected base): resolved. f403410 parametrizes named and detected base, and the test asserts the base_ref and the note.
• C1-2 (--diff-from help wording): resolved.

[pengfei-threemoonslab]

🤖 Engineering review — implementer, addressed cycle 2

Head is now 5b9aa3c2.

Finding	Disposition
C2-1 (P1): without --fail-on, the head's ci.fail_on set verify's exit code	Fixed in 5b9aa3c, as specified (details below).

Code

• build_release_decision (ci/release_decision.py) takes a new optional exit_fail_on_floor.
• When it is set, the list passed to exit_code_for_report is effective_fail_on(ci_mode, fail_on) plus every severity in the floor not already in it.
• Everything else keeps the head's list: blocker_severities, the decision, merge_verdict and fail_policy.fail_on. fail_policy.exit_code and would_fail_ci come from the floored exit code, so they match the process exit.
• ci/exit_policy.py is unchanged.
• The floor travels as an optional keyword (default None): run_scan → _run_scan → _build_final_report → build_report → build_release_decision. scan passes nothing.
• In verify, _exit_policy_base_floor returns the base manifest's declared ci.fail_on whenever --fail-on is absent, with or without --ci-mode.
• When the base's list holds severities the head's effective list lacks, base_notes names them: "The change's ci.fail_on leaves out high, which 'main' declares; a finding of that severity still fails CI (exit 20), while the verdict follows the change's list. Pass --fail-on to override."
• SHIP-VERIFY-POLICY-WEAKENED is not exempt.

Tests

• The test at :513 is replaced by test_a_loosened_fail_on_fails_ci_while_the_verdict_stays_review, with and without --ci-mode advisory. It asserts:
  • the verdict stays human_review_required
  • fail_policy.fail_on keeps the head's list
  • fail_policy.exit_code is 20, would_fail_ci is true, and the process exits 20
  • the note names the dropped severities
  • an explicit --fail-on critical exits 0
• New: test_dropping_fail_on_beside_a_blocking_tool_still_fails_ci covers your r6-shape reproduction (base advisory [high], bound refund tool, fail_on dropped, --ci-mode advisory): blocked, exit 20.
• The floor helper's unit cases now include the floor values. The earlier acceptance cases still pass.

Benchmark. The live scripts/run_governance_benchmark.py output is byte-identical to baseline.v0.2.json (exit 0); the benchmark records no exit code.

Docs

• STABILITY § Exit codes, row 20.
• A floor paragraph under the release decision truth table: the exit column reads 20 for an active finding whose severity the base declares, while the category and rule columns do not move.
• § Verify Orchestrator precedence: the old ":5510" sentence is now true again.
• The verify: read an in-repo --diff-from and ci.mode from the base, never from the PR head #1024 migration note, including who must act in the Action.
• The CHANGELOG entry, the --fail-on help text and the PR body's Decisions taken.

Checks on 5b9aa3c

• ruff, compileall, generate_schemas.py --check and verify_dependency_lock.py: pass.
• regenerate_goldens.py --check: 24 checked, 0 changed.
• pytest exit 0 across: test_release_decision, both governance-benchmark tests, test_adapter_static_only, test_scan, test_cli, the new file (25), and the verify, weakening, policy-split, base/gate-removal, first-adoption and trust-root tests.
• test_shard_partition passes with the re-measured weight (25 cases, 23.4 s).
• ./shipgate verify against the merge base: complete, passed, mergeable.
• origin/main is an ancestor of HEAD.

[pengfei-threemoonslab]

🤖 Engineering review — independent review, cycle 3
Reviewed head: 5b9aa3c

Delta review of b1ca815e..5b9aa3c2 (exit-only base ci.fail_on floor).

No P0–P2 findings.

Severity	File:line	What is wrong	Why	Suggested fix
P3 (non-blocking)	STABILITY.md:408	"What does not change" lists "an explicit --ci-mode and --fail-on"	It can be read as "explicit --ci-mode behaves as before". It does not: --ci-mode advisory without --fail-on went from exit 0 to exit 20 in the case below (c1). The bullet above it says this correctly ("with or without --ci-mode").	Write "a run that passes --fail-on (with or without --ci-mode)".

Reproductions (fresh git repos from samples/agent_weakens_gate, ./shipgate verify --base main --head HEAD, old head b1ca815e → new head 5b9aa3c2)

Case	Flags	Old exit / verdict	New exit / verdict	fail_policy.exit_code / would_fail_ci (new)
c1: base advisory [high]; PR adds bound stripe.create_refund and drops fail_on	none	0 / blocked	20 / blocked	20 / true
c1	--ci-mode advisory	0 / blocked	20 / blocked	20 / true
c1	--ci-mode strict	20 / blocked	20 / blocked	20 / true
c1	--fail-on low	0 / blocked	0 / blocked (override holds)	0 / false
c1	--fail-on "" (with and without --ci-mode advisory)	0 / blocked	0 / blocked (override holds)	0 / false
c1	--no-base	0 / blocked	0 / blocked (no floor)	0 / false
c2: base advisory [critical, high]; PR lowers to [critical] and adds docs.delete (only high: POLICY-WEAKENED)	none	0 / insufficient_evidence	20 / insufficient_evidence	20 / true
c2	--ci-mode advisory	0 / insufficient_evidence	20 / insufficient_evidence	20 / true
c2	--fail-on critical	0 / insufficient_evidence	0 / insufficient_evidence (override holds)	0 / false
c3: strict [critical, high] → strict [critical] + bound delete	none, and --fail-on critical	20 / blocked	20 / blocked	20 / true
c4: same ci block on both sides, no tool change	none	0 / mergeable	0 / mergeable	0 / false
c6: PR tightens [high] → [high, medium] + refund	none	20 / blocked	20 / blocked	20 / true
c7: base declares no fail_on, refund added	none	0 / blocked	0 / blocked (no floor)	0 / false

• The verdict, decision and fail_policy.fail_on are the same on both heads in every case. The process exit equals fail_policy.exit_code and verifier.json head_exit_code in every case. Where the floor changes the exit code, base_notes names the severity that was left out.
• scan with no base gives the same result on both heads: c1 head exits 0 (fail_on: []), and exits 20 with --fail-on high. c2 head exits 0 on both heads.
• Threading: exit_fail_on_floor defaults to None in run_scan, _run_scan, _build_final_report, build_report and build_release_decision. The only caller that passes it is verify's head scan (cli/verify/orchestrator.py:1638). The other run_scan callers (fixture, self_check, baseline, _helpers, scan, verify's base scan at :2045) and run_verify callers (fixture, verify command) do not pass it, and an empty or None floor leaves fail_on as it was. So no other caller gets a different default.
• Docs: the CHANGELOG entry, the STABILITY exit-code row, the truth-table floor paragraph, the precedence paragraph, the migration note and the --fail-on help all match the behaviour above (blocker severities stay {critical} ∪ the head's list; the floor reaches only the exit code; it applies with or without --ci-mode; --fail-on overrides it). The only exception is the P3 above. docs/baseline.md has no stale "not floored" wording.

Commands run

• scripts/run_governance_benchmark.py --out … then cmp against benchmark/agent-pr-governance/results/baseline.v0.2.json: byte-identical. benchmark/ is unchanged against main.
• pytest -m "not perf and not slow" on test_verify_pr_controlled_inputs, test_verify, test_verify_auto_base, test_verify_base_and_gate_removal, test_verify_capability_scope, test_verify_config_binding, test_verify_orchestrator, test_verify_run, test_verify_weakening, test_release_decision, test_ci_recipes, test_distribution_surface_parity, test_governance_benchmark, test_governance_benchmark_baseline: 657 passed.
• ruff check .: all checks passed. compileall -q src tests: ok. generate_schemas.py --check: rc 0. regenerate_goldens.py --check: 0 changed. verify_dependency_lock.py: OK.
• Commits origin/main..5b9aa3c2 and the PR body: no Co-Authored-By, Generated with or Claude-Session trailer. No added text addresses an AI, agent or reviewer.

Earlier findings

• C1-1 (auto-base end-to-end test): resolved
• C1-2 (--diff-from help wording): resolved
• C2-1 (P1, head ci.fail_on set verify's exit code): resolved. The repros above exit 20 with the verdict unchanged, an explicit --fail-on still overrides, and scan is unaffected.

[pengfei-threemoonslab]

🤖 Engineering review — implementer, addressed cycle 3: P3 fixed in 79e6b2b. STABILITY.md's #1024 "What does not change" sentence now reads "a run that passes --fail-on (with or without --ci-mode)". llms-full.txt does not carry this note, so only STABILITY.md changed.

[pengfei-threemoonslab]

🤖 Engineering review — merge readiness

• Reviewed head: 5b9aa3c20930181e98f1467612dd6cdf87d840ae (cycle 3, clean). Current head 79e6b2bd59dfa68e711453d2a3f3e713e0120bb9 differs only by the three-line STABILITY wording fix for cycle 3's P3, checked by the orchestrator.
• Three cycles plus one CI fix. C1-1/C1-2 (P3) resolved; the CI failures on the first head were the line-pinned subprocess allowlist (filed as tests: key the static-only subprocess allowlist by function, not line number #1078) and a governance-benchmark divergence traced to flooring fail_on for the verdict, fixed in code with the baseline kept byte-identical; C2-1 (P1: without --fail-on, the head's ci.fail_on still set the exit code) resolved by flooring fail_on for the exit code only, so verdicts and the benchmark ground truth do not move while a PR can no longer turn exit 20 into 0 through its manifest. Cycle 3 reproduced every case: exit 20 with verdicts unchanged, --fail-on still overrides, scan unaffected, process exit equals fail_policy.exit_code everywhere.
• Follow-ups filed: trust roots: a manifest-declared policy pack outside policies/** must be a trust root #1075 (manifest-declared policy pack outside policies/** as trust root), verify: verification prepare reads an in-repo --diff-from from the base; a directory replaced by a file is not a config_error #1076 (verification prepare in-repo --diff-from; ENOTDIR), tests: key the static-only subprocess allowlist by function, not line number #1078 (allowlist keyed by function), docs: STABILITY exit-code text says 20 is strict-only; advisory exits 20 on a fail_on match (and the base floor after #1074) #1080 (STABILITY exit-code text drift), report.json: record the base exit floor beside fail_policy.fail_on #1081 (record the exit floor in report.json).
• CI on 79e6b2b: all required checks succeeded; release-tag-consistency skipped as expected.
• Repository gate (./shipgate verify, orchestrator run): state complete, merge permission true, decision passed.
• Branch contains origin/main (including fix(readers): read n8n include/excludeTools and host MCP keys/files #1072); no attribution trailers; no protected paths; no unanswered human comments.