refactor: decompose dashboard state boundaries

[bolichen97]

Problem / Motivation

dashboard/state.py concentrated slot buffering, queue delivery, serialization,
interaction state, persistence, registry, and WebSocket transport in two classes
with more than 6,600 lines between them. The facade is heavily monkeypatched and
source-inspected, so changing one concern required reasoning about unrelated
state and made behavior-preserving maintenance unusually risky.

Why it matters

Dashboard state owns concurrency, cancellation, rollback, and on-disk contracts
used by chat, sessions, REST, and WebSocket clients. Keeping those responsibilities
interleaved increases the chance that a local change alters lock scope, wire shape,
or persisted bytes without an obvious failure at the call site.

What changed (motivation → approach → change)

• Kept kiro_crew.dashboard.state as the compatibility facade while extracting
  cohesive owner-driven components for folder storage, slot buffers, queue and
  delivery state, slot projection, approvals/questions, notifications, slot
  registry operations, persistence, and WebSocket transport.
• Preserved facade method signatures and properties, live-container replacement,
  monkeypatch and source-inspection seams, REST/WS JSON fields, file modes and
  bytes, and the existing lock, rollback, cancellation, and close ordering.
• Integrated the latest folder-generation contract through a post-commit callback
  that runs under the repository lock only after persistence is confirmed; no-op
  and failed transactions do not advance the generation.
• Resolve loggers and other injected providers at call time so tests and embedders
  can continue replacing facade dependencies after construction.

Tests

• Added characterization coverage for wholesale container replacement, append
  trimming, replay behavior, queue metadata, exact slot serialization, and source
  links.
• Added exact WebSocket close-order and hub scope/serialization guards.
• Added folder repository coverage for dynamic logging, confirmed-write rollback,
  and folder-generation behavior.
• Relevant dashboard/slot/WebSocket/persistence coverage: 1,755 passed, 2 skipped.
• Final post-rebase characterization, persistence, WebSocket, folder, and security
  posture run: 127 passed, 1 skipped.
• Final source-guard migration and WebSocket hub characterization: 21 passed.
• Security posture omission/registry coverage: 13 passed; documentation self-test
  and the 245-file documentation scan passed.
• Black baseline, isort, flake8, mypy, subprocess encoding,
  lockdown-before-publish, brand, harness-parity, loop-lock, testpaths,
  changelog-history, and focus-cue gates passed locally.

Manual verification

N/A — this is an internal behavior-preserving backend refactor with no rendered
UI delta; facade characterization and transport/persistence tests cover the
observable contracts.

Related Issues

no linked issue: this behavior-preserving refactor was requested directly.

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

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

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

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 279b717

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

[github-actions]

Design Review (Fable 5) — 🟡 CONCERNS

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

Design-Verdict: CONCERNS

Seam-preservation machinery is so faithful it re-creates the coupling the PR set out to remove, now spread across two dynamically-dispatched layers.

Watch

• Components own no state (WebSocketHubOwner protocol fields stay on the facade) and resolve facade methods per call via string getattr — _owner_method("_spawn_ws_send", ...) — while the facade wrappers delegate back to the component. Every behavior now has two homes joined by a call path no static tool (mypy, find-references) can trace, and state.py is still ~7,500 lines. The stated harm — "changing one concern required reasoning about unrelated state" — is only partially removed; cross-cutting changes now require reasoning about both layers plus the dispatch convention.
• _notifications_for/_approvals_for/_questions_for silently discard-and-recreate any replaced coordinator that fails a concrete isinstance check. An embedder's or test's duck-typed replacement is swapped for a fresh empty real one with no error — failing toward "runs fine, state gone," the exact silent-divergence class this refactor claims to reduce.
• The spec declares this "the intended end state, not a migration waypoint," normatively foreclosing ownership migration. If the seam census (which tests actually replace which containers) ever shrinks, that clause keeps the double layer alive past its reason.

Suggestions

• Replace _owner_method string lookups with explicitly injected callables, as NotificationCoordinator already does — same live seams, statically traceable graph.
• Make _X_for accept any non-None replacement (or raise) instead of isinstance-recreating.

[DESIGN-REVIEWED] 279b717

[github-actions]

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of 279b71751d4e6ad641e51fbf28cca42ffd4e09c3 — 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 is in. The extraction is faithful (the folder-generation bump, load-time row filtering, and confirmed-write rollback all exist in the removed base hunks), the spec is updated in the same commit, and the sole production consumer of every new module is state.py — as the design intends. The residue is a set of generalization/second-spelling concerns I counted above.

First-Principles-Verdict: CONCERNS

A faithful facade-preserving decomposition — but it ships two spellings of its own pattern and ten injectable knobs that exactly one construction site ever fills, all with default-equivalent values.

What this change ships

Intent: make the 6,600-line dashboard state module maintainable by extracting nine cohesive components behind unchanged facades — an ADDITION (refactor), and the diff matches the description.

1. Dashboard state logic moves to nine new modules; state.py/_ChatSlot stay the facade — justified
2. Module spec documents the split and end-state ownership, same commit — justified
3. Four new modules classified non-egress in the security-posture registry — justified
4. Every live chat slot now carries three component objects — one consumer, generalized
5. slot_queue_repository.py re-spells the delivery-key hash and 128-entry cap as dead defaults — zero consumers
6. Ten constructor provider params across coordinators/hub — one consumer, generalized
7. Six lazy _X_for(state) self-healing accessors for stateless coordinators — oversized
8. New characterization tests pin container replacement, close order, scope narrowing, folder rollback — justified

Watch

• Two spellings of one pattern inside this PR: FolderRepository is a module singleton (_FOLDER_REPOSITORY, state.py:104) while five equally stateless all-@staticmethod classes (SlotBufferCoordinator, SlotProjection, SlotRegistry, ApprovalCoordinator, QuestionCoordinator) are instantiated per-slot/per-state plus six _X_for heal accessors. Counted: every construction site is in state.py; no other module constructs any of them.
• SlotQueueRepository's defaults (MAX_PENDING_SUBAGENT_DELIVERIES = 128, module _delivery_key) duplicate state.py:350–353, and the single construction site (state.py:2999) overrides all four params with those originals — the defaults are dead and the two copies can silently diverge.

Subtractions

• Drop SlotQueueRepository.__init__'s four provider params; keep one spelling of _delivery_key/the 128 cap in the repository and re-export to state.py, as this PR already does with FOLDERS_FILE (0 consumers of the defaults; 1 construction site, state.py:2999).
• Replace the per-instance _buffers/_projection on _ChatSlot (state.py:2997–2998) and the _registry_for/_approvals_for/_questions_for heal accessors with module singletons matching _FOLDER_REPOSITORY — the classes hold no state, so per-instance copies and isinstance healing buy nothing.
• Drop WebSocketHub's scope_state_provider parameter — its one caller (state.py:154) passes lambda: state, which is the parameter's own default (lambda: owner).

[FIRST-PRINCIPLES-REVIEWED] 279b717

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

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

Review details

The single candidate falsified: websocket_hub.py calls redactors and is correctly placed in NON_EGRESS_REDACTION_MODULES, so test_every_redactor_call_site_is_a_registered_sink_or_allowlisted and test_allowlist_has_no_stale_entries both pass. broadcast_browser_event still redacts at runtime, and state.py retains its own redactor calls (lines 2713/4173 for the slot snapshot, 4821 for the detach notice), so both named-sink invariants hold. The candidate itself concedes "no current runtime redaction is lost" — its stated harm is a future redactor-less egress, which fails requirement (c) (observable wrong outcome now). Dropped.

No findings.

[OPUS-REVIEWED] 279b717

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

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

[bolichen97]

Design review follow-up for 0b473362c:

• Addressed here: the authoritative dashboard spec now enumerates all nine
  extracted modules and states that _ChatSlot / DashboardState remain the
  final compatibility owners of replaceable mutable state, rather than a
  transitional migration layer.
• Accepted as a useful follow-up: component-local owner Protocols. A faithful
  contract spans seven components (the slot projection alone reads roughly 44
  facade members), partial __new__ test states, mutable container replacement,
  and the typed call-time monkeypatch resolver. Adding partial Protocols with
  Any members in this behavior-preserving PR would hide rather than close that
  gap. The dynamic resolver remains intentional for existing instance
  monkeypatch seams; a separate typing change can add complete component-local
  Protocols together with typed resolver names and fake-owner/mypy coverage.

[bolichen97]

Addressed the First Principles blocker in 4a90bb7275ecc4da96251b5539cb54e62c02b1f2:

• Removed facade comments that were falsely satisfying source-inspection tests.
• Kept the live-scope invariant in test_websocket_hub_characterization.py, which scans the real websocket_hub.py implementation and pins all three snapshot reads to all three live narrowings.
• Retargeted the approval producer guard to interaction_coordinator.py, chat_handlers.py, and chat_runner.py; it now counts the four executable producer sites instead of a facade comment.

Focused verification: 21 passed.

[bolichen97]

• Not introduced by this PR — span=11bba1b3166d

Base d7b7d65c30aebec8a50a4b71e04db4b1f49c40bd already filters the same rejected rows in state.py:7439-7450 and writes only the active rows in state.py:7463-7464 and state.py:7784-7787. git blame attributes that predicate to 74f189c86794bf3efa3bcf74a38d3711c880e9d7 (#4793), which is part of the PR base.

This PR moves that existing drop-and-log behavior into FolderRepository; it does not introduce or broaden it. Reverting the filter would reintroduce the non-dict snapshot and unhashable-id crashes that the base behavior prevents. Preserving rejected rows in a new sidecar would instead change the persisted JSON and store state model, contrary to this refactor's explicit disk-byte and behavior-preservation contract. That pre-existing product issue should be handled as a separate behavior-changing fix.

[bolichen97]

Open PR relationship audit

This is a consolidated, point-in-time code-level audit note. It compares complete merge-base diffs and current/merged code; it does not treat a shared topic as duplication or partial coverage as completion.

Relationship findings

• PR #6813 is OVERLAPPING relative to this PR. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #6813: KEEP. The merged refactor is the current home of the eviction policy 6813 removes; deleting the now-unused helper rather than leaving a dead body carrying the defective policy is the right disposition, and no other caller exists in main. Files: src/kiro_crew/dashboard/slot_buffers.py.

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