feat: skip the session reset when kiro-cli hot-reloads MCP config

[CrysisDeu]

Summary

Installing, enabling or disabling an MCP server from the dashboard no longer kills every running session when those sessions reconcile their config live.

kiro-cli 2.10.0+ ships "Config Hot-Reload": a file watcher on ~/.kiro/agents and mcp.json restarts only the changed MCP servers in a running session, keeps the conversation, and applies the edit at the next turn boundary. Kiro Crew's POST /api/mcp/sync still unconditionally called _reset_all_sessions after every MCP config write — a reset whose only purpose was to make kiro-cli re-read a file it already watches.

What changes

• kiro_crew/mcp_hot_reload.py (new) — the gate, keyed to the processes actually running. Every live provider — registered sessions plus the warm pool the reset would drain — must declare LLMProvider.mcp_config_hot_reload as a literal True (new ABC property, default False, harness-parity H14). AcpProvider answers it from the opt-in set ACP_BACKENDS_MCP_CONFIG_HOT_RELOAD (kiro-cli only) and the agentInfo.version ≥ 2.21.0 its own process reported at initialize — the floor is the release the reconcile semantics were observed on, not 2.10.0 where the watcher first shipped; a release in between keeps the always-correct reset until verified, and lowering the floor later is one line. The handshake version is what the process runs; after an in-place kiro-cli upgrade the binary on disk is newer than every process spawned before it, so the file is never consulted. Fails closed: one unhandshaked, older, foreign-harness, or non-declaring process resets everything as before; with nothing running there is nothing to reset.
• Both transports retain agentInfo.version (AcpClient.agent_version, AcpRuntime.agent_version, surfaced through AcpSessionHandle → AcpSessionProvider → AcpProvider.agent_version; shared parser agent_version_from_init in acp/_dispatch.py). SessionManager.warm_providers() snapshots the pool without consuming it.
• POST /api/mcp/sync consults the gate; when it holds, no session is touched and sessions_reset: 0 is the observable outcome. POST /api/sessions/restart stays the unconditional manual path.
• Disable writes disabled: true onto the agent-file entry (_sync_mcp_to_agent, single and batch); enable lifts it. Under the live reconcile a @server ref removed from tools leaves a still-running server's tools mounted, so dropping the ref was never going to be a disable — disabled: true on the entry is what stops the process. The disabled in the kiro-global file cannot stand in: includeMcpJson is pinned false, so kiro-cli never reads it. Side benefit on the cold path: a disabled server is no longer spawned only to sit unmounted.
• list_servers keeps a row (marked disabled, built from the agent entry's full spec, source agent) for a server disabled that way — otherwise the row vanished the moment the user disabled it, leaving nothing to re-enable from.
• Harness parity: new membership set in acp_backends.py (re-exported from acp/types.py), registered in test_harness_parity.py (subset check + test_mcp_config_hot_reload_is_opt_in) and the H6 row.
• slack/gateway.py kiro-cli floor check reuses parse_kiro_cli_version instead of its own split.
• Docs (same commit): docs/architecture/mcp.md gains a "Live reconcile" section and an updated troubleshooting entry; acp-client.md, harness-parity.md, overview.md updated.

How the kiro-cli semantics were established

Probed against kiro-cli 2.21.0 over ACP with an isolated HOME (agent file edited on disk while one session stayed live, no restart):

Edit to the agent file	Observed
add mcpServers entry	_kiro.dev/mcp/server_initialized in ~0.5 s, process spawned
disabled: true on the entry	process exits, tool gone
remove the entry	process exits, tool gone
disabledTools on a running server	tool hidden
add @server ref to tools (server already running)	honoured at the next turn (/tools before a turn still shows the old set)
remove @server ref only (server keeps running)	tools stay mounted, even after a turn → why disable writes disabled: true
in-session /mcp add slash command	registry-only in 2.21 ("No MCP registry configured") — not a usable alternative

initialize on 2.21.0 returns agentInfo: {name: "Kiro CLI Agent", version: "2.21.0"} — the field the gate reads.

Not ACP-standard: the reconcile is a kiro-cli feature (the _kiro.dev/* extension namespace), which is why it is gated by positive harness membership plus a per-process version floor rather than assumed.

Verification

• Local: isort, flake8, mypy (CI-parity pin 1.14.1) on the changed modules, black gate, subprocess-encoding gate, harness-parity gate, brand gate, docs-lint — all exit 0. Tests are left to CI by host policy (no local test runs on this dev host).
• New tests: test/test_mcp_hot_reload.py (parser, agent_version_from_init, pure gate, per-provider gate, live-sessions gate incl. the version-skew case); new cases in test_mcp_sync_agent.py (gate both arms over a fake session registry, disable marker single/batch, enable lifts it, fail-closed) and test_mcp_discovery.py (disabled row survives); test_harness_parity.py extended.
• No frontend change.

Review dispositions

Round 1 — Design (version skew) and First Principles (unused response field, parser consolidation, floor provenance): addressed in the second push. Round 2 — GPT (H14: capability probed off AcpProvider instead of declared on LLMProvider; function-local imports; stale docstring): addressed in the third push. Round 3 — Design (floor granted to unverified releases): floor raised to the probed 2.21.0 in the fourth push; Design (inferred vs observed reconcile): documented as a trade-off, follow-up named below. Per-lane disposition comments on the PR.

Follow-ups (not in this PR)

• Confirmation-based hardening: the gate infers the reconcile from the provider's declared capability; kiro-cli's _kiro.dev/mcp/server_initialized notification is the observable signal and is not yet consulted, so a watcher that fails at runtime reads as success (recovery today: the troubleshooting entry + POST /api/sessions/restart).
• PUT /api/agent/config and the computer-use toggle still reset sessions unconditionally; both could take the same gate.
• The Capabilities header's "Apply & Restart" button copy still implies a restart is needed for MCP changes.

[github-actions]

Design Review (Fable 5) — 🟡 CONCERNS

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

Design-Verdict: CONCERNS

Sound gate, but its premise — "the reset only made kiro-cli re-read a file" — is false for MCP-gateway sessions, whose servers are session/new-injected and outrank that file.

Watch

• MCP-gateway blind spot. When mcp_gateway.enabled is on, a live session's poolable servers are broker stubs "injected … at ACP session/new, where they outrank the same-named entries in the agent spec" (acp/client.py). The new gate keys only on backend + handshake version, so it passes for exactly those sessions — yet the 2.21.0 probe validated file-edit reconcile only. Whether the watcher's disabled: true handling stops a same-named injected stub is unverified; if it doesn't, a disable reports sessions_reset: 0 and the server stays mounted — the precise silent failure the PR names as its worst outcome. Gateway is opt-in/default-off, which is why this is a Watch, not a Block.
• The version floor grants the skip to every future kiro-cli release open-ended; a release that changes reconcile semantics fails silently in the same direction. The named follow-up (consult _kiro.dev/mcp/server_initialized as confirmation) is the real fix — treat it as due, not optional.

Suggestions

• Have _mcp_hot_reload_active (or the provider property) answer False while the MCP gateway is enabled until injection-vs-watcher semantics are probed — one config read, preserves the fail-closed story.

[DESIGN-REVIEWED] 2156c7c

[github-actions]

First Principles Review (Fable 5) — ✅ PASS

Premise-level review of 2156c7ceb9496ba38523db553f5b50a24a9753ce — 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.

First-Principles-Verdict: PASS

Every item traces to a probed kiro-cli behavior or a documented invariant; the one riding-along change (Slack parser) is itself a deletion.

What this change ships

Intent: stop killing every running conversation when the user installs or toggles an MCP server from the dashboard — a FIX (removal of an over-broad reset).

1. MCP config changes no longer restart every chat when all live processes reconcile the file — justified (cause-level: the reset existed only to force a re-read)
2. Skip gated per-process on handshake version ≥ 2.21.0 — justified (floor is the probed release; measured)
3. Disabling a server now writes disabled: true into the agent file — justified (probed: dropping the ref leaves tools mounted; includeMcpJson pinned false at agent.py:2490, so the existing global flag cannot stand in)
4. A disabled server keeps a visible row to re-enable from — justified
5. New capability declaration (LLMProvider.mcp_config_hot_reload, ACP_BACKENDS_MCP_CONFIG_HOT_RELOAD) — justified (mandated shape: H6 opt-in sets, ABC-with-safe-default per AGENTS.md)
6. Both transports retain agentInfo.version — justified; one end consumer, but the hops are the layering, not generalization
7. Warm pool snapshot warm_providers() — justified (1 consumer, minimal form)
8. Slack version check reuses the shared parser — rides along, net deletion
9. Manual restart endpoint stays unconditional — justified escape hatch
10. Docs in same commit — mandated

Watch

• Two sibling unconditional resets remain (grepped _reset_all_sessions: computer_use.py:798, agents.py:1195) — declared follow-ups, accepted-and-deferred; the gate generalizes to both.

Subtractions

• Inline provider_hot_reloads into live_sessions_hot_reload (1 consumer: mcp_hot_reload.py:130); keep the literal-True rule as a comment.
• Drop the DeclaresMcpHotReload Protocol and type against LLMProvider: its stated reason — keeping application code off kiro_crew.providers — is already false in this layer (grepped from kiro_crew.providers under dashboard/: 6+ files, e.g. dashboard/chat_handlers.py:112).

[FIRST-PRINCIPLES-REVIEWED] 2156c7c

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

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

Review details

The candidate claims the re-enable path is asymmetric with the disable path: disable marks both alias and name via _mark_agent_entries_disabled(cfg, (alias, name)), while re-enable pops disabled only from mcp_servers.get(alias).

Verifying the three-part bar:

• The asymmetry is real in code (mcp.py:308 lifts only from _existing = mcp_servers.get(alias); mcp.py:373 marks both). But it only matters when alias != name, which mcp_server_alias (mcp_utils.py:302) makes true only for slash-bearing names, and only when a slash-form dict entry ns/foo coexists with the dash alias ns-foo in the agent config.
• (a) fails: the dashboard's enable path only ever writes mcp_servers[alias] (the dash form, mcp.py:287); it never creates a slash-form dict entry. Coexisting slash+dash dict entries arise only from legacy/manually-edited config, which the discovery pass itself could not confirm occurs in practice.
• (c) fails: even given that state, the functional server is the alias entry ns-foo, which is re-enabled and mounted via @ns-foo. A leftover disabled: true on the slash entry ns/foo is inert — kiro-cli cannot resolve @ns/foo at all (the very reason aliasing exists), so the server works after re-enable. The claimed "session keyed off the legacy name stays off" is an "if a caller were to" scenario.

No grounded defect in the changed lines survives, and I found nothing else meeting the bar.

No findings.

[OPUS-REVIEWED] 2156c7c

Verdict parsed from the review's SHA-scoped output markers for commit 2156c7ceb9496ba38523db553f5b50a24a9753ce.

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

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

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

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 2156c7c

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

[bolichen97]

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 #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. Shared file and shared membership-set convention, nothing shared in behaviour; at most a trivial merge in the same region. Files: src/kiro_crew/acp_backends.py. The two independent directions used different labels; the matrix conservatively retains OVERLAPPING for coordination.

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

[bolichen97]

Approved after deep review (feat: escalation — MCP hot-reload gate).

Security checklist: no auth/authz, trust-boundary, sandbox, or secret-handling changes. The one new input surface is agentInfo.version off the local ACP initialize response, type-guarded in agent_version_from_init and parsed by an anchored regex in parse_kiro_cli_version.

The new gate is a skip, so what matters is that it fails closed — and it does on every path: non-member backend, missing handshake version, below the 2.21.0 floor, any exception in _mcp_hot_reload_active, and a non-literal True all fall back to the reset that was always correct.

SessionPool.warm_providers() is synchronous drain-and-requeue mirroring _pool_pids, so it is atomic against the event loop: no lost provider, FIFO preserved. The disabled: true marker on the agent entry is the load-bearing half — without it the skip would silently turn a disable into a no-op for running sessions.

Non-blocking follow-ups: (1) re-enable lifts disabled only from the alias entry while disable marks alias + legacy slash key; (2) skip is inferred not confirmed — _kiro.dev/mcp/server_initialized hardening already named in the PR docs.

All 5 AI reviewers and 29 CI checks green on 2156c7c.

Verdict: SAFE_TO_MERGE.