fix(members): serve the Crew Members page from the React Query cache (#9418)

[NicholasRBowers]

Problem / Motivation

Every visit to the Crew Members page starts from scratch. The member list goes blank and then fills back in, the DM column shows its "Opening thread…" placeholder again, and the drawer's Recent activity and Wake sources blocks re-skeleton for data that was on screen seconds ago. If a wake-sources fetch fails once, the block stays failed until a full remount — it is never retried. And a crew created or renamed anywhere else in the dashboard does not appear on this page until it fully remounts, because the roster lives in component state that the app's cache invalidation cannot reach.

Why it matters

The page hand-rolls its data layer with useState+useEffect, which website/AGENTS.md forbids ("Data fetching is React Query"). Beyond the rule: users see reload flashes on every visit, duplicate requests go out for data other pages already cache under ['cron-jobs'] / ['webhooks', …], and the roster silently disagrees with the crew editor after a save.

What changed (motivation → approach → change)

The page's five fetch sites move onto React Query. The roster is cached under ['kirocrew-agents', 'members-roster']. That key is a child of the key the WebSocket refresh frames already invalidate, and invalidation matches by prefix, so crews created or renamed elsewhere now reach this page with zero new invalidation calls. It cannot share the bare key: that entry stores a different response shape, and two queryFns under one key let whichever mounts first decide the other's shape.

The thread get-or-create POST becomes a cached query keyed by exact member name with staleTime: 0. A cached thread mounts instantly when you return to a member, and the idempotent POST re-runs behind it to repair whatever the backend lost since — every open still goes through the endpoint, as the page's identity rules require. The drawer's activity block gets a per-member cached query. The wake-sources block reads the same three cache entries the Schedule page and the crew editor already keep (['cron-jobs'], the crew editor's ['webhooks', 'crew-editor'] entry, and the shared ['default-agent'] definition), so opening the drawer dedupes against them, and a failed read is retryable instead of latched forever. The star toggle becomes a useMutation with the full optimistic protocol: cancel the in-flight roster refetch before the flip, revert per-name on error, reconcile with an invalidation on settle.

Two behaviors were preserved deliberately. The roster's display order is committed per membership, not per refetch — the roster now refetches on every refresh frame, and re-sorting on a last_active_ts advance would move rows under the cursor mid-click, opening the wrong member's durable DM thread. And every drawer block keeps its failed-vs-empty distinction: a failed refetch keeps showing the last good data rather than replacing it with an error banner (data === undefined && isError, the same spelling the patrol block already uses). Error surfaces this diff touched now render through ErrorNotice (wake, activity, roster load, thread open), with askAgent off only above the DM composer, which may hold an unsent draft. Non-fetch effects — the unread drain, the countdown tick, the URL-to-member sync — are unchanged.

One small visible behavior change beyond the flash removal, called out on purpose: a webhook row under "Wake sources" now shows the "(paused)" marker when the store-wide webhook kill switch is off, not only when the token's own switch is. The row's silenced state calls the shared webhookCanCallIn predicate (wakesCrew.ts), which encodes both switches — the same rule the Webhooks page and the crew editor already apply — so this drawer stops contradicting them about whether an external webhook can call in. A test pins it.

flowchart LR
  subgraph Before
    WS1[WS refresh frame]:::ctx --> K1["invalidate ['kirocrew-agents']"]:::ctx
    K1 -.-> X1[roster in useState<br/>unreachable]:::removed
  end
  subgraph After
    WS2[WS refresh frame]:::ctx --> K2["invalidate ['kirocrew-agents']"]:::ctx
    K2 --> R2["roster cache<br/>['kirocrew-agents','members-roster']"]:::added
    R2 --> O2[order pinned to membership]:::added
  end
  classDef added fill:#DCFCE7,stroke:#16A34A,color:#14532D,stroke-width:2px
  classDef removed fill:#FEE2E2,stroke:#DC2626,color:#7F1D1D,stroke-dasharray:4 3
  classDef ctx fill:#E0F2FE,stroke:#0284C7,color:#0C4A6E
  linkStyle 1 stroke:#DC2626,stroke-dasharray:4 3
  linkStyle 3,4 stroke:#16A34A,stroke-width:2px

Loading

🟩 added · 🟥 removed · 🟦 unchanged

The refresh frame's invalidation now reaches the roster, and a refetch updates row content without moving rows.

Tests

• serves the roster from cache on remount — rows render synchronously at the remount's first paint; the blank-then-repopulate flash is the bug this locks out.
• invalidating the ['kirocrew-agents'] prefix refetches the roster in place — the WebSocket refresh path reaches the roster's child key, and existing rows never blank during the refetch.
• a refresh-frame refetch never reorders the roster; a membership change re-sorts it — row content updates in place, order holds; adding a crew re-sorts by recency.
• a failed wake-sources fetch recovers on the next retry signal — the old one-shot fetch latched its failure for the page's life.
• a failed wake refetch keeps the known-good list on screen — a transient blip on a shared key must not replace correct data with an error banner.
• the store-wide webhook kill switch marks bound tokens "(paused)" — pins the one declared visible behavior change, through the shared webhookCanCallIn predicate.
• reopening the drawer revalidates activity — cached entries render instantly, and the background refetch lands newly recorded activity (nothing invalidates this key, so the reopen must revalidate).
• Star tests updated for the useMutation protocol: the write dispatches async, the flip is asserted optimistic while the write is still unsettled, and the settle-side reconciling refetch is served post-write server truth.
• All 83 pre-existing MembersPage tests pass unchanged in behavior (two star tests adapted to async dispatch; error-surface testids preserved).

Manual verification

Full frontend suite green (30,407 passed), npx tsc -b and eslint clean. Screenshots below captured through the existing capture-members-page.mjs harness against this branch; its frames 01–06 verified their asserted states (frame 07 and one frame-01 label check fail identically on pristine main — pre-existing capture drift, not this diff).

Screenshots / video

Visually unchanged except two things: reload flashes are gone, and a webhook row under Wake sources shows "(paused)" when the store-wide kill switch is off (declared and tested above). Layout, spacing, and copy are untouched.

More surfaces (mobile, light, wide, driving sessions)

Related Issues

Fixes #9418

Pattern harvest

Rule candidate: review-prompt
Pattern: a page-level useState fetch cache is unreachable by queryClient.invalidateQueries — any surface holding server state outside React Query silently diverges from every invalidation site.

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]

Intent: Move the Crew Members page's five fetch sites (roster, thread get-or-create, drawer activity, wake sources, star write) onto React Query so cached data serves instantly on return, the app's existing invalidation reaches the roster, and failed reads are retryable.
Not a goal: No visual or layout change, no new invalidation sites elsewhere, no change to the page's non-fetch effects (unread drain, countdown tick, URL-to-member sync), and no change to the thread endpoint's always-POST repair contract.

[github-actions]

Design Review (Fable 5) — ✅ PASS

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

Design-Verdict: PASS

Moves page state onto the caches that invalidation already reaches — root-cause fix for the flash, the latched failure, and the stale roster, with each shared-key shape verified.

I checked the load-bearing claims: useWebSocket does invalidate the ['kirocrew-agents'] prefix (useWebSocket.ts:65), the ['cron-jobs'] and crewWebhooksQueryKey queryFns match their existing consumers' exact spelling, webhookCanCallIn is the shared predicate the crew editor already applies, the adapted star tests pin the same behavior async rather than weakening a pin, the one visible behavior change is declared and tested, and temp-screenshots/ is an established repo convention. The order-pinned-per-membership ref and the deliberate not-bare-key choice preserve real properties (mid-click misroute, key/shape collision) rather than gold-plating.

[DESIGN-REVIEWED] d60e31e

[github-actions]

First Principles Review (Fable 5) — ✅ PASS

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

First-Principles-Verdict: PASS

Verify the one unverifiable manual claim: that capture frame 07 and the frame-01 label check fail identically on pristine main (pre-existing drift).

Not justified as shipped

• 8. rides along — the "(paused)" marker is beyond the [insider] Crew Members page refetches the roster from scratch on every visit (useState+useEffect instead of React Query) #9418 fix; declared, tested, and its harm stands alone (the drawer asserts a token can call in when webhookCanCallIn — pre-existing, 2 consumers: KiroCrewAgentsPage.tsx, CrewWebhookSection.tsx — says it cannot).
• 9. rides along — ErrorNotice conversions are beyond the fix; mandated by website/AGENTS.md (errors-use-error-notice), including the required /* No hand-off */ comment by the draft-holding composer.
• 10. rides along — 10 committed PNGs under temp-screenshots/; established evidence convention (8 prior commits touch that tree on base).

What this change ships

Intent: stop the Crew Members page re-fetching from scratch on every visit by moving its five fetch sites onto React Query — a FIX (issue #9418; mandated by website/AGENTS.md "Data fetching is React Query").

1. Returning to the page shows the roster instantly, no blank-then-refill flash — justified
2. Crews created or renamed elsewhere now appear without a remount — justified
3. A failed Wake-sources read retries on the next refresh instead of latching until remount — justified
4. Reopening a member mounts its DM thread instantly; the repair POST still runs behind it — justified
5. Reopening the drawer shows cached Recent activity, then refreshes it — justified
6. A failed refetch keeps last-good data on screen instead of an error banner — justified
7. Rows no longer reorder on refresh frames; only membership changes re-sort — justified
8. A webhook row shows "(paused)" when the store-wide kill switch is off — rides along
9. Roster/thread/activity/wake errors render through ErrorNotice with the agent hand-off — rides along
10. Ten screenshots committed under temp-screenshots/members-react-query/ — rides along

Claims checked: useWebSocket.ts:65 is the one non-test ['kirocrew-agents'] invalidation site; the ['cron-jobs'] queryFn is byte-identical to ExecutionsView.tsx:38; crewWebhooksQueryKey's queryFn matches CrewWebhookSection.tsx:79; defaultAgentQuery pre-exists and mandates spreading.

[FIRST-PRINCIPLES-REVIEWED] d60e31e

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

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

Review details

No findings.

[OPUS-REVIEWED] d60e31e

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

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

[github-actions]

UX Review (Fable 5) — 🟡 CONCERNS

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

I have everything I need: the diff is confined to MembersPage.tsx (a React Query migration), its tests, and ten committed screenshots. The user-visible deltas are the four error surfaces moving to ErrorNotice, the webhook "(paused)" marker now honoring the store-wide kill switch, and flash-removal on revisit. The blind read ran and reconciles cleanly against every control that IS shown; none of the diff's changed states (errors, kill-switch pause) appear in any screenshot, and the recordings list is empty (no lens-13 surface needs one — all changed states are async lifecycle).

UX-Verdict: CONCERNS

The refactor's only visible deltas — four ErrorNotice states and the kill-switch "(paused)" marker — appear in no screenshot, so nobody has seen them rendered.

Watch

• Shot-09 (06c-driving-sessions-empty-dark.png) shows fixer's status as "56y ago" — the blind reader: "obviously wrong… this number would make me distrust the panel." A near-epoch last_active_ts renders an absurd age (rare × trust-eroding × persistent while it holds); fix the screenshot fixture's timestamp, or floor timeAgo output for pre-product dates.

Evidence gaps

• The four error states the diff restyles onto ErrorNotice (roster_load_failed, thread_open_failed, activity_error, wake_error) — one screenshot of any of them rendered inline would close this.
• The declared visible change — a webhook row showing "(paused)" via the store-wide kill switch (webhookCanCallIn) — no screenshot shows it; shot-01's "ci-callback webhook" row is unpaused.

[UX-REVIEWED] d60e31e

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

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

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] d60e31e

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

[NicholasRBowers]

self-added: yes
mechanism: activityQuery gains staleTime 0 (stale-while-revalidate on drawer reopen)

• span=450af0265e68 — activityQuery inherits staleTime Infinity, so newly recorded activity stays absent — disposition: fixed in d60e31e.

Fixed as suggested: the activity query now sets staleTime 0, so reopening the drawer (or switching back to a member) revalidates while the cached entries render instantly — no re-skeleton, and fresh activity lands in the background.
No invalidation path exists for this key (no WS frame carries activity), so revalidate-on-observe is the correct freshness driver; the roster/wake keys keep staleTime Infinity because WS invalidation covers them.
Pinned by the new test "reopening the drawer revalidates activity: cached entries render, then fresh data lands".

[NicholasRBowers]

self-added: yes
mechanism: kill-switch "(paused)" rendering documented in the body, pinned by a test, and routed through the shared predicate

• Undocumented, untested kill-switch "(paused)" rendering in a stated-as-visually-unchanged refactor — disposition: fixed in d60e31e, per the Watch item's own clears-when.

Both clears-when conditions are met: the PR description now declares the behavior change explicitly (What changed + Screenshots sections name the "(paused)" marker under the store-wide kill switch), and the new test "the store-wide webhook kill switch marks bound tokens '(paused)' via the shared predicate" pins it with switch_on: false and an enabled token.
The Suggestion is also taken: the row's silenced state now calls !webhookCanCallIn(tk, wakeSwitchOn) from wakesCrew.ts instead of re-spelling the predicate inline, so the drawer, the Webhooks page and the crew editor share one definition of "can call in".

[NicholasRBowers]

self-added: yes
mechanism: silenced predicate replaced by shared webhookCanCallIn; MEMBERS_ROSTER_QUERY_KEY export dropped

• Undeclared rider: kill-switch "(paused)" state hand-spelling webhookCanCallIn — disposition: fixed in d60e31e, taking both Subtractions.

The Watch item's clears-when is met verbatim: the row's silenced state is now !webhookCanCallIn(tk, wakeSwitchOn), joining the helper's three existing consumers, so both switches stay encoded in one place.
Both Subtractions taken: the inline predicate is gone, and MEMBERS_ROSTER_QUERY_KEY is no longer exported (module-private; nothing outside the file referenced it).
The rider itself is now declared: the PR body's What changed and Screenshots sections name the "(paused)" behavior change and the test that pins it, so a reviewer can find it from the description.

[NicholasRBowers]

self-added: no

• The four ErrorNotice states and the kill-switch "(paused)" marker appear in no screenshot — disposition: rebutted (answered without a new push; a push would re-arm every lane on an otherwise-green PR).

The changed states are transient async lifecycle states, and each is pinned by a deterministic test that asserts the RENDERED DOM: the wake/activity/roster/thread error branches assert ErrorNotice mounts with its testId, and "the store-wide webhook kill switch marks bound tokens '(paused)'" asserts the marker text on the row. What those states LOOK like is not new design introduced by this PR: ErrorNotice is the app's one shared error surface (its visual identity is established across ~all pages), and "(paused)" reuses the exact annotation the same list already renders for a token-level pause (visible in the committed shot 02's wake list on main's fixtures).
The "56y ago" in shot 06c is the capture harness's own fixture (last_active_ts: 1000 in capture-members-page.mjs, unchanged on main) rendering a near-epoch date — pre-existing, not introduced or touched by this diff. Worth a follow-up fixture fix in the capture script; out of this PR's scope, which deliberately does not modify the capture harness.
If the maintainer wants rendered evidence of the error/paused states before merging, say so and I will capture and attach them as a follow-up commit — deferred here only because a new head restarts all five review lanes on a PR whose every other lane is PASS.

[NicholasRBowers]

🤖 Kiro Crew Auto-Pipeline [operator: NicholasRBowers#a942f9ca] — REVIEW-READY at head d60e31e: all 65 checks green, PR Readiness passed, GPT + Opus clean, Design + First Principles PASS, UX CONCERNS answered with a recorded disposition, 0 unresolved threads. Single commit, mergeable. Awaiting human maintainer review; auto-merge not armed.

[NicholasRBowers]

🤖 Kiro Crew Auto-Pipeline — closing as superseded. @CrysisDeu's #9442 merged the same React Query migration for this page (Fixes #9418) while this PR awaited maintainer review — thank you for landing it! The two implementations converged independently on the same design, down to the identical ['kirocrew-agents', 'members-roster'] key, the data === undefined && isError failed-refetch guards, and the cancel-before-flip star mutation.

One hazard this PR's review rounds hardened that the merged version does not carry: orderedMembers on main re-sorts by last_active_ts on every refetch, and the roster now refetches on WS refresh frames, focus, and a 30s staleTime — so an active member's timestamp advancing can move rows under the cursor mid-click, opening a different member's durable DM thread. This PR pinned the order to membership changes (re-sort only on add/remove/rename; content updates in place) with a regression test. Filed as a follow-up issue with details and the tested approach for whoever picks it up.

No hard feelings on the race — the goal was the fix, and it shipped.