refactor(members): move the Crew Members data layer onto React Query

[CrysisDeu]

Fixes #9418.

Summary

The Crew Members page held its data in useState filled by useEffect — the roster, the drawer's Recent activity and Wake sources, and the thread-open answers — so every visit started from an empty list, waited on the network, and re-issued every request. website/AGENTS.md: "Data fetching is React Query, never useState + useEffect."

This moves the page's data layer onto React Query, following the repository's existing key conventions:

Data	Before	After
Roster (GET /api/members)	useState([]) + useEffect	useQuery under ['kirocrew-agents', 'members-roster'], staleTime 30s (new website/src/api/membersQuery.ts). Lives under the registry prefix so the WS refresh invalidation and the crew editor's save reach it.
Recent activity	per-name useState map + useEffect	useQuery(['member-activity', slug, name]), drawer-gated
Wake sources	one Promise.all in useEffect, never retried after a failure	the shared ['cron-jobs'], crewWebhooksQueryKey, defaultAgentQuery entries — one fetch serves the crew editor, Schedule and this drawer
Thread open (POST …/thread)	.then into a slots map	useMutation on every open (the endpoint is the idempotent creator/repairer), outcome cached under ['member-thread', name] and read through a skipToken query — the cached thread mounts at once, the re-POST repairs behind
Star	optimistic setMembers + .catch revert	useMutation: the row is locked synchronously on click, onMutate cancels in-flight roster refetches then patches the row; onSuccess / onError cancel again (a refetch started during the PUT could otherwise land its pre-write snapshot after the final patch) and then re-apply the confirmed / reverted value

Outside the page:

• KiroCrewAgentsPage invalidates the ['kirocrew-agents'] prefix after a registry write instead of refetching only its own query, so a crew created or deleted there reaches the roster (its own query is active, so it still refetches exactly as before).
• useWebSocket forgets every unobserved ['member-thread'] entry on reconnect (forgetUnobservedMemberThreads, counting observers — the page reads the open member through a skipToken query, which react-query classifies as inactive, so type: 'inactive' would have cleared the mounted pane too; unit-pinned in membersQuery.test.ts): a dropped socket is the one client-visible sign the gateway may have restarted (which drops an unmessaged member slot while its binding survives), so the next open of those members waits for the endpoint again, as before this PR; the page re-confirms the observed one on the same reconnect. (Their idle lifetime is react-query's default gcTime; the sweep is the control.)
• AutoNudgePopover and this page share one cronJobsQuery definition (website/src/api/cronJobsQuery.ts) for ['cron-jobs']. ExecutionsView keeps its identical inline spelling for now: migrating it removed a covered statement from a coverage-baselined file and tripped the per-file floor by 0.1% — the baseline is shrink-only by design, so that move waits for the file to gain tests.
• The four bare role="alert" boxes this page had (roster, thread failure, activity, wake) and the slug-collision notice all render through ErrorNotice (AUTOSDE errors-use-error-notice).

One defect found by the first CI round and fixed here: the roster being a store means the thread endpoint confirming a key (which patches the row) can re-render on React's sync lane before the default-lane setActiveName from the open commits; the URL-sync effect then re-ran with the old name and opened the same member twice (and, for a stand-in open, overwrote the remembered member). The effect now guards on a ref activate writes synchronously (activeNameRef); the two pre-existing cases that caught it are the regression pins.

Behaviour kept: default / remembered member (#8546), gone-member notice, needs_you marker (#8614), auto-patrol badge (#8936), driving sessions (#8591), presence-dot resolution — all now render from cache immediately on a return visit.

While the roster read has failed the member count shows a dash rather than "0 members" above the failure (UX review), and slug_collision now reads "shares its short name with {{name}}, whose conversation already uses it. Rename one of the two members…" — plain words, and this page's vocabulary is members (UX review; 12 locales).

One new string (12 locales + regenerated en-XA): pages.membersPage.thread_repair_failed — "Couldn't reconnect this conversation. Reopen the member to try again." — shown when the repair POST fails over a cached, still-mounted thread, instead of "Could not open this member's conversation" above a visibly open conversation (UX review). It makes no claim about the messages below, which may be none.

Screenshot Evidence

Both recordings: land on the dashboard home → open Crew Members → leave through the router (SPA back, never a reload) → open Crew Members again. The second open is the one under test.

Before (main @ 6fbb06bcde845731aacf179f32398782d78ba041): the return visit shows 0 members / Pick a member, then the list, then Opening the conversation…, then the drawer skeletons.

After (this PR): the return visit renders the roster, the thread and the drawer on the first frame and refreshes behind.

Decisive frames (return visit, ~0.5 s after the click): before · after.

Error states (the four surfaces moved onto ErrorNotice, three with Ask the agent): a return visit whose repair POST fails over the cached, still-mounted thread — now worded as a failed reconnect, not a failed open — with the drawer's activity and wake-source reads failing on a cold open. Produced by aborting those three requests at the browser (page.route → abort, i.e. the fetch rejects exactly as on a dropped connection); everything else is the pod's real response.

While a repair has failed, the pane suppresses its own "Session ready. Type a message to start." hint (new optional hideEmptyHint prop on ChatPane), so the column does not say "go" one line under a notice that says "broken". The cold-open failure — no cached key, empty column, so no draft to lose — keeps the original "Could not open…" copy and offers Ask the agent (askAgent={!activeSlot}; the hand-off stays off only while a cached thread is mounted underneath):

The other two ErrorNotice surfaces: the roster load failure (GET /api/members aborted at the browser on a cold open) and a real slug collision — the pod is seeded with two crews, oncall and Oncall, that share one slug; oncall is opened first and binds the thread, then opening Oncall makes the endpoint answer with the other owner and the page refuses to mount (no stub):

Provenance: recorded from 3173bb61f8c358b03070cac5249d645507b3eb46 (after — the bundle was rebuilt from this exact tree before recording; earlier revisions of this PR had re-run pod provision, which skips an existing dist, so their "after" GIF came from the round-1 bundle — same flow, but the provenance line overstated it, corrected here) and 6fbb06bcde845731aacf179f32398782d78ba041 (before, the merge base) · two kirocrew pod instances built from those checkouts, seeded with a six-crew config.json (the pod's agent sync added the package crews) · mode: GET /api/members and POST …/thread delayed 1200 ms through page.route so the localhost round-trip is visible at 12 fps — responses are the pod's real responses, only later; no data is stubbed · real server, no model turn is involved in this flow · Playwright webm → PNG frames → GIF (Pillow, global palette + per-frame differencing, so each GIF is under 1 MB and loads inline), verified frame-by-frame on the encoded GIFs: the 0 members frames appear in the "before" return visit only. Both pods were re-seeded before this recording and the light theme pinned in the scenario so the pair is like for like.

Requests issued during the second open (captured by the same Playwright run, page.on('request'), /api/* only):

	before	after
GET /api/members (roster)	1	0 (cached, refetched only after 30 s / on focus / on invalidation)
GET /api/crons, GET /api/webhooks, GET /api/agents (wake sources)	3	0 (shared cache)
GET /api/members/{slug}/activity	1	0 (cached)
POST /api/members/{slug}/thread	1	1 (by design — every open repairs)
POST /api/agents/sync + GET /api/agents (agent sync, not this page)	2	2
total	8	3

Tests

Written, not run locally (this branch's rule is tsc / eslint / i18n gate only; CI runs vitest):

• MembersPage.test.tsx: mocks moved from kirocrewAgents to defaultAgent; four new pins under roster cache (React Query) — a second mount renders the cached roster synchronously with no empty state and, inside the 30 s stale window, issues no request; a second mount mounts the cached thread while the re-POST hangs; invalidating ['kirocrew-agents'] refreshes the roster in place; a refetch failure after a good read keeps the last roster.
• MembersPage.test.tsx also pins the repair-failure copy: a failed re-POST over a cached thread keeps the thread mounted and says "Couldn't reconnect…", never "Could not open".
• MembersPage.filters.test.tsx: star cases wait for the async onMutate (the flip and the PUT now sit a microtask after the click, behind cancelQueries); the pending case additionally pins that a 2xx star does not refetch the roster.

Local: tsc -b clean, eslint clean on the touched files, check-i18n-strings at baseline, phantom-classes gate green; the two test files type-check under a vitest-globals tsconfig.

Pattern harvest

The Members page was hand-built as a sibling of the Sessions sidebar rather than a consumer of its list infrastructure. In one week that surfaced as five independent drift fixes — steer-only composer (#8852), Quote/Ask (#8947), drag-handle / surface parity and list-shell background (#9428), and now data fetching (#9418). Each was found by a PM reading the shipped page, because there is no shared contract to drift from. Follow-up: #9441 — make the roster reuse the sidebar's list shell, row primitive and data layer.

Notes for reviewers

• The thread open stays a mutation on purpose: POST …/thread is the only creator/repairer of member slots and must run on every open; the cache decides what renders while it is out, it never replaces the POST. Storing the outcome under a query key (written by setQueryData, read by a skipToken query) is what makes the cached thread mount instantly on a return.
• Roster ordering re-sorts only when the cache changes (return visit, focus after the stale window, a registry write) — never on a live message — so rows still do not move under the cursor mid-conversation.
• Avoids the lines fix(dashboard): one list-shell recipe for sessions sidebar and members roster #9428 (drag handle / surface parity) and feat(members): steer-only composer for Crew Members DM threads #8852 (composer) are changing; conflicts, if any, resolve on main as the base.

[github-actions]

First Principles Review (Fable 5) — ✅ PASS

Premise-level review of 3173bb61f8c358b03070cac5249d645507b3eb46 — 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 claims verified. The ExecutionsView inline twin at ExecutionsView.tsx:37-38 is behaviorally identical to the new shared definition, useConnected exists, every new surface has counted consumers, and every item in the diff is declared in the description. The core migration is mandated by a documented invariant (website/AGENTS.md:76) with a linked issue (#9418), and the ErrorNotice conversions satisfy the blocking errors-use-error-notice AUTOSDE rule.

First-Principles-Verdict: PASS

Verify ExecutionsView.tsx:37 — the one surviving inline ['cron-jobs'] twin — stays byte-identical to cronJobsQuery until it migrates; the divergence risk is the one the new module documents.

Not justified as shipped

• 6. rides along — not the migration, but mandated by blocking AUTOSDE rule errors-use-error-notice once these lines were touched; declared.
• 8. rides along — a relabel, but the harm is checkable in-diff: the old copy said "thread"/"crews" beside sibling strings saying "conversation"/"members"; declared.
• 9. rides along — declared; "0 members" above a load failure asserts what is unknown.

What this change ships

Intent: make the Crew Members page render from cache on a return visit instead of blank-then-refetch — a FIX (issue #9418, mandated by website/AGENTS.md's React Query rule).

1. Return visit shows roster, thread and drawer instantly, refreshing behind — justified
2. A crew written elsewhere now reaches the roster in place (registry-prefix invalidation) — justified
3. Cached member threads are forgotten on socket reconnect, except the open one — justified
4. Star flip lands a microtask after the click, with in-flight roster reads cancelled around the write — justified
5. Wake sources share the crew editor's and Schedule's queries; one fetch serves all three — justified
6. Four bare error boxes and the collision notice now render through ErrorNotice with agent hand-off — rides along (AUTOSDE-mandated)
7. New "Couldn't reconnect" copy when a repair fails over a mounted thread (12 locales) — justified
8. Collision copy reworded: "identifier"→"short name", "crews"→"members" — rides along (in-diff vocabulary mismatch)
9. Member count shows a dash, never "0 members", while the roster read has failed — rides along (declared)
10. ChatPane gains hideEmptyHint (1 consumer: MembersPage.tsx:1378) — justified

Watch

• The ['cron-jobs'] dedup has 1 counted unfixed sibling (grep 'cron-jobs': shared def + ExecutionsView.tsx:37) — accepted-and-deferred with a named mechanism (shrink-only coverage baseline). Clears when: ExecutionsView gains tests and spreads cronJobsQuery.

[FIRST-PRINCIPLES-REVIEWED] 3173bb6

[github-actions]

Design Review (Fable 5) — ✅ PASS

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

Design-Verdict: PASS

Right shape: the page joins the repo's mandated React Query pattern, and the trust-sensitive thread-key cache carries explicit reconnect invalidation, re-confirmation, and pins.

Suggestions

• crewWebhooksQueryKey now has three consumers each restating queryFn: () => api.webhooks() inline (MembersPage, KiroCrewAgentsPage, CrewWebhookSection) — the exact divergence hazard cronJobsQuery's comment names; extract the same shared query object while you're here.

[DESIGN-REVIEWED] 3173bb6

[github-actions]

UX Review (Fable 5) — ✅ PASS

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

UX-Verdict: PASS

Every reworded notice survived the cold read — the blind reader understood each failure state and knew the next step without seeing the code.

Watch

• This diff puts "Ask the agent" on five error surfaces at once; the blind reader dared to click it everywhere but could not predict the action — "does it send a message somewhere on my behalf?" (Could-not-tell list). Error-state-only frequency, hesitation on the sole remedy, every failure. The label lives in the shared ErrorNotice, so the fix (a verb+outcome label or tooltip) is component-level follow-up, not this PR.

Suggestions

• In the roster-failure state (06-roster-load-error.png) the filter pills still assert "Mine 0 · Built-in 0 · From packages 0" one line under the new "—" count — the PR's own rationale ("would state as fact what is only unknown") applies to the pill counts; dash or hide them under loadError in MembersPage.tsx.

[UX-REVIEWED] 3173bb6

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

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

Review details

The candidate list contains no candidates, and my independent checks of the highest-risk new logic all hold:

• defaultAgentQuery/cronJobsQuery/crewWebhooksQueryKey consumers share identical queryFns/shapes — no first-observer shape divergence.
• wakeLoaded/wakeFailed correctly treat '' (a resolved default agent) as answered.
• The reconnect re-POST (hadConnectionRef false→skip on first connect, re-POST only on true→false→true) matches its contract; useConnected is a plain store selector.
• forgetUnobservedMemberThreads preserves the open entry via getObserversCount(), and the skipToken read does register an observer (pinned by membersQuery.test.ts).
• The star useMutation cancels in-flight refetches in onMutate/onSuccess/onError before each per-row patch; the synchronous setStarPending lock guards the double-click race.

Nothing survives to the 80+ bar, and I found no groundable Step-2 addition.

No findings.

[OPUS-REVIEWED] 3173bb6

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

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

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

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

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 3173bb6

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

[CrysisDeu]

Round 2 (pushing shortly) addresses every lane:

Frontend Tests (4) — three MembersPage cases red, all real, all mine:

• The new "second mount refreshes in the background" pin asserted a second GET /api/members; the roster carries its own 30 s staleTime, so a remount inside that window must NOT refetch — that is the request the user stopped paying for. The pin now asserts exactly that (no request, cache rendered), and the refresh-behind path stays pinned by the invalidation case.
• "URL naming a member wins" (2 POSTs) and "gone member … SAYS so" (memory overwritten): a real defect this PR introduced. The roster is now a React Query store, so the thread endpoint confirming a key (which patches the roster row) re-renders on React's sync lane — and that render can commit BEFORE the default-lane setActiveName from the open, so the URL-sync effect re-ran with the old name and opened the same member twice (for a stand-in open, also writing the memory it must leave alone). Fixed by guarding the effect on a ref activate writes synchronously (activeNameRef), not on the lagging state. The two existing cases are the regression pins.

GPT F1 (errors-use-error-notice) — the four bare role="alert" boxes (roster, thread failure, activity, wake) now render through ErrorNotice (inline, askAgent on the three read failures; off on the thread failure, since the cached composer may sit under it). The slug-collision line is a fact about the roster, not an error, and stays a separate role="status" notice.

GPT F2 (star cache vs server) — fixed rather than overridden: the row's pending lock is now taken synchronously in toggleStar before the mutation's async onMutate, and onSuccess re-applies the confirmed value so a refetch that started after the cancelQueries and resolved with the pre-write row cannot leave the wrong star showing.

Design Review (instant-mount of a cached thread key across a restart) — a dropped-then-restored socket is the one client-visible sign the gateway may have restarted, so: useWebSocket now forgets every unobserved ['member-thread'] entry on reconnect (the next open waits for the endpoint again, as before this PR), and the page re-confirms the observed one on that same reconnect through the same idempotent POST every open makes. MEMBER_THREAD_GC_MS makes the entries' lifetime an explicit, documented policy in membersQuery.ts.

First Principles subtractions — MEMBERS_ROSTER_STALE_MS is module-private now, and the three inline ['cron-jobs'] spellings (ExecutionsView, AutoNudgePopover, MembersPage) share one cronJobsQuery definition in website/src/api/cronJobsQuery.ts.

[CrysisDeu]

Round 3 (c9ec2f8): GPT F1 was right — a skipToken observer does not make its query active, so removeQueries({ type: 'inactive' }) on reconnect would have cleared the OPEN member's entry, unmounted the pane and dropped the draft. The sweep is now forgetUnobservedMemberThreads in membersQuery.ts, which removes only entries with zero observers; membersQuery.test.ts pins exactly that combination (a subscribed skipToken observer keeps its entry, an unobserved one is dropped). Backend Tests (Windows) (3) is a backend shard on a frontend-only diff; re-run with this push.

[CrysisDeu]

Round 4 (dfbea9e, rebased on main): GPT F1 — the thread-failure ErrorNotice now carries the rule's No hand-off: comment naming the draft it protects (the DM composer of the cached thread mounted under it). Coverage Gate — the per-file floor tripped on ExecutionsView.tsx (46.3% vs 46.4% allowed) because migrating it to cronJobsQuery removed a covered statement from a baselined file; the baseline is shrink-only by design, so ExecutionsView keeps its inline spelling (noted in cronJobsQuery.ts) and the shared definition serves AutoNudgePopover and the Members page.

[CrysisDeu]

Round 7 (bfeef08, rebased on main) takes all three UX items: (1) the reconnect copy drops the self-refuting middle clause — now "Couldn't reconnect this conversation. Reopen the member to try again." in all 12 locales (+en-XA), test updated; (2) two more evidence captures — 06-roster-load-error.png (roster ErrorNotice with Ask the agent) and 07-slug-collision.png (a REAL collision: the pod is seeded with oncall + Oncall, the second open is refused and the notice names the owner); (3) both GIFs re-encoded with a global palette + per-frame differencing — 0.77 MB and 0.81 MB instead of 3.7 / 4.0 MB — from freshly re-seeded pods with the light theme pinned, so the pair is like for like. 05-error-states.png re-captured with the new copy.

[CrysisDeu]

Round 8 (be2417a, rebased on main). UX, all four: (1) the pane no longer says "Session ready" under a failed-repair notice — new optional hideEmptyHint prop on ChatPane, set by the Members page while activeThreadFailed; 05-error-states.png re-captured; (2) 08-cold-open-failure.png added — thread POST failed with nothing cached, empty column under "Could not open…"; (3) slug_collision says "Rename one of the two members" (12 locales; 07 re-captured); (4) the member count shows a dash while the roster read has failed (06 re-captured, test pinned). First Principles: MEMBER_THREAD_GC_MS deleted — it restated react-query's default; the reconnect sweep is the control. Frontend Tests (1) on round 7 failed in FileChangeChipsAnim.test.tsx (untouched by this PR) and was re-run; this push re-runs everything anyway.

[CrysisDeu]

Round 9 (3173bb6). GPT F1 — fixed rather than overridden: onSuccess and onError now await cancelQueries(roster) before re-applying the final row, so a refetch that started during the PUT cannot land its pre-write snapshot on top of the confirmed star. UX — the cold-open failure notice offers Ask the agent (askAgent={!activeSlot}; the hand-off stays off only while a cached thread with a possible draft is mounted underneath) — 08 re-captured; and slug_collision now uses plain words ("shares its short name with {{name}}, whose conversation already uses it…", 12 locales) — 07 re-captured.