fix(dashboard): deliver every chat_message through one identity-carrying door

[NicholasRBowers]

Problem / Motivation

The dashboard can render the same chat reply multiple times — the reported case (#5981) is a channel (WeChat) reply appearing 5 times in a row. The investigation on the issue traced the dominant producer to backend call sites that append a row to the chat window and then hand-build a second broadcast_ws("chat_message", ...) frame for the same row, with no meta.mid on it.

Why it matters

slot.append already delivers one live frame carrying the row's minted meta.mid (via _on_message → _broadcast_chat_message) whenever no HTTP stream reader is active — so the manual frame ships the same row twice. The client's redelivery guard (isRedeliveredMessage, chatSlice.ts) keys on meta.mid and deliberately declines mid-less frames rather than guessing, so each extra copy renders as a new bubble. One mid-less frame per emitted segment of a multi-step channel turn fits the "5 identical bubbles" report. Every user of the dashboard alongside an active channel session is exposed.

What changed (motivation → approach → change)

Symptom: duplicate bubbles → root cause: a family of append-then-manual-broadcast double-emits whose second frame carries no identity → change: one chokepoint, append_and_surface (src/kiro_crew/dashboard/state.py), that delivers every appended row through exactly one identity-carrying door: append's own mid-carrying delivery when no reader is draining, or a single manual frame carrying the row's ts + meta (mid included) when slot._has_reader suppresses append's callback — the conditional pattern handlers/files.py already pioneered, now with identity on the frame.

Complete branch table of every chat_message-emitting site audited (16 candidates):

Site	Class	Change
chat_orchestrator.py cancelled-latch stop msg	assistant double-emit	→ helper
chat_orchestrator.py stage-complete msg	assistant double-emit	→ helper
chat_orchestrator.py stage summary	assistant double-emit	→ helper
chat_orchestrator.py plan-cancelled msg	assistant double-emit	→ helper
chat_orchestrator.py Go/Go All label	user row; manual frame was sole but mid-less delivery	→ helper, broadcast_user=True
chat_handlers.py orchestration-stopped	assistant double-emit	→ helper
chat_runner.py conversation-cleared	assistant double-emit	→ helper; slot_clear reordered BEFORE the append (the wipe must precede the confirmation or it erases the row it announces)
chat_utils.py compaction chokepoint	assistant double-emit	→ helper, kind=compaction preserved on frame+meta
workflow_inject.py workflow result	assistant double-emit	→ helper; window_mid still read off the append for the durable copy
handlers/files.py file card	already conditional, frame mid-less	→ helper; reader frame gains ts+meta.mid
slack/handler.py Slack→dashboard user mirror	user row, sole but mid-less	→ helper, broadcast_user=True (module-level import: dashboard.state was already in this module's import chain via chat_utils)
crew_chat.py crew reply forward	deliberate broadcast=False + manual frame; frame shipped the pre-append meta (no mid)	frame now ships the APPENDED row's meta
chat_runner.py MCP-auth update, tool-patch update	chat_message_update (different event)	unchanged
chat_runner.py compacting status	wire-only status frame, no append	unchanged

The persistence half of #5981 (channel rows saved without their mid, re-minted on every rebuild) was fixed separately in #7646; with both halves in place, the frontend sweep in #5982 catches any residual same-mid duplicate — the three changes are complementary.

Tests

• test/test_midless_broadcast_dedup.py (new): pins the helper contract — single delivery with no reader; exactly one identity-carrying frame (meta.mid + ts) with a reader; user-row broadcast_user semantics (channel-typed rows deliver once, locally-typed rows stay optimistic); extra fields (e.g. kind=compaction) riding the reader frame; and the compaction site end-to-end. Red-before proven: with the old chat_utils site restored, the site test fails assert 2 == 1 on the double delivery.
• Existing tests that pinned the old always-broadcast behavior re-pinned to the new contract (test_workflows_inject.py, test_workflows_service.py, test_outbox_notify_broadcast.py, test_slack_handler_more_coverage.py, test_handler_link_intercept.py, test_dashboard_chat.py); their slot doubles updated to mirror the real append contract (return the row, mint a mid, model _on_message/_has_reader).
• Full backend suite: 81,381 passed; the only failures are the 99 host-environment failures byte-identical to pristine main on the same host.

Manual verification

N/A — unit coverage sufficient: the delivery-count and identity assertions exercise the exact seam the bug lives in, and the changed messages are system strings with no rendering change.

Related Issues

Fixes #5981

Pattern harvest

Rule candidate: semgrep
Pattern: hand-built broadcast_ws("chat_message", {...}) payload without meta — any chat_message frame that does not carry the appended row's meta.mid defeats the client's redelivery guard and renders duplicates; new code must route through append_and_surface.

Checklist

• At most two commits (one is the norm), with a Conventional Commits title (feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)
• Existing tests pass and new tests added for new functionality
• Self-review completed; code follows project style guidelines
• Documentation updated (if applicable)
• No secrets, credentials, or internal references in the diff

[NicholasRBowers]

🤖 Kiro Crew [operator: NicholasRBowers#a942f9ca]

Backend companion to #5982, per the maintainer ruling on that PR's First Principles escalation: this PR fixes the dominant duplicate producer (mid-less chat_message double-emits) and carries the Fixes #5981 closer; #5982 merges as partial hardening (same-mid sweep) with its trailer downgraded to a reference. The persistence half landed earlier in #7646.

Pre-push review: both model-pinned local lanes (GPT gpt-5.6-sol on the codex-review contract, Opus claude-opus-4.8 on the claude-review contract + base-ref AUTOSDE rules) returned no blocking findings on this diff.

Credit: the root-cause investigation that identified the mid-less broadcast producer grew out of isotope14's work on #5982.

[github-actions]

Design Review (Fable 5) — 🟡 CONCERNS

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

Design-Verdict: CONCERNS

Sound root-cause fix, but the helper's two doors still emit different frames — the exact divergence axis this PR exists to kill.

Watch

• append_and_surface's reader-suppressed frame is not equivalent to the append-door frame from _broadcast_chat_message: it skips the non-user re-redaction pass ("the reader-suppressed frame below does not"), and extra fields ride only that frame. workflow_inject already hits this — kind: "workflow_result" now rides only the reader path, dropped from the common no-reader delivery (inert today, no frontend consumer, but an undocumented behavior change vs. the old unconditional frame; the compaction site had to duplicate kind into both meta and extra to dodge it). Future callers of "the ONE door" will inherit a frame whose bytes depend on whether a reader happens to be attached — a rarely-tested path, and display-redaction sits on the untrusted-content boundary.

Suggestions

• Make the reader-suppressed frame byte-identical to the append door: apply the same non-user redaction pass and fold extra into meta (or drop extra), so callers cannot reintroduce per-door divergence through the helper built to prevent it.

[DESIGN-REVIEWED] a37acb9

[github-actions]

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of a37acb9849447f1e676195a716ecd212e94a5d04 — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All evidence gathered and counted. Final assessment:

First-Principles-Verdict: CONCERNS

The fix is cause-level with zero unfixed siblings, but the helper's extra= channel reproduces a door-asymmetry both of its uses make redundant.

What this change ships

Intent: stop the dashboard rendering the same chat reply as multiple bubbles (#5981) — a FIX.

1. Nine append-then-manual-broadcast sites now deliver each row once, with identity — justified (the fix).
2. Frames sent while a stream reader is active now carry ts + meta.mid — justified.
3. Go/Go All labels and Slack-mirrored user rows reach other windows with identity — justified (were sole-but-midless).
4. "Conversation cleared." confirmation now survives the wipe (clear reordered first) — justified, required by the conversion.
5. Crew reply forwards now dedupe against the window rebuild (frame ships appended row's mid) — justified.
6. New chokepoint append_and_surface — justified (10 call sites counted across 7 files).
7. extra= field channel on the helper — declared, both uses redundant (see Subtractions).
8. Workflow-result rows lose their kind marker on the no-reader door — undeclared; 0 consumers of that kind.
9. Unrelated reformat (slack/handler.py:2256) + two black-baseline prunes — rides along, undeclared.

Watch

• Same root cause, one door left: the legacy SSE relay strips meta from chat_message frames (handlers/updates.py:1981-1988), so identity does not survive that path. Grepped chat_message serializers: WS keeps meta (state.py:7755), this one does not. Out of the emit-site family the PR audited, but a mid-less door remains.
• extra rides only the reader-suppressed frame, so any field passed through it exists on one door and not the other — the asymmetry the mid fix just removed, reintroduced for auxiliary fields.

Subtractions

• Drop the extra= parameter from append_and_surface (2 consumers). chat_utils.py:407 passes extra={"kind":"compaction"} duplicating its own meta={"kind":"compaction"} — every frontend read already falls back to meta.kind (chatSlice.ts:1039,4614, completedTurns.ts:31, options.ts:183). workflow_inject.py:178 passes kind="workflow_result", which 0 frontend consumers read (SYSTEM_NOTICE_KINDS is {'compaction','session_reload'}; grepped workflow_result in website/src — only hint-text stripping and a different WS event). Route kind through meta, which rides both doors.

[FIRST-PRINCIPLES-REVIEWED] a37acb9

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

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

Review details

All three candidates are self-admittedly latent/unreachable, and I've confirmed each dies under falsification:

• Candidate 1 (reader branch broadcasts user rows regardless of broadcast_user): every role="user" caller (chat_orchestrator.py:1055, slack/handler.py:2541) passes broadcast_user=True. No caller reaches the asymmetric branch with a default user row — (a) fails, latent only.
• Candidate 2 (workflow_result kind dropped on no-reader path): confirmed the kind is lost on the _on_message path, but no frontend consumer keys on kind === 'workflow_result' (SYSTEM_NOTICE_KINDS is only compaction/session_reload; grep for workflow_result in website/src is empty) — (c) has no observable outcome.
• Candidate 3 (_on_message unset + no reader drops delivery): get_or_create_slot (state.py:6144) wires _on_message on every minted slot; no production path yields an in-use slot with _on_message is None — (a)/(b) fail.

No grounded Step 2 finding: the compaction path retains meta.kind via the meta= argument (so the frontend's m.meta?.kind fallback works), and crew_chat.py correctly ships the appended row's minted meta.mid.

No findings.

[OPUS-REVIEWED] a37acb9

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

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

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

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

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] a37acb9

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

[NicholasRBowers]

🤖 Kiro Crew [operator: NicholasRBowers#a942f9ca]

• fixed span=1473b5c806c7 — function-local append_and_surface import in slack/handler.py moved to the module imports in aa6ed0125.

Rationale: the finding holds — there is no import cycle to defer around. dashboard.state's only slack reference is a type-checking-only import of slack.outbound, and this module already imports kiro_crew.dashboard.chat_utils at module level, which itself imports dashboard.state — so the deferral bought nothing. The same push also fixes the black-format red on test_midless_broadcast_dedup.py and chat_runner.py (CI's --target-version py310 output adopted verbatim; slack/handler.py graduated from the black baseline and was pruned from it).

[github-actions]

UX Review (Fable 5) — ⏭️ skipped

Revision a37acb9849447f1e676195a716ecd212e94a5d04 touches no user-facing surface (no changes under website/ or committed screenshots), so the UX review was skipped. Advisory — does not block merge.

[NicholasRBowers]

🤖 Kiro Crew [operator: NicholasRBowers#a942f9ca]

• rebutted (stale-base diff artifact; remedied by rebase) — "Half this diff is feat(aws-control): move the library removal onto the cloud listing #7810's aws-control relocation, already on main as eee3f2259 — an undeclared rider."

Rationale: the branch never contained that work. Verified locally against the commit graph at review time: git merge-base --is-ancestor eee3f2259 aa6ed0125 is FALSE (the aws-control commit is neither an ancestor of the head nor of the branch's merge-base dc35c192), and git diff origin/main...aa6ed0125 names exactly the 16 files in this PR's table — zero aws-control, i18n, docs, or PNG paths. What happened: #7810 merged to main at 20:56 UTC, ~40 minutes after this branch's last push, so the review lane's diff window (computed against the pre-#7810 merge ref) attributed main's own new commits to this PR. The head is now a37acb984, rebased past eee3f2259 with a content-identical diff (only hunk offsets moved; gates re-run green: 768 affected tests, black gate, compile), so the recomputed merge ref can no longer manufacture the rider. The baseline-prune + reformat hunk in slack/handler.py the verdict calls "undeclared, harmless" is declared: it is the fix-round-1 change recorded in the GPT disposition above (span=1473b5c806c7) and in the PR body's slack-row note.

[NicholasRBowers]

🤖 Kiro Crew [operator: NicholasRBowers#a942f9ca]

• accepted-and-deferred — Watch: the reader-suppressed frame is not equivalent to the append-door frame (skips the non-user re-redaction pass; extra fields ride only that frame)

The concern is legitimate: the helper built to kill per-door divergence should not itself carry a second axis of divergence. It is deferred rather than fixed in-PR because it is outside this PR's charter and inert on this head: the divergent bytes are limited to auxiliary fields with zero frontend consumers (workflow_result kind is read by nothing in website/src; the compaction site already duplicates kind into meta, which rides both doors), the row content itself is identical on both doors, and the security-charter lane (GPT 5.6) passed this head with no blocking findings. Fixing it means changing helper semantics for all 10 call sites — a follow-up-sized change, not a patch onto a green head.

Tracked in #8044 (label deferred-finding, assignee NicholasRBowers, Due 2026-09-17): drop extra= (both consumers redundant) and apply the same re-redaction pass to the reader-suppressed frame, making the two doors byte-identical — the exact remedy this review's Suggestions section proposes.

[NicholasRBowers]

🤖 Kiro Crew [operator: NicholasRBowers#a942f9ca]

• accepted-and-deferred — Watch: the extra= channel rides only the reader-suppressed frame, and the legacy SSE relay (handlers/updates.py:1981-1988) strips meta, leaving one mid-less door outside the audited emit-site family

Both halves are legitimate and both are deferred with tracked owners rather than fixed in-PR, because neither is in this PR's charter (converting the 16 audited emit sites so every emitted chat_message frame carries the appended row's meta.mid — done, and this review confirms "cause-level with zero unfixed siblings" within that family):

extra= asymmetry → #8044 (label deferred-finding, assignee NicholasRBowers, Due 2026-09-17). The review's own falsification shows both current uses are inert (compaction duplicates kind into meta; workflow_result has 0 frontend consumers), so nothing user-reachable diverges on this head — the issue adopts this review's Subtractions remedy verbatim (drop extra=, route kind through meta).

SSE relay strips meta → #8045 (label deferred-finding, assignee NicholasRBowers, Due 2026-09-17). That door is a relay/serializer, not an emit site, which is why the 16-site audit did not cover it; it needs its own change (preserve meta in the SSE frame serialization) with its own test, on top of this PR's invariant rather than inside it.

The rides-along items (#8 workflow_result kind on the no-reader door — subsumed by #8044; #9 slack/handler.py reformat + two black-baseline prunes — mechanical output of the repo's own black gate on the committed diff, not discretionary edits) are noted in the PR body's branch table and Pattern harvest.

[NicholasRBowers]

🤖 Kiro Crew [operator: NicholasRBowers#a942f9ca]

Review-ready at a37acb984. All checks green, both AI review lanes pass (GPT 5.6: no blocking findings; Opus 4.8: no blocking findings), no unresolved threads, and every raised concern carries its own disposition:

• GPT round-1 finding (function-local import) — fixed in aa6ed0125.
• First Principles round-2 block (stale-base rider claim) — rebutted with commit-graph evidence; remedied by rebase onto current main.
• Design CONCERNS (reader-frame divergence via extra=) — accepted-and-deferred → Unify append_and_surface's two frame doors: drop extra=, apply re-redaction to reader-suppressed frame #8044.
• First Principles CONCERNS (extra= asymmetry + legacy SSE relay stripping meta) — accepted-and-deferred → Unify append_and_surface's two frame doors: drop extra=, apply re-redaction to reader-suppressed frame #8044 / Legacy SSE relay strips meta from chat_message frames — last mid-less door after #7981 #8045.

Not merging — merge timing stays with maintainers. This PR carries the Fixes #5981 closer; #5982 is the frontend hardening companion (review-ready separately).

[bolichen97]

Approving on the strength of a full readiness audit of every open PR against main, not a
line-by-line reading of this diff — recording that plainly so the next reader knows what this
stamp does and does not cover.

Verified against this exact head SHA:

• readiness: passed present, and PR Readiness — the one required status context on main
  (ruleset protected-branches) — is success on this head.
• No check run on this head is failure, cancelled, timed_out or still in flight. Skipped
  jobs are path-filtered conditionals, none of them required.
• mergeable: true, and the head is not far enough behind main for its green CI to describe a
  base that no longer exists.
• No surviving reviewer CHANGES_REQUESTED: any such review is on an older commit and therefore
  already dismissed by dismiss_stale_reviews_on_push.
• Every issue comment, inline review comment and review thread was read and classified. Nothing
  left is an unresolved human change request — the remainder is bot review-lane output, resolved
  or outdated threads, explicitly non-blocking suggestions, and author status notes.

Auto-merge (squash) is armed, so this lands once every other ruleset requirement is met.