Skip to content

fix(tasks): source flips serve each side's own work-item cache instead of stale shared entries#8738

Draft
moseoh wants to merge 3 commits into
stablyai:mainfrom
moseoh:fix/work-items-cache-eviction-scope
Draft

fix(tasks): source flips serve each side's own work-item cache instead of stale shared entries#8738
moseoh wants to merge 3 commits into
stablyai:mainfrom
moseoh:fix/work-items-cache-eviction-scope

Conversation

@moseoh

@moseoh moseoh commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Summary

Related to #8726 (item B). Stacked on #8727 — the first commit here is that PR; review the last two commits. Draft until #8727 lands, then this rebases to its own two commits.

Toggling the Tasks Upstream/Origin selector without pressing the manual refresh button showed stale or empty lists: after one flip the rows could stay on the previous source while the pager shrank to the new source's count; after flipping again the list could go empty and stay empty. Only the refresh button recovered. Reproduces on main for both the Issues and PRs tabs.

Two defects compound:

  1. The flip's cache eviction matched nothing. TaskPage keys its work-item cache with a task-source scope prefix (github:local:…::repoId::limit::query) even for plain local repos, but setIssueSourcePreference evicted only keys starting with bare repoId::/repoPath:: — a complete no-op for every Tasks-page entry, and the same mismatch silently skipped scoped entries in evictGitHubRepoCaches and the in-flight dedupe clearing. The post-flip effect then kept serving the old source's still-fresh (60s TTL) cache, while the forced refetch's correct result reached the store cache but never the visible rows (the page/cache reconciliation only patches rows present in both).
  2. Both sources fought over one cache identity. Upstream and origin route to different repos, but their results shared a single key — which is why eviction was load-bearing at all.

This PR fixes both layers:

  • Commit 1 — make repo-scoped eviction actually match scoped keys. matchesRepoCacheKey now also matches the ::repoId:: segment boundary, covering task-source-scoped and execution-host-scoped keys. This independently fixes evictGitHubRepoCaches (repo removal) skipping every scoped entry.
  • Commit 2 — make the flip not need eviction at all. The work-item cache key now includes the repo's effective source side: repoId::<upstream|origin>::limit::query. Upstream and origin entries coexist, so a flip just reads the other side's key — served instantly while still fresh, refreshed by the existing forced fetch. Pre-flip in-flight requests get their own dedupe slot, removing the collision class the eviction dance guarded against. setIssueSourcePreference drops its eviction and keeps the defensive in-flight clear plus the nonce bump that re-runs the Tasks fetch effect.

The key segment is the effective side, not the raw preference: auto routes upstream-first exactly like an explicit 'upstream' pick (per #8727), so both share one entry and pinning the already-highlighted Upstream pill is a cache hit rather than a spurious refetch of identical data.

Action Before After
Flip Upstream→Origin rows stay upstream, pager shows origin count origin rows (instant if cached ≤60s, else fetched)
Flip back within 60s often permanently empty until manual refresh previous upstream rows instantly, no network call
Pin the highlighted Upstream pill under auto n/a (was origin-routed for PRs) cache hit — no refetch
Repo eviction (evictGitHubRepoCaches) scoped entries silently survived evicted

Screenshots

before.mov
after.mov

Testing

  • pnpm lint
  • pnpm typecheck
  • pnpm test — 5 new regression tests (written first, red on the old behavior): scoped-key repo eviction; flip keeps the other side's entries + bumps the invalidation nonce; post-flip fetch does not dedupe onto a pre-flip in-flight request; toggling back within TTL is a cache hit (2 network calls for 3 states); auto and explicit 'upstream' share one entry. Store, TaskPage, github-component, and src/main/github suites: 2,325 tests pass.
  • pnpm build
  • Verified in the dev app on a fork: every toggle shows the selected source's rows; toggling back within a minute renders instantly.

AI Review Report

Adversarial race-trace of the toggle path (Codex, plus the authoring agent's verification of each load-bearing claim against the code):

  • Root cause confirmed statically — the scope-prefixed key shape (getTaskSourceCacheScopeworkItemsCacheKey) vs the bare-prefix eviction was proven by construction, then reproduced as a red unit test; no flaky manual repro required.
  • Interleaving trace — the stale/empty symptoms come from the optimistic repo patch and the repos:changed echo re-running the fetch effect around the forced refetch, whose .then is cancelled; with per-side keys the surviving non-forced run reads the correct side's entry, so the trace is benign now.
  • Matcher false positives::repoId:: containment requires the repo id (a UUID) at a :: segment boundary; cross-repo collision would need one repo's UUID embedded in another's scope, which the scope construction cannot produce.
  • Persisted caches — legacy-format keys restored from disk are never selected again and age out through the bounded cache; repo evictions still match them via the repoId segment.
  • SSH/remote hosts — execution-host-scoped keys (hostId::repoId::…) had the same eviction escape and are covered by the same matcher fix; key construction threads through the existing host/source-scope plumbing unchanged.
  • GitLab/Linear/Jira — untouched; the change is GitHub work-item cache identity only.

Security Audit

No new inputs, IPC, command execution, or dependencies — renderer cache-key construction, eviction matching, and tests only.

Notes

  • Stacked on fix(tasks): resolve PR work items upstream-first under 'auto' like issues #8727: the effective-side key normalization (autoupstream) is only correct once auto routes PRs upstream-first. Kept as draft until that lands.
  • Known separate factor: pagination totals still blink on flips — the count is an independent fire-and-forget query with no renderer cache (the effect nulls totalItemCount at dispatch, so the pager runs degraded until the count lands), and count queries always use gh api --cache 120s with no bypass. The row-side fix here doesn't cover it; the natural follow-up is a per-side count cache mirroring this PR's key scheme, plus (optionally) a cache bypass on flips.
  • Pre-existing chip flicker, slightly more visible now: the Upstream/Origin selector renders from the cache entry's sources metadata, so it unmounts during the first fetch of a side (per Route task PR queries by upstream source #5176's "render nothing until metadata arrives" rule) and remounts on response. With per-side entries this brief flicker can happen once per side instead of once per repo. Not addressed here — persisting the raw candidates per repo (or falling back to any cached entry's sources) would keep the chip mounted; happy to follow up.
  • Not addressed — checks/merge badge flicker on refreshes and flips: PR rows' checks/merge badges hydrate lazily per row, and a forced refresh replaces rows with fresh list objects that carry no such metadata, blanking the badges until the lazy reload. An attempt to route forced refreshes through the landing probe's reconcile did not remove the flicker (the fresh rows' status signature differs whenever hydrated metadata is present, so reconcile still replaces them), so it was backed out rather than shipped half-working; a real fix likely needs to carry hydrated fields across row replacement. Tracked as a follow-up.
  • Follow-up candidate: the repo-cache keys remain stringly-typed with three shapes (bare, host-scoped, source-scoped), which is what made the substring matcher necessary. A structured key (or a repoId→keys registry) would make eviction membership explicit; left out to keep this change reviewable.

moseoh added 3 commits July 19, 2026 12:48
…sues

On a fork, 'auto' routed the Tasks PR list/count to origin — the fork's own
PR list, which is almost always empty — while highlighting the Upstream pill
(issues resolve upstream-first). The PRs tab showed 'No matching GitHub work'
until the user explicitly clicked Upstream, and the highlighted pill
contradicted where the data actually came from.

stablyai#5176 routed explicit picks through the preference for both sides but
deferred the 'auto' side. This aligns it: 'auto' resolves PRs upstream-first
exactly like issues; an explicit 'origin' pick still pins PRs to the fork.
Toggling the Tasks Upstream/Origin selector without a manual refresh kept
showing the previous source's rows (or an empty list) until the refresh
button was pressed. The preference-flip eviction and in-flight clearing
matched only bare 'repoId::'/'repoPath::' key prefixes, but TaskPage keys
its work-item cache with a task-source scope prefix
('github:local:...::repoId::limit::query') even for plain local repos —
so the flip evicted nothing, and the post-flip effect kept serving the old
source's still-fresh cache while the forced refetch's correct result only
reached the store cache, never the visible pages.

Broaden the shared repo-cache-key matcher to also match scoped keys
(':: + prefix' segment boundary) and route setIssueSourcePreference's
inline eviction through it. This also covers execution-host-scoped keys
('hostId::repoId::...') that escaped repo evictions the same way.
…ides coexist

Scope the work-item cache key with the repo's effective source side
('repoId::<upstream|origin>::limit::query'). Upstream and origin results
now live in separate entries, so a source flip reads the other side's key —
served instantly while still fresh — instead of evicting shared state and
refetching from scratch. Pre-flip in-flight requests also get their own
dedupe slot, removing the collision class the eviction dance guarded
against; setIssueSourcePreference keeps only the defensive in-flight clear
and the nonce bump that re-runs the Tasks fetch effect.

The key segment is the effective side, not the raw preference: 'auto'
routes upstream-first exactly like an explicit 'upstream' pick (previous
commit), so both share one entry and pinning the already-highlighted pill
is a cache hit rather than a spurious refetch. Legacy-format persisted keys
are never selected again and age out through the bounded cache; repo
evictions still match them via the repoId segment.
@moseoh
moseoh force-pushed the fix/work-items-cache-eviction-scope branch from 3cadfe3 to ec1f718 Compare July 19, 2026 03:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants