Skip to content

resolve-review is not Codex-interchangeable: git_metadata_write pin hard-rejected, and historical Codex runs silently skipped fix application #4282

Description

@Trecek

Problem

resolve_review is expected to be backend-interchangeable — any run_skill step routable to Codex the same way review_pr is (both are pinned identically in operator config). It is not:

  1. As of Rectify backend pin bypasses and propagation hardening #4275, the pin is hard-rejected at kitchen open. open_kitchen(name="remediation") fails recipe validation when agent_backend.recipe_overrides.remediation.resolve_review: codex is set.
  2. Before Rectify backend pin bypasses and propagation hardening #4275, the pin was accepted but never functioned. Session telemetry shows every Codex resolve_review run completed without applying or committing a single fix — zero fix-application across all 18 runs, reported as success in 17.

There is currently no supported configuration that runs review-resolution work on the Codex backend. This issue requests a codex-compatible resolution path.

Operator-set design constraints (2026-07-17)

These constraints are set by the repo operator and bound the solution space — do not gold-plate beyond them:

  1. Threat model is worker siloing, not adversarial defense. This repo is not defending against hackers, malware, or hostile prompt-injection. The value of .git/ write protections here is keeping each dispatched worker siloed to its own worktree — not blocking a malicious actor.
  2. Parity standard: Codex is held to the same trust bar as Claude Code — no higher. Claude Code worker sessions already run pre-commit + git commit in-session with full filesystem access. Any Codex-enablement design should match that trust level, not exceed it with adversarial-grade hardening (hook suppression, plumbing-only commits, confused-deputy defenses). Such hardening was evaluated during review and explicitly descoped; revisit only if the repo's threat model changes.
  3. Consequence: for Option A, the tool-design requirements below are correctness/siloing requirements (stay inside the calling step's worktree), not security mitigations.

Evidence ledger

Every claim below was verified first-hand during the investigation (report: .autoskillit/temp/investigate/investigation_remediation_resolve_review_codex_pin_2026-07-17_224753.md) and survived a subsequent 3-agent adversarial validation round (2026-07-17) in which every flagged discrepancy was re-checked against the primary source; corrections from that round are incorporated below.

E1 — Rejection, reproduced live (2026-07-17):

[backend-incompatible-skill] step 'resolve_review': skill 'resolve-review' requires backend 'codex' to satisfy: git_metadata_writable=True required (via 'git_metadata_write') but backend has git_metadata_writable=False. (origin: agent_backend.recipe_overrides.remediation.resolve_review)

E2 — Pin origin: ~/.autoskillit/config.yaml agent_backend.recipe_overrides.{implementation,remediation}.resolve_review: codex. Hand-edited config; no code path writes this key (sole write gateway write_config_layer()src/autoskillit/config/settings.py:616-632 — has exactly two call sites, both in cli/_init_helpers.py).

E3 — Capability asymmetry between the sibling skills:

  • src/autoskillit/skills_extended/review-pr/SKILL.md:4: uses_capabilities: [agent_model, github_api_write, run_skill]
  • src/autoskillit/skills_extended/resolve-review/SKILL.md:4: uses_capabilities: [agent_model, git_metadata_write, github_api_write, run_skill, test_check]
  • The git_metadata_write requirement is real: SKILL.md:611-618 (Step 4) instructs in-session git add / pre-commit run / git commit per accepted finding.
  • Only git_metadata_write hard-blocks: it is the only capability with required_backend_property (git_metadata_writable, src/autoskillit/core/types/_type_constants_registries.py:436-445); github_api_write has codex_status="fix-required" but no backend property and is dispatch-advisory (_type_constants_registries.py:395-403, 446-454).

E4 — Codex genuinely cannot write .git/ in its sandbox (declared at src/autoskillit/execution/backends/codex.py:637, evidence comment added by e880352 / #4121):

  • Upstream source, independently fetched and verified 2026-07-17: codex-rs/protocol/src/permissions.rspub const PROTECTED_METADATA_PATH_NAMES: &[&str] = &[".git", ".agents", ".codex"] (via named constants; .git confirmed verbatim), applied per writable root by default_read_only_subpaths_for_writable_root(). The escape-hatch predicate has_explicit_write_entry_for_metadata_path() (call sites verified first-hand; function body confirmed in adversarial re-fetch of the raw file) lifts protection only when user policy grants an explicit write-access entry covering the protected path.
  • Enforcement was deliberately strengthened through 2026: PR #19846 "Enforce protected workspace metadata paths" (merged 2026-04-28), #19847 (Seatbelt/macOS), #19852 (bwrap/landlock). Maintainer on #15505 (comment thread fetched via API, quote verbatim, 2026-03-26): Linux previously allowing .git writes "was a missing feature on Landlock which is why it was working before".
  • Official sandbox modes (sandboxing docs, quotes independently verified): workspace-write = "read files, edit within the workspace, and run routine local commands inside that boundary"; danger-full-access = "runs without sandbox restrictions. This removes the filesystem and network boundaries." approval_policy=never does not lift .git protection — an architecture-level inference from the same docs' explicit separation of controls ("Sandboxing and approvals are different controls that work together"), not a verbatim doc sentence.
  • Note: .git reads are permitted in workspace-write (protection is read-only mount, not hidden) — git log/git status work in-session; only the write half of the workflow is blocked.

E5 — Enforcement lineage in this repo (verified via git show): 56ae370b0 (#4243) added per-step pins with, by design, no feasibility validation → e32a2232b (#4275) closed that (commit Summary, verbatim): "explicit backend pins (REQ-RES-001) suppressed rerouting, leaving no validation that the pinned backend satisfies hard capabilities" → 9da15e9b3 (#4279) added origin/remedy provenance + doctor Checks 37/38 (Check 37 confirmed flagging both pins). Firing rule: src/autoskillit/recipe/rules/rules_backend_compat.py:89-110.

E6 — The pin never worked (~/.local/share/autoskillit/logs/sessions.jsonl, tallied first-hand; re-tallied identically in adversarial round): 147 resolve_review step records. Codex: 18 runs (2026-07-14→17, all backend_override_source=explicit_config), 17 "success" — all 18 with git_writes_detected=false, file_changes_count=0. Claude Code: 129 runs, 103 with git_writes_detected=true.

  • Dispatch precondition (corrected in adversarial round): resolve_review dispatches when review_pr returns changes_requested or approved_with_comments (remediation.yaml:1324-1331; needs_human routes to check_review_posted, not resolve_review). Fix application is still the empirically common outcome — 103/129 Claude Code runs committed.
  • Detector semantics: git_writes_detected compares pre-session vs post-session HEAD SHA (execution/headless/_headless_git.py:70-82) — it detects any committed change and is blind only to pathological commit-then-reset sequences.
  • Interpretation caveat (added in adversarial round): session records carry no verdict field, so the data cannot distinguish "Codex sessions ACCEPTed findings but could not commit" from "Codex sessions systematically classified everything REJECT/skip" (e.g., a backend-specific bias in Step 3.5 intent validation). Either explanation is a defect: 0/18 fix-applications on Codex vs 103/129 on Claude Code, on the same dispatch conditions, means the Codex pin never delivered the skill's core function.

Key structural facts for the fix

  • Only Step 4 of the skill needs git writes. 12 of 13 named workflow steps (arg parsing, PR lookup, comment fetch, classification, ACCEPT/REJECT/DISCUSS intent validation, thread resolution, inline replies, reject-pattern persistence, verdict report) need only GitHub API + reasoning — codex-satisfiable. Step 4 (apply+commit, SKILL.md:611-618) is the sole git_metadata_write consumer; Step 5 uses the test_check MCP tool (codex_status="works-as-is").
  • The surrounding git ops are already host-side. pre_review_rebase (remediation.yaml:1393-1404, run_pythonqueue_ejected_fix, recipe/_cmd_rpc_merge.py:21-34) rebases server-side; re_push_review pushes via the push_to_remote MCP tool. Only git add/git commit remain in-session.
  • Server-side git commits already exist today. perform_merge runs git commit --no-verify -m "chore: untrack generated files" in the MCP server process (src/autoskillit/server/git.py:303, verified); server/tools/tools_ci_watch.py:384 commits --allow-empty server-side. MCP tools execute via the server's injected SubprocessRunner (server/_subprocess.py:47-86), not in the agent session.
  • The MCP server is already registered into Codex sessions. CodexBackend declares mcp_config_capable=True (codex.py:598) and ensure_codex_mcp_registered() (execution/backends/_codex_config.py:307) writes the AutoSkillit MCP server into the Codex session's config.toml — so a new server-side tool is reachable from Codex resolve-review sessions.
  • Gap: no MCP tool stages/commits arbitrary edited files with a caller-supplied message (full server/tools/ registry inventoried; adversarially re-confirmed — the only two server-side commit sites repo-wide are the narrow hardcoded-message ones above).
  • Precedent for per-step sandbox overrides exists: the github_api_write codex fix is applied per-call as -c sandbox_workspace_write.network_access=true (codex.py:840-847, required_sandbox_overrides at _type_constants_registries.py:452); sandbox_mode is already a per-call parameter of build_skill_session_cmd (codex.py:716-847).
  • Capability truthfulness is machine-enforced: tests/arch/test_skill_backend_annotations.py regex-scans SKILL.md prose for capability-implying commands — _CAPABILITY_PATTERNS["git_metadata_write"] (lines 28-32) matches git … commit -m, git worktree add, git checkout -b, git rebase; test_forward_check_capabilities_declared fails any skill whose prose contains these without declaring the capability. Any fix that drops git_metadata_write from resolve-review must also rewrite Step 4 (and the context-limit protocol at SKILL.md:69-79) to the tool-call form — the guard makes a stale declaration or stale prose unshippable.

Options

Option A — Server-side commit tool (recommended): add a commit_files(paths, message, cwd)-style MCP tool; resolve-review Step 4 calls it instead of in-session git add/commit; drop git_metadata_write from the skill's uses_capabilities.

  • Result: pin validates, skill becomes fully codex-interchangeable, single-skill workflow preserved. Codex sessions already reach MCP tools (mcp_config_capable=True, registration verified above); .git reads stay in-session (permitted), so commit_sha retrieval for Step 6.5 replies is unaffected.
  • Ecosystem precedent (verified): the reference MCP git server exposes git_commit/git_add executing in the server process, outside any client sandbox (modelcontextprotocol/servers git README — 12 tools, uvx/docker deployment, independently fetched). SWE-bench-style harnesses likewise keep VCS mutation in the runner, not the agent.
  • Worktree-containment requirement (siloing, per operator constraints): the tool must resolve every requested path to absolute form, enforce containment within the calling step's worktree, and reject .git/-internal paths. Verified cautionary precedent for the implementation mechanism: CVE-2026-27735 (advisory fetched first-hand) — the reference mcp-server-git git_add tool staged files outside the repository because GitPython's repo.index.add() performs no working-tree boundary check for relative paths; the fix (v2026.1.14) switched to repo.git.add(), i.e. the git CLI, which rejects out-of-tree paths natively. Design rule: shell out to git -C <worktree> add -- <paths> (CLI-native containment) plus explicit path validation; do not use library-level index APIs. This is a siloing/correctness rule here — a worker staging files outside its worktree is exactly the cross-area interference the .git protections exist to prevent in this repo.
  • Hooks (per operator constraints): repo hooks are trusted — Claude Code sessions already run them in-session today. The tool should simply run pre-commit run --files <paths> and a normal git commit server-side, preserving AGENTS.md §3.1 discipline and gitleaks coverage (.pre-commit-config.yaml:89-92) with no --no-verify or plumbing workarounds. (The existing server/git.py:303 --no-verify call is a narrow hardcoded-message precedent, not the pattern to generalize.) Step 4's in-session pre-commit run --files moves server-side accordingly; first-run hook-env installs may need network (typically cached in provisioned worktrees).
  • Capability modeling: register the new tool as its own SKILL_CAPABILITY_REGISTRY entry (codex_status="works-as-is", like test_check) so resolve-review's uses_capabilities stays machine-truthful after dropping git_metadata_write, and the arch scanner (test_skill_backend_annotations.py) has a declared home for the new call pattern.

Option B — Split the skill: resolve-review-assess (codex-safe: fetch/classify/intent-validate/reply) + resolve-review-apply (git half, auto-routed or claude-pinned).

  • 12/13 steps land in the assess half; the skill already writes its analysis artifact "BEFORE any code changes are made" (SKILL.md:530-535), so the handoff artifact largely exists.
  • Well-precedented recipe pattern: prepare_pr → compose_pr (capture: prep_path handoff) and review_pr → resolve_review (output_dir JSON handoff). Per-step pin granularity supports pinning the assess half to codex while only the apply half requires git (_resolve_backend_override, server/_guards.py:475-552).
  • Cost: two dispatches per resolution cycle, verdict/retry routing must be re-threaded (capture: verdict, on_result routing, skip_when_false moves to the apply half only), and ACCEPT replies need the apply half's commit_sha fed back.
  • Under the operator-set constraints above, B's advantage over A is architectural purity (no new privileged tool; each half's capability declarations are exactly true for its backend) — not security necessity. If B is pursued, prototype the assess→apply artifact handoff first to measure the second dispatch's latency and context-reload cost before committing to the re-threading work.

Option C — Codex-side sandbox rule granting .git write for this step (upstream escape hatch).

  • Source-level mechanism exists upstream: has_explicit_write_entry_for_metadata_path() lifts protection when user policy grants an explicit write rule for the metadata path (permissions.rs, verified). A community-reported working config exists on #15505 (upgraded from "unverified" in adversarial round: comment thread fetched via API first-hand — JohnnyBill, 2026-05-01, custom permissions profile granting .git write under project roots, "worked for me"); no documented first-class toggle exists.
  • Upstream trajectory (nuance added in adversarial round): shipped changes have only tightened enforcement (#19846/#19847/#19852) — but OpenAI did attempt a first-class scoped relaxation: PR #25664 "feat: allow limited git writes in workspace sandbox" + PR #17036 "feat: enforce allow_git through permissions", built explicitly to fix #15505 — both auto-closed unmerged 2026-06-16 (states verified via API first-hand). So a sanctioned allow_git toggle was attempted and stalled, not rejected; Option C rests on an undocumented-but-real mechanism that upstream has neither committed to nor removed.
  • Would follow the established required_sandbox_overrides "fix-required" pattern (network_access precedent, codex.py:840-847) — but requires making git_metadata_writable conditional rather than static, and three enforcement points read the static field (recipe/rules/rules_backend_compat.py, server/tools/_auto_overrides.py:43, fleet/_api.py:136), plus tests/execution/backends/test_codex_capabilities_fields.py:33-36 hard-asserts False. Also deliberately reintroduces the hooks/config code-execution surface the Codex sandbox exists to remove. Highest risk.

Option D — danger-full-access for this step: rejected. Removes filesystem and network boundaries entirely (official docs, quote in E4); violates the sandbox premise the backend declaration and conformance tests encode.

Option E — Status quo: leave resolve_review unpinned; auto-routing (worker_routable=True, R0 #4176) runs it on claude-code. Zero code, correct today — but abandons the interchangeability requirement this issue exists to restore. Documented here as the interim operator guidance, not the resolution.

Acceptance criteria

  1. A supported configuration exists in which resolve_review work is dispatched to the Codex backend and review fixes are actually applied and committed (not silently skipped), with open_kitchen validation passing.
  2. The E6 failure mode becomes structurally impossible, using the signal the skill already emits: an invocation with accept_count > 0 and fixes_applied == 0 (SKILL.md Step 7 emits fixes_applied = accept_count - skipped_in_fix_phase, verified) must not adjudicate as unqualified success. The guard must not key on git_writes_detected == false alone — that would false-positive legitimate all-REJECT / already_green runs.
  3. Capability declarations remain truthful: if Step 4 no longer needs in-session .git writes, git_metadata_write is removed from uses_capabilities and Step 4 / context-limit-protocol prose is rewritten to the tool-call form (enforced automatically by test_forward_check_capabilities_declared); if any path still commits in-session, validation must still fail-closed for infeasible pins.
  4. Doctor Check 37 and rules_backend_compat Check B behavior remain intact for genuinely infeasible pins.

Related

#4275 (pin feasibility enforcement), #4279 (provenance + doctor checks), #4243 (per-step pins), #4176 (R0 auto-route), #4121 (codex .git sandbox verification — code comment at codex.py:630-637), #4268 (pre-#4275 validation-gap record, staged), #4194.
Investigation report: .autoskillit/temp/investigate/investigation_remediation_resolve_review_codex_pin_2026-07-17_224753.md

Evidence tiering: claims are first-hand-verified unless explicitly marked otherwise. Web sources independently re-fetched for: codex permissions.rs, sandboxing docs, MCP git server README, #15505 comment thread, PRs #25664/#17036, CVE-2026-27735 advisory. Adversarial validation round (3 agents, 2026-07-17): one claim corrected (E6 dispatch precondition), one hedge upgraded (#15505 workaround), one nuance added (stalled upstream allow_git PRs), plus the arch-test/pre-commit-policy considerations folded into Option A and the ACs; all adversary findings were re-verified against primary sources before incorporation, and two adversary flags were themselves rejected on re-check (the e32a223 quote is verbatim from the commit Summary; no recipe/_auto_overrides.py path error existed). Third round (2026-07-17): an external deep-research report was evaluated; its one verifiable new engineering claim (CVE-2026-27735, git CLI vs library-API containment) was source-verified and incorporated, and its security-first framing was replaced by the operator-set design constraints section above.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugExisting behavior is brokenrecipe:remediationRoute: investigate/decompose before implementation

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions