Skip to content

feat: add staged acp adapter admission - #6307

Open
kyleseaman wants to merge 1 commit into
mainfrom
feat/pluggable-acp-backends
Open

feat: add staged acp adapter admission#6307
kyleseaman wants to merge 1 commit into
mainfrom
feat/pluggable-acp-backends

Conversation

@kyleseaman

@kyleseaman kyleseaman commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

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)

  • Kept agent.provider fixed to acp and moved the existing
    agent.acp_backend selection seam onto a registry-derived allowlist. Kiro
    remains the unconditional default; unusable persisted choices degrade to Kiro
    with a diagnostic reason.
  • Added positive harness identities and explicit capability membership for
    session sharing, steering, protocol dialects, internal sandboxing, models,
    usage, reasoning effort, MCP delivery, and session metadata.
  • Kept the admitted set at Kiro CLI and KAS. Claude, Codex, goose, OpenCode, pi,
    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.
  • Kept registry adapters operator-installed and fail-closed. Npm adapters must
    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 uvx execution and binary
    downloads remain unselectable.
  • Made the Settings install probe share successful and failed npm resolution
    results for one request, avoiding a timeout per adapter while retrying on the
    next request after an install or toolchain repair.
  • Added fail-closed routing for tool governance. Unknown or unroutable adapter
    tool calls are refused unless the operator enables the single named
    agent.acp_backend_allow_ungated_tools opt-out; the UI discloses that this
    weakens Kiro Crew enforcement. The option is reserved for a future admitted
    adapter and has no effect on the current Kiro/KAS set.
  • Hardened adapter process ownership by protecting runtime PID records, matching
    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.
  • Added backend-aware models, diagnostics, usage, session behavior, chat labels,
    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.
  • Updated the repository governance text to permit the ACP client adapter seam
    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

  • Full backend pytest suite: 73,512 passed, 381 skipped, 6 expected failures,
    and 1,044 subtests passed.
  • Static gates: scoped Black, subprocess encoding, isort, flake8, Linux mypy
    (1,165 source files), harness parity, brand, docs lint, and diff hygiene.
  • Focused ACP, session, subagent, dashboard, and rebase-sensitive integration
    suites: 4,538 passed and 4 skipped.
  • Full frontend Vitest suite: 1,636 files passed; 25,780 tests passed, 1 expected
    failure, and 3 skipped.
  • Electron suite: 1,404 passed, 1 skipped.
  • Production frontend build: passed; 516 assets precompressed.
  • Regression coverage includes exact install/version verification, uvx
    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

  • Exercised the experimental adapter picker at desktop and mobile widths.
  • Verified the capability disclosure view and selected-backend state.
  • Verified the default Kiro path remains selected when the preview is disabled.

Screenshots / video

ACP adapter settings on desktop

ACP adapter capability disclosure

Mobile layout

ACP adapter settings on mobile

Related Issues

Related to #1693, but does not add API-key or multi-provider selection.

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

@kyleseaman
kyleseaman requested a review from a team August 27, 2026 13:29
@kyleseaman
kyleseaman requested a review from a team as a code owner August 27, 2026 13:29
@kyleseaman
kyleseaman requested a review from dwu96 August 27, 2026 13:29
@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 27, 2026
@github-actions

github-actions Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

UX Review (Fable 5) — 🟡 CONCERNS

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

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

  • Preview promise vs. shipped set. The toggle/tab description ("Claude Code as an alternative to Kiro CLI…") names a backend ACP_BACKENDS_SELECTABLE limits to Kiro CLI + KAS; every user who enables the preview for Claude Code lands on a card whose only trace of it is the muted "Not enabled in this build" footer link. High frequency (the preview's whole audience) × dead-end expectation × every visit. Fix: describe what the toggle actually reveals ("Choose the harness for new sessions — Kiro Agent Service today; Claude Code and Codex are listed but not yet enabled"). The three committed screenshots compound it: they show Claude Code/Codex as selectable radio rows with "Adapter not installed" badges and an older header string — a state unreachable in this build, and the shipped footnote appears in none. Re-capture against the shipped set.
  • Silent failure on "Switch adapter." commit() has try/finally with no catch and no error state; if patchConfig rejects, the modal closes exactly as it does on success, the radio snaps back, and nothing says why. Rare × misleading (closure signals success) — surface the failure in the modal or an inline ErrorNotice.

Suggestions

  • Wire the top-bar harness readout (App.tsx harness segment, cursor-default) to ACP_BACKEND_ROUTE when the preview is on — AcpBackendCard's own comment claims this deep-link exists, and the surfaced fact currently offers no path to change it.

[UX-REVIEWED] 7e3e273

Comment thread src/kiro_crew/acp/doctor.py Fixed
@github-actions

github-actions Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Design Review (Fable 5) — 🟡 CONCERNS

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

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

  • The path every Kiro session runs today (acp/client.py +1311, _dispatch.py +479, session handle, hooks, PID authority) is rewritten to serve backends no operator can select — ACP_BACKENDS_SELECTABLE stays {Kiro, KAS} — so the regression risk lands now while the benefit waits on admission, and the withheld adapters (built on seams the code itself notes are @alpha, "a release could move this") will drift before production ever exercises them.
  • AGENTS.md's "do NOT re-add the public registration glue" becomes "a shipped goal" in the same diff that ships that glue; the direction change is disclosed, but the rule a reviewer would enforce is edited by the change under review — it needs explicit maintainer sign-off independent of this PR.

Suggestions

  • Split goose/OpenCode/pi (withheld even from the named release-blocker track) into their own admission PRs; they add drift surface without advancing this stage.
  • Move temp-screenshots/*.png (~760 KB) out of the tree — the PR body already pins them by SHA, and a merged path named "temp" never gets cleaned.

[DESIGN-REVIEWED] 7e3e273

@github-actions

github-actions Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

GPT 5.6 Review — ✅ no blocking findings

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

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 7e3e273

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

@kyleseaman
kyleseaman force-pushed the feat/pluggable-acp-backends branch from c9a68df to 102d95e Compare August 27, 2026 13:42
@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 27, 2026
@github-actions

github-actions Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of 7e3e273956b53391b85568486b06420315db0af0 — 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. The change inventory is verified: ACP_BACKENDS_SELECTABLE stays {kiro, kas} (both Routing.AGENT_SPEC, structurally ROUTED), so the opt-out key and the five adapter integrations are unreachable at HEAD by the persisted-config path — matching the author's own framing. The screenshots directory turned out to be an established repo convention (250 capture scripts write there), so that is not a finding. Verdict and findings follow.

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 ships

Intent: let operators discover, evaluate, and eventually select third-party ACP adapters without weakening the Kiro path — an ADDITION, honestly framed as staged.

  1. Experimental "ACP Adapters" settings tab with registry discovery and capability disclosure — justified, but ships before anything is admittable
  2. Selectable set unchanged ({kiro, kas}); adapter picks degrade to Kiro — declared
  3. Claude/Codex/goose/OpenCode/pi integrations implemented but withheld — zero reachable consumers
  4. New config key agent.acp_backend_allow_ungated_tools, default off — author-stated "no effect on the current Kiro/KAS set"
  5. Opt-in fetch of the upstream ACP Registry, 6h cache, no implicit execution — justified
  6. Fail-closed adapter launch (exact npm/Volta resolution; uvx/binary refused) — derived from the untrusted-code boundary
  7. .claude/.credentials.json / .codex/auth.json join the sensitive-path deny list — justified
  8. Backend-scoped model namespaces; switching clears all model picks — justified
  9. Claude cost/rate-limit readouts plumbed to the dashboard popover — emitted only by the withheld backend; unreachable
  10. PID-record hardening (start-time identity, exact name match, libproc) — justified, also fixes existing Kiro/KAS records

Watch

  • The opt-out key is permanent public surface with no reachable effect: config validation restricts acp_backend to {kiro, kas}, both Routing.AGENT_SPEC → structurally ROUTED, so enforce() returns before allow_ungated is read. The description concedes it: "reserved for a future admitted adapter and has no effect on the current Kiro/KAS set." Once shipped (config-baseline, 12-locale "Allow Ungated Tools" label, docs), it must be honored forever.
  • goose/OpenCode/pi have no named release blocker — only "until basic end-to-end evidence exists" — unlike Claude (settings-cleanup blocker) and Codex (ungated passive reads). Their hand-written ROUTED verdicts assert admission evidence the description says is still pending.

Subtractions

  • Defer agent.acp_backend_allow_ungated_tools (key in config/loader.py, config-baseline entry, UI label, doc text) to the PR that admits the first backend that can resolve non-ROUTED; ToolGateUnroutable can refuse unconditionally until then.
  • Defer acp/goose.py, acp/opencode.py, acp/pi.py (~585 lines plus descriptors and 5 test files) to their admission PRs — descriptor_for_registry_adapter in acp/backends.py already yields the correct fail-closed UNVERIFIED descriptor for any registry adapter without a hand-written module, so discovery and disclosure lose nothing.

[FIRST-PRINCIPLES-REVIEWED] 7e3e273

@kyleseaman
kyleseaman force-pushed the feat/pluggable-acp-backends branch from 102d95e to d04fea2 Compare August 27, 2026 14:02
@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 27, 2026
@kyleseaman
kyleseaman force-pushed the feat/pluggable-acp-backends branch from d04fea2 to 6c4c458 Compare August 27, 2026 14:21
@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 27, 2026
@kyleseaman
kyleseaman force-pushed the feat/pluggable-acp-backends branch from 6c4c458 to 8a0085e Compare August 27, 2026 14:29
Comment thread src/kiro_crew/acp/doctor.py Fixed
@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 27, 2026
@kyleseaman
kyleseaman force-pushed the feat/pluggable-acp-backends branch from 8a0085e to 9367de8 Compare August 27, 2026 14:38
@github-actions github-actions Bot removed the readiness: action required A blocking check or review needs attention label Aug 27, 2026
@bolichen97 bolichen97 mentioned this pull request Aug 29, 2026
5 tasks
@bolichen97

Copy link
Copy Markdown
Collaborator

ACP coordination note from the actual full diffs of #5349, this PR, and #6390:

  • feat: add Codex ACP backend #5349's Codex-specific ACP client/provider path is functionally replaced by this PR's broader BackendDescriptor registry, adapters, staged admission, capability/tool gate, and Codex model/effort/auth support. Please do not land feat: add Codex ACP backend #5349 as a parallel Codex special case.
  • feat(acp): select an ACP interface per crew #6390 is not a duplicate: it adds a per-crew arbitrary absolute command/args/env interface, but it competes with this PR for backend discovery, provider selection, factory routing, session inheritance, and model/effort handling.

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.

@bolichen97

Copy link
Copy Markdown
Collaborator

Audit note — three open PRs are building the same ACP-harness seam

This is a consolidation request, not a duplicate finding: #5349, #6307 and #6777
all add a non-kiro-cli ACP harness at the same seams, and they disagree with each other in
ways that mean only one design can land. Each of the three also carries substantial work
the other two do not, so none of them should simply be closed.

What the audit verified by reading all three merge-base diffs:

  • feat: add Codex ACP backend #5349feat: add staged acp adapter admission #6307 — Verified on both cached heads that these implement one capability -- drive the operator-installed codex-acp adapter as a harness chosen at agent.acp_backend, with a dashboard picker -- at the same seams with mutually exclusive contracts. Both add ACP_BACKEND_CODEX and PROVIDER_LABEL_CODEX to acp/types.py, then disagree on ACP_BACKENDS_SELECTABLE (each pinned by its own test); both rewrite DefaultProviderRegistry.create_factory with incompatible bodies (feat: add Codex ACP backend #5349 an if cfg.agent.acp_backend == ACP_BACKEND_CODEX branch, feat: add staged acp adapter admission #6307 _create_adapter_dispatch_factory on Dialect.KIRO/SPEC); both add SessionManager.acp_backend with contradictory semantics (restart-only snapshot that writes cfg.agent.acp_backend back, vs a live read plus live_harness); both add the same unlanded kiro-readiness bypass so a foreign-adapter operator is not told to sign into kiro-cli; both add codex adapter/sign-in rows to doctor; and both ship a harness selector carrying the same 'new chats only' disclosure (aiBackendPanel.* vs acpBackend.*). Since the earlier rounds ran, main landed the plumbing itself: feat(config): one registry for selectable ACP backends instead of three literals #6593 added acp_backends.py (BASELINE_SELECTABLE_BACKENDS, register_selectable_backend, one selectability gate) and feat(acp): add a dormant Codex ACP backend seam and name the harness-onboarding sequence #7813 added the dormant codex seam (_is_codex, PROTOCOL_VERSION_CODEX, _resolve_codex_acp_bin, a _spawn branch, _codex_session_mcp_servers, plus ACP_BACKENDS_EFFORT_VIA_CONFIG_OPTION / _MODEL_VIA_CONFIG_OPTION / _KIRO_SLASH_COMMANDS replacing the is_claude_backend negations), with codex known but withheld from the baseline. That landed shape is feat: add staged acp adapter admission #6307's -- in-place dispatch, membership sets, one gate, a create_factory documented to register nothing -- and it contradicts each of feat: add Codex ACP backend #5349's three signature choices: the create_factory special case (its standing, non-overridable BLOCK-MERGE), the CodexAcpClient spawn-path fork, and the restart-only snapshot (main computes _bg_runtime_backends() per call). Neither PR closes either documented admission blocker: feat: add Codex ACP backend #5349 has no tool gate or read routing at all, and neither touches agent_sdk/backend_install.py, the exact gap landed main names as the reason codex is not selectable. So only one design can land and feat: add staged acp adapter admission #6307 is the base -- but it is stale against main's landed seam and still carries an unaddressed CHANGES_REQUESTED whose item 3 asks for Codex in the first admitted set, which is what feat: add Codex ACP backend #5349's remainder feeds.

  • feat: add staged acp adapter admission #6307feat(acp): add OpenCode harness #6777 — Both PRs are still OPEN (the issue/PR reference check), so neither can have superseded the other. Reading the two merge-base diffs directly, I confirmed nine collisions inside the SAME functions, not merely the same files: build_permission_event's two rawInput fallback chains are byte-identical additions on both sides; api_sessions_usage gains the identical return web.json_response({"usage": {"available": False}}) at the same position ahead of reject_if_kiro_unverified; api_kiro_prerequisite_status gains the same asdict(PrerequisiteStatus(...)) + legacy_idle_operation() pre-probe bypass; reject_if_kiro_unverified is scoped off the same pre-existing ACP_BACKENDS_KIRO_IDENTITY_STORE set by two mutually exclusive shapes (inline live-slot resolution vs. a new selected_backend_uses_kiro_identity() helper plus a keyword on the function); AcpClient._spawn gains an OpenCode arm and protocol 1 twice by incompatible schemes (PROTOCOL_VERSION_CLAUDE renamed to PROTOCOL_VERSION_SPEC and mapped per-Dialect vs. a new PROTOCOL_VERSION_OPENCODE and a third elif); LLMProvider gains one new property under two names (backend vs acp_backend) with the same None default and the same rationale, and AcpProvider.is_opencode_backend is added twice with identical bodies; and the managed-MCP projection onto an adapted harness is implemented twice, in acp/spec_servers.py and mcp_gateway/session_servers.py, both reduced to (name, command, args, env) with the same user-server-exclusion rationale. test/test_harness_parity.py carries directly opposed edits. Only one design can own each of those seams, so a consolidation decision is owed. the first adjudication's nomination of feat: add staged acp adapter admission #6307 as survivor is nevertheless wrong: feat: add staged acp adapter admission #6307 pins ACP_BACKENDS_SELECTABLE == frozenset({KIRO, KAS}) with an equality assertion and its own docs row reads | OpenCode | opencode | opencode auth login | Described, not selectable |, and selectable_ids() returns that frozen set, so "opencode" can never be persisted and every non-Kiro branch it adds (models, usage, prerequisite, readiness) is unreachable code at its HEAD. Current state cuts under both: acp_backends.py landed between the two merge bases (absent at e48ea42, present at 588ae92), main now ships BASELINE_SELECTABLE_BACKENDS = {kiro, claude, kas} with claude deliberately selectable, and both branches rewrite that same line in opposite directions - feat: add staged acp adapter admission #6307 to {kiro, kas}, feat(acp): add OpenCode harness #6777 to {kiro, kas, opencode} - which is the one decision the consolidation has to make.

Why this needs a decision rather than a merge order

The three collide inside the same functions, not merely in the same files, and each pins its
own answer with its own test — so whichever lands second does not conflict textually so much as
contradict a test the first one added. In particular the ACP_BACKENDS_SELECTABLE / selectable-set
question is answered three different ways.

Two things must not be lost whichever design wins

From #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 note

Nothing 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 permission: "ask" into the session work_dir (opencode.json or .opencode/opencode.json, never ~/.config/opencode) and reads it back, because OpenCode's own default is permissive so an unseeded work_dir emits no session/request_permission and privileged tool calls never reach Crew's PreToolUse gate - I grepped #6777's whole diff and it has no counterpart (0 hits for opencode.json), so OpenCode must not become selectable without this. From #6777: AcpClient.opencode_mcp_identity / remember_opencode_mcp_servers, which reverse OpenCode's sanitize(server)+''+sanitize(tool) wire key with fail-closed prefix-collision handling (#6307's parser handles only the mcp___ shape); website/src/components/KiroPrerequisiteGate.tsx's switchBackendMutation escape hatch plus its five en.json keys and 52 test lines, the only first-run path a kiro-cli-less operator can reach; the AgentBackendTab.tsx fourth row; _cold_opencode_models (opencode models, 1 MB cap, ^[^\s/]+/[^\s]+$, dedup) so the picker fills before any session exists; agent.py's MANAGED_MCP_SERVER_NAMES + materialized_agent_spec capped reader; the opencode addition to scripts/check_harness_parity.py's HARNESS_LITERAL; and the live evidence in the PR body (opencode 1.18.23, real ses* session, stream_events -> end_turn) that is precisely what maintainer iamwhatever's CHANGES_REQUESTED point 3 asked for before any adapter is admitted.

Suggested next step: a maintainer picks the seam design (the registry-dispatch shape vs. the
per-backend branch shape), then the other two rebase onto it as harness additions rather than
as competing seams — which is also what the harness-parity rule in AGENTS.md asks for
("an added harness ADAPTS, it does not widen").


From a repository-wide duplicate/overlap audit of every pull request open against main (2026-09-02, 330 PRs, one reviewer per PR). Each PR was read as its full merge-base diff plus its description and every comment and review, then compared against each candidate PR's own diff and against origin/main at 1a765b88ceb7. This PR is not being closed — the note is informational. If the reading is wrong, please correct the reasoning rather than just the conclusion.

@bolichen97

Copy link
Copy Markdown
Collaborator

Open PR relationship audit

This 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

  • PR #5349 is OVERLAPPING relative to this PR. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #5349: MERGE_DISCUSSION. The maintainer overlap audits on 2026-08-29 and 2026-09-02 name 6307 as the broader competing implementation of this capability and ask for a seam-design decision before either proceeds; both PRs carry work the other lacks, so neither should simply be closed. Files: src/kiro_crew/platform/defaults.py, src/kiro_crew/acp/types.py, src/kiro_crew/dashboard/kiro_readiness.py.
  • PR #6777 is OVERLAPPING relative to this PR. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #6777: MERGE_DISCUSSION. Both are open, so neither supersedes the other, and they collide inside the same functions with contradictory contracts (selectability, permission-event parsing, readiness scoping, the ABC property name, the managed-MCP projection module). PR #6307 is broader (registry-driven staged admission across five adapters plus a routing verdict) and deliberately withholds OpenCode selectability; PR #6777 ships OpenCode selectable with material work PR #6307 lacks (the sanitized _ identity reversal with collision fail-closed, the first-run escape hatch, the cold opencode models picker). A maintainer has to pick the seam design before either lands; PR #6777's OpenCode-specific pieces should then be re-landed on top of it, and its permission-routi… Files: src/kiro_crew/acp_backends.py, src/kiro_crew/acp/_dispatch.py, src/kiro_crew/dashboard/kiro_readiness.py, src/kiro_crew/dashboard/handlers/kiro_prerequisite.py (+3 more). The two independent directions used different labels; the matrix conservatively retains OVERLAPPING for coordination.
  • PR #7617 is OVERLAPPING relative to this PR. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #7617: MERGE_DISCUSSION. Independent keys in a shared handler; at most a mechanical rebase, no behavioural interaction. Files: src/kiro_crew/dashboard/handlers/core.py.
  • PR #7628 is OVERLAPPING relative to this PR. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #7628: KEEP. Different user goals on the same rendering gate; neither subsumes the other and both are wanted. Note the merge order for the App.tsx hunk. Files: website/src/App.tsx, src/kiro_crew/dashboard/handlers/sessions.py.
  • PR #7963 is OVERLAPPING relative to this PR. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #7963: MERGE_DISCUSSION. Neither PR subsumes the other, but the credential-floor change and the Codex routing model are duplicated implementations of the same security decision, and the two heads conflict irreconcilably (add/add on both new test files). A maintainer has to pick which module owns the routing verdict — 6307's acp/tool_gate.py inside the acp package with a local opt-out, or 7963's leaf acp_tool_gate.py with no opt-out plus the OS-level mask — before either can be reviewed on its own merits. 7963 additionally carries the install probe and the sandbox mask that 6307 lacks, so the likely outcome is that whichever lands second drops its duplicated half rather than being closed. Files: src/kiro_crew/security.py, src/kiro_crew/acp_tool_gate.py, test/test_acp_tool_gate.py.
  • PR #8215 is OVERLAPPING relative to this PR. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #8215: KEEP. Highest file overlap of any open PR (8 shared files) and the same ABC-extension seam, but entirely different capabilities; the only interaction is ordinary co-editing of one class. Files: src/kiro_crew/providers/base.py.
  • PR #8255 is OVERLAPPING relative to this PR. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #8255: KEEP. Two competing descriptor models for the same subsystem, one 24-file and one 247-file. Whichever lands second inherits a redesign, so the descriptor-authority question (code-only admission vs operator config data) needs deciding before either merges. Files: src/kiro_crew/acp/harness_descriptor.py + harness_registry.py vs src/kiro_crew/acp/backends.py + acp/registry.py.

No PR, Issue, label, branch, or review state was changed by the relationship-note portion of this audit.

@bolichen97

Copy link
Copy Markdown
Collaborator

@kyleseaman we are not closing this. Main's merged RFC docs/request-for-change/rfc-crew-agent-sdk-boundary.md names this PR and says it should land on its own merits. But the seam moved underneath it: main now ships the selectable-backend registry as src/kiro_crew/acp_backends.py (plugin-extensible via register_selectable_backend, with claude and codex already selectable), the fail-closed routing gate as src/kiro_crew/acp_tool_gate.py, the install probe as src/kiro_crew/agent_sdk/backend_install.py, the picker as website/src/pages/developer/AgentBackendTab.tsx, adapter cost parsing as acp/_dispatch.py::parse_usage_cost, and PID-recycle pinning as platform_compat.kill_process_tree_pinned.

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 (AcpRateLimit), and bills_kiro_credits, which docs/system-specs/modules/agent-host-contract.md already specifies.

Please rebase onto those modules and narrow the PR to that remainder. Two blockers first: agent.acp_backend_allow_ungated_tools is pinned as an absence by test/test_acp_tool_gate.py::test_no_local_opt_out_exists, so a per-install opt-out cannot come back; and ACP_BACKENDS_SELECTABLE is still {kiro, kas} at 7e3e273, so every non-Kiro branch here is unreachable. The PR is 247 files, 1562 commits behind main, mergeable_state dirty.

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 acp/opencode.py writes permission: "ask" into the session work_dir opencode.json, which contradicts #9013's premise that no enforceable routing exists. @rnoack1, it widens the identity signal #7163 documents as kiro-cli-only. @welikoiwanenko, #8003 is complementary (per-run amount versus per-harness flag). @rubencu, #7617 conflicts only textually in dashboard/handlers/core.py.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge conflict Branch has merge conflicts with its base — author must resolve before merge readiness: passed Eligible automated validation passed for the current revision

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants