feat: add staged acp adapter admission - #6307
Conversation
UX Review (Fable 5) — 🟡 CONCERNSUX-level review of UX-Verdict: CONCERNS The preview toggle sells "Claude Code as an alternative," but the build withholds Claude Code — the headline backend is only reachable as a footnote. Watch
Suggestions
[UX-REVIEWED] 7e3e273 |
Design Review (Fable 5) — 🟡 CONCERNSDesign-level review of Design-Verdict: CONCERNS Fail-closed staged admission is the right shape, but five unselectable adapters and a self-authorizing governance rewrite ride one 26k-line review unit. Watch
Suggestions
[DESIGN-REVIEWED] 7e3e273 |
GPT 5.6 Review — ✅ no blocking findingsGPT 5.6 completed its review of This comment is updated in place on each push. Review detailsNo findings. False positive or not applicable? A repository writer can comment: |
c9a68df to
102d95e
Compare
First Principles Review (Fable 5) — 🟡 CONCERNSPremise-level review of All evidence gathered. The change inventory is verified: First-Principles-Verdict: CONCERNS Everything downstream of admission ships before any admission: five adapter integrations, a forever config key, and dashboard readouts no user action can reach at HEAD. What this change shipsIntent: let operators discover, evaluate, and eventually select third-party ACP adapters without weakening the Kiro path — an ADDITION, honestly framed as staged.
Watch
Subtractions
[FIRST-PRINCIPLES-REVIEWED] 7e3e273 |
102d95e to
d04fea2
Compare
d04fea2 to
6c4c458
Compare
6c4c458 to
8a0085e
Compare
8a0085e to
9367de8
Compare
|
ACP coordination note from the actual full diffs of #5349, this PR, and #6390:
Please use this PR's registry/admission boundary as the common architecture, then define how #6390's per-crew external selection maps to descriptors and one validation/spawn/model/effort/session-inheritance path. Mechanical stacking would leave two competing ACP configuration models. |
Audit note — three open PRs are building the same ACP-harness seamThis is a consolidation request, not a duplicate finding: #5349, #6307 and #6777 What the audit verified by reading all three merge-base diffs:
Why this needs a decision rather than a merge orderThe three collide inside the same functions, not merely in the same files, and each pins its Two things must not be lost whichever design winsFrom #5349, carry before consolidating: (1) website/src/pages/settings/AiBackendPanel.tsx plus website/src/test/AiBackendPanel.test.tsx -- the Settings -> System -> AI Backend picker with per-backend descriptions and the restart/new-chat disclosure (aiBackendPanel.restart_new_chats_description), together with its command-palette wiring in website/src/components/commandPalette/settingsRegistry.gen.ts and settingsTabLabel.ts; this is closer to the maintainer's 'the provider switch has its own page, keep consistent with the internal version' than #6307's preview-gated Developer -> ACP Adapters tab (AcpBackendCard.tsx). (2) src/kiro_crew/dashboard/kiro_readiness.py::running_backend_requires_kiro_prerequisite -- keep its fail-closed rule (an absent or unknown backend still demands kiro readiness) as the behaviour #6307's live resolver must preserve. (3) src/kiro_crew/cli_doctor.py's actionable codex rows ('Codex ACP adapter not found', Fix: npm i -g @agentclientprotocol/codex-acp), folded into acp/doctor.py::_report_adapter, and reused as the basis for the missing agent_sdk/backend_install.py codex probe that landed main names as codex's selectability blocker. (4) From test/test_acp_backend_codex.py, the design-independent cases only: codex advertised-model gating, session/new MCP descriptor wiring, session resume, and the auth-required / readiness boundary. Do NOT carry CodexAcpClient, CodexAcpProvider / create_codex_provider_factory, the create_factory codex branch, the ACP_BACKENDS_SELECTABLE edit, CODEX_EFFORT_CONFIG_ID = 'reasoning_effort', or the restart-only SessionManager.acp_backend snapshot -- each is superseded by, or contradicted by, landed main. One security-relevant noteNothing needs harvesting for a closure - neither PR is closed. For the consolidation, two items must not be lost whichever design wins. From #6307: src/kiro_crew/acp/opencode.py's ensure_routed_settings() with PERMISSION_ASK / PERMISSION_BYPASS_VALUES, which writes Suggested next step: a maintainer picks the seam design (the registry-dispatch shape vs. the From a repository-wide duplicate/overlap audit of every pull request open against |
Open PR relationship auditThis is a consolidated, point-in-time code-level audit note. It compares complete merge-base diffs and current/merged code; it does not treat a shared topic as duplication or partial coverage as completion. Relationship findings
No PR, Issue, label, branch, or review state was changed by the relationship-note portion of this audit. |
|
@kyleseaman we are not closing this. Main's merged RFC What is still genuinely missing on main, and worth keeping: upstream ACP Registry discovery with exact npm/Volta version resolution, the goose/OpenCode/pi resolvers, the capability-disclosure layer, the plan-quota readout ( Please rebase onto those modules and narrow the PR to that remainder. Two blockers first: Also needing a decision: #8255 models harness descriptors as validated config data where this PR freezes them in code, and #6777 and #5349 move the selectable set in other directions. Counterparts: @atomsbaza, this PR's Posted from the 2026-09-08 open-PR relationship audit (read-only, one auditor per PR); reply here if any of this is wrong. |
Problem / Motivation
Kiro Crew's ACP client had only fixed Kiro/KAS identities. It had no
registry-driven way to discover operator-installed adapters, describe their
capability gaps, or enforce one fail-closed admission path before a backend could
become selectable. Adding an adapter otherwise required another hard-coded path
or a local patch.
Why it matters
A staged ACP admission foundation lets maintainers evaluate adapters against the
same provider, session, and governance contracts without weakening the
first-class Kiro path. This PR intentionally keeps the admitted set at Kiro CLI
and KAS while the documented Claude and Codex release blockers remain open. No
adapter is downloaded or executed implicitly.
Scope decision
The ACP client seam and staged-admission approach are intentional. This PR does
not claim that Claude, Codex, goose, OpenCode, or pi are released backends. It
lands the shared discovery, disclosure, safety, and lifecycle contracts needed
to validate them; each backend must separately satisfy its documented admission
conditions before joining the selectable set.
What changed (motivation → approach → change)
agent.providerfixed toacpand moved the existingagent.acp_backendselection seam onto a registry-derived allowlist. Kiroremains the unconditional default; unusable persisted choices degrade to Kiro
with a diagnostic reason.
session sharing, steering, protocol dialects, internal sandboxing, models,
usage, reasoning effort, MCP delivery, and session metadata.
and upstream ACP Registry entries are discoverable and described but withheld
until their release blockers are closed. Claude remains withheld until session
cleanup preserves pre-existing project settings; Codex remains withheld
because passive reads are not permission-routed.
resolve an exact globally installed package/version and verified Node entry;
Volta package/bin metadata must agree before the package-pinned Node runtime
launches the verified entry directly. Cache-only
uvxexecution and binarydownloads remain unselectable.
results for one request, avoiding a timeout per adapter while retrying on the
next request after an install or toolchain repair.
tool calls are refused unless the operator enables the single named
agent.acp_backend_allow_ungated_toolsopt-out; the UI discloses that thisweakens Kiro Crew enforcement. The option is reserved for a future admitted
adapter and has no effect on the current Kiro/KAS set.
short process names exactly, and binding records to high-resolution process
start identities. Legacy or recycled records fail closed instead of gaining
kill authority; the shared macOS identity probe now uses libproc precision.
and a dedicated experimental ACP Adapters settings tab. Backend-owned model
namespaces never inherit Kiro agent-file pins, and switching adapters atomically
clears global, role, crew, and live-slot model selections. Third-party adapters
on Windows require the existing unsandboxed-exec opt-in.
while retaining the ban on second providers and API-key paths.
This deliberately does not add API-key providers or a second provider selector;
that separate question remains outside this ACP client seam.
Tests
and 1,044 subtests passed.
(1,165 source files), harness parity, brand, docs lint, and diff hygiene.
suites: 4,538 passed and 4 skipped.
failure, and 3 skipped.
rejection, POSIX and Windows Volta layouts, retry-after-install behavior, and
one failed npm probe shared across all adapter rows in a request.
Manual verification
Screenshots / video
Mobile layout
Related Issues
Related to #1693, but does not add API-key or multi-provider selection.
Checklist
feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)Contribution License Agreement