feat(acp): wire Codex in behind an enforced tool-permission route

[iamwhatever]

Problem / Motivation

Codex could be spelled by the core but not chosen. The vocabulary, the spawn path, the adapter resolution and the per-harness protocol literal all landed in #7813; what was missing was everything that decides whether offering the switch is honest.

Two things were missing, and they were separate reasons:

• backend_install.py had no probe for the adapter. A build offering the switch could not tell an operator what was absent when a session failed to start — the install row could only read unknown.
• Nothing in this core established how a harness is made to ask before it runs a tool. Kiro Crew's PreToolUse gate — the bundled denied-command rules, the sensitive-path block, the governance ceiling — runs from exactly one place, HookManager.on_tool_call, reached only from the permission-request branch of the dispatch parser. A harness that does not send session/request_permission per tool call is a harness where none of those controls execute. codex-acp's default agent mode permits writes inside the workspace without asking, so selecting it would have put the operator's narrowing silently out of circuit.

Separately, and independent of Codex: ~/.claude/.credentials.json was not on the sensitive-path floor while Claude Code is in BASELINE_SELECTABLE_BACKENDS. That was a live agent-readable OAuth token, not a hypothetical.

Why it matters

A picker entry is a promise. Offering a harness whose tool calls skip the gate means an operator who narrowed their denied-command rules, or who relies on the sensitive-path block, gets neither for the calls that harness self-approves — with nothing in the UI or the logs saying so. And an install row reading unknown hands a failed session to the operator with nothing to act on.

The Claude token gap needed no Codex work to matter: any agent fs_read could lift it and impersonate the operator against that vendor.

What changed (motivation → approach → change)

The credential floor (independent of Codex, and it stands on its own merits). ~/.codex/auth.json and ~/.claude/.credentials.json go on the sensitive-home floor. Kiro Crew never reads either — it only checks that one EXISTS so it can name the right sign-in command — so nothing legitimate loses access. Only the token leaf is classified, never the adapter directory: the siblings (codex config.toml, claude settings*.json) stay readable because routing diagnosis reads them and they carry no credential.

Both adapters honour a home override, so the resolved-root helper becomes a _ResolvedRoots NamedTuple carrying the resolved home, the logical (unresolved) home, and the override roots each adapter honours — KIROCREW_HOME, CLAUDE_CONFIG_DIR, CLAUDE_HOME — and the builder re-anchors each leaf under them. That tuple is both the anchor set and the TTL cache key on purpose: a root the builder anchors on but the key omitted would serve targets computed for the previous value, the fail-open shape the resolved-home key already exists to prevent. Two details are load-bearing and easy to "clean up" wrongly:

• Both home spellings are anchored. A host that symlinks /home/u → /local/home/u resolves one and reports the other, so anchoring only the resolved form leaves the logical path undenied.
• Override roots are read VERBATIM — no .strip(). _resolved_env_root now matches _valid_override_home(). Stripping anchored the floor on <root> while the process actually ran from "<root> ", denying nothing.
• Targets are deliberately not casefolded. A casefolded path denies nothing on a case-sensitive filesystem.

The route. acp_tool_gate resolves one verdict per harness, so the refusal message, a doctor row and any later dashboard surface cannot disagree. For Codex the client verifies session/new advertised mode=read-only and applies it after session/new|load, before any prompt can run. Not advertised → INDETERMINATE (Kiro Crew cannot tell); write rejected → BYPASSED (an observed failure). Both refuse identically, because a guarantee that lapses when evidence is missing is not a guarantee.

There is no local opt-out. An earlier revision carried an agent.acp_backend_allow_ungated_tools bool that started the session anyway with a warning. It is gone, and its absence is now asserted by a test: a local config bool can undercut a governance ceiling set above the operator, which is exactly the control the bypass would be waiving. The refusal is unconditional and carries the three unenforced controls by name plus a remedy.

The verdict and the apply ship together deliberately: routing_verdict returns ROUTED on a promise, because the option lives on a session that does not exist yet. Ported without its caller, the harness would report routed while running its own default mode — the one silent-bypass hole in this design.

The credential homes. The standard sandbox tier deliberately leaves the AWS, SSH and kube home directories readable because kiro-cli needs its AWS config file for the credential_process path; hiding them fleet-wide would degrade the first-class harness. An enforced adapter authenticates through its own token store and has no claim on any of them, so they are denied to its child through the existing extra_hidden_dirs extension point — additive, scoped to one harness, and it does not override the operator's chosen sandbox mode.

The mask is derived, not enumerated: security.sandbox_credential_targets() reuses the read gate's own anchor rules — sensitive_home_dirs() minus the adapter's own leaf, re-anchored under every override root — so a leaf added to the floor is covered with no edit in the ACP layer. It is empty for every harness this core does not enforce, so their spawn arguments are unchanged. On macOS the profile emits both a subpath and a literal deny for each entry: most of that list is plain files rather than directories, and whether a subpath rule alone covers a non-directory was asserted in three comments in this tree and contradicted in a fourth, with no test exercising sandbox-exec either way — so the mask no longer depends on which reading is right.

New behaviour change: selecting Codex with agent.sandbox="off" now refuses to start. wrap_argv returns from its mode == "off" branch before it applies extra_hidden_dirs, so the mask — Codex's only compensating control, since ACP v1 cannot prompt on a passive read — silently evaporated. acp_tool_gate.enforce_sandbox_floor() fails that combination closed at spawn time. It keys on the effective tier from sandbox.effective_sandbox_mode(), so a governed min_level floor that already raises the tier does not produce a false refusal, and the one branch where the mask legitimately does not apply (nested-sandbox passthrough) is enumerated in sandbox.credential_mask_applies(), beside the branch it describes rather than re-derived in the gate. A host with no sandbox backend fails closed unconditionally, reading neither the unsandboxed-exec opt-in nor the governance floor: both are mutable config read at the preflight and acted on at the spawn, so either one moving in that window would drop the refusal that made an allowance safe to report.

Adapter work stays behind the adapter's own seam. The refuse-then-mask preflight runs inside the pre-existing elif self._is_codex: arm of AcpClient._spawn, not behind a new gate on the shared construction path: harness-parity H13 asks whether the Kiro path changed, and both a new conditional and a new awaited step there count as the change. Kiro reaches wrap_argv_async with neither. The cost of that placement is that a future enforced harness needs its own call, so a ratchet test pins one preflight call site per enforced harness — forgetting it fails CI instead of shipping an unmasked adapter.

A gate refusal no longer costs a wasted spawn. AcpToolGateUnroutable documents itself Non-retryable — the refusal is a configuration fact, so a respawn re-reads the same answer — but it subclasses AcpError, so ensure_ready's transport retry ladder caught it, tore the child down, respawned, hit the identical refusal and only then raised. It now has its own handler ahead of the generic one, which also keeps the reconnect budget for the transport faults it was meant for. _sandbox_preflight also translates the leaf module's plain ToolGateUnroutable into that type at the boundary, the way the session-routing path already did: raised raw it was neither an AcpError nor the class the dedicated handler names, so a sandbox-floor refusal escaped ensure_ready uncaught and skipped the cleanup every other refusal path runs.

The probe, and the flip. _probe_codex reports missing with the component and the command that installs it. One component, not claude's two: codex-acp ships its own compatible Codex binary. Only with the route and the probe together does the baseline entry stop being a switch ahead of its answer, so NOT_SHIPPED_SELECTABLE is empty again — the state its own docstring calls healthy.

Known gap, stated rather than closed

ACP v1 offers no way to make an adapter ask for a passive READ. mode=read-only covers commands and changes; reads emit no session/request_permission, so the sensitive-path block does not see reads this harness performs. That does not close, and this PR does not claim it does.

What made it dangerous was the combination with readable credential homes, and that combination is what the sandbox change removes. The SSH directory is included deliberately and it has a cost: git-over-SSH inside a Codex session stops working, because the private key is no longer readable. Leaving private keys readable would not close what this exists to close, and a harness landing new has no established workflow to break. An operator who needs it back has the sandbox mode to lower, deliberately — and lowering it all the way to off is now a refusal rather than a silent downgrade.

Second known gap, accepted with a recorded override: on Linux the mask is point-in-time. Both launcher mask loops are guarded on existence (if os.path.isdir(target) / if os.path.isfile(target)), because a bind mount needs a mountpoint. A floor leaf that does not exist at spawn therefore gets no mount at all, so one created during the session — ~/.git-credentials from a credential helper, ~/.npmrc from a registry login — is readable by the child for the rest of that session. macOS is not affected: its rules are path patterns and apply whether or not the file exists.

This is a property of the Linux bind-mount mechanism shared by every consumer of extra_hidden_dirs and by the standard tier's own hidden dirs — an absent ~/.aws created mid-session is equally unmasked for kiro-cli — so it is not introduced here. It is disclosed rather than waved away, because the mask is the only compensating control for an enforced adapter, which makes absence cost more here than elsewhere. Closing it means changing the Linux masking mechanism itself (overlaying the parent, or a path-based rather than mountpoint-based deny) for all consumers; creating the leaf to manufacture a mountpoint is not an option, since it would leave a stray empty credential file on the host that could itself break the operator's auth.

Scope: Claude is declared but not enforced

Claude is SEEDED_SETTINGS and is not enforced here, and the reason is a read-back gap rather than a missing writer. _write_claude_local_settings (acp/client.py, called from the claude arm of _spawn) does seed permissions.defaultMode into <work_dir>/.claude/settings.local.json — but the write is conditional on ownership: it touches only a file Crew created this session whose bytes are still the ones Crew wrote, and otherwise leaves the path alone. Nothing then reads back whether the adapter honoured the mode, so a bypassPermissions already sitting in a user's own settings.local.json or ~/.claude is neither detected nor stripped. That is what cannot be established, so the verdict stays INDETERMINATE.

routing_verdict reports that honestly as INDETERMINATE rather than upgrading it to a ROUTED this core cannot earn. What is scoped is whether a verdict refuses, not whether it is told truthfully — which keeps a shipped harness working while leaving its gap visible. Widening the scope means implementing a mechanism, not editing an allowlist. Claude's unrouted state on public builds is a real finding this PR documents but does not fix.

Tests

• test_acp_backend_credentials.py (13, new): both leaves on the floor, both default locations blocked, siblings still readable, each override root anchored one variable at a time, and the override roots proven to be cache-key fields. Revert-verified: 10 fail with the change removed.
• test_acp_tool_gate.py (47, new): split along the axis that matters — what a verdict SAYS versus whether it REFUSES. Pins that Claude is INDETERMINATE and not refused, that INDETERMINATE refuses alongside BYPASSED for an enforced harness, that the refusal names all three controls and a remedy, that no local opt-out exists, and five malformed-advertisement shapes each failing closed with their own reason. Also covers the sandbox floor (off refuses an enforced adapter; a governed floor still starts; nested passthrough refuses), the derived mask (whole read-gate floor covered, adapter's own token still readable, override-relocated credentials re-anchored, paths absolute), and the new test_every_enforced_harness_reaches_the_spawn_preflight ratchet.
• test_harness_parity.py (34): test_codex_is_selectable_and_answerable and test_codex_tool_calls_are_gated_before_it_is_offered assert selectability, the probe and the route TOGETHER — either alone is the failure mode, so a change dropping one fails here.

2362 tests pass across the gate, credentials, parity, client, client-coverage, security and sandbox-profile suites. test_tool_gate_refusal_does_not_retry_the_spawn pins the single spawn, test_sandbox_preflight_translates_the_gate_refusal pins the boundary translation, and test_extra_hidden_file_leaf_also_gets_a_literal_deny pins the file-shaped deny rule; all three are revert-verified.

Manual verification

16 diff-scoped gates pass: harness-parity, sync-io-in-async, subprocess-encoding, agent-sdk-boundary, brand-name, feature-map, changelog-history, focus-cue, loop-bound-locks, testpaths-coverage, builtin-skill-scope, vendor-manifest, lockdown-before-publish, black, docs-lint and scrub-lint. mypy is clean across 1282 source files.

Not run locally, disclosed rather than claimed: no live Codex session. This host has no codex-acp installed — which the new probe correctly reports as missing. The refusal, apply and mask paths are covered by unit tests against advertised/unadvertised config payloads and synthetic override roots, but the end-to-end "a real codex-acp session starts and asks" path is unexercised here. config-baseline.json was regenerated by the repo's own scripts/generate_config_baseline.py.

Screenshots / video

Why no screenshot: no rendered surface changes. The new config field is schema-driven and appears in Settings from the generated baseline; the dashboard's backend switch already renders from the live registry (union of server answers) rather than a hard-coded list, so Codex appearing there is data, not markup.

Related Issues

no linked issue: continues the agent_sdk boundary work tracked by docs/request-for-change/rfc-crew-agent-sdk-boundary.md, and builds on the Codex seam merged in #7813. Reference implementation for the routing model is #6307, which reached the same verdict on Codex's read gap.

Pattern harvest

Rule candidate: review-prompt — a picker entry and an armed gate are separate facts, and shipping one without the other is the defect. Both times a harness reached BASELINE_SELECTABLE_BACKENDS in this repo, something that justified admitting it was missing: Claude's OAuth token was off the sensitive-path floor, and its routing was never established on public builds. The generalizable check is that adding an id to a selectable set requires naming, in the same change, what makes the offer honest — an install probe that can explain a failure, and a routing verdict that is earned rather than assumed.

Second candidate: harness-parity — a compensating control that a lower sandbox tier silently discards is not a control. The mask was correct and still evaporated at mode == "off", because the tier that disables the sandbox returns before the extension point that applies it. Any control delivered through the sandbox needs a spawn-time check that the tier in force actually delivers it.

Checklist

• Squashed to a single commit, with a Conventional Commits title
• Existing tests pass and new tests added for new functionality
• Self-review completed; code follows project style guidelines
• Documentation updated (harness-parity intro + H8, harness-onboarding worked example, baseline docstring)
• No secrets, credentials, or internal references in the diff

[github-actions]

Design Review (Fable 5) — 🟡 CONCERNS

Design-level review of bc3dcc0001e2779fccc3fb293e39213e21267006 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: CONCERNS

Sound, fail-closed design throughout — but on hosts with no sandbox backend the picker offers a harness that can never start, failing the PR's own "a picker entry is a promise" bar.

Watch

• detect_backend knows only namespace (Linux userns) and sandbox-exec (macOS). On native Windows — a supported platform — and on userns-denied Linux, credential_mask_applies answers False unconditionally, so enforce_sandbox_floor refuses every Codex session while _probe_codex reads installed and the refusal's remedy ("a host with a working backend") is one that operator cannot perform. Failing closed is right; offering the switch where it can never succeed repeats the unknown-install-row defect this PR exists to close, one layer up.
• The end-to-end path is unexercised (disclosed): the pinned ("mode", "read-only") spelling was never checked against a live codex-acp. If the real adapter advertises differently, every real session refuses — fail-closed, but broken-on-arrival for all users, invisible to the 2362 green tests.
• ROUTED rests on advertised spelling, not observed behavior: an adapter version keeping mode=read-only but changing what it prompts for bypasses silently, since nothing correlates tool-call notifications with preceding permission requests.
• Description fidelity: "The new config field is schema-driven and appears in Settings" survives from the removed opt-out revision — no config field or baseline change is in this diff.

Suggestions

• Fold sandbox-backend availability into _probe_codex (or the doctor row the leaf module was built for), so a no-backend host learns pre-session rather than per-spawn.

[DESIGN-REVIEWED] bc3dcc0

[github-actions]

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of bc3dcc0001e2779fccc3fb293e39213e21267006 — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All counts are verified. The change is well-grounded overall, but it ships a truth-reporting API layer (routing_verdict, the Verdict enum, the SEEDED_SETTINGS state) and a sandbox.effective_sandbox_mode helper that nothing in src/ consumes — the enforcement half that actually runs only ever asks the one-bit "is it SESSION_CONFIG?" question. Here is the review:

First-Principles-Verdict: CONCERNS

The enforcement half is derived and consumed; the verdict-reporting half (routing_verdict, Verdict, effective_sandbox_mode) ships for a doctor/dashboard that doesn't exist yet — zero consumers.

What this change ships

Intent: let an operator actually choose Codex as a harness without their security controls silently going out of circuit — an ADDITION, carrying one independent FIX (the Claude/Codex OAuth-token floor gap).

1. Codex appears in the backend picker — justified
2. Codex install row says missing/installed + the npm command, not unknown — justified
3. Codex sessions are forced to mode=read-only before the first prompt, refused otherwise — justified
4. Selecting Codex unsandboxed (off, no backend, or nested) refuses to start — justified, declared
5. Codex's child loses read access to every credential home, breaking git-over-SSH there — justified, declared
6. Agent file tools can no longer read ~/.codex/auth.json / ~/.claude/.credentials.json, overrides included — justified (independent fix)
7. macOS sandbox emits literal+subpath denies for masked entries — justified
8. A tool-gate refusal no longer burns a respawn — justified
9. routing_verdict / Verdict enum / SEEDED_SETTINGS reporting state — zero consumers
10. sandbox.effective_sandbox_mode — zero consumers, undeclared-as-dead

Capped at 10; docs/spec updates and the black-baseline prune of test_sandbox_cc_mode.py (an undeclared but AGENTS.md-sanctioned rider) omitted.

Watch

• The description says the sandbox guard "keys on the effective tier from sandbox.effective_sandbox_mode()", but the shipped enforce_sandbox_floor calls credential_mask_applies() instead; effective_sandbox_mode has 0 callers in src+test (grep effective_sandbox_mode) — a leftover from the revision the tests describe reverting.
• One root cause spans item 9: the truth axis has no runtime reader. routing_verdict: 0 src consumers (grep routing_verdict, hits only its own module). enforce_runtime_routing's verdict: parameter is never read in the body — both client call sites pass a Verdict the function ignores; the wording difference lives entirely in reason. The Routing distinctions AGENT_SPEC/SEEDED_SETTINGS/UNVERIFIED are consumed only by routing_verdict. Named consumer ("doctor row, dashboard GET") is future tense — inherited, not derived.

Subtractions

• Delete sandbox.effective_sandbox_mode (sandbox.py:5304) — 0 consumers; credential_mask_applies already owns the clamp.
• Drop the verdict parameter from enforce_runtime_routing and the two verdict= arguments in acp/client.py:3604,3620 — the parameter is never read.
• Defer routing_verdict + the Verdict enum (and the SEEDED_SETTINGS branch it alone distinguishes) to the PR that lands its doctor/dashboard consumer; enforcement needs only routing_for, is_enforced, session_config_issue.

[FIRST-PRINCIPLES-REVIEWED] bc3dcc0

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed bc3dcc0001e2779fccc3fb293e39213e21267006 — this comment is updated in place on each push.

Review details

I've verified the exception hierarchy (AcpTimeoutError/AcpProcessDied both subclass AcpError), the broad except AcpError in the new _apply_session_permission_routing, its conversion to a BYPASSED verdict → AcpToolGateUnroutable, and that the dedicated AcpToolGateUnroutable handler in ensure_ready is placed before the retry handler and does not retry. The asymmetry with step 5 (_apply_startup_model, whose transport faults fall through to the retrying except (AcpTimeoutError, AcpError)) is real. It fails closed and self-heals on the next prompt, so it is advisory, not blocking.

No blocking findings.

FINDING — src/kiro_crew/acp/client.py:3609 — the broad except AcpError around await self.set_config_option(...) classifies a transient transport fault (AcpTimeoutError/AcpProcessDied, both AcpError subclasses) as verdict=BYPASSED, raising the non-retryable AcpToolGateUnroutable; every other init step retries such a fault once via except (AcpTimeoutError, AcpError), so a flaky RPC at this one write fails the prompt with a misleading "would not reach the security gate" message instead of the transparent respawn-and-retry → Fix: re-raise transport-class errors (AcpTimeoutError, AcpProcessDied) unchanged before the enforce_runtime_routing(BYPASSED) call so the transport ladder retries them, treating only a genuine JSON-RPC option/value rejection as BYPASSED.

[OPUS-REVIEWED] bc3dcc0

Verdict parsed from the review's SHA-scoped output markers for commit bc3dcc0001e2779fccc3fb293e39213e21267006.

False positive or not applicable? A repository writer can comment:
/ai-review override fable bc3dcc0001e2779fccc3fb293e39213e21267006: <one-sentence reason>

[github-actions]

GPT 5.6 Review — ✅ human override accepted

Human judgment by @iamwhatever overrides the GPT 5.6 finding for bc3dcc0001e2779fccc3fb293e39213e21267006; the recorded reason is authoritative for this commit.

This comment is updated in place on each push.

The model was not re-run because an authorized human decision supersedes it.

False positive or not applicable? A repository writer can comment:
/ai-review override gpt bc3dcc0001e2779fccc3fb293e39213e21267006: <one-sentence reason>

[iamwhatever]

• BLOCKING — src/kiro_crew/acp_tool_gate.py:515 — the credential mask evaporates on the no-backend + opted-in spawn path, so enforce_sandbox_floor fails open — fixed in b03868a9297f2267e55e2f82b44325504e4089d3 (span=cb3e020ed5e0).

Legitimate, and the trace is exactly right:

effective_sandbox_mode returns "standard", so the guard returns without refusing → wrap_argv reaches backend=="none" with opted_in true and no floor, takes the opted-in branch and return argv, None — extra_hidden_dirs is never applied.

The guard tested a tier when the thing it depends on is whether the mask is applied, and those are not the same question. Rather than add the second branch to the gate — which would leave the same drift open for a third — the branch knowledge now lives beside the branches, in sandbox.credential_mask_applies(mode), and enforce_sandbox_floor asks that instead:

if credential_mask_applies(mode):
    return
raise ToolGateUnroutable(...)

I enumerated every wrap_argv return to make sure this closes the chain rather than one more link. Exactly two outcomes hand back an unwrapped child: the off tier, and backend == "none" with unsandboxed exec opted in and no floor mandating a sandbox. namespace and sandbox-exec both thread extra_hidden_dirs through, and the remaining backend == "none" case raises SandboxUnavailableError, which needs no guard because nothing starts — so the predicate deliberately returns True there, leaving that path's host-specific remedy text intact instead of replacing it with a vaguer refusal.

Two tests pin both halves: test_no_backend_with_opt_in_refuses_an_enforced_adapter (revert-verified — the old != "off" predicate passes that configuration straight through) and test_no_backend_without_opt_in_is_left_to_the_sandbox.

[iamwhatever]

• BLOCKING — src/kiro_crew/acp_backends.py:100 — Codex can start without its required credential mask — fixed in b03868a9297f2267e55e2f82b44325504e4089d3 (span=77b30efb5807).

The defect is legitimate and converges with the Opus lane's finding on the same commit:

No sandbox backend + sandbox_allow_unsandboxed_exec=true -> Codex spawn -> wrap_argv returns the unwrapped child, leaving sensitive credential paths readable.

Fixed at the guard rather than by the proposed remedy. enforce_sandbox_floor now asks sandbox.credential_mask_applies(mode) — "will extra_hidden_dirs actually be applied" — instead of testing one tier, so both mask-dropping paths refuse: the off tier, and backend == "none" with unsandboxed exec opted in and no governance floor. I enumerated every wrap_argv return to confirm those are the only two; namespace and sandbox-exec apply the mask, and the remaining no-backend case already raises SandboxUnavailableError.

On the proposed fix — "Remove Codex from the baseline until every unwrapped sandbox path refuses it" — the second clause is now satisfied, so the precondition for removal no longer holds. Removing Codex from BASELINE_SELECTABLE_BACKENDS would also delete the feature this PR exists to add, when the actual gap was a guard covering one of two branches; closing the branch is the narrower change and is what the codebase's own fail-closed idiom asks for here.

Covered by test_no_backend_with_opt_in_refuses_an_enforced_adapter, which is revert-verified: with the previous effective_sandbox_mode(mode) != "off" predicate restored, that configuration passes the guard and the test fails.

[iamwhatever]

• BLOCKING — src/kiro_crew/acp/client.py:3251 — Codex sandbox preflight blocks the gateway event loop — fixed in 2166c93a7cd9a441f776c7836f7af71e6daed737 (span=0dfda262228b).

Legitimate, and self-inflicted by the previous round's guard:

First cold Codex spawn on macOS -> detect_backend() -> synchronous subprocess.run() -> gateway tasks and heartbeat stall.

enforce_sandbox_floor was called inline from the async ensure_ready, and its predicate probes for a sandbox backend — a cold probe shells out. This repo has an explicit no-blocking-call-on-event-loop rule and the call sat on the wrong side of it.

Fixed by offloading, and by taking the second half of the suggested fix too: the credential-target resolution also touches the filesystem (home plus every env-override root), so both steps now run in one worker thread rather than two hops:

adapter_hidden_dirs = await asyncio.to_thread(
    _sandbox_preflight, self.backend, self._sandbox_mode
)

_sandbox_preflight refuses first, then resolves the mask, so the refusal still happens strictly before the spawn and ToolGateUnroutable propagates out of the thread unchanged. An unenforced harness still gets an empty mask and byte-identical spawn arguments.

Verified: scripts/check_sync_io_in_async.py passes in scope, and test_no_blocking_call_on_loop.py (19 tests) plus test_acp_client.py are green — 836 tests across the touched suites.

[iamwhatever]

/ai-review override gpt d6d6221d8b74ba0f4b7ba4b0e42d3e39fca4c1a9: Same accepted residual re-scoped to the current head — an ACP adapter reading its own on-disk OAuth token is inherent to running Claude Code or Codex at all, is not closable in this repo because codex-acp offers no off-disk credential channel, and this change strictly reduces total credential exposure by putting both adapter tokens on the read-gate floor for the first time and masking every other floor credential from the Codex child.

[github-actions]

AI-review override not recorded: d6d6221d8b74ba0f4b7ba4b0e42d3e39fca4c1a9 is not the current PR head. Re-run the command with d6d6221d8b7493195455b9aed00842c90bb4f698.

[bolichen97]

Open PR relationship audit

This is a consolidated, point-in-time code-level audit note. It compares complete merge-base diffs and current/merged code; it does not treat a shared topic as duplication or partial coverage as completion.

Relationship findings

• PR #5349 is OVERLAPPING relative to this PR. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #5349: MERGE_DISCUSSION. 7963 is the current-upstream continuation of the same capability on the landed seam and additionally closes the two documented admission blockers this PR leaves open; landing both would produce two Codex client paths, one of them ungated. Files: src/kiro_crew/acp_backends.py, src/kiro_crew/acp/client.py.
• PR #6777 is OVERLAPPING relative to this PR. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #6777: MERGE_DISCUSSION. Textual collision on the baseline-selectable statement, plus a policy dependency: PR #7963 defines what a harness must demonstrate before it becomes selectable, and PR #6777 admits OpenCode with no routing verdict at all. Files: src/kiro_crew/acp_backends.py, src/kiro_crew/acp_tool_gate.py.
• This PR is OVERLAPPING with PR #6307. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #7963: MERGE_DISCUSSION. Neither PR subsumes the other, but the credential-floor change and the Codex routing model are duplicated implementations of the same security decision, and the two heads conflict irreconcilably (add/add on both new test files). A maintainer has to pick which module owns the routing verdict — 6307's acp/tool_gate.py inside the acp package with a local opt-out, or 7963's leaf acp_tool_gate.py with no opt-out plus the OS-level mask — before either can be reviewed on its own merits. 7963 additionally carries the install probe and the sandbox mask that 6307 lacks, so the likely outcome is that whichever lands second drops its duplicated half rather than being closed. Files: src/kiro_crew/security.py, src/kiro_crew/acp_tool_gate.py, test/test_acp_tool_gate.py.
• This PR is OVERLAPPING with PR #6663. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #7963: MERGE_DISCUSSION. Complementary features on the same function whose interaction is undefined. Whichever lands second should state whether expose_docker_config applies to a harness whose reads bypass the PreToolUse gate, and ideally assert it. Files: src/kiro_crew/sandbox.py.
• This PR is OVERLAPPING with PR #7140. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #7963: MERGE_DISCUSSION. Whichever lands second must extend credential_mask_applies to cover the other's new wrap_argv outcomes, or 7963's compensating control silently stops applying on the newly reachable configurations. Files: src/kiro_crew/sandbox.py. The two independent directions used different labels; the matrix conservatively retains OVERLAPPING for coordination.
• This PR is OVERLAPPING with PR #8255. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #7963: MERGE_DISCUSSION. Not a functional conflict, but 8255 would relocate the routing tables 7963 introduces. Landing 7963 first is cheaper (8255 absorbs two more fields); the reverse order forces 7963 to be re-expressed as descriptor data. Files: docs/system-specs/modules/harness-parity.md.

No PR, Issue, label, branch, or review state was changed by the relationship-note portion of this audit.

[iamwhatever]

/ai-review override gpt af985b8: Two residuals accepted by the maintainer and now documented in the PR — an ACP adapter reading its own on-disk OAuth token is inherent to running Claude Code or Codex at all and is closable only upstream, and the Linux mask being point-in-time is a property of bind-mount masking shared by every extra_hidden_dirs consumer and by the standard tier itself, with no fix available at that layer; the third finding on this head, the contradictory Claude-settings claim, is fixed rather than accepted.

[iamwhatever]

• BLOCKING — src/kiro_crew/acp_backends.py:117 — Linux mask misses credentials created after session start — rebutted on af985b8ef7a5ec82bedf2f3e96431a90aa99be5f (span=77b30efb5807).

The mechanism is real and I verified it rather than arguing from the diff. Both launcher mask loops are guarded on existence — if os.path.isdir(target) at sandbox.py:3074 and if os.path.isfile(target) at :3118 — because a bind mount needs a mountpoint. An absent floor leaf matches neither branch and gets no mount at all, so one created during the session is readable by the child for the rest of it. The sibling READONLY_DIRS loop deliberately uses os.path.exists and its comment names this exact failure ("would silently skip every ceiling FILE — the caller asks for it to be sealed, gets no error, and it stays writable"), so the distinction was already known one loop away.

Scope worth stating precisely. macOS is not affected: after the previous round its rules are subpath plus literal path patterns, which apply whether or not the file exists. On Linux the realistic instances are ~/.git-credentials from a credential helper and ~/.npmrc from a registry login — both plausible mid-session.

The maintainer has accepted this as a disclosed residual and a /ai-review override gpt af985b8ef7a5ec82bedf2f3e96431a90aa99be5f is recorded alongside this comment. Three things make that the proportionate answer rather than the offered remedy:

It is a property of the Linux bind-mount masking shared by every consumer of extra_hidden_dirs and by the standard tier's own hidden dirs, not something this change introduces. An absent ~/.aws created mid-session is equally unmasked for a kiro-cli spawn today. Removing Codex would apply a bar the existing sandbox does not meet for any harness, while leaving that behaviour in place.

There is no fix at this layer. A non-existent path cannot be a bind mountpoint, and manufacturing one by creating the leaf would leave a stray empty credential file on the operator's host after the session — an empty ~/.netrc can itself break the auth it was meant to protect. Closing it properly means changing the Linux masking mechanism for all consumers (overlaying the parent, or a path-based rather than mountpoint-based deny), which is its own change.

I am not waving it away on the strength of "pre-existing", though: the mask is the only compensating control for an enforced adapter, so absence costs more here than it does for a harness whose tool calls reach the gate anyway. That is why it is now written into the PR's Known-gap section with the mechanism named, rather than left implicit — and why the general fix is worth tracking separately rather than dropping.

[iamwhatever]

• BLOCKING — src/kiro_crew/acp/client.py:4722 — Codex routing changes the Kiro construction path — rebutted on af985b8ef7a5ec82bedf2f3e96431a90aa99be5f (span=0dfda262228b).

This is a repeat of an already-adjudicated finding, not a new defect, and the evidence has not moved. Re-checked against the current base rather than restated: on origin/main, _initialize_session spans 4398-4586 and already contains Codex-specific arms — elif self._is_codex: at 4441 and at 4473, self._is_codex at 4467, and PROTOCOL_VERSION_CODEX if self._is_codex at 4404 — alongside if self._is_kiro: further down. Per-backend arms are that method's pre-existing shape on the first-class path, so a positive routing predicate there is the established idiom rather than a novel intrusion in service of an adapter.

It also cannot move earlier. Those existing Codex arms run while the session/new request is being built, and the permission option must land after session/new and after the model apply (both write config options, and the permission one must be last) while still being pre-prompt — which is what makes the placement a guarantee rather than a best effort.

And the one alternative that removes the conditional is strictly worse against this same anchor: self-guarding inside _apply_session_permission_routing and calling it unconditionally puts a new awaited step on the kiro path, which H13's own FLAG list names explicitly alongside a new conditional. Rounds 4 through 6 on this span already demonstrated that pattern — each fix satisfied one clause of the anchor by violating another — and the resolution was to move the adapter's work behind the adapter's own seam in _spawn, which is where it now lives.

Same-span stall, recorded as the rule asks: this span has now blocked rounds 5 (H13, unconditional to_thread), 6 (H13, the gate itself), 8 (the macOS mask rule shape, a different subject sharing the span because span identity is path plus reviewer/kind) and this one — four rounds, past the three-round trigger. The invariant that makes the whole span unreachable, and which the code now satisfies: an adapter's work lives behind the adapter's own seam, and the shared construction path gains neither a conditional nor an awaited step in service of it. The one predicate left is in a method whose per-backend branching predates this change.

Not deferred and not waived: if there is a placement for a post-session/new adapter step that satisfies H13, I will take it. I could not find one, and reverting the hook removes the routing enforcement this change exists to add.

[iamwhatever]

• FINDING — src/kiro_crew/acp_tool_gate.py:23 — the Claude settings claim contradicts _spawn — fixed in af985b8ef7a5ec82bedf2f3e96431a90aa99be5f (span=d633f1076c61).

Correct, and it was my error. _write_claude_local_settings is defined at acp/client.py:3077 and called directly from the claude arm of _spawn at :3738 (and again at :4508), so "the public core does not write claude's settings file" was simply false — and it was not reached through getattr, as I had also written.

The contradiction was in three places, not one, and the third is the reason this was worth more than a comment edit:

1. the acp_tool_gate module docstring the finding names;
2. acp_tool_gate.py:260 — the user-visible INDETERMINATE reason string returned by routing_verdict, which read "this core does not write the harness's permission settings". That is what a doctor row or a refusal message shows an operator, so the false claim was being surfaced, not just recorded;
3. the Routing.SEEDED_SETTINGS docstring in acp_backends.py, which additionally claimed the method was "attached by an internal companion".

All three now describe the actual limitation, which is a read-back gap rather than a missing writer: the method does seed permissions.defaultMode, but the write is conditional on ownership — it touches only a file Crew created this session whose bytes are still the ones Crew wrote, and otherwise leaves the path entirely alone — and nothing afterwards confirms the adapter honoured the mode. So a bypassPermissions already sitting in a user's own settings.local.json or ~/.claude is neither detected nor stripped, which is what cannot be established and why the verdict stays INDETERMINATE. That reasoning is documented on _write_claude_local_settings and on CC_PERMISSION_MODE_BYPASS in acp/types.py:194; the gate was contradicting its own module rather than the code being unclear.

The same false sentence was in the PR description's "Scope: Claude is declared but not enforced" section and is corrected there too.

Worth noting it strengthens rather than weakens the surrounding argument: an unenforced harness is unenforced because its precondition is unverifiable, which is a sharper claim than "nobody writes the file". It does not touch the mask asymmetry — adapter_hidden_credential_dirs returns () for Claude because SEEDED_SETTINGS is outside ENFORCED_ROUTINGS, which is independent of who writes the settings.

Verified: 16 local gates green including docs-lint, mypy clean across 1280 files, 2361 tests pass across the touched suites, single commit on the current origin/main.

[github-actions]

Human judgment recorded

@iamwhatever marked the gpt AI finding as false positive, not applicable, or explicitly accepted for af985b8ef7a5ec82bedf2f3e96431a90aa99be5f.

Two residuals accepted by the maintainer and now documented in the PR — an ACP adapter reading its own on-disk OAuth token is inherent to running Claude Code or Codex at all and is closable only upstream, and the Linux mask being point-in-time is a property of bind-mount masking shared by every extra_hidden_dirs consumer and by the standard tier itself, with no fix available at that layer; the third finding on this head, the contradictory Claude-settings claim, is fixed rather than accepted.

This decision applies only to this commit. A new push requires a new judgment.

[iamwhatever]

/ai-review override gpt bc3dcc0: Two residuals accepted by the maintainer and documented in the PR — an ACP adapter reading its own on-disk OAuth token is inherent to running Claude Code or Codex at all and is closable only upstream, and the Linux mask being point-in-time is a property of bind-mount masking shared by every extra_hidden_dirs consumer and by the standard tier itself, with no fix available at that layer; re-scoped to the current head after a rebase onto main.

[github-actions]

Human judgment recorded

@iamwhatever marked the gpt AI finding as false positive, not applicable, or explicitly accepted for bc3dcc0001e2779fccc3fb293e39213e21267006.

Two residuals accepted by the maintainer and documented in the PR — an ACP adapter reading its own on-disk OAuth token is inherent to running Claude Code or Codex at all and is closable only upstream, and the Linux mask being point-in-time is a property of bind-mount masking shared by every extra_hidden_dirs consumer and by the standard tier itself, with no fix available at that layer; re-scoped to the current head after a rebase onto main.

This decision applies only to this commit. A new push requires a new judgment.