feat(mcp): show what a session's own MCP servers reported

[buluoray]

Problem / Motivation

The dashboard has three MCP views and none of them answers the question a user
actually asks. /api/mcp/active reads an agent spec off disk; /api/mcp/probe
records whether the gateway can start a server in its own process. Both
describe the HOST. Nothing describes a session.

The reported symptom: a user's MCP server showed a Kiro Crew scope badge and
Online in Settings → MCP, while their chat session insisted the tool did not
exist. With no signal saying why, the agent invented an architectural
explanation ("that server is only injected into Kiro CLI sessions, Kiro Crew runs
its own MCP set"), repeated it across turns, cited its own earlier answer as
evidence, and sent the user to change environment settings to fix a problem that
was not there.

The frames that could have answered it already arrive. Both ACP transports
receive _kiro.dev/mcp/server_initialized / server_init_failure /
oauth_request at session init and both already reduce them — but only to build
a timeout error string. On the happy path AcpClient._drain_notifications
collapsed them into one log line ("ACP: MCP servers loaded: %s", not even
distinguishing the three kinds) and cleared the buffer.

Why it matters

A confabulated architectural explanation is worse than a missing tool. It sounds
authoritative, cannot be refuted from inside the session, and the dashboard was
supplying it an objective-looking anchor: a green Online badge that means
"the gateway started this server itself" and nothing about any session. The user
in this report believed the agent because the UI appeared to confirm it.

What changed (motivation → approach → change)

Goal: make a session able to state what it actually mounted, and stop a
host-level probe from reading as a session fact.

Approach. Capture the registration frames where they are otherwise dropped,
keep them as per-session state, and render them beside the configured list rather
than merged into it — the two facts stay distinguishable because conflating them
is the defect.

Frames carry params.sessionId and the shared runtime already de-interleaves
them per session (_finish_session_init), so this is genuinely per-session on
both transports, not per-process. The one exception is a session/new timeout,
where no sessionId exists yet; that path keeps its existing runtime-wide error
string untouched.

Built:

• src/kiro_crew/acp/mcp_session_report.py — one accumulator (ready / failed /
  awaiting-auth, the roster actually sent on the wire, redacted failure reasons).
  It owns all normalization, so the two capture sites are one line each.
• Capture in each transport's own init drain (AcpClient._drain_notifications,
  AcpSessionHandle.drain_init) because that is where the frames are consumed;
  live updates ride the AcpEvents both transports already emit, so the
  mid-turn path stays single.
• Published onto the slot and pushed as an mcp_report_update delta, following
  the todo_update precedent, and invalidated on session reset — deliberately
  unlike that precedent, which never clears (_todo has no reset anywhere in
  state.py). A report is evidence; one describing a torn-down session is worse
  than none.
• Chat MCP panel: with a report in hand each server's mark shows what THIS session
  reported. The mark is a ring where the tool rows use filled dots, because
  both vocabularies live in this one panel and colour is already spent on state —
  without a shape difference "loaded this session" and "started in this session"
  are the same green dot, and "deferred" and "no report yet" are the same hollow
  one. Its own legend sits beside the tool legend, plus a line naming where each
  half of the view comes from and a line for servers the session started that the
  configured list cannot show.
• Settings → MCP: the Online badge gains the caveat two words cannot carry.

Three properties are load-bearing and tested:

• An unreported server renders as not reported, never not mounted. Both
  drains are time-bounded and a late frame still arrives mid-turn. Claiming
  absence here would just move the wrong answer to a new surface.
• A frame the runtime fanned out because it named no session is refused
  (JsonRpcMessage.fanout_no_owner): at most one recipient produced it and
  nothing says which, so recording it would credit this session with another
  session's server.
• A server's state moves. failure → initialized is the normal shape of a
  server that needed authorization, so a name lives in exactly one bucket and a
  recovered server stops showing the reason it failed with.

What this deliberately does NOT do

A per-session tool list. ACP's initialize / session/new responses carry no
advertised tool names (a repo-wide grep for availableTools|tool_names|tools/list
finds nothing), so that needs a kiro-cli protocol change or Kiro Crew issuing
tools/list itself, plus a way to separate resident from Tool-Search-deferred
tools. Server granularity answers the reported case; tool granularity is not
worth that cost yet.

Also untouched: website/src/pages/chat/McpInfoButton.tsx, which reimplements
this view inline and mislabels agent config as "Session MCP servers". Nothing in
the app renders it — the only references are its own test file — so fixing it
would grow this diff for a surface no user sees.

Tests

• test/test_mcp_session_report.py (27) — accumulation rules: bucket movement,
  stale-reason clearing, the unowned-fanout refusal, nameless/non-registration
  frames ignored, redaction and caps, None vs empty payload, superset semantics.
• test/test_mcp_session_report_wiring.py (21) — both capture paths, the accessor
  not draining, _reset_state dropping it, slot set/clear, projection key,
  publish + broadcast, live-event folding, and that a declined reset keeps
  the report (the session it describes is still live).
• test/test_acp_runtime.py (+4) — drain_init records; a pre-switch stale
  backlog is drained but NOT credited; a post-backlog report is; create_session
  records the wire roster.
• website/src/test/mcpSessionReport.test.tsx (24) — derivation, the per-state
  mark (ring colour, and a dashed ring for unreported — not just tooltips), the
  extras line, the reducer storing a null report rather than ignoring it, and a
  guard asserting the RULE that the session vocabulary shares no mark with the
  tool vocabulary, so this collision cannot come back by a colour edit.
• Updated test/test_chat_slot_facade_contract.py (new ordered projection key),
  src/kiro_crew/dashboard/ws_event_scope.py (the new event classified
  slot-scoped, like todo_update) and its frontend mirror in
  website/src/app-sdk/index.ts — three gates caught this diff, and each is a
  contract change that should be reviewed rather than waived. The app-SDK one is
  the symmetric half: a test pins the two tables in sync, so classifying the
  event on the backend alone would have left app tokens unable to subscribe.
• Changed one existing expectation: McpTab.test.tsx asserted that only
  needs_auth carries a hover explanation. That test sits in the needs_auth
  block as an anti-scope-creep guard, and Online overclaiming is precisely what
  this PR fixes, so it now asserts the stronger property — Online explains
  itself, and the other non-needs_auth statuses still carry no hint.

Mutation-verified (13/13 caught). Backend: unowned-fanout guard, stale
backlog, bucket movement, stale reason, retire clear, buffered capture, empty-vs-
absent payload, clearing on a declined reset. Frontend: unreported default, the
mark driven by session state rather than the configured flag, hidden extras,
roster-as-report, ignoring null, and the vocabulary-collision guard (reverting
started to the tool dots' bg-ok reddens it). One frontend mutation initially
survived — the test asserted the tooltip but not the mark itself, which is the
signal a user actually reads; the assertion was strengthened until it failed.

Manual verification

Full backend suite run twice and compared as sets: the only failures that
disappeared between runs were the two this diff introduced (both fixed above),
and the sole new one (test_browser_cli_install) passes 76/76 in isolation — a
parallel-run flake in a file this diff does not touch. The remaining ~106 are
this host's pre-existing environmental set (AF_UNIX path too long,
classify_source reading the real HOME path shape, an assertion on a path that
contains the data-home directory name). Frontend: 1699 test files green, 647 i18n
tests green including every per-language style test.

Screenshots / video

Both variants of the panel, dark theme. github-mcp stands in for the reported
case: it is in the agent's configuration, and this session never reported it —
previously indistinguishable from the servers that did start.

Captured from a harness rendering the real McpToolsPanel with backend-shaped
props, not from a live session — stated plainly so the evidence is not read as
more than it is. The payload shape it is given is the one pinned by the backend
tests and the McpSessionReport type; what the frame proves is the rendering and
the copy, not the end-to-end wiring (that is what the wiring tests cover).

The frame earned its place twice. Its first version rendered every label blank
(the harness had not initialized i18n), which hid the honest wording that is half
of this change. Its second version showed the marks — and that is how the
vocabulary collision above was found: two legends stacked with indistinguishable
swatches. Both were fixed before this frame.

Related Issues

Relates to #2946, which records the probe-vs-session divergence this makes
visible. Complementary to #7143 (servers that should be mounted and are not on
the KAS backend) and #7328 (an agent not knowing what to do when policy blocks a
call) — three parts of the same illegibility, no overlap.

Pattern harvest

Rule candidate: review-prompt
Pattern: a status label sourced from one subject (the host) rendered on a surface
the reader will attribute to another (their session), with no wording that names
the subject.

Checklist

• At most two commits (one is the norm), with a Conventional Commits title
• Existing tests pass and new tests added for new functionality
• Self-review completed; code follows project style guidelines
• Documentation updated (if applicable) — N/A: no spec covers this surface;
  the rationale lives in the new module's docstring
• No secrets, credentials, or internal references in the diff

[github-actions]

Design Review (Fable 5) — 🟡 CONCERNS

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

Design-Verdict: CONCERNS

Sound, well-reasoned design; ships with spec drift on a documented subsystem and a "temp" screenshot binary committed into the source tree.

Watch

• The checklist's "N/A: no spec covers this surface" is contradicted by docs/system-specs/modules/acp-client.md § Notification Buffering, which documents exactly the _drain_notifications / drain_init flow this PR extends (frames now also feed a per-session report; the stale-backlog mechanism changed from queue-emptiness to a caller-measured count). AGENTS.md mandates the spec update in the same commit; without it, the next acp/ contributor works from a doc that omits a consumer of these frames — and the new contract surface (mcp_report projection key, mcp_report_update WS event, LLMProvider.mcp_session_report) lives only in docstrings.
• temp-screenshots/mcp-session-report/panel-before-after.png (118 KB, new in this commit) exists only to host the PR-description image via a commit-pinned raw URL. Once merged it is permanent git history in a directory named "temp-", and deleting it later breaks the description. Upload the image to the PR description directly and drop the file.

Suggestions

• _drop_orphaned_mcp_report inlines a variant of chat_utils.effective_session_key because of an import cycle; extracting the derivation into a cycle-free module would remove the drift hazard the docstring itself acknowledges — a divergence silently clears live reports or preserves stale ones.
• Delete the unrendered McpInfoButton.tsx rather than leaving it: it reimplements this exact view with the config-as-session mislabeling this PR exists to fix, and revival by a future contributor reintroduces the defect.

[DESIGN-REVIEWED] 828c569

[github-actions]

UX Review (Fable 5) — 🟡 CONCERNS

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

UX-Verdict: CONCERNS

Honest, well-worded session states — but the failure reason, the one actionable fact the feature captures, is reachable only by hovering an 8px dot.

Watch

• Failure reason lives only in title={sessionLabel: sessionReason} on the w-2 h-2 ring (McpToolsPanel.tsx). No affordance says a tooltip exists, the hover target is 8px, and title never fires on touch or keyboard — so the user the feature was built for sees "Failed to start in this session" and still can't learn why. Frequency: exactly the target scenario; impact: diagnosis blocked; persistence: every failure. Smallest fix: render the redacted reason as a muted text line under a failed server's row (matching the extras-line pattern already in this diff).
• The panel now stacks source note + 3-item tool legend + 4-item session legend (~6 explainer lines, screenshot shows the after-panel ~1.5× taller) above a 4-row list, inside a dropdown a daily user reopens constantly. Habituated users pay the height forever. Fix: keep the legends, demote mcp_session_source_note to a title on the session legend row.

Suggestions

• "Waiting for authorization" surfaces a fact with no next step; append "— sign in from Settings → MCP" to mcp_session_awaiting_auth.
• online_help: replace "mounted" (internal vocabulary) with "started" — the session-panel strings already use it.

[UX-REVIEWED] 828c569

[github-actions]

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of 828c56962fb371b939df9637d10b5c7d8447c3c9 — 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 checks are done. Assembling the review.

First-Principles-Verdict: CONCERNS

Every load-bearing item traces to the reported confabulation defect, but the diff exports a helper nothing calls and leaves the dead duplicate of this very view in the tree.

What this change ships

Intent: let a user see which MCP servers their chat session actually started, so a host-level "Online" badge stops standing in for a session fact — a FIX (of a reported misinformation defect) delivered mostly as new surface.

1. Chat MCP panel marks each server with what THIS session reported (ring vocabulary, legend, source note) — justified
2. Panel lists servers the session started that the configured list can't show — justified
3. Settings → MCP "Online" badge gains a hover caveat naming it a host check — justified
4. Backend keeps a per-session report of registration frames both transports previously dropped — justified
5. Report invalidated by session identity at the single slot projector — justified, cause-level
6. New mcp_report slot field + mcp_report_update WS event, scope-classified both sides — justified (gated contract)
7. drain_init's ignore_queued_reports flag replaced by a caller-measured frame count — justified (fixes two counted races)
8. MCP events now carry ownership (runtime_global) via a positive session-id test — justified
9. LLMProvider.mcp_session_report() default + SessionMcpReport Protocol — method justified (AGENTS.md mandates ABC-with-safe-default); the Protocol is a second spelling
10. mcpSessionStartedCount exported from website/src/lib/mcpSessionReport.ts — undeclared, zero consumers

Watch

• mcpSessionStartedCount has ZERO consumers — grepped mcpSessionStartedCount across website/src: 1 hit, its own definition; even the new test file never imports it.
• SessionMcpReport Protocol (providers/base.py) duplicates two method signatures of the concrete McpSessionReport for exactly one runtime consumer, the isinstance in chat_runner._session_mcp_report. Its stated purpose — "without taking an ACP-layer edge" — is already false of both files: providers/base.py and chat_runner.py each import kiro_crew.acp.types today, so the concrete import is available at no new edge.
• The description says McpInfoButton.tsx "mislabels agent config as 'Session MCP servers'" and "nothing in the app renders it" — verified: <McpInfoButton renders only in its own test file; the other 31 grep hits are the barrel export and test mocks. Leaving it standing keeps a dormant copy of the exact conflation this PR exists to remove.

Subtractions

• Delete mcpSessionStartedCount from website/src/lib/mcpSessionReport.ts (0 consumers, counted above).
• Drop the SessionMcpReport Protocol in providers/base.py; import McpSessionReport for the annotation and the isinstance (1 consumer: chat_runner._session_mcp_report).
• Delete website/src/pages/chat/McpInfoButton.tsx, its barrel export in pages/chat/index.ts:5, and test/McpInfoButton.test.tsx (0 production render sites) — a removal that finishes the PR's own stated job.

[FIRST-PRINCIPLES-REVIEWED] 828c569

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed 828c56962fb371b939df9637d10b5c7d8447c3c9 — this comment is updated in place on each push.

Review details

The candidate claims switching the three MCP registration events to runtime_global = not self._owns_mcp_frame(msg) can cause a subagent's idle-stall clock to falsely read as stalled, because for a lone session a sessionless MCP frame now yields runtime_global=True and skips _touch_activity.

I verified the mechanical facts: run.py:1121 does if not event.runtime_global: await self._manager._touch_activity(info), and _owns_mcp_frame (session_handle.py:634) is a strict params.get("sessionId") == self._session_id test, so a sessionless frame does set runtime_global=True and skip the touch — a real behavior change from the prior always-touch default.

But (c), an observable wrong outcome, does not survive:

• MCP registration frames are consumed by drain_init at session establishment, before the subagent's turn-level event loop at run.py:1089 runs; they only reach that loop in the rare mid-turn case (e.g. a post-OAuth server_initialized).
• For the loss of one activity touch to invent a false stall, that MCP frame would have to be the sole liveness signal across the entire stall window while the subagent is otherwise active — but an active subagent emits text/tool events continuously, each of which touches activity. The stall clock trips only on a genuine no-event hang, which one skipped init-time touch cannot manufacture.
• The run.py comment establishes the exclusion is intended: sessionless fanned-out frames are another tenant's traffic and must not reset a wedged batch's clock; marking MCP sessionless frames runtime_global is consistent with that intent, not a regression against it.

Every step of the harm chain is a "could"/"if a caller were to" (lone subagent, sessionless own-server frame, arriving mid-turn, sole signal in the window) — the candidate's own note concedes it could not confirm the divergence window occurs. Below the 80 bar; dropped.

No independently grounded new defect was found in the changed lines.

No findings.

[OPUS-REVIEWED] 828c569

Verdict parsed from the review's SHA-scoped output markers for commit 828c56962fb371b939df9637d10b5c7d8447c3c9.

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

[github-actions]

GPT 5.6 Review — ✅ human override accepted

Human judgment by @buluoray overrides the GPT 5.6 finding for 828c56962fb371b939df9637d10b5c7d8447c3c9; the recorded reason is authoritative for this commit.

This comment is updated in place on each push.

The model was not re-run because an authorized human decision supersedes it.

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

[buluoray]

Disposition — GPT 5.6 on 44ec479d5 (all four accepted, one remedy declined)

BLOCKING · reset conversations retain a destroyed session's report — REAL, fixed differently.
Correct finding. I had cleared the report at two call sites (_reset_slot_session and the
queued discard) and missed at least six more: the reset-conversation route
(chat_handlers.py:3645), chat_runner.py:3106/10041/10043, and
handlers_channel.py:542/547 — plus the cron reaper and task runner.

The suggested remedy — remove the projection until every teardown path clears the
report — is declined: it deletes the feature to fix its invalidation, and the
premise ("clear at every path") is the shape that failed. _reset_slot_session's own
docstring says why: six call sites each remembering an extra line is how one gets
missed, and I proved it.

Fixed at the single enforcement point instead: DashboardState.serialize_slots
drops a report whose slot has no live session, keyed with the same derivation as
chat_utils.effective_session_key so a channel-born slot is checked on its linked
key. That projector is the only path both /api/chat/slots and the WebSocket
snapshot share, so no teardown — existing or newly added — can serve a stale report.
Failing to know is not failing to none: with no session manager to ask, the report is
left alone rather than dropped on a guess. Four tests, two mutation-verified
(removing the validation, and making the no-manager case fail closed).

FINDING · _failures[name] bypasses the 64-server cap — REAL, fixed as suggested.
Past the cap the name was dropped from _failed while its reason was still stored, so
the dict grew unbounded in exactly the case the cap exists to bound. Now gated on the
name actually landing in the bucket. Mutation-verified.

FINDING · failed/awaiting servers rendered as "Also started" — REAL, fixed as suggested.
The copy says these started; a failed server did not. mcpSessionExtraServers is now
ready-only. Named cost: a server absent from the configured list that FAILED to start
is consequently not surfaced there — an accepted limitation, documented in the
function, not a silent one. Mutation-verified.

FINDING · a configured-only report reverted to green configured indicators — REAL, fixed as suggested.
The best catch of the four: a session that sent its roster but had not been reported on
yet fell back to the enabled-flag green dots — reasserting the exact false all-clear this
PR exists to remove. mcpSessionHasReport now treats any non-null report as present, so
every row correctly reads "no report from this session yet". My own test had encoded the
wrong intent ("a roster alone is configuration") and was rewritten to state why the
opposite is right. Mutation-verified.

New head a76f1e2eda368503c767d97a81ab1edfdb0b19e3. 18/18 mutations caught overall;
mypy (1217 files), flake8, isort, the four baselined gates, harness-parity and brand
gates green; 1699 frontend test files green. Screenshot re-pinned to the new SHA (200).

[buluoray]

Disposition — GPT 5.6 on 8b1ee0658 (both accepted; one remedy declined)

BLOCKING · ownerless fan-out contaminates every session report — REAL, and worse than reported.
Verified. AcpEvent.runtime_global exists precisely for this and its own docstring says
"a consumer must not read such an event as ITS OWN activity". I built that refusal on the
FRAME path (mcp_session_report.record_frame, tested) and never applied it to the EVENT
path — the same defect on the other side of a symmetric pair.

The finding actually understates it: all three MCP event constructions in
session_handle.py omitted runtime_global entirely, so the flag read False on
frames the runtime had fanned out, in the same while loop where the subagent roster
right above them does carry it. The flag was not merely unchecked, it was lying — so
gating on it without fixing the source would have been dead code.

The remedy — remove these live-report calls — is declined: they are the only path by
which a server that finishes init after an OAuth callback ever reaches the report, which
is the mid-turn recovery this feature exists for. Removing them would freeze the report at
its init-time answer and keep showing a recovered server as unreported.

Fixed instead by carrying the provenance and refusing on it, mirroring what the
compaction path in the same file already does (surface the event, gate only the mutation):
session_handle passes msg.fanout_no_owner on all three MCP events, the three
chat_runner call sites forward it, and McpSessionReport.record_event refuses it — kept
in the report module so both ownership rules live together. Five mutations, all caught,
including one that initially survived: dropping the keyword at the runner's call site
left both halves looking correct while every co-tenant recorded the frame. That gap is now
covered by a behavioural test plus a guard pinning that each call site forwards it.

BLOCKING · _NAME_CAP = 96 truncates a valid name — accepted, with a correction.
Raised to 128. I do not accept the premise that 97–128-character names are a real range —
these are config keys and the repo's own caps for comparable names run 40/48/64/128/200,
so 96 was arbitrary and 128 is no less so. What makes the finding right is the mechanism:
the frontend matches these names EXACTLY, so a truncated name matches nothing and its row
reads "no report" for a server that did report. The cap only has to stop a pathological
name, so it belongs well above any real one. Note the failure direction was conservative —
"no report", never a false all-clear — so this was a concern rather than a fire; it is
fixed because the correct bound is free, not because the range is plausible.

Also caught while landing this: running black on the touched files reformatted 260 lines
of session_handle.py (a .github/black-baseline.txt entry) around a 3-line change.
Reverted to the prior bytes and re-applied only those 3 lines, so the diff there is 3 lines.

New head 235870d6473df09813b3b189d1a43637c869b4fe. 23/23 mutations caught overall;
1622 backend tests in the affected modules, mypy (1217 files), flake8, isort, black,
sync-io, harness-parity and brand gates green. Screenshot re-pinned.

[buluoray]

Disposition — GPT 5.6 on 235870d64 (accepted; fixed at the invariant, not the call site)

BLOCKING · failed resume frames contaminate the replacement session — REAL, and the worst direction.
Verified in client.py: the session/load attempt calls set_roster(...), then
_wait_for_response drains notifications — so the attempt's MCP frames are already
recorded when the load raises and the client falls through to
_new_session_following_substitution(). set_roster only assigned configured; it never
cleared the buckets, so a ready server from an attempt that never came up was published
as the replacement session's own. That is a false all-clear — the exact reading this PR
exists to remove — rather than the conservative "no report" the other findings landed on.

Not fixed as suggested. Clear buffered frames before the fallback session/new patches
one ordering and leaves the next one open (a second load attempt, or any future
establishment path). The real invariant is that a report never spans two session
attempts, and every one of the four callers is already a session-establishment point — so
the clear belongs in the one function all four go through, not in any of them.

set_roster is therefore now begin_session, and it discards the prior attempt's buckets
and failure reasons along with setting the roster. The rename is the point: a function
called "set the roster" reads as safe to call anywhere, and someone doing that is how this
bug comes back. Two mutations, both caught — reverting to a bare roster assignment, and
clearing the buckets while leaving the failure reasons behind.

This is the third round on the same theme (a report must describe exactly one session) and
the third distinct mechanism: teardown paths, ownerless fan-out, and now a failed resume.
Worth naming as a pattern rather than three coincidences — each was a place the report's
lifetime was implied by a caller's discipline instead of enforced by the report itself. All
three are now enforced in the report or at the single projector.

New head 6b2c5f1dfad5fd459e24f9f384a67e29cc3c6f7e. 25/25 mutations caught overall;
1623 backend tests in the affected modules, mypy (1217 files), flake8, black, isort green.
Design, UX and First Principles all PASSED on the previous SHA with no outstanding finding.
Screenshot re-pinned.

[buluoray]

Rebased onto main (26 commits) — 07ccf35ef

The previous head reached all checks green (65 success, 5 skipped, zero failures) with both
blocking lanes clear: GPT 5.6 ✅ and Opus 4.8 ✅ no blocking findings, Design PASS, UX and First
Principles advisory CONCERNS. Backend Tests (3.10, 3) and its downstream Coverage Gate both
cleared on a same-SHA rerun, which is what the earlier disposition predicted (that job's own
annotation was fetch failed, a network step, not an assertion).

Then main advanced 26 commits and the PR went CONFLICTING, which silently stops every
pull_request-event workflow from dispatching — so the rebase is required, not cosmetic.

Two conflicts, both orthogonal, both kept whole:

• providers/base.py — main added resolve_billing_stats, this branch added the
  SessionMcpReport protocol. Both retained.
• providers/acp.py — the same two names in one import block. Both retained, isort-ordered.

Post-rebase: mypy (1222 files), flake8, isort, black, agent-SDK boundary, sync-io and
harness-parity gates green; 1686 tests pass across the affected modules.

One inherited failure, verified not ours.
test_security_posture.py::TestGateSideLogRedactorSpelling::test_the_census_holds_no_slack fails
on a clean origin/main worktree with the identical assertion —
dashboard/handlers/files.py: 1 sites, census says 3. Neither that file nor
_BASELINE_LOG_SITE_CENSUS appears in this PR's diff (git diff origin/main...HEAD returns zero
matches for both). Someone reduced the log sites in files.py without lowering the census
baseline. Not touched here — that baseline belongs to whoever removed the sites.

[buluoray]

Frontend Lint & Type Check red on 07ccf35ef — main is one over its own ratchet

Measured, not inferred. CI runs npx eslint src/ --max-warnings 659:

tree	warnings
origin/main (clean throwaway worktree)	660
this branch	660

Identical, so this diff contributes zero warnings — main itself is one over the ceiling it
pins. Every annotation on the failing job names a file this PR never touches
(apps/command-bar/CommandBarOverlay.tsx, apps/code-review-sage/*, apps/aws-control/*,
apps/auto-improvement/SetupPanel.tsx, app-sdk/ChatEmbed.tsx, App.tsx). The ratchet was
pinned at 659 by b2a16804a; a later merge added a warning without bumping it.

Not fixed here, and deliberately not by bumping the ceiling: the workflow's own comment says to
ratchet --max-warnings DOWN toward zero, so raising it to 660 inside an unrelated PR would hide
main's regression under this change. Two PRs are already in flight against exactly this —
#7508 (remove an unused eslint-disable pushing main over budget) and #7480 (burn one
warning down to the ceiling) — so the repo's own rule applies: don't open a third duplicate, let
the earliest land and rebase onto it.

Everything else on this SHA is green so far (36 success, 5 skipped, 1 neutral, zero other
failures) with 24 checks still running, including both blocking review lanes. The pre-rebase head
e4ed9ab01 had reached all checks green with GPT 5.6 ✅ and Opus 4.8 ✅ no blocking findings.

[buluoray]

Round 7 — all three GPT 5.6 BLOCKING findings were real, all three fixed. f49ab4f1d

Each was verified against source before accepting, and each fix is mutation-verified with the
original bug's exact shape (4/4 caught this round).

1. Ownership was derived from the runtime's fan-out counter

The deepest of the three, and the bot's anchor was right for a reason worth stating: runtime_global
was set from msg.fanout_no_owner, which main's runtime raises only at
if len(_queues) > 1. Its original consumer — the subagent idle-stall clock at
subagent_manager/run.py — is right to treat a lone session as the sole owner of whatever
arrives. This view is not: it publishes server names and failure reasons as "what THIS session
mounted", so a co-tenant emitting a sessionless frame before registering its own queue had its
server attributed here, with the flag clear.

Fixed at the predicate rather than by widening main's flag, which would have changed that clock's
behaviour for every session on the runtime. New AcpSessionHandle._owns_mcp_frame is a positive
test — the frame must NAME this session — and it is the single spelling used by both the event path
and the raw-frame path, so the two cannot answer ownership differently. Main's own
subagent_list construction is untouched.

record_frame's ownership argument is now required, not defaulted. The exclusive transport
(AcpClient) states owned=True with the reason it holds (one session per process), so a third
transport has to answer the question instead of inheriting a yes. That is what turned this from a
patch into a fix: the fail-closed signature reddened 36 call sites immediately.

2. Backlog exhaustion was detected by emptiness, not depth

Real, and the consequence the bot named is the one in the code: line 3013 skips
record_frame for a stale-classified frame. Exhaustion was tested as "the queue went empty", so an
active agent that refilled the queue before the backlog drained kept the flag set for the whole
drain — every report it made was skipped, leaving the session at "no report" for as long as its
servers kept talking. Replaced with a qsize() snapshot spent one frame at a time; later arrivals
cannot extend it.

3. An empty retry preserved an obsolete failure reason

Real. The store was only written when the new failure carried text, so a retry that failed
silently left the first attempt's reason standing and the panel presented it as this failure's own
— the same stale-evidence defect this PR exists to remove, one layer in. An empty error now clears
the stored reason; the server stays in failed, it just carries no reason it never gave.

What the mutation round taught

M1's first mutation survived, and correctly: my sessionless-frame test exercises the raw-frame
path, while the mutation changed the event constructions. Splitting it proved both paths
independently — the event path by the ownership guard (now also rejecting the old fan-out-flag
spelling by name), the raw path by the behavioural test. Worth recording that the event path rests
on a source-text guard rather than a behavioural assertion; the guard's failure message names the
exact defect, but a behavioural event test would be stronger and is the honest next improvement.

Verification

flake8, isort, black, mypy (1222 files), agent-SDK boundary, sync-io, subprocess-encoding and
harness-parity gates green. 1857 tests pass across the affected modules.

Frontend Lint & Type Check is still red for the reason measured in the previous comment: clean
origin/main is 660 warnings against its own pinned ceiling of 659, this branch measures the same
660, and #7508 / #7480 are already in flight against it.

[buluoray]

Round 8 — the one remaining BLOCKING was real, and its fix moved the measurement. 0df57cebc

GPT 5.6's finding landed on the same span round 7 touched, so it deserves a straight answer about
whether this is a third patch on one mechanism or a different defect. It is a different one, and the
remedy is the one I wrongly concluded last round did not exist.

Round 7 replaced an emptiness test with a depth snapshot, and I reasoned at the time that a queue
holding one pre-switch frame and one post-switch frame is indistinguishable at drain start — so I
accepted over-counting as inherent. That was wrong about WHERE the information lives, not about the
ambiguity. At drain start the two are genuinely indistinguishable; at the moment the caller is about
to send set_mode, they are not: everything queued right then is pre-switch by construction.
kiro-cli can emit the switched-to agent's registrations before it answers set_mode, which is exactly
the window that made a drain-time qsize() count them as stale and consume them without recording —
a false "no report" for the rest of the session.

So the count is no longer taken inside drain_init. AcpSessionHandle.queued_frame_count() is read
by the caller immediately before the request goes out, in both create_session and load_session,
and passed in as stale_report_frames. The parameter changed from a boolean to a count deliberately:
ignore_queued_reports=True reads as safe to pass from anywhere, while a count cannot be supplied
without deciding when it was measured.

Mutation-verified with the original bug's exact shape — restoring the drain-time
self._queue.qsize() read reddens the new regression test, which stages one pre-switch frame, takes
the caller's count, then enqueues the active agent's frame before draining.

Verification

flake8, isort, black, mypy (1222 files), agent-SDK boundary, sync-io, subprocess-encoding and
harness-parity gates green. 1853 tests pass across the affected modules.

Two inherited reds, neither ours, one already fixed upstream

• Frontend Lint & Type Check: clean origin/main measures 660 eslint warnings against its own
  pinned ceiling of 659; this branch measures the same 660, so the diff contributes zero. fix(ci): repair two cross-merge breakages reding main's own tip #7508 and
  fix(ci): burn one eslint warning down to the ratchet ceiling #7480 are in flight against it and neither has landed yet.
• The shard-3 test_the_census_holds_no_slack failure reported earlier is now fixed on main by
  ecab0babe (fix(ci): re-measure the files.py log-site census after #7293 #7492). Both will clear together on the next rebase, which is deliberately deferred
  until the ratchet fix lands so one rebase clears both rather than burning a review cycle per red.

[buluoray]

Round 9 — third finding on the report-lifetime span. Root-cause disposition, not another patch. 0df57cebc

Opus 4.8 is ✅ no blocking findings on this SHA. GPT 5.6's remaining BLOCKING is on the reset /
report-lifetime span for the third time (round 1: the projection was retained after a reset;
round 5: session-identity binding), so it gets an answer about the invariant rather than a fourth
edit to the same mechanism.

The mechanism, verified in source

The claim is that api_chat_slot_reset_conversation discards the provider without publishing a
snapshot or a null delta. That part is true — the endpoint calls
state.sessions.discard_conversation(key, replay=replay), logs, and returns. It does not call
clear_mcp_report() and does not broadcast.

The conclusion drawn from it — "dashboard keeps showing the retired session's MCP state" — is not.
Correctness here does not rest on the call site. DashboardState._drop_orphaned_mcp_report runs
inside serialize_slots, which is the single projector both /api/chat/slots and the WebSocket
snapshot go through, and it asks for identity, not liveness:

provider = self.sessions.get_provider(session_key)
live_id = getattr(provider, "session_id", "") if provider is not None else ""
if live_id != slot._mcp_report_session_id:
    slot.clear_mcp_report()

Two things follow, and I checked the second because it is the one that would have made the finding
right:

1. push_slots_update() has 166 call sites, each routed through that projector.
2. The window this finding is actually about — after the discard, before the next message, when
   there is no live session — resolves live_id to "", which cannot equal a real session id,
   so the report is dropped. The no-session case fails CLOSED.

That is why round 5 replaced per-teardown clearing with an identity check in the first place, and the
guard's own docstring names this endpoint among the paths it closes: review had already found the
reset funnel, the reload and reset-conversation routes, the queued discard, a channel handler, the
cron reaper, the task runner and a project change each skipping a clear.

What is actually residual

Delta latency, not stale state: between the discard and the next of those 166 pushes, an already-
open dashboard still holds the previous delta. Bounded, and closed by the next push.

Why the proposed remedy is declined

"Revert slot-level report caching until every successful discard synchronously publishes a null
report" removes the projection in order to fix the projection's invalidation. Round 1 proposed the
same shape and it was declined for the same reason: the cached payload is what lets a reader tell
"configured" from "started in this session", which is the entire point of the change. Reverting it
restores the conflation this PR exists to remove — and it would trade a bounded staleness window for
a permanently wrong answer.

The one judgment I am not making alone

The residual latency has three possible answers, and picking among them is a design call:

1. Accept it. Correctness is the projector's; the delta is a courtesy. No code change.
2. Add the courtesy call at this endpoint. One line, consistent with the six paths that already
   do it — but it is the seventh call site, and _reset_slot_session's own docstring says six sites
   each having to remember an extra line is how one gets missed. This finding is that prediction
   coming true.
3. Move invalidation onto the teardown seam so no call site can forget, the same shape as the
   identity binding. This is the actual root cause — and it puts a dashboard-level concern inside
   SessionManager, which today knows nothing about slots or WebSocket broadcasts. That layering
   change is larger than this PR and should not ride along in it.

I am stopping the babysit loop here rather than pushing a fourth edit to this span. Everything else
on 0df57cebc is green or accounted for: Frontend Lint & Type Check is main's own eslint ratchet
(clean origin/main measures 660 against its pinned 659; this branch measures the same 660, so the
diff contributes zero — #7508 and #7480 are in flight), and the shard-3 census failure reported
earlier is already fixed on main by ecab0babe (#7492).

[buluoray]

Disposition for GPT 5.6 BLOCKING on d205d2bcb (mcp_session_report.py:250, full-bucket transition erases a tracked server) — verified real, fixed in ab6ed1ae2.

The mechanism was confirmed against source: _record removed name from its current bucket unconditionally, then checked the target's capacity — a tracked server transitioning into a full bucket fell out of both, silently vanishing from the report.

The suggested fix (check target capacity before removing) was not adopted verbatim: refusing the move leaves the server displayed in its old state — e.g. still failed with a stale reason after it actually initialized — which is exactly the stale-evidence defect this PR exists to remove. The fix distinguishes the two cases instead:

• A new name past the cap is still dropped whole (the anti-spam bound is unchanged; letting fresh names displace tracked servers would defeat the cap).
• A tracked server transitioning into a full bucket always lands: the oldest target entry is evicted and degrades to "no report" — an absent claim rather than a wrong one. The evicted entry's failure reason is removed with it, so the reasons dict stays bounded by the buckets it describes.

Both behaviors are pinned by tests mutation-verified against the original bug's exact shape (revert to remove-then-refuse → both tests red).

[buluoray]

Disposition for GPT 5.6 BLOCKING on 9b5a841c3 (runtime.py:2920, late pre-switch frames credited to the new mode) — mechanism verified real, remedy declined: it reinstates this reviewer's own prior blocking finding.

The ambiguous window is real: a frame arriving while set_mode awaits cannot be classified by queue position alone. But the suggested fix — "snapshot after set_mode completes, treat all ambiguous frames as stale" — is the exact defect this same review flagged earlier on this PR (session_handle.py:3013: the boolean stale_backlog dropping the switched-to agent's own registrations). kiro-cli emits the new mode's registrations before answering set_mode; a post-completion snapshot counts them as stale, they are consumed without being recorded, and registration frames are one-shot — the panel reads "no report" for the rest of the session. The in-code comment at the measurement site documents precisely this.

The two failure directions are not symmetric:

• Record ambiguous frames (current): a slow pre-switch report can be transiently attributed to the new mode — bounded by the set_mode round-trip, and self-correcting where it matters (the new mode's own registration for the same server overwrites the bucket entry).
• Drop ambiguous frames (suggested): an eagerly-registering server on the new mode is silently absent for the whole session, with no later frame to repair it.

Within this protocol, the window is undecidable — registration frames carry no mode/epoch attribution. The pre-send measurement point was chosen (in response to this reviewer's earlier finding) to take the bounded, self-correcting failure over the permanent one. The real fix is protocol-level (frames carrying a mode epoch), which is out of this PR's scope.

Flip-flopping the measurement point between the two directions this review has now demanded in successive rounds would leave whichever bug the latest round happens to name. Keeping the documented tradeoff.

[buluoray]

/ai-review override gpt 9b5a841: the suggested post-set_mode snapshot reinstates this review's own earlier blocking finding (session_handle.py:3013 -- new-mode registrations consumed unrecorded, permanent false no-report); the current pre-send point takes the bounded self-correcting failure over the permanent one, documented at the measurement site.

[github-actions]

Human judgment recorded

@buluoray marked the gpt AI finding as false positive, not applicable, or explicitly accepted for 9b5a841c399d7648774c3e04b58a75f8ed2d8f13.

the suggested post-set_mode snapshot reinstates this review's own earlier blocking finding (session_handle.py:3013 -- new-mode registrations consumed unrecorded, permanent false no-report); the current pre-send point takes the bounded self-correcting failure over the permanent one, documented at the measurement site.

This decision applies only to this commit. A new push requires a new judgment.

[buluoray]

/ai-review override gpt 0184c95: re-posting the drain-watermark disposition after rebase (override is SHA-scoped) -- the suggested post-set_mode snapshot reinstates this review's own earlier blocking finding (new-mode registrations consumed unrecorded, permanent false no-report); the pre-send point takes the bounded self-correcting failure, documented at the measurement site.

[github-actions]

Human judgment recorded

@buluoray marked the gpt AI finding as false positive, not applicable, or explicitly accepted for 0184c9504d20060392eb0fb2ac30bdbb7ce47e4f.

re-posting the drain-watermark disposition after rebase (override is SHA-scoped) -- the suggested post-set_mode snapshot reinstates this review's own earlier blocking finding (new-mode registrations consumed unrecorded, permanent false no-report); the pre-send point takes the bounded self-correcting failure, documented at the measurement site.

This decision applies only to this commit. A new push requires a new judgment.

[buluoray]

Frontend Lint & Type Check on 0184c9504 is main-inherited, not this PR's: clean origin/main (4e3a854) measures 604 eslint warnings against its own pinned --max-warnings 603"; this branch also measures 604, i.e. zero contribution (all annotations point at website/src/apps/design-critique/, untouched by this diff — git diff origin/main...HEAD` has 0 matches). PR #7569 is burning the ceiling to zero; this lane clears on the next rebase after it lands.

[buluoray]

/ai-review override gpt 828c569: re-posting the drain-watermark disposition after conflict rebase (override is SHA-scoped) -- the suggested post-set_mode snapshot reinstates this review's own earlier blocking finding (new-mode registrations consumed unrecorded, permanent false no-report); the pre-send point takes the bounded self-correcting failure, documented at the measurement site.

[github-actions]

Human judgment recorded

@buluoray marked the gpt AI finding as false positive, not applicable, or explicitly accepted for 828c56962fb371b939df9637d10b5c7d8447c3c9.

re-posting the drain-watermark disposition after conflict rebase (override is SHA-scoped) -- the suggested post-set_mode snapshot reinstates this review's own earlier blocking finding (new-mode registrations consumed unrecorded, permanent false no-report); the pre-send point takes the bounded self-correcting failure, documented at the measurement site.

This decision applies only to this commit. A new push requires a new judgment.

[chenmingwei23]

Approving. Reviewed the diff in three parallel passes (accumulator, frontend, dashboard
integration) and then separately adjudicated the two findings a bot had already
disposed of, since approving endorses those dispositions too.

What I think is the strongest part of this

Two places choose "structurally impossible to get wrong" over "enumerate every case",
and both land correctly.

The ownership test. fanout_no_owner is only set when more than one queue is
registered (runtime.py), so a LONE session receiving a sessionless frame leaves that
flag False -- trusting it would credit a co-tenant's server to a solo session. Using
the positive sessionId == self._session_id test instead is the only correct read, and
it explains why the two capture sites look inconsistent (owned=True hardcoded on the
exclusive transport, the strict test on the shared runtime) when they are in fact the
same rule applied to two different tenancy models.

Report invalidation. Rather than clearing at every teardown point, the report is
identity-stamped and validated at the single read-time projector
_drop_orphaned_mcp_report. Because both /api/chat/slots and the WS snapshot go
through serialize_slots, no teardown path can bypass it -- including reset-recreates-
under-the-same-key, which a liveness check would have missed. The clear_mcp_report()
calls correctly demote to courtesy delta pushes.

Also confirmed, since a report carrying server names and failure reasons invites the
question: mcp_report is not written to history.py or chat_persistence.py or any
transcript, so none of it reaches disk. No cross-slot path. mcp_report_update lands
in Tier-1 slot-scoped filtering like todo_update. And appSdkEventScope.test.ts
genuinely enforces the two-table parity by reading ws_event_scope.py at runtime, so
adding the event to one side only would have failed.

The two dispositions, independently checked

The GPT override is justified. I recovered the original finding text from the
disposition thread rather than reasoning from the override wording. GPT wanted the
watermark snapshot after set_mode returns. That position loses new-mode registration
frames PERMANENTLY: kiro-cli can emit the switched-to agent's registrations before it
answers set_mode, those frames get counted as pre-switch, if not stale_backlog:
skips recording them, and registration frames are one-shot -- the drain has already
consumed the frame, so nothing replays it and the panel reads "no report" for the rest
of the session. The kept pre-send position fails only transiently, and self-corrects
because _record removes a name from every bucket before placing it, so the new mode's
own registration overwrites the misattributed entry. The tradeoff is documented at both
the measurement site and the drain site, more fully than the override reason claims.
There is no third position: frames carry no mode epoch, so the set_mode round-trip
window is genuinely undecidable inside this protocol, and deferring that to a protocol
change is the right scope call.

The Opus dismissal reaches the right disposition on incomplete verification. The
candidate it dismissed -- that routing these three MCP events through
runtime_global = not _owns_mcp_frame(msg) makes a lone session skip _touch_activity
-- is mechanically REAL. Before this diff the three events yielded with no
runtime_global field at all, defaulting to False, so run.py:1121 touched. After, a
lone session with a sessionless frame yields True and skips. Opus confirmed the guard
exists but did not evaluate that the guard's input value flipped for this event class.

It is still not a defect, for a reason worth stating explicitly because it is the
opposite of the candidate's fear: skipping a touch cannot INVENT a stall, it can only
fail to reset the clock -- and before this diff these frames were counted as this
session's activity incorrectly, so a co-tenant's broadcast could reset a genuinely
wedged subagent's clock. That is a false negative this change removes. Net safety
improves. The SubagentInfo.last_activity writers are turn-start, this gated touch,
and three tool-approval finally blocks, so the quiet MCP-init window is indeed
uncovered now -- but a session that is actually working emits other stream events that
feed the clock, and one that is truly idle should measure as idle.

Notes -- all non-blocking, none a change request

None of these five is introduced by this diff. Recording them so they are not
rediscovered from scratch.

1. Session state is title-only. The per-server mark conveys started / failed /
   awaiting-auth / no-report by colour plus ring shape plus a title, with no
   aria-label, sr-only or role. The span is not focusable, so a keyboard user
   gets no tooltip, the button's accessible name is just the server name, and the
   failure reason never reaches a screen reader. The pre-existing tool rows use the
   identical title-only pattern, so this matches the panel's convention rather than
   regressing it -- which is also why I would scope a fix to the WHOLE panel rather
   than to the half this PR adds. Fixing only the new marks would leave a stranger
   inconsistency than either end state.

2. A bare filesystem path survives redaction. _clean runs
   redact_exfiltration_urls and redact_credentials, neither of which touches an
   ordinary path, so failed to open <a path under the user's home> reaches the DOM
   verbatim (capped). Information disclosure, not credential leak -- the credential
   side is thorough and, importantly, redaction runs BEFORE the cap, so a credential
   cannot be split across the truncation boundary and survive in halves. If reason
   strings are considered sensitive that belongs in security.py, not here.

3. The key derivation is duplicated. _drop_orphaned_mcp_report reimplements the
   active-turn / linked-session / dashboard: fallback chain instead of sharing
   chat_utils.effective_session_key, due to an import cycle. The whole no-leak
   guarantee depends on that derivation matching the one the report was stamped under,
   so a future edit to one will not propagate. A cross-referencing comment at both
   sites would be cheap insurance.

4. The pre-send watermark has one narrower residual than the documented one. Its
   self-correction relies on the new mode re-registering the same server to trigger the
   bucket overwrite. If a transiently-misattributed pre-switch server is ABSENT from
   the new mode's roster, the overwrite never fires and the entry persists until some
   later frame touches it. Still the safe direction -- shows a server that is not there
   rather than a false all-clear -- and bounded to a mode switch that drops a server,
   so much narrower than the permanent bug the alternative position creates. Worth a
   line at the measurement site, which already documents the wider case well.

5. The runtime_global change has no behavioural test. The wiring guards assert
   the three sites use the strict test and that exactly three do, but by source
   inspection -- nothing exercises whether a lone-session MCP frame trips the reaper.
   Given the analysis above the answer is no, but that answer currently lives in review
   threads rather than in a test.

The bug this fixes is worth the size: an agent confabulating an architectural
explanation, citing its own earlier turns as evidence, and sending the user to change
unrelated settings, with a green Online badge supplying the false anchor. Rendering
an unreported server as "not reported" rather than "not mounted" is the property that
keeps the fix from just relocating the wrong answer.