feat(acp): advertise clientCapabilities in the initialize handshake

[kyleseaman]

Problem

KiroCrew's ACP initialize request omits clientCapabilities entirely. Both
transports (AcpClient._initialize_session and AcpRuntime) send only
protocolVersion + client name/version, so the agent applies the all-false
default and treats us as a client that supports no optional capability at all.

Why it matters

Capability negotiation is the mechanism by which the agent decides whether it
may issue a richer client-bound request. Sending nothing is indistinguishable
from declining everything, so any capability that lands upstream stays dark for
us until someone notices and edits two separate call sites.

The concrete instance is ACP elicitation — a general-purpose form prompt
(elicitation/create, mode: form|url, a requestedSchema supporting enum /
oneOf labelled single-select and type: array multi-select). kiro-cli 2.14.0
already compiles that schema and gates it on clientCapabilities.elicitation.

Fix (symptom → root cause → change)

Symptom: the agent never offers elicitation, and there is no way to tell
whether that is an upstream gap or our own omission.

Root cause, established by probe rather than by reading code. I drove
kiro-cli over ACP with a stub MCP server whose tool issues an
elicitation/create back to the host, declared
clientCapabilities.elicitation, and had the model call it. The agent replied:

{"jsonrpc":"2.0","id":1000,"error":{"code":-32601,"message":"elicitation/create"}}

So there are two independent gaps: the MCP→ACP forwarding path is
unimplemented in 2.14.0 (upstream), and we advertise nothing (ours). This PR
fixes only the second — the part we own.

Change: one shared ACP_CLIENT_CAPABILITIES dict in acp/types.py, sent by
both transports.

Key	Value	Why
fs.readTextFile / fs.writeTextFile	false	We serve no fs/* handler; advertising them would invite requests that hit _reject_unknown_server_request.
terminal	false	Same — the agent uses its own tools.
elicitation	{form: {}, url: {}}	Forward-bet: costs nothing today, lights up the moment upstream ships the bridge.

Shared dict rather than two literals because the two transports build their
initialize params independently — that is precisely how a capability added to
one silently stays dark on the other.

Consequence stated plainly, not buried: once the bridge lands, inbound
elicitation/create will be rejected by _reject_unknown_server_request until a
handler is wired. That is the same failure mode as today, but then attributable
to us rather than upstream. Declaring the capability is what makes the work
visible instead of silent.

Tests

test/test_acp_client_capabilities.py (3 tests):

• test_elicitation_is_declared — the exact shape kiro-cli gates on. Reverting
  the dict fails it.
• test_fs_and_terminal_stay_false — guards against advertising a handler we do
  not serve, which would turn silence into inbound errors.
• test_both_acp_transports_send_capabilities — asserts both call sites, so
  adding a capability to one transport only cannot pass. Asserted on source
  because neither params dict is reachable without spawning a real agent
  subprocess; encoding="utf-8" is explicit because read_text() defaults to
  the locale codec (cp1252 on the Windows shards) and these files contain em
  dashes.

The existing ACP suite (900 tests) passes unchanged — no test asserted an exact
initialize params shape, so adding the key breaks nothing.

Manual verification

Done, and it is what produced the root cause above: a stub MCP server plus an
ACP driver script confirmed the handshake accepts
clientCapabilities.elicitation cleanly, and that an MCP server's
elicitation/create is still answered -32601 method not found by kiro-cli
2.14.0. Worth re-running when kiro-cli updates, to detect the day the bridge
lands.

Screenshots

N/A — no user-visible UI change. This is a wire-protocol field in the
initialize request.

Notes

Split out of #464 (ask_question), where this shipped as an unrelated
forward-bet. Separating it keeps that PR's review surface to the feature itself
and lets this stand on its own merits. There is no functional coupling between
the two.

[github-actions]

Design Review (Fable 5) — 🟡 CONCERNS · blast radius: medium

Advisory design-level review of d126f87117d66ee514dd2f2e8367130aa3ad3d2d — updated in place on each push; does not block merge.

Design-Verdict: CONCERNS
Design-Blast-Radius: medium

Advertising elicitation with no handler contradicts this PR's own fs/terminal rationale — it schedules a client-attributable failure on upstream's release clock.

Watch

• The diff keeps fs/terminal false because "advertising them would invite requests we have no handler for," yet declares elicitation: {form, url} under the same condition. Once kiro-cli ships the bridge, MCP elicitation/create requests reach _reject_unknown_server_request — and by declaring support, KiroCrew also forfeits whatever non-elicitation fallback kiro-cli would use for a false client. The doc accepts this consequence, but the lower-risk shape is to declare the capability in the same change that wires the handler (or gate it behind detecting kiro-cli's bridge). Failure timing is otherwise controlled by an external release you don't ship with.
• The spec notes the handshake strictly validates shape (-32602 Invalid params); nothing states the new clientCapabilities key was verified against the oldest kiro-cli version users still run. One quick check against the minimum supported binary would close the risk of breaking every session at initialize.

[DESIGN-REVIEWED] d126f87

[github-actions]

Arbiter — ✅ no blocking findings

Arbiter found no unresolved long-term items that require action before merging d126f87117d66ee514dd2f2e8367130aa3ad3d2d.

Second-order review for d126f87117d66ee514dd2f2e8367130aa3ad3d2d; this comment is updated in place on each push.

Review details

Both files read. Judging the three sub-threshold themes against the narrow bar.

1. Weak source-grep transport test (Claude MEDIUM, GPT MEDIUM). A test that asserts a string appears in source can go green while the wire key is misspelled or removed from the params dict. This is a test-coverage-quality gap — the shipped behavior is correct, and strengthening the test is fully reversible in a later change with no migration, rollout, or contract cost. Fails both tests; follow-up.

2. Advertising elicitation with no handler (Claude MEDIUM, GPT MEDIUM, design CONCERN). The advertisement is a single client-side constant (ACP_CLIENT_CAPABILITIES in types.py). Is it a one-way door? No — KiroCrew ships its own client; dropping or gating the key later is a one-line change with no data migration, breaking API change, or coordinated rollout (the agent binary already tolerates the capability being absent — that's today's state). Is it concrete harm this diff can trigger in production? No — the diff's own doc establishes that kiro-cli 2.14.0 does not yet route elicitation/create over ACP, so the -32601 failure path is unreachable today; it only becomes reachable on an external future release, at which point the constant can be flipped or the handler wired before or promptly after. The failure mode is also identical to today's (-32601), only the attribution changes. Deliberate, documented forward-bet; reversible; follow-up.

3. clientCapabilities key not verified against the oldest supported kiro-cli (design CONCERN). This is the closest call: if the minimum supported binary strictly rejected unknown initialize keys with -32602, every session would fail at handshake — real availability harm. But the finding is a hypothetical ("nothing states … was verified"), not a demonstrated trigger: clientCapabilities is a standard ACP initialize field (the previous behavior was merely omitting it, and the agent "assumed the all-false default" — implying the field is understood), and no reviewer produced evidence any supported version rejects it. Under the rule that an unverified maybe is not concrete harm — and the fix, if it ever bit, is a trivially reversible one-line revert, not a locked-in contract — this routes to a pre-release verification follow-up rather than a block.

Nothing else in the findings approaches the bar.

Arbiter-Verdict: PASS

No sub-threshold finding meets the long-term-impact bar.

Suggested follow-ups (open as issues — non-blocking)

• Verify clientCapabilities against the oldest supported kiro-cli (design reviewer) — a strict-validating old binary would fail every session at initialize; one manual handshake check against the minimum supported version closes the risk, and a revert of the key is trivial if it bites. Track as a pre-release checklist item for the ACP client.
• Strengthen the capabilities transport test (Claude + GPT) — replace the source-grep assertion in test/test_acp_client_capabilities.py with a params-capture assertion (e.g. sent_params["clientCapabilities"] == ACP_CLIENT_CAPABILITIES via the existing _send_request / _send_and_await stubbing patterns in test/test_acp_client.py and test/test_acp_runtime.py); safe to wait because the shipped wire behavior is currently correct and this only hardens regression detection.
• Resolve the elicitation forward-bet before kiro-cli ships the bridge (Claude + GPT + design reviewer) — either wire an elicitation/create handler or gate/drop the advertised capability; safe to wait because the failure path is unreachable until an external kiro-cli release, and the constant in src/kiro_crew/acp/types.py is a one-line change. Open an issue pinned to the kiro-cli bridge release so the timing isn't controlled solely by upstream's clock.

[ARBITER-REVIEWED] d126f87

False positive or not applicable? The PR author or a repository writer can comment:
/ai-review override arbiter d126f87117d66ee514dd2f2e8367130aa3ad3d2d: <one-sentence reason>

For a broader accepted-risk deferral, apply defer-longterm and explain why.

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

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

This comment is updated in place on each push.

Review details

Review pass 1

Severity: MEDIUM -- src/kiro_crew/acp/types.py:76 -- "elicitation": {"form": {}, "url": {}} advertises a capability both dispatchers reject with -32601 when exercised -> Fix: omit elicitation until its request handler exists.
Severity: MEDIUM -- test/test_acp_client_capabilities.py:44 -- Checking only that "ACP_CLIENT_CAPABILITIES" appears in each source file passes if the handshake field is removed but the import remains -> Fix: mock each transport and assert the actual initialize parameters.
[CODEX-REVIEWED] d126f87

Review pass 2

No issues found - LGTM.
[CODEX-REVIEWED] d126f87

Review pass 3

No issues found - LGTM.
[CODEX-REVIEWED] d126f87

False positive or not applicable? The PR author or a repository writer can comment:
/ai-review override gpt d126f87117d66ee514dd2f2e8367130aa3ad3d2d: <one-sentence reason>

[github-actions]

Opus 5 Review — ✅ no blocking findings

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

No blocking findings — 2 advisory items.

MEDIUM — test/test_acp_client_capabilities.py:41 — the transport test asserts "ACP_CLIENT_CAPABILITIES" in src.read_text(...), which passes on the mere from ... import line and never checks the wire key is spelled clientCapabilities or that it lands in the initialize params — so a typo'd/renamed key ships green. Its stated reason ("neither params dict is reachable without spawning a real agent subprocess") is contradicted by test/test_acp_client.py:798 (stubs _send_request and captures the initialize params) and test/test_acp_runtime.py:2257 (monkeypatches _send_and_await to capture params) → Fix: add assert sent_params["clientCapabilities"] == ACP_CLIENT_CAPABILITIES inside the existing test_initialize_protocol_version_per_backend, and drop the source-grep test.

MEDIUM — src/kiro_crew/acp/types.py:76 — "elicitation": {"form": {}, "url": {}} advertises a capability with no handler, contradicting the rationale three lines above for keeping fs/terminal false ("advertising them would invite requests we have no handler for"); when kiro-cli ships the ACP bridge, an MCP server's elicitation/create reaches _reject_unknown_server_request / send_error(-32601) (client.py:3652, session_handle.py:1070) instead of the server taking its capability-absent fallback path → Fix: drop the elicitation key until a handler exists (keeping fs/terminal as-is), or land it in the same PR that wires the handler.

Verdict recorded via the action's structured output for commit d126f87117d66ee514dd2f2e8367130aa3ad3d2d.

False positive or not applicable? The PR author or a repository writer can comment:
/ai-review override fable d126f87117d66ee514dd2f2e8367130aa3ad3d2d: <one-sentence reason>