Rectify: resolve-review Codex interchangeability + silent-success immunity (#4282) by Trecek · Pull Request #4287 · TalonT-Org/AutoSkillit

Trecek · 2026-07-18T20:58:45Z

Summary

Issue #4282 has two failure planes: the operator pin resolve_review: codex is hard-rejected at kitchen open (since #4275, because resolve-review declares git_metadata_write), and before that fix, 18 Codex runs reported success with zero fixes applied because the success adjudicator is infra-signal-only.

Three phases deliver architectural immunity:

Phase A — Outcome-contract adjudication. Contract-declared output fields (accept_count, fix_failures) are parsed server-side, typed, recorded in telemetry, and evaluated against per-skill outcome_invariants in a consolidated post-session gate. Failed fix application (fix_failures > 0) demotes success. Legitimate all-skipped runs carry a success qualifier. Consolidates 4 duplicated zero-write demotion sites into one shared helper.
Phase B — Backend-neutral git mutation. Server-side commit_files(paths, message, cwd) MCP tool executes staging, pre-commit hooks, and commit in the server process. resolve-review Step 4 calls it instead of in-session git add/commit. git_metadata_write dropped from uses_capabilities; Codex backend pin validates and functions.
Phase C — Generalization + drift closure. Sibling skills (resolve-research-review, resolve-claims-review) migrated to commit_files. Frontmatter capability parsing fail-closed via SkillInfo.invalid_reason — unknown capability names mark the skill invalid with ERROR findings in recipe validation.

Closes #4282

Implementation Plan

Plan file: .autoskillit/temp/rectify/rectify_resolve_review_codex_immunity_2026-07-18_124500_combined.md

🤖 Generated with Claude Code via AutoSkillit

Add post-session outcome invariant evaluation: - OutcomeInvariantSpec type alongside WriteBehaviorSpec - Contract-field parser (_headless_outcome.py) extracts KEY = value tokens - Consolidated zero-write + invariant gate replaces 4 duplicated demotion sites - resolve-review emits accept_count and fix_failures output tokens - skill_contracts.yaml: outcome_invariants + success_qualifiers for resolve-review - RetryReason.OUTCOME_INVARIANT with routing rule (on_failure) - sessions.jsonl gains outcome_fields, outcome_invariant_violated, outcome_qualifier - Schema version 4 → 5 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Phase A test fixes: - Fix import layer violation: resolve skill_contract via caller threading instead of deferred IL-2 import in IL-1 module - Add outcome_fields/outcome_invariant_violated/outcome_qualifier to SkillResult - Use regex (not re) in _headless_outcome.py per project convention - Update test fixtures: RetryReason count 16→17, schema_version 4→5, headless module set, tool registration Phase B foundation: - commit_files MCP tool in tools_git.py (headless-tagged, no _require_enabled) - commit_files capability in SKILL_CAPABILITY_REGISTRY (works-as-is) - _CAPABILITY_PATTERNS entry for commit_files Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

- resolve-review SKILL.md: replace git add/commit with commit_files tool calls in Step 4 and context-limit protocol; drop git_metadata_write, add commit_files - Remove skip_when_false guards from resolve_review steps in remediation.yaml, implementation.yaml, merge-prs.yaml (keep on resolve_pre_review_conflicts) - Add fixes_applied capture to all three recipe resolve_review steps - Update tool registration test (add commit_files) - Regenerate contract cards and compiled recipe JSON Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

- resolve-research-review: replace git commit with commit_files, drop git_metadata_write, add commit_files capability - resolve-claims-review: same migration - Remove skip_when_false guards from research-review.yaml resolve steps - Add fixes_applied capture to both research-review resolve steps - Frontmatter fail-closed: SkillInfo gains invalid_reason field; unknown capability names mark the skill invalid instead of being silently dropped - Regenerate contract cards and compiled recipe JSON Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

- Add invalid_reason to derived_fields in schema exhaustiveness test - Assert invalid_reason is set in unknown_capability_pruned_with_warning test Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

- Remove optional: true from resolve_review steps (optional-without-skip-when) - Remove fixes_applied captures from recipe steps (dead-output violations) - Add commit_files to TOOL_SUBSET_TAGS (food truck visibility) - Add invalid_reason to SkillInfo derived_fields set (schema exhaustiveness) - Update unknown_capability test to assert invalid_reason Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

- Layer violations: move recipe imports to TYPE_CHECKING in _headless_outcome.py - Tool registry: commit_files added to HEADLESS_TOOLS, TOOL_SUBSET_TAGS - ToolContext: add SkillContractResolver protocol, wire through context/factory - InMemoryHeadlessExecutor: add skill_contract param to match protocol - TYPE_CHECKING inventory: update cross-layer guard expected set - Doc counts: kitchen-tagged 39→40, headless 2→3, total MCP tools 58→59 - Line budgets: headless/__init__.py 525→530, _headless_execute.py 630→635 - commit_files: extract path validation to server/git.py (REQ-CNST-008) - Recipe diagrams: re-pin content hashes after YAML edits - Doctor standing pins: swap fixture to still-infeasible skill - Re-export SkillContract through recipe/__init__.py (cross-package import) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…ed wiring Addresses audit findings RECT-011 through RECT-060: Tests (RECT-011–018, RECT-032): - tests/execution/test_outcome_invariants.py: outcome invariant evaluation, contract-field parser, success qualifiers, recovery paths, evidence regression - tests/server/test_tools_commit_files.py: commit_files tool path containment, pre-commit hooks, auto-fix retry, error envelopes Sibling outcome invariants (RECT-060): - skill_contracts.yaml: add accept_count/fix_failures outputs + outcome_invariants + success_qualifiers for resolve-claims-review and resolve-research-review - Both sibling SKILL.md files: emit accept_count and fix_failures tokens Runtime fail-closed (RECT-052): - rules_backend_compat.py: ERROR finding when recipe step references a skill with invalid_reason set (unknown capabilities) Test parametrization (RECT-054): - Remove research-review from git_metadata_write step guard test (no longer has any git_metadata_write steps after migration) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…-018 Subagent-authored comprehensive test suite with: - Full _build_skill_result integration tests for E6 shapes (RECT-011/012) - Counter-tests: all-REJECT, full-success, legitimate all-skipped (RECT-013) - Recovery path tests via STALE/IDLE_STALL (RECT-014) - Contract-field parser unit tests (RECT-015) - Token-emission sync cross-check (RECT-016) - Contract loader validation failures (RECT-017) - Evidence semantics regression (RECT-018) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

- skill_contracts.yaml: add flake_suspected/ci_only_failure pattern examples for sibling contracts - rules_backend_compat.py: use getattr for invalid_reason (SimpleNamespace compat) - recipe re-exports: OutcomeInvariantEntry, SuccessQualifierEntry, SkillOutput added to contracts.py, __init__.py, and __all__ - test_outcome_invariants.py: import from recipe package (not submodule) - Regenerate contract cards and compiled recipes Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

- Add tests/execution/test_outcome_invariants.py to the recipe layer's cascade map and layer-boundary allowlist (imports autoskillit.recipe). - Fix stale line-number pin in pyright suppression allowlist after recipe/__init__.py re-export additions shifted the ignore comment. - Set invalid_reason=None on the MagicMock skill_info in test_admission_dispatch_agreement_with_explicit_pin — an unset MagicMock attribute auto-vivifies truthy, short-circuiting the capability-mismatch check that getattr(..., "invalid_reason", None) now correctly guards in production. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

- RECT-039: Error envelope when pre-commit config exists but no binary found - RECT-038: validate_commit_paths now uses resolve_contained_path from core - RECT-048: Add capture fixes_applied to resolve_review recipe steps - Add test for missing pre-commit binary case Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

resolve_contained_path requires file existence and checks hardlinks/size/ world-writable which are inappropriate for staging-time path validation. Revert to inline realpath + startswith check with .git component guard. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…pins - server/git.py: revert resolve_contained_path (too strict for staging paths) - Recipe diagram hashes updated for remediation/implementation/merge-prs - Pretty-output test snapshot re-pinned for capture: fixes_applied addition Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Trecek

AutoSkillit PR Review — Verdict: changes_requested

Trecek

AutoSkillit review: blocking issue found — readOnlyHint: True on a mutating tool. See inline comment on tools_git.py:636.

The outcome-field parser previously used finditer over the full session result text, where last-match-wins. If LLM prose earlier in the output contained a line matching a declared field name in KEY = value syntax, it could overwrite the authoritative final output block. Now walks backward from the end, skips trailing non-field lines (gate tags like %%ORDER_UP%%), collects the contiguous run of field lines, and parses only those. Directly encodes the design contract that structured output tokens are always the final content. Adds regression tests with adversarial prose preceding the field block. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Trecek and others added 14 commits July 18, 2026 09:00

fix: Phase C test fixes — SkillInfo.invalid_reason coverage

2221ca1

- Add invalid_reason to derived_fields in schema exhaustiveness test - Assert invalid_reason is set in unknown_capability_pruned_with_warning test Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Trecek commented Jul 18, 2026

View reviewed changes

Comment thread src/autoskillit/server/tools/tools_git.py

Trecek commented Jul 18, 2026

View reviewed changes

Comment thread src/autoskillit/execution/headless/_headless_outcome.py

Trecek commented Jul 18, 2026

View reviewed changes

Comment thread src/autoskillit/execution/headless/_headless_result.py

Trecek mentioned this pull request Jul 18, 2026

review-pr: subagent prompts omit annotated diff → findings return line:null → inline comments dropped #4289

Closed

Trecek merged commit aedce3c into develop Jul 18, 2026
3 checks passed

Trecek deleted the impl-rectify-codex-immunity-20260718-084158 branch July 18, 2026 22:28

This was referenced Jul 21, 2026

fix_failures counts transient commit retries → spurious outcome_invariant halt #4309

Open

check_review_posted false negative: codex review-pr writes receipt to flat dir #4311

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Rectify: resolve-review Codex interchangeability + silent-success immunity (#4282)#4287

Rectify: resolve-review Codex interchangeability + silent-success immunity (#4282)#4287
Trecek merged 15 commits into
developfrom
impl-rectify-codex-immunity-20260718-084158

Trecek commented Jul 18, 2026

Uh oh!

Trecek left a comment

Uh oh!

Uh oh!

Trecek left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

Trecek commented Jul 18, 2026

Summary

Implementation Plan

Uh oh!

Trecek left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Trecek left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant