crueber/walhub

Watch

Fork 0

Milestone reassignment shows stale membership in milestone-filtered issue lists until reload (client cache invalidation gap) #318

New issue

Closed

opened 2026-09-11 11:24:05 +00:00 by crueber · 3 comments

crueber commented

2026-09-11 11:24:05 +00:00

Owner

What's wrong

After removing a milestone from an issue and assigning a different one, opening the milestones page and clicking View issues for the new milestone navigates to the issue list filtered by that milestone — and the list shows stale membership (the issue appears under its old milestone grouping, or the newly-assigned issue is missing) until a full page reload fixes it.

Reproduction path

On an issue, remove its milestone and assign milestone B (the sidebar picker PATCHes the thread).
Navigate to the milestones page; milestone B's card shows the linked-issues list (and/or its "View issues" button).
Click through to /:owner/:name/issues?milestone=B — the filtered list is wrong on first paint; a browser reload corrects it.

Root cause analysis (code evidence — stale cache invalidation, two cooperating gaps)

The server is not the problem: PatchIssue updates the thread, calls s.updateIndex(..., cardOf(th)) (internal/issues/service.go:527 — Card.Milestone included, model.go:107), and moveMilestone adjusts the milestone counters. The issues-list endpoint is writeJSON with Cache-Control: no-store (internal/issues/http.go:416-424, :149). The stale read is client-side, in the useData promise-cache, via two gaps:

Gap 1 — milestone-PATCH invalidation never reaches the other pages' caches. The issue SSE frame maps to invalidation keys ["issue:{full}:{num}", "issues:{full}:*"] (web/src/lib/collab.js:43) — the issues:{full}:* prefix WOULD cover both stale surfaces: the milestones page's MilestoneIssues entry (key issues:{full}:milestone:{id}, Milestones.jsx:25) and the issues list window (key issues:{full}:{JSON(query)}, Issues.jsx:41). But useCollabStream subscribers are per-page and the issue page filters its frames: useCollabStream(..., ["issue","issue_event"], (frame) => Number(frame.num) === Number(num())) (Issue.jsx:364) — the accept callback is applied before invalidateCollab (collab.jsx:26-30), which is fine for same-issue frames, but the mutation's own invalidation therefore only runs in the tab that receives the frame. Navigating client-side (Solid router, no reload) carries the old useData LRU entries over: the milestones page renders MilestoneIssues from a pre-mutation cached window, and the filtered issues list on the new route can hit a cached window keyed identically to a pre-mutation query.

Gap 2 — the milestones page doesn't subscribe to issue frames at all. Milestones.jsx has no useCollabStream — so if the milestones page is open (or revisited client-side within the TTL/LRU horizon), its MilestoneIssues lists and counts never invalidate on issue mutations. The file header says "Refetches after every save" — but only milestone saves invalidate; issue-side membership changes (PATCH milestone on an issue) don't touch this page's caches.

Why a reload fixes it: a full reload drops the in-memory useData LRU and re-fetches through no-store endpoints — correct data. That matches the reported symptom exactly.

Fix direction

Invalidate on mutation, at the mutation site (primary fix). When the issue page's milestone picker (and label/assignee pickers — same class) completes a PATCH, invalidate the affected list keys directly, not just the thread entry: issues:{full}:* (all list windows) plus milestones:{full} (the milestones page counts). The mutation knows what it touched; don't rely on the SSE round-trip through a page that may unmount mid-navigation. Precedent: landedVisible in Import.jsx invalidates cross-page keys after its own mutation.
Subscribe the milestones page to issue frames (useCollabStream(full, repoClient, ["issue"]) — no accept filter) so its MilestoneIssues windows and counts self-heal while mounted, matching Issues.jsx:52.
Audit the accept-before-invalidate pattern: filtering frames on the issue page means frames for other issues' mutations don't invalidate shared list caches in that tab. The filter should gate refetching the thread, not cache invalidation — or the invalidation for issues:{full}:* should run regardless of frame.num. This is the systemic half; verify against invalidateCollab and keep the narrow fix minimal.

Acceptance criteria

The repro path shows correct membership on first paint: remove milestone A → assign B → milestones page → View issues for B → the issue appears under B, and A's list no longer shows it, with no reload.
The milestones page's inline issue lists and open/closed counts update without reload when an issue's milestone changes while the page is mounted.
Issue-page PATCH invalidates issues:{full}:* and milestones:{full} at the mutation site (not dependent on SSE delivery to a still-mounted page).
The accept-filter on the issue page's stream no longer suppresses shared-cache invalidation (or an equivalent narrow fix is documented).
No new endpoints; the server path is already correct (index + card update verified) — this is a client cache-invalidation fix.
Headless test for the invalidation key coverage (PATCH → expected invalidated key set), per the collabKeys test precedent in web/test/unit/.

## What's wrong After removing a milestone from an issue and assigning a different one, opening the milestones page and clicking **View issues** for the new milestone navigates to the issue list filtered by that milestone — and the list shows **stale membership** (the issue appears under its old milestone grouping, or the newly-assigned issue is missing) until a full page reload fixes it. ## Reproduction path 1. On an issue, remove its milestone and assign milestone B (the sidebar picker PATCHes the thread). 2. Navigate to the milestones page; milestone B's card shows the linked-issues list (and/or its "View issues" button). 3. Click through to `/:owner/:name/issues?milestone=B` — the filtered list is wrong on first paint; a browser reload corrects it. ## Root cause analysis (code evidence — stale cache invalidation, two cooperating gaps) The server is not the problem: `PatchIssue` updates the thread, calls `s.updateIndex(..., cardOf(th))` (`internal/issues/service.go:527` — `Card.Milestone` included, `model.go:107`), and `moveMilestone` adjusts the milestone counters. The issues-list endpoint is `writeJSON` with `Cache-Control: no-store` (`internal/issues/http.go:416-424`, `:149`). The stale read is client-side, in the `useData` promise-cache, via two gaps: **Gap 1 — milestone-PATCH invalidation never reaches the other pages' caches.** The `issue` SSE frame maps to invalidation keys `["issue:{full}:{num}", "issues:{full}:*"]` (`web/src/lib/collab.js:43`) — the `issues:{full}:*` prefix WOULD cover both stale surfaces: the milestones page's `MilestoneIssues` entry (key `issues:{full}:milestone:{id}`, `Milestones.jsx:25`) and the issues list window (key `issues:{full}:{JSON(query)}`, `Issues.jsx:41`). But `useCollabStream` subscribers are per-page and the **issue page filters its frames**: `useCollabStream(..., ["issue","issue_event"], (frame) => Number(frame.num) === Number(num()))` (`Issue.jsx:364`) — the `accept` callback is applied *before* `invalidateCollab` (`collab.jsx:26-30`), which is fine for same-issue frames, but the mutation's own invalidation therefore only runs in the tab that receives the frame. Navigating client-side (Solid router, no reload) carries the old `useData` LRU entries over: the milestones page renders `MilestoneIssues` from a pre-mutation cached window, and the filtered issues list on the new route can hit a cached window keyed identically to a pre-mutation query. **Gap 2 — the milestones page doesn't subscribe to `issue` frames at all.** `Milestones.jsx` has no `useCollabStream` — so if the milestones page is open (or revisited client-side within the TTL/LRU horizon), its `MilestoneIssues` lists and counts never invalidate on issue mutations. The file header says "Refetches after every save" — but only *milestone* saves invalidate; issue-side membership changes (PATCH milestone on an issue) don't touch this page's caches. **Why a reload fixes it:** a full reload drops the in-memory `useData` LRU and re-fetches through `no-store` endpoints — correct data. That matches the reported symptom exactly. ## Fix direction 1. **Invalidate on mutation, at the mutation site (primary fix).** When the issue page's milestone picker (and label/assignee pickers — same class) completes a PATCH, invalidate the affected list keys directly, not just the thread entry: `issues:{full}:*` (all list windows) plus `milestones:{full}` (the milestones page counts). The mutation knows what it touched; don't rely on the SSE round-trip through a page that may unmount mid-navigation. Precedent: `landedVisible` in `Import.jsx` invalidates cross-page keys after its own mutation. 2. **Subscribe the milestones page to `issue` frames** (`useCollabStream(full, repoClient, ["issue"])` — no `accept` filter) so its `MilestoneIssues` windows and counts self-heal while mounted, matching `Issues.jsx:52`. 3. **Audit the `accept`-before-invalidate pattern**: filtering frames on the issue page means frames for *other* issues' mutations don't invalidate shared list caches in that tab. The filter should gate *refetching the thread*, not *cache invalidation* — or the invalidation for `issues:{full}:*` should run regardless of `frame.num`. This is the systemic half; verify against `invalidateCollab` and keep the narrow fix minimal. ## Acceptance criteria - [ ] The repro path shows correct membership on first paint: remove milestone A → assign B → milestones page → View issues for B → the issue appears under B, and A's list no longer shows it, with no reload. - [ ] The milestones page's inline issue lists and open/closed counts update without reload when an issue's milestone changes while the page is mounted. - [ ] Issue-page PATCH invalidates `issues:{full}:*` and `milestones:{full}` at the mutation site (not dependent on SSE delivery to a still-mounted page). - [ ] The `accept`-filter on the issue page's stream no longer suppresses shared-cache invalidation (or an equivalent narrow fix is documented). - [ ] No new endpoints; the server path is already correct (index + card update verified) — this is a client cache-invalidation fix. - [ ] Headless test for the invalidation key coverage (PATCH → expected invalidated key set), per the collabKeys test precedent in `web/test/unit/`.

crueber added the

issues

label

2026-09-11 11:24:05 +00:00

crueber added this to the v1 milestone

2026-09-11 11:24:05 +00:00

crueber referenced this issue from a commit

2026-09-11 11:55:51 +00:00

Fix #318: cross-page cache invalidation for thread mutations (08 §4)

crueber referenced this issue from a pull request that will close it,

2026-09-11 11:55:55 +00:00

Fix #318: cross-page cache invalidation #321

crueber commented

2026-09-11 11:55:59 +00:00

Author

Owner

Fix ready for review: #321 (branch fix/issue-318). Mutation-site invalidation (invalidateIssueLists) on all 8 issue-page mutation tails, milestones page subscribes to issue frames, issue frame map gains milestones:{full}, issue-page accept filter dropped. Tests green (14/14 targeted; full suite green except pre-existing smoke.test.js environmental hang, identical on clean main); vite build passes; browser check open (loopback blocked). Not merged — awaiting review.

Fix ready for review: #321 (branch `fix/issue-318`). Mutation-site invalidation (`invalidateIssueLists`) on all 8 issue-page mutation tails, milestones page subscribes to `issue` frames, `issue` frame map gains `milestones:{full}`, issue-page accept filter dropped. Tests green (14/14 targeted; full suite green except pre-existing `smoke.test.js` environmental hang, identical on clean main); vite build passes; browser check open (loopback blocked). Not merged — awaiting review.

crueber commented

2026-09-11 12:05:10 +00:00

Author

Owner

Review of PR #321 (fix/issue-318), verified in scratch worktree /tmp/pr321 (since removed):

PASS — prefix scope (data.js:326-338): issues:{full}: covers both stale surfaces — Issues.jsx:39 query windows AND Milestones.jsx:32 milestone:{id} entries — plus milestones:{full} counts. Per-repo scoping confirmed by test (other repo + thread keys untouched). No over-invalidation across repos/threads.

PASS — all 8 mutation tails routed via afterMutation (Issue.jsx): comment, commentAndClose, close(reason), reaction menu, summary chips, generic patch (title/body/reopen), labels, milestone. No missed path: assignees are display-only (Issue.jsx:508, no mutation UI), events pagination is a read.

PASS — accept-filter drop safe: per-key self-scoping holds. This page caches exactly one issue:{full}:{num} + its events window; other nums' keys miss cache = silent no-ops. Own frames still invalidate; removal only ADDS shared-prefix invalidation. No page misses its own updates.

PASS — no Milestones double-invalidate problem: pages never mount simultaneously (one route); Milestones SSE goes through scheduleInvalidate (coalesced per-tick set). Only redundancy is the issue page's own SSE echo refetching list keys afterMutation already hit — 1 extra coalesced batch per human-rate click, single-flighted. Bounded, acceptable.

PASS — P7/no polling: no new timers; Milestones reuses useCollabStream SSE; afterMutation is synchronous direct invalidate.

PASS — storm risk: 8 tails x prefix is human-rate (button clicks), each invalidates only the handful of cached keys under the repo prefix. SSE-burst coalescing untouched. No sequential store round trips added (client cache only).

PASS — no new deps (package.json untouched; imports are existing modules). Doc entry (08_ui_sdk.md) matches code, including the pull-pages follow-up note. Whitespace re-indent of the #311 bullet is churn but harmless.

VERIFY: node tests 592/592 pass (320+272 across two batches; full glob in one call exceeds the 180s runner window — batch 2 alone takes ~170s due to timer-based SDK tests, pre-existing). New issue-invalidation.test.js 4/4 + collab-lib pin pass. vite build + esbuild SDK bundle succeed (chunk-size warning pre-existing). No browser drive (node tests + reasoning, per review brief).

No fixes pushed — nothing to fix. MERGE RECOMMENDATION: ready to merge.

Review of PR #321 (fix/issue-318), verified in scratch worktree /tmp/pr321 (since removed): PASS — prefix scope (data.js:326-338): `issues:{full}:` covers both stale surfaces — Issues.jsx:39 query windows AND Milestones.jsx:32 `milestone:{id}` entries — plus `milestones:{full}` counts. Per-repo scoping confirmed by test (other repo + thread keys untouched). No over-invalidation across repos/threads. PASS — all 8 mutation tails routed via afterMutation (Issue.jsx): comment, commentAndClose, close(reason), reaction menu, summary chips, generic patch (title/body/reopen), labels, milestone. No missed path: assignees are display-only (Issue.jsx:508, no mutation UI), events pagination is a read. PASS — accept-filter drop safe: per-key self-scoping holds. This page caches exactly one `issue:{full}:{num}` + its events window; other nums' keys miss cache = silent no-ops. Own frames still invalidate; removal only ADDS shared-prefix invalidation. No page misses its own updates. PASS — no Milestones double-invalidate problem: pages never mount simultaneously (one route); Milestones SSE goes through scheduleInvalidate (coalesced per-tick set). Only redundancy is the issue page's own SSE echo refetching list keys afterMutation already hit — 1 extra coalesced batch per human-rate click, single-flighted. Bounded, acceptable. PASS — P7/no polling: no new timers; Milestones reuses useCollabStream SSE; afterMutation is synchronous direct invalidate. PASS — storm risk: 8 tails x prefix is human-rate (button clicks), each invalidates only the handful of cached keys under the repo prefix. SSE-burst coalescing untouched. No sequential store round trips added (client cache only). PASS — no new deps (package.json untouched; imports are existing modules). Doc entry (08_ui_sdk.md) matches code, including the pull-pages follow-up note. Whitespace re-indent of the #311 bullet is churn but harmless. VERIFY: node tests 592/592 pass (320+272 across two batches; full glob in one call exceeds the 180s runner window — batch 2 alone takes ~170s due to timer-based SDK tests, pre-existing). New issue-invalidation.test.js 4/4 + collab-lib pin pass. vite build + esbuild SDK bundle succeed (chunk-size warning pre-existing). No browser drive (node tests + reasoning, per review brief). No fixes pushed — nothing to fix. MERGE RECOMMENDATION: ready to merge.

crueber closed this issue

2026-09-11 12:06:05 +00:00

crueber commented

2026-09-11 12:06:06 +00:00

Author

Owner

Fixed by PR #321 (review clean; prefix scope + all-tails + no-storm verified; 592/592), merged. Closing.

crueber referenced this issue from a commit

2026-09-11 12:06:06 +00:00

Merge pull request 'Fix #318: cross-page cache invalidation' (#321) from fix/issue-318 into main

crueber referenced this issue from a commit

2026-09-11 12:25:38 +00:00