fix(dashboard): carry the slots offender note on every read path

[javenciu]

Problem / Motivation

#8888 added _slots_serialization_note, so when the open-slots projection picks up a value json.dumps cannot serialize, the coalesced slots broadcast fails with a note naming the exact slot and field instead of a bare Object of type X is not JSON serializable. But the broadcast is only one of four places that serialize that projection. The three sibling read paths still fail bare:

• the dashboard-user WS frame (_slots_ws_frame, covering both of its send sites) — this one serializes the enriched projection, so a poisoned value that only enrichment adds raises here and nowhere else;
• the WS connect snapshot — which sits under an except Exception: pass, so a broken snapshot today means an empty sidebar with zero evidence anywhere;
• GET /api/chat/slots — web.json_response runs the dump internally, out of reach of the diagnostic.

Why it matters

The offender note exists precisely because these failures are data-dependent and intermittent: whichever path serializes the poisoned projection first is the one that fails, and three of the four paths still produce the undebuggable bare TypeError (or, on the connect snapshot, silence). The FP review on #8888 called this out and sanctioned the follow-up; this PR completes the seam so every read path fails with the same named-offender diagnostic.

What changed (motivation → approach → change)

All three remaining paths now dump explicitly and annotate failures through the shared helper:

• _slots_serialization_note gains a path keyword (default keeps the wired broadcast site byte-identical) so the note names which read path raised, and its two unreachable shape-check branches are removed — every caller passes serialize_slots() output, which is list-of-dicts by construction, and any surprise still degrades to the generic note through the defensive except instead of raising.
• _slots_ws_frame wraps its dump, annotates with path="ws-frame", and re-raises — one seam covering both of its send sites (fan-out and owner projections).
• GET /api/chat/slots dumps explicitly and returns web.Response(text=..., content_type="application/json") — json_response is Response(text=dumps(...)), so the healthy response is unchanged; the failure path now carries the note.
• The WS connect snapshot dumps explicitly, annotates with path="ws-connect-snapshot", and also logs a WARNING before re-raising — a deliberate addition, disclosed here: the outer except Exception: pass swallows the annotated exception, so without the log line the note would vanish with it. One log line on a today-silent failure is the minimal honest diagnosability; the exception flow itself (swallow-and-continue) is unchanged. The snapshot also now sends the pre-dumped string via send_str — the established shape on every other slots send path (send_json is send_str(dumps(...))).

Test doubles: the WS fakes gain a send_str that parses back to the dict frames existing assertions read; one double's folders_generation had returned a raw MagicMock, which a fake send_json never serialized and the real dump now rightly rejects.

Tests

Contract-generated, reusing the _poison_slots_projection helper and REST app fixtures from the #8888 suite (test/test_open_slots_persistence.py, +219 lines):

• REST: poisoned projection → the TypeError carries the note naming slot, field, and GET /api/chat/slots (fails-before: bare TypeError from inside json_response);
• WS connect: poisoned snapshot → caplog WARNING carries the note, nothing is sent, connection proceeds (fails-before: no log record, silent empty sidebar);
• _slots_ws_frame direct: poisoned enriched projection → TypeError note with [ws-frame] (fails-before: bare);
• exoneration control: clean slots + poisoned envelope extras → the note says no slots entry offends (the diagnostic must not miscredit);
• degradation pin post-shrink: helper(42) → generic note, no raise;
• benign controls: all four paths healthy → byte-identical outputs (REST body equality against json_response, WS frame dict equality).

Fails-before reproduced at the pristine base (this suite copied onto 3a6478967): 5 failed / 69 passed — the three named-offender assertions, the path-label test, and the connect-snapshot log assertion all fail exactly as designed. Full 7-section gate run (targeted 345 passed / seam-neighbor 295 passed / mypy / flake8 / isort / black-baseline / docs-lint) green at the exact commit.

Manual verification

N/A — the seam is deterministic (same poisoned projection in, same annotated failure out) and the benign controls pin byte-identical healthy behavior on all four paths.

Screenshots / video

Why no screenshot: backend-only diff (dashboard serialization diagnostics; no rendered UI change). The only user-visible delta is a clearer error annotation in logs/API error payloads, covered by the attack tests above.

Related Issues

Follow-up to #8888 (same seam, deferral named in that PR's review round); completes the offender-note coverage started there. Diagnostic class as #8745.

Pattern harvest

Pattern: a diagnostic added at one serialization site of a shared projection leaves sibling read paths (REST handler, WS snapshot, WS frame) failing bare — the fix is to route every dump of that projection through the one annotating helper, with a path label so the note also says where.
Rule candidate: review-prompt — "when a serialization diagnostic wraps one call site, list every other place the same object is serialized (grep the projection builder's callers); require either shared-helper routing or an explicit rationale per uncovered path, and flag any covered path that sits under a swallowing except without a log line."

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) — N/A: the helper docstring is the doc-of-record for this seam and is updated in the diff
• No secrets, credentials, or internal references in the diff

Contribution License Agreement

Per the template placeholder (CLA text pending): offered under the same terms as my prior merged contributions to this repository (#8835, #8888).

[github-actions]

Design Review (Fable 5, fork) — ✅ PASS

Design-level review of af4999700fa826d3f86995a80a2ad0d0ce2a1250 via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Both load-bearing claims verified against the base tree: the connect block genuinely sits under a bare except Exception: pass (src/kiro_crew/dashboard/ws.py:713), and _slots_serialization_note currently annotates only the broadcast dump (state.py:7844). The PR does what it says on all four paths, with fails-before evidence and healthy-path byte-equality controls.

Design-Verdict: PASS

Sanctioned follow-up that completes the #8888 diagnostic seam correctly; each read path now fails with a named offender instead of a bare TypeError.

Suggestions

• The seam is still convention, not structure: four sites hand-copy the try: json.dumps … add_note … raise block, so a fifth slots read path added later fails bare again — a tiny _dump_slots_with_note(payload, slots, path) in state.py would make the routing the PR's own "pattern harvest" prescribes structural.
• The residual "zero evidence" root cause is the bare except Exception: pass at ws.py:713: every non-serialization failure in that ~85-line connect block (filtering, folder tree, the send itself) still vanishes silently. A follow-up logging at that swallow (excluding disconnect races) would cover all failure classes, not just this one.

[DESIGN-REVIEWED] af49997

[github-actions]

GPT 5.6 Review (fork) — ✅ no blocking findings

Reviewed af4999700fa826d3f86995a80a2ad0d0ce2a1250 via the fork AI-review pipeline; updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] af49997

[github-actions]

Opus 4.8 Review (fork) — ✅ no blocking findings

Reviewed af4999700fa826d3f86995a80a2ad0d0ce2a1250 via the fork AI-review pipeline; updated in place on each push.

Review details

I have enough to decide. Let me verify the assert-related claim by checking how the function is actually reached in production — whether any caller could pass a non-list.

The diff shows _slots_serialization_note is only ever invoked from except (TypeError, ValueError) handlers, and every call site passes a list projection: _slots_serialization_note(payloads, ...), _slots_serialization_note(slots, ...), _slots_serialization_note(slots_data, ...). All of these are serialize_slots() output (list-of-dicts by construction).

Candidate 1 falsification:

• (a) The only input that triggers the bug is a non-list argument, which the candidate itself concedes no production caller supplies — plus the interpreter must run under -O/PYTHONOPTIMIZE. That is a caller-contract violation that does not occur in practice; (a) fails.
• (c) Even when it triggers, the "wrong outcome" is only a misleading diagnostic note string attached to an exception that still propagates unchanged. This is a diagnostic-only path (it runs solely after a serialization failure has already occurred). No crash, no data loss, no changed control flow. The cosmetic difference in a traceback note under a non-default flag is not an observable wrong outcome on a real path.

The candidate's own confidence is "low," and the failing-test-under--O sub-claim falls in the deterministically-owned "missing/failing tests" category, which is excluded. The candidate dies under falsification.

Step 2: The remaining behavioral changes are all serialization-equivalent to what they replace — web.Response(text=json.dumps(payloads), content_type="application/json") is byte-identical to web.json_response(payloads); send_str(json.dumps(frame)) equals send_json(frame); the connect-snapshot re-raise lands in the same swallowing except Exception: pass it did before, now merely adding a WARNING log. No grounded new defect at 80+.

No findings.

[OPUS-REVIEWED] af49997

[github-actions]

First Principles Review (Fable 5, fork) — ✅ PASS

Premise-level review of af4999700fa826d3f86995a80a2ad0d0ce2a1250 via the fork AI-review pipeline — 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.

Verification complete. The base confirms every load-bearing claim: the helper exists with exactly one consumer at base (state.py:7844, the broadcast), _slots_ws_frame has two production send sites (state.py:7918, state.py:8041), the connect snapshot sits under except Exception: pass (ws.py:713), and the REST handler returned web.json_response(payloads) on the same projection. I also grepped for other dumps of the projection: the only remaining one, _serialize_for_client (websocket_hub.py:210), serializes the per-app filtered subset and only runs after the now-annotated dumps succeed in the same flow, so it cannot fail from the same cause — not an unfixed sibling.

First-Principles-Verdict: PASS

A sanctioned follow-up that routes three bare-failing dumps of one projection through the one existing diagnostic, deleting dead branches along the way.

What this change ships

Intent: make every slots read path fail with the named-offender diagnostic #8888 gave only the broadcast — a FIX (completing a deferred fix).

1. GET /api/chat/slots failures now name the offending slot and field — justified (defect class api_chat_slot_create 500s (bare StopIteration) when config.agents is empty — main CI red since #6465 #6522/Failure semantics: a slots-broadcast exception 500s an already-committed slot create #8745)
2. Dashboard WS frame failures carry the note, covering both send sites via one seam — justified
3. WS connect snapshot failure now leaves a WARNING with the note (was silent empty sidebar) — justified, explicitly declared
4. Every note names which read path raised (path label) — justified; 3 counted callers plus the default
5. Connect snapshot sends a pre-dumped string (send_str) — declared mechanical consequence, healthy bytes unchanged
6. Two unreachable shape-check branches deleted from the helper; surprises degrade to the generic note — justified deletion, pinned by test
7. REST healthy response built by explicit dump instead of json_response — declared, byte-identity tested

The one dump of this projection left unwrapped, _serialize_for_client (websocket_hub.py:210), serializes the per-app filtered subset and runs only after the annotated dumps succeed, so the same poison cannot reach it — no uncovered sibling. The helper is reused, not respelled (1 consumer at base, 4 after); no new knob, key, or concept ships.

[FIRST-PRINCIPLES-REVIEWED] af49997

[bolichen97]

Tech Lead review — APPROVE.

Verified the completeness claim against the head tree (af49997) rather than the PR body. Every dump of the slots projection is now annotated:

site	path label
_do_slots_broadcast bare list (state.py:7846)	slots-broadcast (default, byte-identical to #8888)
generic dashboard-user frame via _slots_ws_frame (state.py:8049)	ws-frame
owner frame via _slots_ws_frame (state.py:7926)	ws-frame
GET /api/chat/slots (chat_handlers.py:1118)	GET /api/chat/slots
WS connect snapshot (ws.py:699)	ws-connect-snapshot + WARNING

Independently grepped for sibling read paths beyond the four the PR names. The only other json.dumps({"type": "slots", ...}) is the app-token frame in websocket_hub.py:210, and it is genuinely not an uncovered path: _send_ws_all is reached only after _slots_ws_frame (or the broadcast dump) has already dumped the same projection, filtered is a subset of it, extras is slots_envelope_extras → {"yolo": bool}, and any residual failure there already logs a WARNING with exc_info in _send_ws_all. The SSE path consumes the pre-dumped note["slots"] string from the annotated broadcast dump, so it inherits the note. broadcast_ws_subagent_subscribers never carries msg_type == "slots".

The removed shape-check branches degrade correctly rather than raising: under -O a non-list input reaches enumerate/.get and lands in the defensive except Exception, returning the generic note. The web.json_response → web.Response(text=dumps(...)) swap is behaviour-preserving on the healthy path, and the connect-snapshot send_json → send_str(pre-dumped) swap matches every other slots send site.

Scope is tight and coherent: 6 files, +365/-55, backend-only, one commit, no unrelated churn. All checks green (0 failures), all five AI lanes clear, readiness: passed.

Design Review's suggestion to extract a _dump_slots_with_note(payload, slots, path) so a fifth read path cannot regress is a real improvement and matches this PR's own pattern harvest — taking it as advisory follow-up, not a merge blocker, since the four sites here are correct as written.