Skip to content

feat(developer): bring back the agent-backend switch, with KAS - #6398

Merged
bolichen97 merged 1 commit into
mainfrom
feat/agent-backend-switch
Aug 28, 2026
Merged

feat(developer): bring back the agent-backend switch, with KAS#6398
bolichen97 merged 1 commit into
mainfrom
feat/agent-backend-switch

Conversation

@iamwhatever

@iamwhatever iamwhatever commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

What

Adds Developer → Agent Backend: a three-way switch over the agents that can run a session, writing agent.acp_backend.

Why

The core has been able to run three agents for a long time — Kiro CLI, KAS and Claude Code — and shipped no way to pick between them. agent.acp_backend was already validated end to end (_normalize_acp_backend against ACP_BACKENDS_SELECTABLE) and already published in the config schema, but the UI that used to switch providers was removed when the website collapsed to KiroACP-only. The field was switchable in principle and unreachable in practice.

This is not a revival of the old ProviderPanel: Bedrock model ids, the Claude Code migration wizard, and a one-member provider enum stay gone.

What each option says

Each agent gets one derived status line, not a prose capability sentence.

An earlier revision of this branch wrote per-agent claims — OS sandboxing, one process shared across sessions, mid-turn steering, subagent progress — directly in the view layer, where nothing could contradict them. They were wrong in the way unmeasured claims usually are, so they are gone.

The vocabulary is now borrowed from the ACP-adapter card in #6307 rather than invented a second time:

row line why that one
Kiro CLI Default. All features supported. its descriptor is all-supported
KAS (kiro-agent) Experimental selectable, and not all-supported
Claude Code Not enabled in this build this build cannot run it

Which line a row gets is derived, not authored: not-enabled is read off the same schema that gates the buttons, so a build widening the enum flips Claude Code's row from not-enabled to Experimental with no edit here. There is a test for that.

Per-capability detail — which individual feature is supported, degraded or unverified per backend — needs the descriptor table that owns those facts (#6307's acp/backends.py). It is deliberately not restated in this view, because a copy here could disagree with the gateway and nothing would go red.

All three lines are always shown: the reader is choosing between the agents, so showing only the selected one's status would hide the comparison the control exists for.

The choices come from the server

The selectable set is read from GET /api/config/schema, which the backend derives from the same field metadata the PATCH /api/config/kirocrew validator uses — so the enabled options and the values the wire accepts cannot disagree.

Claude Code is the case that makes this worth doing: this build does not include it. Hiding it would imply it does not exist; enabling it would produce a 400 from a control that looked live. So it is listed, disabled, and says which it is. A build that ships another agent lights it up with no frontend change — there is a test for that.

While the schema request is still in flight every option stays enabled rather than flashing disabled and then live; the PATCH allowlist is the real gate, so an optimistic enable can only ever cost one visible refusal.

Backend (two minimal changes)

  • agent.acp_backend joins _EDITABLE_CONFIG. Without it every save came back field not editable. The values duplicate ACP_BACKENDS_SELECTABLE as a literal because reaching kiro_crew.acp.types at module scope executes the kiro_crew.acp package init (client + runtime) while the dict is being built — the same import cycle config.loader._normalize_acp_backend defers for. test/test_agent_backend_editable.py fails in both directions if the two ever drift, and also pins the schema enum to the same set.
  • The field joins the refresh_defaults() list. A new session then picks the agent up without a gateway restart, and a pre-warmed Kiro CLI process cannot serve a session that asked for KAS. Deliberately not reload_provider_factory(): switching the default agent must not kill in-flight turns, and live sessions keep the agent they started on.

Shared component

SettingsButtonGroup gains per-option disabled, so a known-but-unavailable choice can stay visible instead of the control silently changing shape between builds. Additive and backwards-compatible.

Placement

Under Developer, not Settings, on purpose: gen-settings-registry.mjs scans pages/settings/, and indexing this into Settings search would advertise it as an ordinary preference. It changes which agent binary runs.

Drive-by test hardening

ChatSidebar.tagFilter's remount case waited for one session to appear and then synchronously asserted another was gone — a barrier the test never established. Any branch that changes React's batching (adding i18n keys is the common one) flips it red, which is exactly what this branch did locally. The absence now gets its own waitFor.

i18n

10 keys across all 12 catalogs, en-XA regenerated (the reword retired 5 keys and added 4, so the tab is one key lighter than the previous revision). Claude Code, KAS and kiro-agent are added to the do-not-translate glossary — the changed-passthrough gate asks for exactly that instead of a worked-around translation, and every occurrence of the three is new in this PR, so no existing locale value changes meaning. Four locale style gates were caught and fixed locally: a dropped Kiro product name in German (\bKiro\b needs a boundary, and Kiros has none), the formal आप that style/hi.md §4 forbids, 回合 where style/zh-CN.md §2 pins turn to 轮次, and — new in this revision — three stacked in one zh-CN clause, which style/zh-CN.md §3 caps at two (启动时的代理 became 启动时所用的代理 with the clause split on ).

Verification

Gate Result
tsc -b --force clean
vitest run (full) 25,263 passed, 1 failed¹
eslint src/ --max-warnings 664 0 errors, 663 warnings
npm run i18n:check exit 0 (all 10 checks)
black / flake8 / isort / mypy clean
backend pytest (-n 4) 111 passed
new tests 14 frontend, 4 backend

¹ DiagramLightbox.zoom.test.tsx needs fs/promises.glob (Node 22). Local Node is 20, and it fails identically without this diff — nothing in it touches that sweep.

Evidence

Captured from a throwaway pod. Each step re-reads GET /api/config/kirocrew afterwards, so the evidence shows the write landing, not only the control repainting.

1. Shipped defaultagent.acp_backend == "", Kiro CLI pressed, Claude Code disabled and saying so.

Default: Kiro CLI selected

2. Switched to KAS — a real click, and the server then reports agent.acp_backend == "kas".

KAS selected and persisted

3. Switched back — config returns to "".

Restored to Kiro CLI

4. At a 320px viewport — the selector wraps to two rows instead of overflowing the card.

320px: the selector wraps to two rows

A screenshot alone cannot prove the absence of overflow — a clipped row still looks fine — so the harness measures it: the track's scrollWidth equals its clientWidth (244), the document does not scroll horizontally (320 == 320), no button's right edge passes the track, and the buttons land on 2 rows.

The harness also forces a click on the disabled Claude Code option, past the disabled attribute, so the check is that nothing is written rather than that the DOM refused the pointer:

[1-default]  persisted agent.acp_backend = ""
             Kiro CLI          {"pressed":"true","disabled":false}
             Claude Code       {"pressed":"false","disabled":true}
             KAS (kiro-agent)  {"pressed":"false","disabled":false}
[claude]     forced click on the disabled option: "" -> ""
[2-kas]      persisted agent.acp_backend = "kas"
             Kiro CLI          {"pressed":"false","disabled":false}
             Claude Code       {"pressed":"false","disabled":true}
             KAS (kiro-agent)  {"pressed":"true","disabled":false}
[3-restored] persisted agent.acp_backend = ""
             Kiro CLI          {"pressed":"true","disabled":false}
             Claude Code       {"pressed":"false","disabled":true}
             KAS (kiro-agent)  {"pressed":"false","disabled":false}

Because the point of this revision is the copy, the harness reads the rendered <dl> back rather than leaving it to the eye, and separately asserts the removed prose is absent from the live DOM — not merely absent from the bundle:

status  Kiro CLI          "Default. All features supported."
status  Claude Code       "Not enabled in this build"
status  KAS (kiro-agent)  "Experimental"

[copy] stale prose still rendered: []

The [copy] line searches the page text for OS sandbox, steered mid-turn, Anthropic and Not available in this build; the empty list is the assertion that none of the retired wording survived into the shipped bundle.

Follow-up (not in this PR)

There is currently no per-agent setting to surface: the one genuinely agent-scoped knob, MCP Tool Search, already has a toggle in Developer → Config, and reasoning effort is a model-family setting in Settings → Chat rather than an agent one. If Tool Search moves here later it should move, not be mirrored — it is a no-op on the Claude backend, so presenting it as global is slightly wrong today.

no linked issue: this restores a control the UI lost when the website collapsed to KiroACP-only; there was no filed issue tracking it. The #6307 references above are to that PR as the source of the status vocabulary, not to an issue this PR resolves. The one issue this work did produce — #6502 — is a deferred follow-up that this PR deliberately does NOT fix, so it must stay open on merge.

@iamwhatever
iamwhatever requested a review from a team August 27, 2026 21:11
@iamwhatever
iamwhatever requested a review from a team as a code owner August 27, 2026 21:11
@iamwhatever
iamwhatever requested a review from pepmach August 27, 2026 21:11
@github-actions github-actions Bot added the readiness: checking Automated validation is still running label Aug 27, 2026
@github-actions

github-actions Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

UX Review (Fable 5) — ✅ PASS

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

UX-Verdict: PASS

A developer-surface switch that says what it changes, scopes it ("new sessions only"), explains the dead option, and recovers cleanly from both failed reads and failed writes.

Suggestions

  • Tab/page subtitle "Which agent new sessions start — Kiro CLI, Claude Code, or KAS" (en.manual.json agentBackend.description) drops its preposition and reads broken on a cold read; use "Which agent runs new sessions — …".
  • "Not enabled in this build" renders text-warn (backend-1-default.png) — a permanent, non-actionable fact of the build painted in alarm color on every visit; text-muted matches the other status rows and reserves warn for problems.
  • The card restates "Agent Backend" directly under the identical page title (backend-1-default.png shows it three times in one viewport); the SettingsButtonGroup label could carry the description alone, e.g. "Default agent for new sessions".

[UX-REVIEWED] 2bc6c0a

@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 2bc6c0a71bdf98e726216877dd01e2db7fb3415f and found no blocking issues.

This comment is updated in place on each push.

Review details

FINDING -- website/src/i18n/locales/en.json:6325 -- "already open keeps the one it started with" is false after provider recycle because recreation uses the new global factory -> Fix: say switching does not restart currently running providers. (origin: validation)
[GPT-REVIEWED] 2bc6c0a

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

@github-actions

github-actions Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of 2bc6c0a71bdf98e726216877dd01e2db7fb3415f — 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. Composing the review.

First-Principles-Verdict: CONCERNS

The switch earns its place, but the PATCH allowlist ships a third copy of the selectable-backend set when an importable second copy already sits in core.py's imports.

What this change ships

Intent: let an operator pick which agent new sessions run, from the dashboard — an ADDITION.

  1. New Developer → Agent Backend tab switching the session agent — justified
  2. agent.acp_backend becomes saveable from the dashboard — justified; enum literal is a second spelling
  3. A saved switch reaches the next session without a gateway restart — justified, mechanism-level
  4. Claude Code shown disabled with a reason instead of hidden — declared, argued from the 400-vs-invisible dilemma
  5. One derived status line per agent, no authored capability claims — justified
  6. Shared button group gains per-option disable + screen-reader reason — one consumer each, minimal for the need
  7. Every existing button group (4 consumers) now wraps at narrow widths — rides along, declared only as this tab's evidence
  8. 10 strings in 12 languages, 3 glossary terms — justified by documented i18n invariant
  9. A flaky sidebar test now waits for the absence it asserts — declared drive-by, cause-level fix
  10. Four evidence screenshots under temp-screenshots/ — documented repo convention

Watch

  • "A build that ships another agent lights it up with no frontend change" holds only for the three ids hardcoded in AgentBackendTab.tsx (''/'claude'/'kas'); a fourth backend id gets no row at all, and the test widens only to 'claude'.
  • The flex-wrap max-w-full edit reflows 4 existing SettingsButtonGroup consumers (grep SettingsButtonGroup, non-test: ShortcutsPanel, SttSettings, ChatPanel, DisplayPanel) — behavior change outside the feature, verified only on this tab.

Subtractions

  • Drop the ["", "kas"] literal in _EDITABLE_CONFIG (core.py:1851) — the selectable set now has 3 spellings (acp/types.py:129, config/loader.py:1620, this one). The comment's import-cycle rationale covers acp.types only; config.loader is already imported at core.py:28, so read the field's metadata["enum"] from AgentConfig instead, and the parity test shrinks to one direction (metadata ↔ ACP_BACKENDS_SELECTABLE).

[FIRST-PRINCIPLES-REVIEWED] 2bc6c0a

@github-actions

github-actions Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Opus 4.8 Review — ✅ no blocking findings

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

Review details

No findings.

[OPUS-REVIEWED] 2bc6c0a

Verdict parsed from the review's SHA-scoped output markers for commit 2bc6c0a71bdf98e726216877dd01e2db7fb3415f.

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

@github-actions

github-actions Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Design Review (Fable 5) — 🟡 CONCERNS

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

Design-Verdict: CONCERNS

Sound, well-gated control over an already-validated field — but the module spec now documents a switching story this PR quietly obsoletes.

Watch

  • kas-backend.md § Switching backends documents the only switch path as kirocrew config set … # then: kirocrew restart ("restart reaps the prior runtime process"). This PR adds a second path with different semantics — PATCH + refresh_defaults(), no restart, in-flight turns preserved — and updates no spec. An operator following the doc will restart needlessly; worse, the doc's claim that restart is what reaps the prior runtime is now unverifiable against the new path (does refresh_defaults() also stop a live KAS runtime, or only drain the pool?). Update kas-backend.md (and the dashboard/config spec row) in this same PR per the same-commit spec rule, stating what the drain does and does not reap.
  • The per-row copy is only half derived: not-enabled comes from the schema, but "Default. All features supported." vs "Experimental" is authored in the view keyed on value === KIRO. That's the exact copy-vs-source drift the PR's own rationale rejects — if a backend graduates, nothing goes red. Acceptable now (safe direction: others default to the weaker claim), but it should follow the descriptor table when feat: add staged acp adapter admission #6307's card gets an API.

[DESIGN-REVIEWED] 2bc6c0a

@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
@iamwhatever
iamwhatever force-pushed the feat/agent-backend-switch branch from 6e935d8 to 725ecab Compare August 27, 2026 22:25
@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: passed Eligible automated validation passed for the current revision and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Aug 27, 2026
chenmingwei23
chenmingwei23 previously approved these changes Aug 27, 2026
@iamwhatever
iamwhatever force-pushed the feat/agent-backend-switch branch from 725ecab to e4b0329 Compare August 28, 2026 00:06
@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: passed Eligible automated validation passed for the current revision labels Aug 28, 2026
@iamwhatever
iamwhatever force-pushed the feat/agent-backend-switch branch from e4b0329 to 9eb75b8 Compare August 28, 2026 00:13
@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 28, 2026
@iamwhatever
iamwhatever force-pushed the feat/agent-backend-switch branch from 9eb75b8 to 36530c0 Compare August 28, 2026 00:54
@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 28, 2026
@iamwhatever

Copy link
Copy Markdown
Collaborator Author

Round 2 on 36530c04d.

GPT — narrow-viewport overflow (blocking): fixed. The segmented control's track was inline-flex … w-fit with no wrap, so at 320px a group whose labels are words rather than single glyphs pushed past the card and hid choices off-screen. It now carries flex-wrap + max-w-full, which fixes it for every SettingsButtonGroup consumer rather than just this one.

Measured rather than eyeballed, at 320px: track scrollWidth == clientWidth (244), document scrollWidth == clientWidth (320, so no horizontal page scroll), no button's right edge past the track, and the buttons lay out on 2 rows. Screenshot backend-4-narrow.png plus the harness numbers are in the PR body.

Frontend Lint (Copy/paste detection): inherited, cleared by rebase. The two jscpd clones were both in website/scripts/capture-*.mjs, none of them files this branch touches. capture-aws-control.mjs did not even exist on this branch — it and its clone pair arrived from main, and main's HEAD is now #6434 (fix(ci): dedupe capture-aws-control's hand-rolled server via serveDist), which fixes exactly that pair. Rebased onto it; 0 behind.

Design Review suggestion (advisory, not taken here): a test asserting every schema-advertised value has a rendered option would catch a genuinely new backend id being silently hidden. Agreed that the gap is real, but the fix belongs with whatever adds a fourth id — a test today would assert over a three-id set that the component already covers exhaustively, so it would pass without constraining the case it is meant to catch.

Screenshots re-captured against a correctly-staged build (an earlier set was captured from a stale dist and showed superseded copy). Local gates on this head: tsc clean, eslint 0 errors / 663 of 664, i18n:check exit 0, jscpd exit 0, vitest 25,301 passed with only the pre-existing Node-22 fs/promises.glob failure.

@github-actions github-actions Bot added readiness: passed Eligible automated validation passed for the current revision and removed readiness: checking Automated validation is still running labels Aug 28, 2026
@iamwhatever
iamwhatever force-pushed the feat/agent-backend-switch branch from 36530c0 to 9435b7a Compare August 28, 2026 06:45
@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: passed Eligible automated validation passed for the current revision labels Aug 28, 2026
@iamwhatever

Copy link
Copy Markdown
Collaborator Author

Round 3 on 9435b7a5c. The three prose lines are gone; the wording now comes from #6307 rather than from this component.

Why they were wrong, not just imprecise. The retired lines asserted per-agent facts — "isolates what it runs in an OS sandbox", "shares one process across sessions", "can be steered mid-turn", "does not report subagent progress yet" — in the view layer, where nothing could contradict them. Nothing in this branch measured any of it, and "Amazon internal only" was flatly untrue. A claim with no source of truth behind it will drift the moment a backend changes, silently, so removing them is the fix rather than editing them.

What replaces them, borrowed from #6307's ACP-adapter card:

row line grounds
Kiro CLI Default. All features supported. _KIRO's descriptor is {cap: Level.SUPPORTED for cap in ALL_CAPABILITIES}
KAS (kiro-agent) Experimental _KAS is experimental=True and not all-supported
Claude Code Not enabled in this build not in this build's schema enum

Those are #6307's own strings (all_features_supported, experimental, and the not-enabled wording), and the group description is its applies_to_new_sessions sentence with "harness" read as "agent", which is this tab's own noun.

What I deliberately did NOT copy. #6307's card also shows {{count}} of {{total}} features behave differently, are unavailable, or are not verified behind a "Show what changes" expander, over a 10-capability table with four levels. That count comes from acp/backends.py's descriptors, which this PR does not touch and should not duplicate — a hardcoded count here would be a number I invented, and a copied table could disagree with the gateway with nothing going red. So the per-capability detail stays where it is owned, and this tab says only what it can substantiate. Same reason the row status is derived rather than authored: not-enabled reads off the same schema that gates the buttons, so widening the enum flips Claude Code's row with no edit here.

Evidence. Screenshots re-captured (the old four showed the retired copy, so they were stale). The harness now reads the rendered <dl> back instead of leaving the copy to my eye, and separately asserts the removed prose is absent from the live DOM, not merely from the bundle:

status  Kiro CLI          "Default. All features supported."
status  Claude Code       "Not enabled in this build"
status  KAS (kiro-agent)  "Experimental"

[copy] stale prose still rendered: []

Behaviour is unchanged and re-proven on the new build: "" -> "kas" -> "" round-trips through GET /api/config/kirocrew, a forced click past Claude Code's disabled attribute still writes nothing, and at 320px the track's scrollWidth == clientWidth (244) on 2 rows with no horizontal document scroll.

Gates, after rebasing onto main HEAD 17dfa28ce: tsc clean, eslint 0 errors / 663 warnings, i18n:check all 10 checks green, jscpd 0 clones (the previous revision still carried one inherited pair; #6445 on main removed it), vitest 1611 files passed with only the known Node-22 DiagramLightbox glob failure, and test_agent_backend_editable.py 4 passed. Two frontend tests added: one pinning the three derived lines and the absence of the retired prose, one pinning that widening the schema enum re-derives Claude Code's row.

One locale gate caught this revision: style/zh-CN.md §3 caps at two per clause and my first zh string stacked three. Fixed by splitting the clause on and rewording to 启动时所用的代理.

The core has been able to run three agents for a long time -- Kiro CLI, KAS and
Claude Code -- and shipped no way to pick between them. `agent.acp_backend` was
validated end to end (`_normalize_acp_backend` against `ACP_BACKENDS_SELECTABLE`)
and published in the config schema, but the UI that used to switch providers was
deleted when the website collapsed to KiroACP-only, so the field was switchable in
principle and unreachable in practice.

This adds Developer > Agent Backend: a segmented control over the three agents,
writing `agent.acp_backend`.

Two things it does NOT do. It does not resurrect the old `ProviderPanel` --
Bedrock model ids, the Claude Code migration wizard and a one-member provider enum
are all gone. And it does not invent a vocabulary: the selectable set is read from
GET /api/config/schema, which the backend derives from the same field metadata the
PATCH validator uses, so the enabled options and the values the wire accepts cannot
disagree. Claude Code is the case that makes that worth doing -- this build does
not include it. Hiding it would imply it does not exist; enabling it would produce
a 400 from a control that looked live. It is listed, disabled, and says which it
is. A build that ships another agent lights it up with no frontend change
(asserted by a test).

Each agent's row carries one DERIVED status line rather than a prose capability
sentence. An earlier revision of this branch asserted per-agent facts -- OS
sandboxing, one process shared across sessions, mid-turn steering, subagent
progress -- in the view layer, where nothing could contradict them, and they were
wrong in the way unmeasured claims usually are. The vocabulary now comes from the
ACP-adapter card rather than being invented a second time: `Default. All features
supported.` for the backend whose descriptor is all-supported, `Experimental` for
one that is not, and `Not enabled in this build` for one this build cannot run.
Which line a row gets is derived, not authored -- not-enabled is read off the same
schema that gates the buttons, so a build widening the enum flips Claude Code's row
with no edit here (asserted by a test). Per-capability detail -- which feature is
supported, degraded or unverified per backend -- needs the descriptor table that
owns those facts and is deliberately NOT restated in the view.

Backend, both minimal:

  * `agent.acp_backend` joins `_EDITABLE_CONFIG` -- without it every save came
    back "field not editable". The values duplicate `ACP_BACKENDS_SELECTABLE`
    as a literal because reaching `kiro_crew.acp.types` at module scope executes
    the acp package init (client + runtime) while the dict is being built, the
    same cycle `_normalize_acp_backend` defers for; a parity test fails in both
    directions so the literal cannot drift.
  * the field joins the `refresh_defaults()` list, so a new session picks the
    agent up without a gateway restart and a pre-warmed Kiro CLI process cannot
    serve a session that asked for KAS. Deliberately NOT
    `reload_provider_factory()`: switching the default agent must not kill
    in-flight turns, and live sessions keep the agent they started on.

`SettingsButtonGroup` gains per-option `disabled` so a known-but-unavailable
choice can stay visible instead of the control silently changing shape between
builds -- and, with it, per-option `describedById`. Dimming carries "unavailable"
both visually and through the native `disabled` state, but the REASON is rendered
outside the component, so proximity was the only thing associating them, which is
no association at all for a screen reader. The option now points at its own status
row via `aria-describedby`. That ships with the `disabled` feature rather than
after it: every future consumer that disables an option inherits the same gap.

A failed config read no longer renders as a selection. `?? KIRO` is right for a
config that omits the key -- the shipped default really is Kiro CLI -- and wrong
for a read that FAILED, where the value is simply unknown and defaulting paints
Kiro CLI as pressed, telling an operator running KAS they are on Kiro. The tab now
offers a retry instead of guessing.

It also gains `flex-wrap` + `max-w-full` on the track. It was `inline-flex ...
w-fit` with no wrap, so at 320px a group whose labels are words rather than single
glyphs -- "Kiro CLI" / "Claude Code" / "KAS (kiro-agent)" -- pushed past the card
and hid choices off-screen. A control you cannot see is a control you cannot use.
Every consumer narrows the same way now; the long font-family group benefits too.
Measured rather than eyeballed: at 320px the track's scrollWidth equals its
clientWidth, the document does not scroll horizontally, and the buttons lay out on
two rows.

Placed under Developer rather than Settings on purpose:
`gen-settings-registry.mjs` scans `pages/settings/`, and indexing this into
Settings search would advertise it as an ordinary preference -- it changes which
agent binary runs.

Also hardens `ChatSidebar.tagFilter`'s remount assertion, which waited for one
session to appear and then SYNCHRONOUSLY asserted another was gone. That is a
barrier the test never established, so any branch changing React's batching --
adding i18n keys is the common one -- flips it red. The absence now gets its own
`waitFor`.

"Claude Code", "KAS" and "kiro-agent" are added to the i18n do-not-translate
glossary -- the changed-passthrough gate asks for exactly that rather than a
worked-around translation, and every occurrence of the three is new here, so no
existing locale value changes meaning.
@iamwhatever
iamwhatever force-pushed the feat/agent-backend-switch branch from 9435b7a to 2bc6c0a Compare August 28, 2026 07:10
@iamwhatever

Copy link
Copy Markdown
Collaborator Author

Local GPT-lane mirror (gpt-5.6-sol against codex-review.yml + the base-ref AUTOSDE rules) reported 0 Critical, 0 High, 2 Medium on 5fabb8627. Both are answered below; fixes are in 2bc6c0a71.

  • core.py:2423 — the background runtime ignores agent.acp_backend: accepted-and-deferred.

    "agent.acp_backend", refreshes the provider factory and warm pool, but leaves SessionManager._bg_runtime alive; get_bg_session() also constructs AcpRuntime(...) without acp_backend, so after selecting KAS, title generation and other background sessions continue running Kiro CLI.

    Verified rather than taken on trust, and it holds. AcpRuntime.__init__ defaults acp_backend: str = ACP_BACKEND_KIRO (acp/runtime.py:614); get_bg_session constructs AcpRuntime(agent="kirocrew-lite", sandbox_mode=…, expect_mcp_reports=False) (session.py:1408) and passes no backend, so it takes that default; and _bg_provider_is_kiro() (session.py:1313) gates on agent.provider == "acp", which is unconditionally true because that enum has exactly one member — it never consults agent.acp_backend. So background work (chat titles, suggestions, memory consolidation) does keep running Kiro CLI after a switch to KAS.

    Deferred because the defect is neither introduced nor touched by this PR: no file under src/kiro_crew/session.py or acp/runtime.py is in this diff, and the gap is already reachable today by setting agent.acp_backend in config.json — the field has been validated and schema-published all along, which is the premise of this PR. The fix is background-runtime lifecycle work (thread the configured backend into that constructor, teach _bg_provider_is_kiro to read the backend rather than the provider, and retire a mismatched cached runtime once its handles drain), and retiring a live runtime needs its own evidence about in-flight background turns. That is a separate change with its own tests, not a rider on a settings control. Filing it as a follow-up issue.

    Worth being precise about what this PR claims, since the finding brushes against it: the body says a new session picks the agent up without a gateway restart, which is accurate for foreground chat sessions and is what refresh_defaults() delivers. It does not claim background sessions switch.

  • AgentBackendTab.tsx:98 — a failed config read rendered as a real selection: fixed in 2bc6c0a71.

    "const current = cfgQ.data?.agent?.acp_backend ?? KIRO" … treat failed or unfinished prerequisite queries as authoritative defaults. A config failure falsely shows Kiro selected.

    This half is right and worth fixing. ?? KIRO is correct for a config that genuinely omits the key — the shipped default really is Kiro CLI — but on a rejected read isLoading is false and data is undefined, so the control painted Kiro CLI as aria-pressed="true", telling an operator running KAS that they are on Kiro. The tab now renders a retryable error instead of guessing, and a new test pins both halves: the control does not render at all on a failed read, and the retry re-reads and lands on the real value (kas).

  • AgentBackendTab.tsx:106 — options enabled while the schema is unknown: rebutted.

    a schema failure enables unavailable Claude Code and labels it Experimental until PATCH rejects it. Fix: … keep backend options disabled until the schema enum is available.

    This is the documented, deliberate choice, not an oversight, and reversing it is the worse trade. Flashing every option disabled and then live is what reads as a broken control on a slow load, and the PATCH allowlist — not the UI — is the actual gate, so an optimistic enable can cost at most one visible refusal. _normalize_acp_backend rejects an unselectable value regardless of what the UI let the user click. The behaviour is pinned by leaves every option selectable while the schema is still loading, so adopting the suggestion would mean deleting a test that exists specifically to protect it. The window is also milliseconds in the normal case and self-correcting: the status line re-derives from the enum the moment it arrives.

    The genuinely new wrinkle the finding surfaces is narrower and I'll name it rather than hide behind the above: with the reworded rows, an unknown schema now shows Claude Code as Experimental instead of Not enabled in this build, so the optimism became a positive claim rather than a silent omission. That is a real if minor infelicity in a transient state, and the honest fix is not to disable the control but to distinguish schema-unknown from schema-says-no in the status derivation. Left as-is here because it needs a third status state and this PR is a settings switch; noted in the follow-up issue alongside the first finding.

@iamwhatever

Copy link
Copy Markdown
Collaborator Author

Local Opus-lane mirror (claude-opus-4.8 against claude-review.yml + the base-ref AUTOSDE.yaml / website/AUTOSDE.yaml / AGENTS.md) reported 0 Critical, 0 High, 0 Medium and 2 Low on 5fabb8627. Both Lows are answered below; the fix is in 2bc6c0a71.

  • The disabled option's reason is not programmatically associated with its control: fixed in 2bc6c0a71.

    settings.tsx:416-431 renders the disabled Claude Code <button>, and its explanation Not enabled in this build lives in a separate <dl> … with no link between them. A screen-reader user hears "Claude Code, dimmed button" but not why it is unavailable.

    Rated Low, fixed anyway, and the reason it is not gold-plating is worth stating: per-option disabled is a capability this PR adds to a shared component, and it shipped with no accessible way to convey why a choice is dead. Every future consumer that disables an option would inherit the same gap, so this is completeness of the new feature rather than speculative hardening — the proportionality argument for deferring would have been the wrong one.

    SettingsButtonGroup's option type gains an optional describedById, spread onto the button as aria-describedby; the tab gives each status <dd> a stable id and points the matching option at it. Optional, so a group whose options are all selectable is unchanged. A new test pins the wiring end to end — that Claude Code's aria-describedby resolves to an element actually containing Not enabled in this build, rather than merely that the attribute is present — and pins the empty-string case: KIRO is '', so its id is agent-backend-status-kiro rather than a trailing bare separator.

  • icon: <Terminal size={14} /> instead of className="lucide-inline": rebutted.

    website/AGENTS.md says icons use className="lucide-inline", "never size={N}."

    The convention citation is accurate, and the lane's own caveat is the reason not to act on it: this is an AGENTS.md convention rather than a blocking: true AUTOSDE rule, and size={N} is the prevailing in-repo form. Concretely, DeveloperPage.tsx:42 registers this very tab's icon as Cpu size={16}, matching every sibling tab; switching only the three icons inside this one tab to lucide-inline would make it the single inconsistent member of the group it belongs to, trading a documented-but-unenforced convention miss for a visible inconsistency. If the convention is to be enforced it wants a repo-wide sweep plus a lint rule to hold it, not a one-tab exception — which is a different change from a settings switch.

@iamwhatever

Copy link
Copy Markdown
Collaborator Author

Follow-up for the deferred GPT finding is filed as #6502 (background sessions ignore agent.acp_backend), with the schema-unknown-vs-schema-says-no status nuance recorded alongside it so it is not lost. Deliberately an issue rather than a question for the maintainer: the fix is decided and actionable, it just does not belong in a settings-control PR.

@github-actions github-actions Bot added readiness: passed Eligible automated validation passed for the current revision and removed readiness: checking Automated validation is still running labels Aug 28, 2026
@bolichen97
bolichen97 enabled auto-merge (squash) August 28, 2026 09:12
@bolichen97
bolichen97 merged commit b594fb1 into main Aug 28, 2026
83 of 86 checks passed
@bolichen97
bolichen97 deleted the feat/agent-backend-switch branch August 28, 2026 09:14
@github-actions github-actions Bot removed the readiness: passed Eligible automated validation passed for the current revision label Aug 28, 2026
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