fix: make session_pid importable, and cover Codex in the host contract

[iamwhatever]

Problem / Motivation

Two defects, both found by auditing what the recent provider work actually left
behind.

The host contract does not describe Codex. docs/system-specs/features/agent-host-contract.md
is the document that tells a provider author what a backend must supply besides
speaking ACP, in nine buckets. Codex landed in ACP_BACKENDS_KNOWN and in
BASELINE_SELECTABLE_BACKENDS, so a plain public build serves sessions on it —
with a column in none of the nine buckets. The doc's own New-provider checklist
says "Silence is not an answer", and nothing enforced that, so the omission
shipped. The §5 failure the checklist already described then recurred unchanged
on it: _codex_session_mcp_servers() returns [], so nothing is projected onto
a codex session. Two places in code still infer "nothing is mounted" from
"nothing is projected" — the _codex_session_mcp_servers docstring and the
NO_MIRROR rationale — and are corrected here. (Two sibling comments and
providers.md carried the older "dormant seam no build offers" claim; main
has since rewritten all three (#8905), so they dropped out of this diff on
rebase.)

session_pid could not be imported on its own. import kiro_crew.session_pid
as the first kiro_crew module raised
ImportError: cannot import name '_track_pid' from partially initialized module.
session_pid.py imported kiro_crew.providers.base at module scope for a single
parameter annotation, closing session_pid → providers.base → acp.types → acp/__init__ → acp.runtime → session_pid.

And the RFC that governs this work, rfc-crew-agent-sdk-boundary.md, still said
"nothing proposed here has shipped" while three of its six PRs rested on premises
that are no longer true.

Why it matters

The point of the boundary work is that onboarding a provider should be writing
one driver, not editing the tree. Both defects cut against that directly.

An undocumented column is the expensive kind of missing: the next author
re-derives from code what the previous author knew, and the same class of defect
ships again — which is exactly what happened between CC's empty MCP array and
Codex's.

The import cycle was not cosmetic. It is why four other leaves carry
LLMProvider = Any runtime stubs and why session_pid reaches acp.client
through function-local imports; and the RFC named the wrong cycle
(session_pid ↔ worker_pool, which does not exist), so its PR 4 exit criterion
would have been satisfied without fixing anything.

A stale RFC is worse than no RFC: PR 3 was scoped to build a capability mechanism
that now exists, and PR 6 was scoped to delete a package that has since grown a
new load-bearing layer inside it.

What changed (motivation → approach → change)

Codex gets a column, and a gate replaces the sentence. A stronger "silence is
not an answer" had already been tried and had already failed, so the fix is a
ratchet. test_agent_host_contract_parity.py parses the header row of every
bucket table, resolves each column label through the Column-meaning table to a
backend constant, and fails when the resolved set is not exactly
ACP_BACKENDS_KNOWN. Its scope is deliberately narrow and the doc says so: it
asserts a column exists, not that a cell is true. A column of "unknown" passes.
That is the honest limit of a text gate — it makes the omission visible and a
reviewer makes it answered. All 47 Codex cells were derived from code, and rows
that are genuinely unmeasured say so rather than guessing.

Three attacks on a first draft of that ratchet shaped what it checks, and all
three passed against it. A decoy backend-shaped table placed before the real one,
because the parser took the first table and stopped looking. A continuation table
further down a bucket omitting a backend. And a fabricated
ACP_BACKEND_FAKE = "" row impersonating kiro-cli, which worked because only the
quoted value was read and a set erases the alias. So the contract table must now
be the first table under its heading with a rule row of matching width, a second
backend-shaped table in the same bucket is rejected rather than ignored, every
Column-meaning row's CONSTANT NAME is resolved against acp_backends and its
documented value checked against the real one, and bucket numbering must be
consecutive from 1 — which also stops a legitimately added tenth bucket failing
for the wrong reason.

The cycle is closed by deleting the edge, not by hiding it. Deferring the
import under if TYPE_CHECKING: was the first attempt, and
scripts/check_agent_sdk_boundary.py refused it. That refusal is correct and
worth stating: the gate counts a type-only import as boundary knowledge by
explicit design, and it deliberately offers no opt-out marker, because "this
consumer legitimately needs the layer" is the claim the boundary exists to
refuse. Being refused exposed a better answer — the annotation was wrong to begin
with. _sync_kill_provider reads _client, _proc and _active_proc through
getattr(..., None), none of which the provider ABC declares, so LLMProvider
never described that parameter. It is object now, the import is gone rather than
exempted, and the baseline shrinks from 107 edges to 106 — recorded progress, not
an exemption. session_compaction.py's now-needless LLMProvider = Any goes with
it. session_allocation.py keeps its stub because
ProviderFactory = Callable[..., LLMProvider] is a runtime subscript — noted
rather than silently left.

The RFC is re-audited to v4 against 73d60a83d: PR 1 marked landed with what
it actually shipped, the stale 68-edges/42-files census replaced by the baseline
as single source (the doc contradicted itself on that figure), §2.3 rewritten
because the capability vocabulary moved out of acp/types.py entirely into
acp_backends.py plus the new acp_tool_gate.py, §2.4 rewritten against the real
cycle, a new §2.7 recording Codex as the third cost measurement, PR 3 reframed
from "build the mechanism" to "consolidate it into the SDK", PR 4's wrong exit
criterion replaced, and PR 6 blocked on deciding where providers/mirrors/ lives.

Codex is worth reading as a split verdict, and both halves are recorded. On
mechanism it is the counter-example the boundary work wants: no if chain, but a
Routing enum plus identity-keyed tables read through accessors that fail closed
on an unknown id, with enforcement dispatching on the mechanism rather than the
harness. On declaration it repeats the old pattern exactly — its capability gaps
live in frontend prose behind if (value === CODEX), nothing puts them on the
wire, and a third harness with a caveat adds a third if.

Tests

• test/test_agent_host_contract_parity.py (new, 3 tests) — every id in
  ACP_BACKENDS_KNOWN has a column in every bucket; the Column-meaning table
  resolves every id against the real constants; and a structural self-check on
  bucket numbering, so a renamed or dropped heading cannot make the suite pass
  vacuously. Verified against seven mutations of a scratch copy of the doc: the
  three bypasses above now fail, a legitimate tenth bucket passes, and a deleted
  bucket, a narrowed rule row and the pre-Codex doc all fail.
• test/test_agent_lifecycle_cycle.py (new, 4 tests) — import kiro_crew.session_pid succeeds standalone, and pulls in zero kiro_crew.acp /
  kiro_crew.providers modules. Both run in a fresh interpreter, because inside
  pytest the ACP package is already imported and the cycle cannot be observed. A
  third test pins that the child measures this checkout — an editable install
  pointed a first draft at a sibling worktree. The child's answer is
  sentinel-prefixed so a stray write to stdout cannot read as a leaked module.
  Mutation-verified against origin/main's session_pid.py: 3 of the 4 fail.

That pair is the RFC's PR 4 exit criterion, replacing the one that tested a cycle
which does not exist.

508 tests pass across the touched areas (test_harness_parity,
test_acp_tool_gate, test_acp_backend_credentials,
test_agent_sdk_backend_install, test_pid_lifecycle, test_session_pid_*,
test_session_pool, the compaction suites, and both new files).

The full backend suite was also run locally — 87,785 passed, 89 failed — and every
one of those 89 is attributed rather than assumed. 88 reproduce identically on a
clean origin/main worktree in the same environment (this sandbox's /local/home
is owned by uid 65534, and the host has 16 cores where test_xdist_host_budget
expects a big one). The 89th,
test_public_repo_chip_status.py::test_force_public_to_private_transition_queues_hide_update,
passes in isolation and passes as a whole file on both branches, so it is a
load-order flake under the parallel suite. None of the 15 failing files is touched
by this diff, and CI's own eight backend shards are green on this head.

Manual verification

Both halves of the cycle claim were verified by hand before the tests were
written: on unpatched main, import kiro_crew.session_pid raises the
ImportError above; on this branch it loads with zero agent-layer modules in
sys.modules.

scripts/check_agent_sdk_boundary.py passes with the baseline lowered to 106
edges / 58 files, and its own --test self-check passes (32 violation probes
flagged, 13 clean probes ignored). ./scripts/docs-lint.sh passes over 261
markdown files. mypy is clean on both touched source files, as are black,
flake8 and isort on every touched Python file.

Reviewed before first push by four local lanes (two models, four charters:
factual verification of the cells, adversarial attack on the ratchet, adversarial
attack on the cycle fix and baseline, and cross-document coherence). Every
finding above Low is fixed; the notable ones were the three ratchet bypasses, one
self-contradicting cell (§5 claimed the control plane is never mounted while also
saying pooled stubs can contribute — resolved by describing what the gateway
decides), and a providers.md contradiction that main then fixed independently.

Rebase note. After this PR opened, main moved the host contract to
docs/system-specs/modules/, dropped every file:line citation from its cells,
and added a docs-lint line-ref rule with a shrink-only baseline. The Codex column
was re-applied on top of that version (line numbers stripped to match), the RFC's
v4 citations were rewritten to modules and symbols per the doc's own rule, and the
line-ref baseline shrank by 16 entries the v4 edits had already retired.

CI's own lanes then found two more, both real and both in the new cycle test, and
both are fixed here:

• Backend Lint & Type Check / subprocess-encoding gate. The child was
  spawned in text mode with no encoding=, which decodes with the Windows ANSI
  code page. It now passes **UTF8_TEXT from src/kiro_crew/subprocess_utf8.py
  — the documented correct case for that helper, since the child is a Python
  process we spawn and whose output we control.
• GPT blocking finding (span=1c56ce2df458, security-class). -I isolates
  the child but does not imply -B, so a read-only assertion was compiling
  kiro_crew into src/**/__pycache__ — writing into the checkout it existed to
  inspect. -B is now passed and pinned by a test.

That pin is worth a note, because the obvious version of it does not work. A test
that diffs .pyc filenames and mtimes around the child run passes with -B
removed: pytest imports most of kiro_crew during collection, so the bytecode
already exists and a writing child adds no new file. Measured, not assumed. The
test therefore asks the child what it will do (sys.dont_write_bytecode), which
fails exactly when the flag is dropped.

Related Issues

no linked issue: this is the re-audit that
docs/request-for-change/rfc-crew-agent-sdk-boundary.md asks for in its own
last-audited frontmatter, plus the two defects that audit surfaced. Nothing was
filed first, so there is no issue to close.

Pattern harvest

Rule candidate: review-prompt

Pattern: a module-scope import needed only for a type annotation closes an import
cycle. It is invisible in review because the import looks used, and the module
stays importable as long as something else imports the cycle's other half first —
so the failure surfaces only when that module is imported first, which no test
did. Two follow-ons generalize further. First, the check added here is the
reusable one: for any module a lower layer imports back, assert it imports
standalone and drags in none of that layer. Second, an annotation that forces such
an import is worth re-reading before it is deferred — this one turned out to name
a type the function never required, and two more instances of the same shape sit
in the tree as LLMProvider = Any runtime stubs, one of which this PR deletes.

Checklist

• At most two commits (one is the norm), 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 (if applicable)
• No secrets, credentials, or internal references in the diff

[github-actions]

Design Review (Fable 5) — ✅ PASS

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

Design-Verdict: PASS

Root causes fixed the hard way — cycle edge deleted rather than exempted, and the doc gate is honestly scoped, self-checked, and mutation-tested against its own bypasses.

Suggestions

• The functional §5 gap the gate exposed — a public-build codex session still gets no Crew MCP projection — is now well documented but untracked; file the follow-up so the "unwritten mirror" state cannot quietly ossify.

[DESIGN-REVIEWED] 3a122c2

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

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

Review details

Verification complete. All grep hits for LLMProvider in the three modules that dropped the runtime stub are annotations under from __future__ import annotations (stringified, never evaluated), and the two modules with real module-scope subscription (session_pool.py, session_allocation.py) correctly kept the stub. session_pid.py retains only docstring mentions after the import removal. The remainder of the diff is RFC/spec prose, baseline files, docstring/comment edits (_codex_session_mcp_servers still returns []), and two new test files — no production behavior change, no touched security boundary, no crash or data-loss path.

No findings.

[OPUS-REVIEWED] 3a122c2

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

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

[github-actions]

First Principles Review (Fable 5) — ✅ PASS

Premise-level review of 3a122c22d29a1a0e321e462b029e41cf081730a2 — 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 verification done — Codex is in BASELINE_SELECTABLE_BACKENDS (acp_backends.py:115-117), the providers.base import is gone from session_pid.py, the two retained LLMProvider = Any stubs are exactly the two with runtime subscripts, session_cleanup.py has postponed annotations so its stub deletion is safe, and the repo has 17 existing test_*parity*.py files, so the new gate follows an established pattern rather than inventing one. The pruned acceptEdits docs-lint baseline entry corresponds to the identifier now living in test/test_acp_seed_provenance.py.

First-Principles-Verdict: PASS

Every item traces to a reported defect — an ImportError, a backend shipped with no contract column — and each fix lands at the cause, not the symptom.

What this change ships

Intent: make the host contract describe every backend a public build can select, and make session_pid importable standalone — a FIX (two defects), plus re-auditing the RFC that governs the work.

1. Codex gets a column in all nine host-contract buckets, unmeasured rows saying so — justified
2. New test fails any ACP_BACKENDS_KNOWN backend missing a contract column — justified
3. session_pid stops importing providers.base; kill-helper parameter becomes object — justified, cause-level
4. Three LLMProvider = Any stubs deleted; two survivors re-justified (both runtime subscripts, counted: 2) — justified
5. New fresh-interpreter tests pin standalone import and zero ACP leak — justified
6. "Dormant seam, no build offers codex" claims corrected in docstring, spawn comment, NO_MIRROR — justified
7. RFC re-audited to v4: PR 1 marked landed, PR 3/PR 4/PR 6 rescoped, new §2.7 — declared, partly rides along
8. Boundary baseline shrinks 107→106 edges — mechanical consequence
9. docs-lint baseline pruned, incl. one unrelated acceptEdits line — undeclared, mechanical

Watch

Item 7: the RFC sections beyond the two fixes (PR 3 rescope, §12.5 mirrors, PR 6 gating) are a third job riding in a fix PR. Each corrects a falsifiable stale claim with named harm (a contributor executing PR 3 or PR 6 against premises the tree no longer satisfies), so they earn existence — but they are separable, and AGENTS.md's one-logical-change rule is tight against this shape.

Item 9: the acceptEdits baseline prune is forced by the identifier going live in test/test_acp_seed_provenance.py (grepped: 2 files) — harmless, but the description never mentions it.

[FIRST-PRINCIPLES-REVIEWED] 3a122c2

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 3a122c22d29a1a0e321e462b029e41cf081730a2 and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 3a122c2

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

[iamwhatever]

• fixed — span=1c56ce2df458 — test/test_agent_lifecycle_cycle.py, isolated child writes bytecode into the checkout. Fixed in 035ced860.

The finding holds exactly as written. -I isolates the child's environment but does
not imply -B, so a test whose entire purpose is a read-only assertion about the
tree was compiling kiro_crew into src/**/__pycache__ as it ran. Measured on a
scratch copy: the flags as reviewed produced 12 .pyc files under src/kiro_crew;
with -B added, zero.

Isolated child writes bytecode into the checkout

The fix is -B, and it is the only lever available here — -I implies -E, so
PYTHONDONTWRITEBYTECODE cannot do the job. Both flags now live in one named
_CHILD_FLAGS tuple with a comment saying why neither may be dropped, and
test_the_child_refuses_to_write_bytecode pins it: removing -B fails that test
and only that test.

One thing worth recording, because the obvious pin does not work. A test that diffs
.pyc filenames and mtimes around the child run passes with -B removed — pytest
imports most of kiro_crew during collection, so the bytecode already exists and a
writing child adds no new file. That was measured, not reasoned about: 458 cached
files were present after collection and an -I child changed neither the set nor any
mtime. So the test asks the child what it will do (sys.dont_write_bytecode) instead
of inspecting the directory afterwards.

Verified independently on 035ced860 before this push: the mutation behaves as
described, the other four tests are unaffected, and the same round's
subprocess-encoding gate failure in the same file is fixed alongside it by routing
the spawn through **UTF8_TEXT (src/kiro_crew/subprocess_utf8.py).

[iamwhatever]

• fixed — span=a63bd9ceeb00 — src/kiro_crew/acp/client.py, the control-plane claim contradicts pooled gateway stubs. Fixed in 4de291b6a.

The finding holds, and it is the third instance of one overclaim rather than a new
one. _pooled_mcp_servers() is appended on the session/new path for every
backend, and a pooled stub wraps a server named in the agent spec — so when the
shared gateway is on and it wrapped kirocrew-core, that server does mount on a
codex session. An absolute "none of Crew's own control plane is mounted" is
therefore false in exactly that case, and the same docstring contradicted itself
two sentences later by saying pooled stubs can contribute.

"none of Crew's own control plane ... is mounted" contradicts pooled gateway stubs mounting those servers

The docstring now separates projection from arrival, which is the distinction that
actually holds: nothing is PROJECTED here, the only entries a codex session gets
are the pooled broker stubs appended for every backend alike, so the gateway rather
than this hook decides whether the control plane arrives — and with the gateway off
the session has no MCP tools at all.

Worth recording for the ledger: the identical claim was corrected in
agent-host-contract.md §5 and in the RFC's §2.7 earlier in this PR, after a local
review lane raised it there. This docstring was the instance I missed, and the
suggested fix is the wording adopted. Docstring only — no behaviour change; black,
flake8, mypy, the 34 Python-side ratchet gates and 90 tests across the touched
areas are green on 4de291b6a.

[iamwhatever]

• fixed — span=fb614b38d267 — src/kiro_crew/providers/mirrors/registry.py, the control-plane claim contradicts pooled gateway stubs. Fixed in 8e47c47f9.

The finding holds. This is the fourth site of one inference, handed out one site per
round, so this reply closes the class rather than the instance and enumerates every
branch so a fifth round has nothing left to find.

"none of Crew's own control plane is mounted" contradicts pooled gateway stubs being appended

The invalid step is always the same word: nothing is projected so nothing is
mounted. Projection and arrival are different things — _pooled_mcp_servers()
appends the shared gateway's broker stubs on the session/new path for every
backend, and a stub wraps a server named in the agent spec, so a wrapped
kirocrew-core mounts on a codex session that projected nothing. Unprojected does
not mean absent.

Every site of that claim, and its state as of 8e47c47f9:

Site	State
providers/mirrors/registry.py NO_MIRROR[codex]	fixed here — says what decides arrival, and carries an explicit "do not shorten this to 'nothing is mounted'" so the sentence cannot regress
acp/client.py _codex_session_mcp_servers docstring	fixed in 4de291b6a, disposed of separately in the previous round's own record
agent-host-contract.md §5 Public-core default	fixed before the first push — states that the gateway rather than the harness decides
agent-host-contract.md New-provider checklist	already correct — says "projected", never "mounted"
RFC §2.7, Its sessions get an empty MCP array	fixed before the first push — same wording as §5

The three remaining control plane hits in the tree (acp/session_mcp.py:53,
:223, test_acp_session_mcp.py:303) are a different claim — the historical defect
where a missing spec mounted the control plane unrestricted on the CC/kiro path —
and are correct as written.

Docstring and rationale-string only; no behaviour change. The 34 Python-side ratchet
gates and 90 tests across the touched areas are green on 8e47c47f9.

[github-actions]

UX Review (Fable 5) — ⏭️ skipped

Revision 3a122c22d29a1a0e321e462b029e41cf081730a2 touches no user-facing surface (no changes under website/ or committed screenshots), so the UX review was skipped. Advisory — does not block merge.

[iamwhatever]

The Backend Tests (3.12, 4) red on head 88220070e is inherited from main.

Rebasing onto main after #8828
landed cleared the RemoteCrewPanel red as expected. The one test now failing is

test/test_snapshot.py::TestNotificationCopyWhenNoLiveFileExists::test_a_note_delivered_during_the_copy_survives_the_READER
— AssertionError: the append ran while the copy was still writing, so the two are concurrent rather than ordered

1 failed / 22,235 passed on the shard. This PR touches nothing under snapshot.

Attribution:

• main fails the same way. main@618251772 CI run
  34018323966 —
  same shard, same test, same assertion message. That head carries be7de18fb
  (fix(snapshot): validate notification records the copy branch installs, fix(snapshot): validate notification records the copy branch installs #8576),
  which is also the only snapshot change between this PR's previous green head
  8e47c47f9 and this one; the rebase pulled it in.
• Not a flake here: gh run rerun --failed on this shard alone reproduced it
  identically. It passes 5/5 locally, so it is timing-sensitive under the CI
  runner rather than deterministic everywhere — but it is main's timing
  sensitivity, introduced by fix(snapshot): validate notification records the copy branch installs #8576.
• Coverage Gate is downstream of the shard, not a separate failure.

Not fixing it here: test_snapshot.py is not this PR's file, and
#8723 already has that file and
snapshot.py open (currently DIRTY against main). A patch from this PR would
collide with it and be out of scope. This branch rebases once main is green on
that shard again.

Everything else on 88220070e is green: the other seven backend shards, all four
frontend shards, and GPT 5.6 / Opus 4.8 / Design / First Principles each completed
with no blocking findings.

[iamwhatever]

fixed — both First Principles concerns on 0e26d5193, in 5096507d8.

RFC §2.7 says "All four are corrected in this same PR", but the diff corrects three

Holds. The sentence was true before the rebase and false after it: main rewrote
the two acp_backends.py / acp/client.py spawn-branch comments in #8905 while
this PR was open, so they dropped out of the diff. The RFC now says exactly that —
main fixed two, this revision fixes the remaining two and goes one step further on
both (the "unprojected ⇒ unmounted" inference).

Delete the else: LLMProvider = Any branches and their now-false cycle comments in session_background.py and session_cleanup.py

Holds, and is the same one-line cleanup already applied to session_compaction.py
for the same reason. Both modules have from __future__ import annotations and use
LLMProvider only in annotations, and both else comments named the
providers → acp → runtime → session_pid → providers cycle this PR closes — so
they described a hazard that no longer exists. Deleted. session_allocation.py and
session_pool.py keep theirs, as the review notes: Callable[..., LLMProvider] is
a runtime subscript there. Boundary baseline unchanged at 106 (a TYPE_CHECKING
import was already counted); 34 Python-side gates and 290 tests across the touched
areas green.

[iamwhatever]

fixed — all three First Principles concerns on 5096507d8, in 3a122c22d. They share one root: this PR's own text carried counts and per-file claims that drifted as the PR grew, which is the exact habit the RFC it rewrites warns against. The fix is to stop counting.

session_pool.py and session_allocation.py both keep comments justifying their stub by the closed cycle

Holds. Both stubs stay (each subscripts LLMProvider at module scope, so a name must exist at runtime), but their comments now give that reason plus the boundary gate — not the cycle this PR closed.

The session_pid.py docstring says "four sibling leaves" — present tense, while this PR deletes three

Holds. Number dropped ("sibling leaves"); the test module's docstring had the same count and is fixed the same way.

RFC §2.4 says only session_compaction.py's stub is deleted; the diff deletes three

Holds. §2.4 now names all three deleted stubs and both surviving ones, with the surviving ones' reason stated as the runtime subscript.

Comment-and-prose only; no behaviour change. Boundary baseline unchanged at 106, 34 Python-side gates and 290 tests green.

[bolichen97]

Approved.