Skip to content

fix(acp): ship Claude Code as selectable and hide agents policy forbids - #7301

Merged
bolichen97 merged 1 commit into
mainfrom
feat/claude-code-selectable
Sep 1, 2026
Merged

fix(acp): ship Claude Code as selectable and hide agents policy forbids#7301
bolichen97 merged 1 commit into
mainfrom
feat/claude-code-selectable

Conversation

@iamwhatever

@iamwhatever iamwhatever commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator

Problem / Motivation

Developer → Agent Backend renders Claude Code as permanently unavailable on every
public build
, and the reason it gives — "Not enabled in this build" — is not true.

acp_backends.BASELINE_SELECTABLE_BACKENDS shipped {kiro, kas} and excluded
ACP_BACKEND_CLAUDE, describing it as "a dormant seam ... not something a public build
can serve a session with". That description does not match the code:

  • acp/client.py owns the entire Claude spawn path — the _is_claude branch,
    _resolve_claude_acp_bin, _resolve_claude_code_executable, the argv cache.
  • The adapter it needs is a public npm package, named in the same module as
    CLAUDE_ACP_NPM_PKG (@agentclientprotocol/claude-agent-acp).
  • providers/acp.py constructs it, and PROVIDER_LABEL_CLAUDE labels its sessions.

Nothing about the harness is edition-private. The exclusion removed the switch, not
the capability, so the one thing standing between a public user and a Claude session was
a set literal.

Second problem, in the same panel: an agent the deployment may not select was rendered
greyed with an explanation. That is the wrong treatment for a restriction.

Why it matters

The first is a dead end with a misleading sign on it. #7166 added an install probe so the
panel could say what is missing and how to install it, but that machinery never ran for
Claude Code, because the build gate was checked first and always lost the option. An
operator who has claude and wants the adapter was told the build could not do it.

The second is worse than saying nothing: a greyed chip invites the reader to go find out
how to enable it, and under a managed policy there is nothing they can do from their
machine. Advertising a forbidden option is also the opposite of what the restriction is
for.

Fix (symptoms → root cause → change)

1. Claude Code joins the public baseline.

BASELINE_SELECTABLE_BACKENDS gains ACP_BACKEND_CLAUDE, so the baseline is now every
ACP_BACKENDS_KNOWN id, and the comment states what makes that correct rather than
asserting dormancy.

Usability on a given host is a separate question, and #7166's probe already answers
it: without the adapter the option is disabled with the component named and the command
that installs it. That is the line the user needed all along; it just had a build gate in
front of it.

The test pinning the old set existed "so a widening is deliberate". This is that
deliberate widening, and the assertion now reads the other way round — a future
narrowing has to state its reason next to the removal.

2. An unselectable agent is hidden, not dimmed.

AgentBackendTab now filters the rendered set instead of disabling it, which splits the
two facts onto the two treatments they deserve:

  • Not selectable → the row is gone, chip and status line both. The panel's footer
    sentence is what explains the absence.
  • Rendered but not usable → disabled, on installed === 'missing' || restart_required. Both are things the user can act on, which is why they are shown at
    all, and the status line carries the action.

The not_enabled_in_this_build string is therefore unreachable and is deleted, from
en.json and all 12 other locales.

Two properties are load-bearing:

  • The selected backend is always kept, whatever the verdict. The single gate degrades
    a denied persisted value to the floor on load, so this should not arise; if it ever
    does, a control rendering no pressed chip is a worse failure than one extra row.
  • It fails open. unavailable() is false when neither the schema nor the probe has
    answered, so an in-flight query, a 403 (non-owner) or a 404 (older gateway) hides
    nothing and disables nothing.

3. Everything the widening falsified.

Sixteen sites described the Claude seam as dormant, or the public edition as
Kiro-CLI-only, or referenced the deleted string. Each is corrected in place across
platform/bootstrap.py, platform/interfaces.py, platform/defaults.py,
providers/acp.py, config/loader.py, session.py, dashboard/chat_utils.py,
dashboard/handlers/agents.py, acp/client.py, acp/types.py, acp_backends.py and one
test docstring. Where a comment had a real point underneath the stale adjective it keeps
it — providers/acp.py still excludes the Claude client from the kiro-family runtime set,
and now says why (one process per session, no shared runtime) instead of leaning on
"dormant".

4. kirocrew doctor reported it wrongly, to the user.

Doctor printed claude-acp: ✅ <path> (dormant seam — not used by the public core), and
printed it only when the adapter happened to be installed. Both halves were wrong: an
operator who could select the harness was told the build did not use it, and an operator
who had not installed it was told nothing at all — which is doctor's entire job.

It now reports Claude Code as a real optional backend either way, and takes its verdict
from agent_sdk.probe_backend — the same owner GET /api/acp-backends uses — so doctor
and the panel cannot drift. A half-install names the absent component and the install
command, matching the dashboard line exactly.

5. Four spec docs.

features/claude-code-provider.md was titled "dormant ACP seam"; it is now "a selectable
ACP harness" and separates three questions the old text conflated — registered (build),
permitted (governance scope), usable (probe). features/agent-host-contract.md had the
dormancy premise under its conditional-surface reasoning (eleven places, including host
seam bucket 8 moving from "none — no preflight" to "partial", since the probe now exists
but a declared requirement still does not). modules/harness-parity.md drops the dormant
framing and records that ACP_BACKENDS_KNOWN and the baseline are now coupled.
features/README.md gets the index line for the retitled doc.

6. What Crew gates on this harness, and what a pre-approval skips — stated in the panel.

By default Claude asks and Crew decides: with no matching rule, every tool call reaches
the SDK's canUseTool callback, which is what claude-agent-acp turns into ACP
session/request_permission, which runs hooks.on_tool_call and lands in the SEL record.
A Claude session is governed like any other on that path.

What escapes is a call that was already pre-approved, so it never asks. Allow rules sit
at step 5 of the SDK's six-step evaluation, ahead of the callback at step 6, and
Anthropic's docs state the consequence in bold: "Auto-approved tools never reach
canUseTool."
No callback means no ACP request, so for that one call there is nothing for
Crew to gate or record. It matters here rather than being purely the operator's own choice
because the SDK reads .claude/settings.json from the project directory by default —
a cloned repository can carry allow rules its author wrote — and our core passes nothing
that would change it (_permission_mode is stored and never read; no settingSources, no
PreToolUse hook, no settings seed).

That is documented, intended Claude behaviour, not something this PR breaks — the harness
was already implemented and reachable by any edition that registered it, so the old
exclusion bought invisibility rather than enforcement. So this PR ships it and states the
narrow difference at the point of choice: a standing line on the Claude row, present
whenever the harness is offered and independent of the install probe, in all 12 locales.
Kiro CLI and KAS have no equivalent settings file that can pre-approve past Crew's gate and
deliberately carry no such line — claiming it for them would be false and would train the
reader to skip it.

Closing even the pre-approved case needs one of the two mechanisms Anthropic documents (a
PreToolUse hook, whose deny holds even in bypassPermissions, or excluding project
from settingSources). Whether claude-agent-acp forwards either over ACP is not
answerable from this repository, and is named in the spec as the prerequisite for gating
every Claude tool call rather than every call Claude asks about.

A pinned model reached the adapter in the wrong namespace. Making the backend
selectable put a second consumer behind one model translation that only ever had one.
KiroCrewConfig.acp_effective_model — the factory's own selection, shared with the
spawn-side effort verdict — hardcoded model_registry.to_acp_id, whose own docstring
says it produces kiro-cli ids. So a concrete agent.model handed the claude adapter
a kiro-namespaced id, which its set_config_option rejects, and nothing withheld it: the
pre-wire availability guard in acp.client is deliberately kiro-only, because the two
backends advertise in different namespaces and widening the check would call every
legitimate claude model unusable.

The give-away is that the warm-pool model-switch path in session_allocation already
keyed this translation on the backend, so the same pinned model translated correctly on a
warm claim and incorrectly on a cold start — the outcome depended on whether a pooled
process happened to exist. The fix makes the cold path agree with the warm one rather
than inventing a rule: to_provider_id(m, "claude_code") on the claude backend,
to_acp_id otherwise. auto collapses to "" in both, which is why enabling this
backend by editing config.json works today and why the defect was never on the default
path — the client skips the model send entirely when nothing is pinned.

The gap this does NOT close

AcpClient._claude_session_mcp_servers() returns [] by default, and the
claude-agent-acp adapter does not read kirocrew.mcp.json on its own. So a Claude session
on this build starts with zero MCP tools — the harness works (prompts, streaming,
model/effort, the permission flow) but kirocrew-core, cron and every user MCP server are
absent. kiro-cli is unaffected; it gets its servers via --agent.

This is now stated in the code comment, in claude-code-provider.md under its own
### Known gap heading, and in agent-host-contract.md §5, rather than being left for
someone to discover mid-session. Closing it means translating kirocrew.mcp.json into
that array, which is a separate change with its own tests.

Tests

  • test/test_agent_backend_editable.py — the baseline assertion is rewritten to the new
    intent and additionally pins baseline == ACP_BACKENDS_KNOWN, so adding a known id
    without shipping it, or shipping one without making it known, both fail.

  • The registration-reaches-the-allowlist test can no longer borrow Claude Code as the
    "not yet registered" id, so it constructs that state. What it actually tests is
    unchanged and now unmistakable: the allowlist resolves the registry per call.

  • restore_registry (and the equivalent inline snapshot in test_harness_parity.py) now
    restores _baseline as well as _selectable. register_selectable_backend writes
    both, so restoring one leaked a widened baseline into every later test in the run —
    a real defect, latent until a test needed to narrow the baseline.

  • test/test_harness_parity.py — H3's degradation row keeps claude in the
    parametrize for the opposite reason: it must now survive. The assertion is
    membership-conditional, which is what makes that row prove the gate reads the registry
    instead of hardcoding a verdict.

  • website/src/test/AgentBackendTab.test.tsx (26) — hiding replaces dimming and
    outranks both a missing verdict and the restart line, so neither leaks into the DOM;
    the selected backend stays visible even when it reads as unselectable; widening the
    schema makes a hidden row appear with no edit to the component; a 403 hides nothing;
    and the aria-describedby association is re-pinned on the remaining disabled case (a
    missing install), where it now carries an actionable line rather than a dead end.

  • test/test_doctor_claude_backend.py (3, new) — doctor reports the harness whether or
    not it is installed; it never calls it dormant; and a half-install names the absent
    component with the command on the following line. Mutation-checked: removing doctor's
    missing-component line reddens two of the three and nothing else.

  • test/test_acp_effective_model_backend.py (5, new) — the cold-start model id is
    translated into the namespace of the backend that will run it: a canonical key
    resolves to a claude provider id on the claude backend and stays a kiro id on kiro
    and kas, an explicit model_override is translated too, and auto still pins
    nothing on claude (the default-config path, which is why enabling the backend by
    hand works today). Mutation-checked: restoring the hardcoded to_acp_id reddens
    exactly the two claude-namespace cases.

  • test/test_doctor_claude_backend.py (+2, and two existing assertions retargeted)
    — doctor never labels an install "selectable". It reads the install probe and
    never consults apply_selectable_denials, so claiming selectability would print
    the opposite of the truth to the operator of a policy-denied deployment. The two
    older tests asserted the previous wording incidentally; their intent (an off-PATH
    install is not printed as None, and the harness is never called dormant) is
    unchanged.

Manual verification

  • Rebuilt the SPA and re-ran the committed capture harness
    (website/scripts/capture-agent-backend-probe.mjs) against the real bundle. The first
    attempt reproduced the deleted string, which caught that the harness serves a prebuilt
    dist — the frames below are from a rebuilt one.
  • Ran doctor against this host and read its actual output: claude-acp: ⏭ claude-agent-acp not found (optional agent backend) followed by the npm command. The
    three new tests assert that exact shape rather than a mock's.
  • test_acp_client.py::TestResolveKiroBinEnvOverride::test_spawn_passes_installed_path_through_exact_wrappers
    fails in this worktree. It also fails on a pristine origin/main checkout with none of
    these changes applied (verified in a detached worktree), and this diff touches no spawn
    kwargs — comments only in acp/client.py. Pre-existing, not folded in here.

Screenshots / video

Before this change, on this same host, the Claude Code chip was greyed and its line read
"Not enabled in this build" — no before frame is included because the capture harness now
builds the fixed bundle and cannot produce it.

After, on this host: Claude Code is offered, disabled only because the adapter is absent,
naming the component and the command that installs it.

Claude Code offered but disabled, with the missing component named and its install command

A managed deployment whose policy denies the harness — the row is absent entirely, and
the footer sentence explains it:

Only Kiro CLI and KAS offered; the Claude Code row is absent

The two remaining probe states, unchanged by this PR

Installed on disk, but this gateway process cached its absence — disabled, and worded as
a restart rather than as missing:

Installed but this gateway must restart

unknown leaves the option enabled and must not read as missing — the check failed,
which is not evidence the binary is absent:

Could not check whether this is installed, option still enabled

Related Issues

Follow-up to #7166, which added the install probe and the agent_backend governance
scope. That PR made the two reasons an option can be unavailable visible; this one
fixes the reason itself being wrong for Claude Code, and changes how the policy reason is
presented.

no linked issue: this closes nothing on merge — #7166 is already merged, and the
follow-up work named above (the MCP seat and the pre-approval gate) is not filed yet.

Pattern harvest

Rule candidate: review-prompt
Pattern: a registry entry excluded by a comment asserting a capability the code contradicts

The defect generalizes. BASELINE_SELECTABLE_BACKENDS excluded Claude Code with a
comment claiming a public build "cannot serve a session with it", while
acp/client.py carried the whole spawn path and the adapter was a public npm package.
Nothing mechanical could catch that: the set was internally consistent, the tests
asserted the exclusion, and the comment justified it — so the exclusion outlived its
own reason and rendered a dead option for months. A reviewer prompt can ask the one
question that finds this class: when a registry, allowlist or capability set excludes a
member, does the code that would serve that member actually exist?

The same shape produced the two smaller bugs in this PR. restore_registry restored
only _selectable while register_selectable_backend writes _baseline too, and
doctor collapsed the probe's unknown verdict into "not found" — both cases of a
second piece of state existing but one site behaving as if it did not.

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

@iamwhatever
iamwhatever requested a review from a team August 31, 2026 16:44
@iamwhatever
iamwhatever requested a review from a team as a code owner August 31, 2026 16:44
@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Aug 31, 2026
@github-actions

github-actions Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

UX Review (Fable 5) — 🟡 CONCERNS

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

UX-Verdict: CONCERNS

Hiding policy-denied agents works, but the page header still statically promises "Kiro CLI, Claude Code, or KAS" a denied deployment can't see.

Watch

  • In agent-backend-denied.png, the chip row shows only Kiro CLI and KAS, yet the page subtitle directly above (en.manual.json: "Which agent new sessions start — Kiro CLI, Claude Code, or KAS") still enumerates Claude Code — a managed-deployment user is told an agent exists, finds no trace of it in the panel, and must connect a muted footer paragraph to explain the gap. Managed deployments only × confusion/looks-broken × every visit → moderate. Smallest fix: drop the agent enumeration from the subtitle ("Which agent new sessions start with") so the header stops asserting a set the panel now derives at runtime.

Suggestions

  • Doctor's claude-acp: ✅ resolved off PATH (Claude Code installed) — "resolved off PATH" reads as mechanism; "installed (custom location)" says the same to an operator without ACP-resolver vocabulary.

[UX-REVIEWED] 73ae069

@github-actions

github-actions Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Design Review (Fable 5) — 🟡 CONCERNS

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

I have what I need. Composing the review.

Design-Verdict: CONCERNS

Sound widening with the right controls sequenced first, but it ships a live audit/deny bypass on default-allow, mitigated only by a caveat sentence.

Watch

  • The pre-approval gap is real, not theoretical: the description states "a tool pre-approved in Claude's own settings — including a .claude/settings.json inside a cloned project — never reaches Crew's approval path, so its deny rules and audit log do not see that call." A cloned repo is untrusted external content, and this widening flips the harness from unreachable to offered on every existing deployment whose policy is silent on agent_backend — an admin who composed a ceiling when the build couldn't serve Claude must now add a denial to keep the state they had. The PR names the closing mechanisms (PreToolUse hook, settingSources) but defers investigating whether the adapter forwards them; the adapter is a public npm package, so that is answerable, and a human should decide whether it's a prerequisite rather than a follow-up.
  • Disclosure asymmetry: AGENTS.md now claims "Both are disclosed on the Agent Backend panel," but the panel caveat covers only the permission gap. The zero-MCP-tools gap — the one every Claude session hits (no memory, cron, or core tools) — is docs-and-comment only, so users discover it mid-session, which the PR itself calls the failure mode to avoid.

Suggestions

  • Add a second panel line (or extend the caveat) naming the missing-MCP-tools gap at the point of choice, or correct the AGENTS.md "both are disclosed on the panel" sentence to match reality.

[DESIGN-REVIEWED] 73ae069

@github-actions

github-actions Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of 73ae0699ceecac9b3d7d188edf419449ba06502b — 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 claims verified: probe_backend/BackendInstallState exist in agent_sdk/backend_install.py, register_selectable_backend does write _baseline (acp_backends.py:149) so the fixture fix is real, and the model-translation fix aligns with the two existing backend-keyed sites (session_allocation.py:1197-1206, chat_handlers.py:4413). No unfixed to_acp_id siblings that could hand a kiro id to the claude backend. Final review follows.

First-Principles-Verdict: CONCERNS

Every item names its harm; the one soft spot is hiding the policy-denied row — argued from principle, with no reported user failing at the greyed chip.

What this change ships

Intent: let an operator who has the Claude binaries actually pick Claude Code, and stop the panel lying about why they can't — a FIX.

  1. Claude Code offered on every public build's backend panel — justified, cause-level (the set literal was the whole gap)
  2. Policy-denied agent vanishes from the panel instead of greying — declared; no named person failing; principle-derived
  3. "Not enabled in this build" deleted from 13 locales — justified, falsified by item 1
  4. Standing note on Claude row: pre-approved tools skip Crew's gate — justified (untrusted .claude/settings.json in a cloned repo is a named boundary)
  5. kirocrew doctor reports Claude Code present or absent, with the install command — justified; old line was wrong and silent
  6. A pinned model reaches the Claude adapter in its own namespace on cold start — mechanism-level; matches 2 existing backend-keyed sites (counted: session_allocation.py:1197, chat_handlers.py:4413; 0 unfixed siblings)
  7. Selected backend always rendered even when unselectable — justified defensive floor
  8. ~16 comment sites + 4 specs + AGENTS.md corrected — mandated by the same-commit spec rule
  9. Screenshot scene "missing" replaced by "denied" — rides along with item 2

Watch

  • Item 2's harm after item 1 is one falsifiable string under managed policy; the smaller fix was rewording that string ("restricted by policy") rather than the visible filter plus the kept-selected special case. The description argues dimming is wrong but cites no observed confusion; the footer sentence now carries the whole explanation.
  • Item 6 appears in no visible description section (the intent file truncates at 8000 bytes), so its declared status could not be verified.

Subtractions

  • Drop test_the_reporting_does_not_run_the_whole_doctor (test/test_doctor_claude_backend.py) — every other test in the file already imports and calls _doctor_claude_backend, so assert callable(...) guards nothing they don't.

[FIRST-PRINCIPLES-REVIEWED] 73ae069

@github-actions

github-actions Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Opus 4.8 Review — ✅ no blocking findings

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

Review details

No findings.

[OPUS-REVIEWED] 73ae069

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

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

@github-actions

github-actions Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

GPT 5.6 Review — ✅ no blocking findings

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

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 73ae069

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

@iamwhatever
iamwhatever force-pushed the feat/claude-code-selectable branch from 8493fed to 116e1e1 Compare August 31, 2026 17:21
@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Aug 31, 2026
@iamwhatever
iamwhatever force-pushed the feat/claude-code-selectable branch from 116e1e1 to 5f5aaa0 Compare August 31, 2026 18:36
@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Aug 31, 2026
@iamwhatever
iamwhatever force-pushed the feat/claude-code-selectable branch from 5f5aaa0 to 040d44f Compare August 31, 2026 18:46
@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Aug 31, 2026
@iamwhatever
iamwhatever force-pushed the feat/claude-code-selectable branch from 040d44f to be6c842 Compare August 31, 2026 20:45
@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Aug 31, 2026
@iamwhatever
iamwhatever force-pushed the feat/claude-code-selectable branch from be6c842 to 4880fc3 Compare August 31, 2026 21:49
@iamwhatever

Copy link
Copy Markdown
Collaborator Author
  • span=f44ebe40eb24shutil.which prints the literal None when the adapter resolved off plain PATH — fixed in 4880fc36a.

Correct and precisely reasoned, and it names the exact case I had exercised by hand
earlier in this branch's development: I flipped the probe from missing to installed
by pointing CLAUDE_AGENT_ACP_BIN at a real file, which is step 1 of
_resolve_claude_acp_bin and invisible to shutil.which. So the one install path I
verified manually is the path that would have printed ✅ None.

The probe resolves through the spawn's own resolver — CLAUDE_AGENT_ACP_BIN, a
vendored node_modules, a mise shim — while which only sees plain PATH, so its
miss is not evidence of anything and must not be interpolated as a location. Doctor now
names the path only when it genuinely has one, and otherwise says ✅ resolved off PATH (Claude Code selectable). Two tests, one per branch; mutation-checked by restoring the
interpolation, which reddens the off-PATH one alone.

@iamwhatever

Copy link
Copy Markdown
Collaborator Author
  • span=77b30efb5807 — Claude selection bypasses Crew's security gate — rebutted (mechanism confirmed, prescribed fix declined by the repository owner).

The mechanism is real and I verified it against Anthropic's own documentation rather
than taking it on faith.
Allow rules are step 5 of the SDK's six-step evaluation,
ahead of the callback at step 6, and the docs state the consequence in bold: "Auto-approved
tools never reach canUseTool."
Since canUseTool is what claude-agent-acp bridges
into session/request_permission, a matching allow rule means no ACP request, no
permission_request event, and hooks.on_tool_call never runs. The project setting
source is enabled by default, so a cloned repository can carry those rules. Our side
passes nothing that would change it: AcpClient._permission_mode is assigned in
__init__ and read nowhere, and there is no settingSources restriction, no
PreToolUse hook and no settings seed. Every step of the finding holds.

What is declined is the prescription, and this is now the third round it has been
raised on the same span.
Round 1 and 2 asked for ACP_BACKEND_CLAUDE to be removed
from the baseline; this round asks again. Removing it does not close the bypass — it only
makes the option invisible. Claude Code was already fully implemented in acp/client.py
and reachable by any edition that registered it, so the exclusion this PR removes
constrained the switch, never what the harness does once running. Honouring
.claude/settings.json is documented, intended Claude Code behaviour, not a hole this
diff opens in Crew.

The repository owner's ruling, which this PR implements: a provider is allowed its own
configuration model, and Kiro Crew officially supports the Kiro product lines (kiro-cli
and KAS) — which do run under Crew's approval path and carry no such caveat. Choosing
Claude Code is choosing a different governance model, so the boundary is stated at the
point of choice rather than the choice being withheld.

The residual is disclosed, not silent. The Agent Backend panel carries a standing
line on the Claude row, present whenever the harness is offered and independent of the
install probe, in all 12 locales: "Claude Code normally asks before it acts, and Kiro
Crew gates that request. But a tool pre-approved in Claude's own settings — including a
.claude/settings.json inside the project — never asks, so the deny rules and audit log do
not see it."
Three tests pin it, including that Kiro CLI and KAS do NOT carry it.
docs/system-specs/features/claude-code-provider.md states the evaluation order, the
project-settings source, the four things our core does not pass, and the two mechanisms
Anthropic documents for closing even the pre-approved case (a PreToolUse hook, or
excluding project from settingSources). AGENTS.md was amended in this PR so the
repo's own rule now matches the shipped behaviour instead of contradicting it — that was
Design Review's blocking finding, and it cleared to CONCERNS once the rule was corrected.

Whether claude-agent-acp forwards either mechanism over ACP is not answerable from
this repository and is named in the spec as the prerequisite for gating every Claude
tool call rather than every call Claude asks about. That is the follow-up this finding
correctly identifies. A formal /ai-review override accompanies this disposition.

@iamwhatever

Copy link
Copy Markdown
Collaborator Author

/ai-review override gpt 4880fc3: Mechanism confirmed against Anthropic's SDK docs and accepted; the prescribed fix is declined. Removing ACP_BACKEND_CLAUDE from the baseline does not close the bypass, it only hides a harness that acp/client.py already implements and that any edition could already register — the exclusion constrained the switch, never the harness. Honouring .claude/settings.json is documented, intended Claude Code behaviour. Kiro Crew officially supports the Kiro product lines (kiro-cli, KAS), which do run under Crew's approval path; Claude Code is offered as a provider with its own governance model, stated at the point of choice on the Agent Backend panel in all 12 locales, in docs/system-specs/features/claude-code-provider.md, and in AGENTS.md (amended in this PR so the repo rule matches shipped behaviour). Third round on this same span with the same prescription. Prerequisite for closing even the pre-approved case — whether claude-agent-acp forwards PreToolUse or settingSources over ACP — is named in the spec as follow-up work.

@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Aug 31, 2026
@github-actions

Copy link
Copy Markdown
Contributor

AI-review override not recorded: keep the reason to 500 characters or fewer.

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Aug 31, 2026
@iamwhatever
iamwhatever force-pushed the feat/claude-code-selectable branch from 4880fc3 to dc151b8 Compare September 1, 2026 00:16
@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Sep 1, 2026
The harness was runnable and unreachable: acp/client.py owns its whole spawn
path and the adapter is a public npm package, so the only thing missing was
the switch.
@iamwhatever
iamwhatever force-pushed the feat/claude-code-selectable branch from dc151b8 to 73ae069 Compare September 1, 2026 00:43
@iamwhatever

Copy link
Copy Markdown
Collaborator Author
  • span=fb966fb08c6a — a policy-denied backend is reported as "Claude Code selectable" — fixed in 73ae0699ceecac9b3d7d188edf419449ba06502b.

Legitimate, and it is my wording that was wrong. That branch reads the INSTALL
probe (agent_sdk.probe_backend) and never consults apply_selectable_denials,
so it cannot know whether the deployment may select the backend. On a deployment
whose policy denies agent_backend, doctor printed the exact opposite of the
truth — and it did so on the one surface an operator uses to explain why the
option is missing from the panel, which is where a false claim costs the most.

Now both INSTALLED branches say "Claude Code installed", which is what the probe
actually establishes. This is the same separation the panel already keeps: the
install probe and the governance verdict are two different answers, and the whole
point of #7166 was to stop collapsing them into one.

Pinned by two new cases in test/test_doctor_claude_backend.py asserting
"selectable" not in out for both the on-PATH and off-PATH install. Two older
tests in that file asserted the previous wording incidentally; their own intent
(an off-PATH install is not printed as None, and the harness is never called
dormant) is unchanged, so I retargeted the string and left the tests' purpose
alone.

@iamwhatever

Copy link
Copy Markdown
Collaborator Author
  • span=51d8b5c04192 — the "harness the core does NOT ship" extension claim cannot hold — fixed in 73ae0699ceecac9b3d7d188edf419449ba06502b.

Legitimate, and it is a comment my own change falsified — the same class of defect
this PR exists to correct, so shipping it would have been the joke writing itself.

The two halves compose exactly as the finding says: register_selectable_backend
raises ValueError on any id outside ACP_BACKENDS_KNOWN, and this PR made
BASELINE_SELECTABLE_BACKENDS equal to ACP_BACKENDS_KNOWN. So there is no id the
function accepts that is not already selectable, and no id it accepts for a harness
the core does not ship. Before the widening the sentence was true; afterwards it
describes a path with no reachable input.

Rewritten to say why the hook still exists (the ProviderRegistry protocol
declares it and an edition overrides it) and what it actually buys (reach into the
config gate, the dashboard PATCH allowlist and GET /api/config/schema once the id
is known), plus the part that was missing: adding a genuinely new harness means
widening ACP_BACKENDS_KNOWN, which is a core edit, not something this seam opens
on its own.

I swept for the same claim rather than fixing only the flagged line, and
docs/system-specs/features/claude-code-provider.md carried it too ("with no core
edit") — corrected in the same commit. platform/interfaces.py states the
protocol obligation without asserting an unknown id works, so it needed no change.

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running readiness: passed Eligible automated validation passed for the current revision and removed readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention labels Sep 1, 2026
@bolichen97
bolichen97 merged commit 4e5c25e into main Sep 1, 2026
111 of 119 checks passed
@bolichen97
bolichen97 deleted the feat/claude-code-selectable branch September 1, 2026 05:40
@github-actions github-actions Bot removed the readiness: passed Eligible automated validation passed for the current revision label Sep 1, 2026
billygerhard added a commit to billygerhard/KiroCrew that referenced this pull request Sep 1, 2026
…end architecture

Upstream shipped a parallel backend architecture while this branch was built (kirodotdev#7166 install-state probing + policy-gated selection, kirodotdev#7301 Claude Code selectable, plus the subagent_manager/ and session_* decompositions). This merge adopts it as the foundation: acp_backends.py is the backend-id vocabulary leaf; Claude Code is serviceable (our wave-1 refusal posture dropped as factually superseded); the install probe and governance selectability layers gate the bundled rows. Our registry generalizes their closed set on top: operator descriptors, GenericAdapter serving, HarnessBinding per-chat/spawn/cron selection, and fail-closed capability gates all preserved, with the subagent/session harness logic re-homed into upstream's decomposed components and AgentBackendTab superseded by HarnessPanel carrying its install-state and policy display.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants