fix(acp): name the adapter that spawned, not the seam

[Premshay]

Problem / Motivation

AcpClient._spawn logs Spawned claude-agent-acp (PID n), and _drain_stderr
prefixes adapter stderr with claude-acp, from fixed strings chosen by which
seam the session took. But both seams resolve their binary through a documented
environment override — CLAUDE_AGENT_ACP_BIN (resolution order step 1) and
CODEX_ACP_BIN — and either may point at a dispatch shim, a vendored build, or
a wrapper that is not the seam's own adapter. In that configuration every log
line names a program that did not run.

Why it matters

The override exists precisely so an operator can put a different adapter behind
a seam, and these two lines are the only place the running adapter is named.
They are therefore most misleading in exactly the setup they are most needed
for: an operator reading the log to confirm which engine a session got is told
the seam's default, confidently and wrongly. Downstream automation that parses
the line reaches the same wrong conclusion, and stderr from a non-default
adapter is filed under the wrong program.

What changed (motivation → approach → change)

Symptom: the spawn and stderr labels report the seam, not the adapter.

Root cause: both labels are constants selected by self._is_claude /
self._is_codex, while the resolved command is already in scope as argv and
is the only authority on what launched.

Change: each ACP seam now keeps its stable name and appends the pre-wrap
resolved command (for example, claude-agent-acp via /path/to/index.js). A
leading node or bun launcher yields the entry-script path, avoiding the
ambiguous default index.js label. _spawn retains the adapter label for its
separate stderr task; Claude stderr preserves its established claude-acp
prefix. The kiro-cli branch is unchanged.

Tests

test/test_acp_spawn_label.py:

• a native binary on either seam names itself;
• node / node.exe carrying an entry script yields the script, not the
  interpreter;
• an empty argv, and a bare node with nothing after it, fall back rather than
  producing an empty label;
• an override pointing at a dispatch shim is reported as that shim and
  explicitly not as claude-agent-acp — the regression this fixes.

Manual verification

Confirmed against a live gateway whose CLAUDE_AGENT_ACP_BIN points at a shim
that execs a different adapter: before the change the log read
Spawned claude-agent-acp while the spawned process tree was the other
adapter's; the resolved argv the fix reads is the same value the process tree
shows.

Related Issues

N/A — found while diagnosing a misreported adapter in a live deployment.

Pattern harvest

Rule candidate: review-prompt
Pattern: a log label hardcoded to a branch's nominal identity while the actual
value is already in scope (a resolved argv, a resolved path, a chosen binary).
The line then reports which branch was taken rather than what was done, and is
wrong exactly when an override makes the two differ — the case the reader is
consulting it about.

Checklist

• At most two commits (one is the norm), with a Conventional Commits title (feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)
• 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

Contribution License Agreement

N/A — template placeholder; no CLA wording is supplied.

[github-actions]

GPT 5.6 Review (fork) — 🔴 changes requested (blocking)

Reviewed f08ce4e5bf37730ae04407573d1e21485f5eb01a via the fork AI-review pipeline; updated in place on each push.

1 of 1 blocking finding(s) are security-class and were withheld from adjudication, so the blocking verdict stands.

BLOCKING -- src/kiro_crew/acp/client.py:5171 -- adapter-only state changes the Kiro construction path
_spawn_label = self._spawn_label or f"{KIRO_CLI_BIN} {KIRO_CLI_SUBCMD}"
Default Kiro session -> AcpClient._spawn() -> adapter-only mutable state now selects its process label.
Anchor: harness-parity H13
Fix: Keep resolved labels local to adapter branches and preserve the existing Kiro label and stderr paths unchanged.
[BLOCK-MERGE] f08ce4e
[GPT-REVIEWED] f08ce4e

Adjudication (Opus 4.8) — is blocking on each finding proportionate?

The adjudicable block is empty (0 findings). One fenced finding, F1.

F1 — H13 parity, src/kiro_crew/acp/client.py:5171. Claim: adapter-only mutable state now selects the Kiro construction path's process label.

Conditions to reach the claimed harm (adapter state affecting the Kiro label):

• self._spawn_label/self._stderr_label are set non-empty only inside the claude branch (diff:4888-4890) and codex branch (diff:4915-4917), each guarded by self._is_claude/self._is_codex.
• Those are properties derived from the per-client fixed self.backend (client.py:3317-3323); a Kiro client has both False and never enters either branch.
• Both fields initialize to "" (diff:3135-3136).

Behavior on the Kiro path: _spawn_label = self._spawn_label or f"{KIRO_CLI_BIN} {KIRO_CLI_SUBCMD}" resolves to "" or <constant> = the identical prior constant (diff:5168-5171); stderr resolves to "" or (... KIRO_CLI_BIN) = "kiro-cli", pinned by test_kiro_stderr_keeps_its_existing_prefix. The Kiro construction path is byte-identical — no failure to recover from.

The label is a diagnostic argument to finish_suspended_spawn(label=...) and the stderr log prefix, not a governance decision. The condition under which adapter-only state could reach the Kiro path is not producible: a single AcpClient is bound to one backend and the Kiro branch never writes either field. A human would accept this residual — the parity observation is structural, and the observed Kiro behavior is unchanged.

[ADJUDICATION] f08ce4e total=0 uphold=0 downgrade=0
[GPT-ADJUDICATED] f08ce4e

[ADJUDICATION-FENCED] f08ce4e fenced=1 flagged=1
FLAG F1 src/kiro_crew/acp/client.py:5171 -- The Kiro branch never writes _spawn_label/_stderr_label and the per-client backend is fixed, so "" or <kiro constant> yields the byte-identical prior label (pinned by test_kiro_stderr_keeps_its_existing_prefix); the claimed adapter-state influence on the Kiro path is not producible.
[GPT-ADJUDICATED-FENCED] f08ce4e

🏷️ Fenced finding(s) machine-flagged as likely edge case

The security fence keeps these findings blocking regardless of adjudication; the only clearance path is a human override recorded by a repository writer, who must independently verify a rationale before recording it — it is machine-authored, and a wrong override on a security-class finding ships exactly the class the fence exists to stop. (This lane's comment deliberately carries no override command.)

• F1 src/kiro_crew/acp/client.py:5171 — The Kiro branch never writes _spawn_label/_stderr_label and the per-client backend is fixed, so "" or yields the byte-identical prior label (pinned by test_kiro_stderr_keeps_its_existing_prefix); the claimed adapter-state influence on the Kiro path is not producible.

[github-actions]

Design Review (Fable 5, fork) — ✅ PASS

Design-level review of f08ce4e5bf37730ae04407573d1e21485f5eb01a via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: PASS

Label is derived from the resolved argv — the one authority on what launched — at the right layer, pre-wrap, with the seam prefix preserved for parsers.

Suggestions

• The description claims a leading bun launcher is unwrapped, but _ADAPTER_INTERPRETERS holds only node/node.exe — and neither resolver ever emits a ["bun", script] argv. Fix the description rather than the set.

[DESIGN-REVIEWED] f08ce4e

[github-actions]

Opus 4.8 Review (fork) — ✅ no blocking findings

Reviewed f08ce4e5bf37730ae04407573d1e21485f5eb01a via the fork AI-review pipeline; updated in place on each push.

Review details

No findings.

The sole candidate is a log-label refactor its own author scored low: _spawn_label/_stderr_label stay "" on the kiro path (set only in the _is_claude/_is_codex branches), so self._spawn_label or f"{KIRO_CLI_BIN} {KIRO_CLI_SUBCMD}" resolves to the identical kiro string — no behavioural change. H13 governs the construction path (argv, awaited steps, failure modes); a shared log string adds no conditional, argument, or failure mode to the kiro spawn, and the fallback expresses kiro positively as the default, not as the absence of the adapters. No observable wrong outcome (c) exists, and nothing in the diff touches a security boundary, so nothing else grounds a Step-2 finding.

[OPUS-REVIEWED] f08ce4e

[github-actions]

First Principles Review (Fable 5, fork) — 🟡 CONCERNS

Premise-level review of f08ce4e5bf37730ae04407573d1e21485f5eb01a via the fork AI-review pipeline — 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 evidence gathered. Verifying the sibling claim precisely before writing the review: the kiro-cli branch's label at base client.py:5145/5277 is a constant while kiro_bin (overridable via KIROCREW_KIRO_BIN, kiro_cli.py:251) is in scope at client.py:4954 — the same pattern the PR names as root cause. Grep for bun across src/kiro_crew/acp: 0 hits, while the description claims bun handling.

First-Principles-Verdict: CONCERNS

The named root cause has one counted unfixed sibling: the kiro-cli branch, overridable via KIROCREW_KIRO_BIN, keeps its constant label.

Not justified as shipped

• Item 5 — symptom-level (scope): the PR's own root cause ("labels are constants selected by branch while the resolved command is in scope") holds identically for the kiro branch, left unfixed without saying the defect remains there.

What this change ships

Intent: make the spawn and stderr log lines name the adapter that actually launched, not the seam's default — a FIX.

1. Claude-seam spawn line appends the resolved command ("via /path") — justified
2. Codex-seam spawn line does the same — justified
3. Adapter stderr prefix carries the resolved command on both seams — justified
4. node/node.exe launchers report the entry script, not the interpreter — justified
5. kiro-cli spawn/stderr labels stay constants — symptom-level (see above)
6. Labels are taken pre-sandbox-wrap, so the wrapper never masks the adapter — justified

Watch

• Point patch: same pattern at base client.py:5145 and client.py:5277 for the kiro branch; kiro_bin is overridable (kiro_cli.py:251) and in scope at client.py:4954. Count: 1 sibling, grep KIRO_CLI_BIN|_resolve_kiro_bin_for_spawn. Clears when: the kiro label reads its resolved kiro_bin, or the description states the kiro defect is deliberately left.
• Description claims "a leading node or bun launcher yields the entry-script path"; _ADAPTER_INTERPRETERS is {"node", "node.exe"} — no bun. Grep bun in src/kiro_crew/acp: 0 hits, so the code is right and the claim overstates. Clears when: the description drops the bun claim.

Subtractions

• Drop the self._spawn_label instance attribute — assigned and consumed only inside _spawn (1 consumer, the fallback at the old client.py:5142 site); a local variable suffices. self._stderr_label alone needs to persist for _drain_stderr.

[FIRST-PRINCIPLES-REVIEWED] f08ce4e

[bolichen97]

@Premshay Thanks for the detailed writeup. The defect is real and nothing on main covers it: _adapter_spawn_label and _ADAPTER_INTERPRETERS do not exist there, test/test_acp_spawn_label.py is absent, and both label sites in src/kiro_crew/acp/client.py (_spawn_label in _spawn, _bin_label in _drain_stderr) are still the hardcoded seam ternaries. So the remaining scope is the whole change, and the insertion point is intact, only line offsets moved.

Three things before it can land, audited at 1c90b7a:

1. The blocking H13 finding stands: adapter_argv = tuple(argv) and self._spawn_label = ... else "" sit on the shared spawn path that the kiro-cli seam also runs. Compute and store the label inside the claude and codex branches only.
2. Both advisory lanes agree the interpreter skip degrades the default install: _CLAUDE_ACP_PKG_ENTRY and _CODEX_ACP_PKG_ENTRY are both dist/index.js, so a vendored or mise resolution logs Spawned index.js for both seams, which is less identifying than the constant it replaces. Either drop the argv[1] branch and fall back to the seam constant, or log the seam plus the resolved path.
3. The branch is 105 commits behind main, so it needs a rebase. It also rewrites the exact two ternaries that open feat(acp): register opencode ACP backend — known, gated until enforceable routing exists #9013 (the opencode seam), feat(acp): add OpenCode harness #6777 and feat: add Codex ACP backend #5349 touch. feat(acp): register opencode ACP backend — known, gated until enforceable routing exists #9013 is the live one, so a note there about who owns those two expressions would save a conflict later.

One thing worth declaring in the body: the claude stderr prefix changes from claude-acp to the resolved basename for every claude-seam user, not only when an override is set.

Posted from the 2026-09-08 open-PR relationship audit (read-only, one auditor per PR); reply here if any of this is wrong.