fix_failures counts transient commit retries → spurious outcome_invariant halt

[Trecek]

Problem

The remediation pipeline for PR #4308 halted at escalate_stop because the server-side outcome-invariant gate demoted the resolve-review session (contract: accept_count > 0 requires fix_failures == 0; session reported accept_count=13, fix_failures=2). Session-log forensics showed the two counted failures were transient: the same commit_files call failed twice on pre-commit hooks (doc-counts 59→60 tool mismatch across 4 doc files, plus sandbox read-only-filesystem errors in uv-lock-check/check-contract-freshness) and succeeded on the third attempt (fa7b319e). All 13 accepted fixes were committed. The invariant fired on the skill's spec-mandated counting behavior, not on any terminally-failed fix: SKILL.md Step 4 instructs "A failed call increments fix_failures" (line 619), while the output-field definition describes a per-finding count ("number of ACCEPT findings whose apply/commit attempt errored", lines 880-881/901) — an internal spec inconsistency, and under either reading a transiently-failed-then-recovered fix still reports fix_failures > 0, guaranteeing an invariant violation whenever any commit_files call fails even once.

The halt was substantively deserved for a different, uncovered reason — the session emitted verdict: real_fix despite ending its third test iteration with 4 tests failing. That emission is currently spec-compliant: the verdict decision (SKILL.md:859-861) is purely accept-count-driven with no test-status gate, a seam between Step 5's "exit non-zero" failure path and Step 7's verdict tokens. Separately, the orchestrator's human-facing escalation summary misattributed the failures to "apply_patch line drift." That was a conflation, not pure fabrication: the session log contains five real apply_patch verification failed: Failed to find expected lines events (08:33:54–09:02:37Z), but all were self-recovered edit retries uncorrelated with the two counted commit_files failures (08:54:19/08:54:44Z). The recipe's escalate_stop message is a fixed generic string (remediation.yaml:2799-2803), so the misattribution originated in orchestrator free-text synthesis.

Impact

• Any resolve-review run where a pre-commit hook flakes once (and the retry succeeds) spuriously violates the invariant and halts the pipeline for human intervention. This is spec-mandated, not interpretation-dependent — a transient commit_files failure (hook flake, stale doc counts, sandbox read-only error) deterministically produces fix_failures > 0 under the SKILL.md's own instructions. First trigger among 149 recorded resolve_review runs; guaranteed to recur.
• Sibling skills resolve-research-review and resolve-claims-review declare identical invariants (skill_contracts.yaml:1931-1933, 1972-1974) and share the exposure.
• The signal that actually justified halting (final test iteration red while verdict=real_fix) is not covered by any invariant, and no test-status field exists in the resolve-review contract outputs (skill_contracts.yaml:168-189).
• Diagnosability was degraded by an empty error reason: the failed commits surfaced pre-commit retry failed: with no detail, because tools_git.py:716-718 captures only stderr and the failing hooks wrote their diagnostics to stdout.

Remediation directions (from investigation)

1. Redefine fix_failures in resolve-review SKILL.md (and siblings) to count terminal per-finding failures only; retried-then-successful commits contribute 0 (or add a distinct hook_retry_count field so the invariant stays on terminal failures). ⚠ Must ship with a companion enforcement mechanism — a terminal-only redefinition alone re-opens the silent-success hole aedce3cf9 closed (a session could skip attempts entirely, or self-classify terminal failures as transient); the contract currently has no field distinguishing never-attempted / transiently-failed / terminally-failed. This work must also resolve the SKILL.md's internal inconsistency between the per-call increment rule (line 619) and the per-finding output definition (lines 880-881/901).
2. Add a tests-green outcome invariant: verdict in (real_fix, already_green) requires a session-reported final-test-status field to be passing. This is a new contract field — nothing like it exists in the current outputs — and it targets the spec seam that let real_fix be emitted (per SKILL.md:859-861's accept-count-only verdict decision) while Step 5's failure path was exiting non-zero.
3. Escalation-summary fidelity: ground the orchestrator's human-facing failure explanation in the session's own reported reasons. The fix belongs in sous-chef doctrine / _prompts_orchestrator.py summary guidance — not in the recipe's escalate_stop message, which is a fixed generic string with no per-run content to correct.
4. Secondary/latent: harden the fix-context drift gap (context resolved once before the sequential edit loop, no re-read/bottom-up ordering) — real gap, and the session did hit five self-recovered apply_patch anchor-mismatch retries, but it was not the cause of the counted failures in this incident.
5. doc-counts interplay: a PR adding a tool without updating the 4 doc-count locations guarantees every in-session commit_files trips the hook until the session repairs docs itself; consider surfacing doc-count drift at review time.
6. Separate follow-up candidates surfaced during validation: (a) uv-lock-check/check-contract-freshness failing with Read-only file system (os error 30) during Codex-backend commit_files — a distinct sandbox/infra bug; (b) the empty-reason pre-commit retry failed: error string when hooks write diagnostics to stdout only (tools_git.py:716-718).

Investigation

Prior investigation completed interactively. See below for root cause analysis.

Investigation: PR #4308 Remediation Pipeline Halt — resolve_review outcome_invariant

Date: 2026-07-21
Scope: Root cause of the retry_reason: outcome_invariant failure at the resolve_review step of the remediation recipe run for PR #4308 (kitchen d1a383fd-d119-4ce2-a30e-821446da2ebc, session 019f83ca-704e-7151-9d73-b0054473d0c6), the 2 reported fix_failures, the 4 failing tests in the preserved clone, and whether the halted pipeline can be manually resumed from the still-open orchestrator session.
Mode: Standard

Summary

The pipeline halted because the server-side outcome-invariant gate (accept_count > 0 requires fix_failures == 0, declared in skill_contracts.yaml) demoted the resolve-review session to failure, and sous-chef doctrine routes outcome_invariant unconditionally to on_failure → release_issue_failure → … → escalate_stop. The orchestrator's human-facing explanation — "apply_patch couldn't locate expected lines after prior edits shifted line numbers" — is a conflation: five real apply_patch anchor-mismatch retry events did occur in the session (08:33:54–09:02:37Z), but all self-recovered and none coincide with the counted failures. The actual fix_failures=2 were two failed commit_files attempts of the same fix, rejected by the doc-counts pre-commit hook (docs claimed 59 tools; the PR's new get_recipe_section tool made it 60) plus sandbox read-only-filesystem errors in uv-lock-check/check-contract-freshness; the third attempt succeeded (fa7b319e…). All 13 accepted fixes are committed in the clone. The remaining real problem is 4 failing tests with three distinct causes: a stale golden snapshot, a self-inflicted regression (a later review fix removed the - 64 serialization safety margin an earlier review fix in the same session had added), and one genuinely undiagnosed envelope bug the session explicitly deferred at its 3-iteration cap. Nothing server-side blocks manual resumption: escalate_stop is advisory (action: stop writes no state), the kitchen gate stays open until close_kitchen, and run_skill performs no terminal-pipeline-state check.

Root Cause

Three-layer causal chain:

1. Invariant trigger (proximate). evaluate_outcome_invariants() (src/autoskillit/execution/headless/_headless_outcome.py:120-139, invoked from _headless_result.py:200-211) demoted the session because it self-reported accept_count=13, fix_failures=2, violating the resolve-review contract invariant (src/autoskillit/recipe/skill_contracts.yaml:193-195). Routing is then hard-coded doctrine: sous-chef/SKILL.md:113,161 and cli/_prompts_orchestrator.py:246-249 — outcome_invariant → always on_failure, retries budget not consulted.
2. fix_failures semantics (why the counter was non-zero despite all fixes landing). Per resolve-review SKILL.md Step 4 (lines 597-620): "A failed call increments fix_failures" (line 619). The codex session's commit for the envelope-preservation fix failed twice on pre-commit hooks (08:54:19Z, 08:54:44Z: doc-counts mismatch 59→60 tools across 4 doc files; sandbox read-only errors in uv-lock-check/check-contract-freshness), then succeeded on the third attempt after staging the stale doc files (08:56:03Z). Counting the two transient attempts as fix_failures=2 while also reporting fixes_applied=13 is literal compliance with the per-call rule at line 619 — although the SKILL.md's own output-field definition (lines 880-881/901: "number of ACCEPT findings whose apply/commit attempt errored") describes a per-finding count, an internal spec inconsistency. Neither definition excludes eventually-successful findings, so under either reading the invariant fired on transient, self-recovered failures.
3. Why the halt was nevertheless substantively correct. The session ended its 3rd (final) test iteration with 4 tests failing yet emitted verdict: real_fix — which, absent the invariant demotion, would have routed on_result → pre_review_rebase → re_push_review, pushing a red branch onward. The invariant caught a bad state for a partially wrong reason.

The 4 failing tests (clone verified: 31432 passed, 4 failed):

Test	Root cause
tests/infra/test_pretty_output_recipe.py::test_canonical_recipe_responses_fit_independent_registry_ceilings	Stale golden snapshot — finding #8's fix legitimately grew rendered payload ~25.7KB (load_recipe 186540 vs. hardcoded 160827 at lines 1053-1054, last set by pre-review commit 88a71d35d); no review-fix commit refreshed it.
tests/server/test_tools_recipe_pull.py::test_envelope_priority_fields_share_floor_under_tight_budget	Regression introduced by final review-fix commit d3652a3e6, which removed the - 64 safety margin (src/autoskillit/server/tools/_serve_helpers.py:376) that earlier same-session commit 3fee3f9d3 added; combined-JSON postcondition check now trips fail-closed fallback.
tests/server/test_tools_recipe_pull.py::test_envelope_priority_field_truncation_handles_multibyte_utf8	Same - 64 margin regression.
tests/server/test_tools_recipe_pull.py::test_load_recipe_envelope_pulls_from_its_own_artifact	Genuine undiagnosed bug, self-flagged "needs focused diagnosis". The test was added in-session by 1941a481d ("test(review): cover load_recipe artifact pulls") using the real BACKEND_REGISTRY["codex"](); d3652a3e6 extended real-backend usage further. step_flow_skeleton built from the full recipe (_serve_helpers.py:637-639) overflows the 40,000-byte bound independent of load_recipe's 1,139-byte payload → bare error envelope, KeyError: 'recipe_pull'.

Affected Components

• src/autoskillit/recipe/skill_contracts.yaml:193-195 — invariant declaration when accept_count > 0 require fix_failures == 0 [SUPPORTED]
• src/autoskillit/execution/headless/_headless_outcome.py:120-139, _headless_result.py:200-211 — evaluation + demotion to outcome_invariant_violation [SUPPORTED]
• src/autoskillit/skills/sous-chef/SKILL.md:113,161; src/autoskillit/cli/_prompts_orchestrator.py:246-249 — unconditional on_failure routing doctrine [SUPPORTED]
• src/autoskillit/recipes/remediation.yaml:1354 (resolve_review), :2654 (release_issue_failure), :2799 (escalate_stop, action: stop, stateless) [SUPPORTED]
• src/autoskillit/skills_extended/resolve-review/SKILL.md:597-620 — sequential fix loop with per-call fix_failures increment rule (line 619), conflicting per-finding output definition (lines 880-881/901), accept-count-only verdict decision (lines 859-861), 3-iteration cap (lines 47, 648-657) [SUPPORTED]
• Clone src/autoskillit/server/tools/_serve_helpers.py:376 — removed - 64 margin (regression) [SUPPORTED]
• Clone tests/server/test_tools_recipe_pull.py, tests/infra/test_pretty_output_recipe.py — the 4 failures [SUPPORTED]
• doc-counts pre-commit hook + docs/README.md, docs/execution/architecture.md, docs/execution/tool-access.md, docs/skills/visibility.md — 59 vs 60 tool-count mismatch that caused the counted commit failures [SUPPORTED]

Data Flow

review_pr findings (13, in local_findings_4308.json + diff_context_map handoff) → codex resolve-review session applies fixes sequentially, committing via server-side commit_files (pre-commit hooks run in server process) → session emits outcome fields (accept_count, fixes_applied, fix_failures, verdict) in its result sentinel → server parses fields (parse_outcome_fields) and evaluates contract invariants → violation demotes SkillResult to success=False, retry_reason=OUTCOME_INVARIANT → run_skill envelope returns that to the sous-chef orchestrator → doctrine routes on_failure → release_issue_failure (swaps issue label in-progress → fail) → register_clone_failure → analyze_pipeline_health_error → escalate_stop (prose-only stop; emits failure sentinel; no state written).

Test Gap Analysis

• tests/execution/test_outcome_invariants.py:99-149 covers demotion when fix_failures > 0 but nothing pins the counting semantics — there is no contract-level statement (testable or prose) distinguishing transient attempt failures from terminal per-finding failures. The SKILL.md in fact carries two conflicting definitions — a per-call increment rule ("A failed call increments fix_failures", line 619) and a per-finding output definition ("number of ACCEPT findings whose apply/commit attempt errored", lines 880-881/901); the codex session followed the per-call rule. Neither definition excludes eventually-successful findings, so both conflict with the invariant's stated intent ("Failed fix application demotes success").
• No invariant checks the signal that actually mattered: verdict=real_fix emitted while the session's own final test iteration was red. A tests_passing-style outcome field/invariant would have demoted this run for the right reason.
• The 4 clone test failures are themselves partially a review-loop process artifact: the same 3-iteration session both added (3fee3f9d3) and removed (d3652a3e6) the - 64 margin, with no guard noticing a later fix reverted an earlier fix.

Similar Patterns

• Sibling skills resolve-research-review and resolve-claims-review carry identical invariants (skill_contracts.yaml:1931-1933, 1972-1974) — same attempt-counting exposure.
• Other retry_reasons (zero_writes, early_stop, contract_recovery, thinking_stall, idle_stall) all condition their on_failure fall-through on progress evidence; outcome_invariant is the only unconditional one — by design (logical vs. infrastructure failure).
• Fail-closed evaluation (_headless_outcome.py:133-138): missing/non-numeric require fields also count as violations.

Design Intent Findings

• Outcome-invariant gate: Introduced in aedce3cf9 (2026-07-18), "Rectify: resolve-review Codex interchangeability + silent-success immunity (resolve-review is not Codex-interchangeable: git_metadata_write pin hard-rejected, and historical Codex runs silently skipped fix application #4282) (Rectify: resolve-review Codex interchangeability + silent-success immunity (#4282) #4287)". Stated purpose: 18 Codex runs had reported success with zero fixes applied because success adjudication was infra-signal-only; Phase A parses contract-declared outcome fields server-side and demotes success when fix_failures > 0. Callers/dependents: _headless_result.py (demotion), _headless_execute.py:595 + _headless_result.py:903 (telemetry propagation), sous-chef SKILL.md + _prompts_orchestrator.py (routing doctrine), recipe/_contracts_types.py/_contracts_manifest.py (schema). No ADR/design docs mention it — intent lives in the commit message and doctrine text only (sole other occurrence: a test-index table row in tests/execution/AGENTS.md:130). [SUPPORTED]
• Unconditional on_failure routing: Deliberate — "logical failure, not infrastructure; the skill ran but its core function failed"; a bare re-run is not presumed to correct agent-judgment failures, so the step's retries: 2 budget (which serves stale-type reasons) is bypassed. [SUPPORTED]
• escalate_stop: action: stop is a recipe-schema construct (recipe/schema.py:115) — pure orchestrator instruction to emit the failure sentinel; writes no DB/kitchen/fleet state. Human intervention is the designed escalation endpoint. [SUPPORTED]

Historical Context

• The gate is 3 days old (aedce3cf9, 2026-07-18). Mining all 149 resolve_review records in sessions.jsonl: this is the first-ever run with fix_failures > 0 and the gate's first trigger. Not a recurring pattern (yet).
• aedce3cf9 deliberately shipped detection without remediation of fix-application failure causes; no commit or issue addresses edit-anchor drift or fix_failures counting semantics (git log grep for drift/apply_patch/fix_failure: zero commits; nearest issues PY4 P4-A3-WP1: Extend write_guard.py to intercept Codex's apply_patch tool calls by adding apply_patch to the tool_name guard #2876, resolve_review Write Guard Blocks Source Edits + re_push_review Fails on Upstream Branch Advancement #3161 are unrelated write-guard concerns).
• Latent-but-uninvolved gap: resolve-review's SKILL.md resolves fix context once (Step 3.5, lines 381-384) before the sequential Step 4 edit loop, with no re-read/bottom-up ordering — the drift hazard the orchestrator claimed caused this failure exists structurally but did not cause this incident.
• Not flagged for /rectify as a recurrence, but the attempt-counting ambiguity makes further spurious invariant halts likely on any run where pre-commit hooks flake once.

External Research

Not applicable — all evidence was internal (session rollout logs, clone state, git history). No external library behavior implicated.

Scope Boundary

Investigated: resolve-review session rollout log + artifacts; clone commit history and full task test-check run (4 failures reproduced and diagnosed); outcome-invariant mechanism, contracts, and routing doctrine; remediation recipe step graph; server-side enforcement surfaces (run_skill preflight, pipeline tracker, guards, gate); issue claim/release semantics; historical recurrence across 149 sessions.
Not yet explored: Focused diagnosis of the load_recipe skeleton-overflow bug (test 4) — the correct fix (skeleton budgeting vs. test fixture) needs its own analysis; whether the codex sandbox read-only errors in uv-lock-check/check-contract-freshness hooks affect other commit_files calls; exact provenance of the orchestrator's "apply_patch line drift" wording — RESOLVED during adversarial validation: five real apply_patch verification failed: Failed to find expected lines events occurred in the rollout log (08:33:54, 08:35:40, 09:00:59, 09:02:17, 09:02:37Z), all self-recovered and none coinciding with the counted commit_files failures; the session's own report attributes the 2 failures solely to pre-commit doc-count staleness. The orchestrator's free-text summary conflated the uncounted edit retries with the counted failures [SUPPORTED].

Recommendations

1. Immediate (this pipeline): Resume from the still-open orchestrator — nothing blocks it. Fix the 4 tests in the clone (snapshot refresh; restore/re-derive the - 64 serialization margin in _serve_helpers.py:376; diagnose the skeleton-overflow bug), re-claim the issue (claim_issue with allow_reentry=true — release_issue_failure swapped the label to fail), then re-enter at pre_review_rebase (resolve_review's on_success target); check_review_loop will naturally decide whether another review round runs. See final assessment for a paste-ready prompt.
2. Redefine fix_failures counting in resolve-review SKILL.md (and siblings): count terminal per-finding failures only; a fix whose commit eventually succeeds after hook-triggered retries must contribute 0. Alternatively add a distinct hook_retry_count field so the invariant stays on terminal failures.
3. Add a tests-green outcome invariant: verdict in (real_fix, already_green) should require a session-reported final-test-status field to be passing — this targets the signal that actually justified halting this run.
4. Escalation-message fidelity: the escalate_stop summary presented a fabricated-sounding mechanism ("apply_patch couldn't locate expected lines") not found in session evidence. Escalation text should quote the failing session's own reported reasons verbatim.
5. Harden the latent drift gap (independent of this incident): instruct bottom-up (descending line) application or mandatory re-read of any file already edited this session before the next anchored edit.
6. Pre-commit interplay: the PR added a 60th tool without updating the 4 doc-count locations, guaranteeing every commit_files in the session tripped doc-counts until the session repaired docs itself. Consider surfacing doc-count drift at review time (review-pr dimension) rather than at commit time inside fix loops.

---

Adversarial validation (2026-07-21)

Three independent adversarial reviewers (repo-code citations, session-log/clone evidence, reasoning/completeness) attacked this issue's claims; every disputed or load-bearing citation was then re-verified manually against the actual files, logs, and commits. No claim was refuted outright. Corrections applied above as a result:

• fix_failures counting is spec-mandated per-call (SKILL.md:619), not loose interpretation — and the SKILL.md carries a conflicting per-finding definition at lines 880-881/901 (internal spec inconsistency). Recurrence on any transient commit_files failure is deterministic, not probabilistic.
• verdict: real_fix with red tests is spec-compliant (the verdict decision at SKILL.md:859-861 is accept-count-only) — a spec seam between Step 5's exit-non-zero path and Step 7's verdict tokens, not a lying session (contrast the invariant's own test name, test_lying_model_real_fix_with_fix_failures_demoted).
• The "apply_patch line drift" narrative was a conflation, not fabrication: five real, self-recovered apply_patch anchor-mismatch retries occurred in the session (08:33:54–09:02:37Z), uncorrelated with the counted failures.
• Remediation direction 1 gained the silent-success-hole caveat (terminal-only redefinition without companion enforcement re-opens what aedce3cf9 closed); direction 3 was re-aimed from the fixed escalate_stop YAML message to orchestrator summary guidance.
• Provenance fix: the failing load_recipe integration test was introduced by in-session commit 1941a481d, not d3652a3e6.
• Minor: outcome_invariant does appear in one non-design doc (tests/execution/AGENTS.md:130, a test-index row); the "149 resolve_review runs" figure re-confirmed by field-filtered count.

Post-investigation status (2026-07-21, ~07:45 PT)

Remediation of the 4 failing tests is actively in progress in the preserved clone by a follow-up session: commits dbe99feb5 ("fix(review): reserve envelope serialization overhead at finalize" — finalize-only 64-byte serialization reserve, canonical-maxima refresh, and an active-recipe-name guard so load_recipe for a non-active recipe emits a name-only skeleton instead of bloating past the bound) and f77898ecb ("fix(review): compensate test budgets for envelope serialization reserve") have landed on the PR branch. A concurrent full-suite check raced with that session's live edits and is not authoritative; final green-ness will be determined by the follow-up session's own verification. This issue documents the pipeline-halt mechanism, which is independent of that in-flight remediation.