Skip to content

patch cache authoritatively after merge/close/toggle-draft#30

Merged
AntonNiklasson merged 1 commit into
mainfrom
an/cache-authoritative-patches
May 6, 2026
Merged

patch cache authoritatively after merge/close/toggle-draft#30
AntonNiklasson merged 1 commit into
mainfrom
an/cache-authoritative-patches

Conversation

@AntonNiklasson
Copy link
Copy Markdown
Owner

Summary

Fixes stale dashboard state after mutations: previously, merged PRs would reappear on the next poll because the post-mutation fire-and-forget resync re-fetched from GitHub's search index, which is eventually consistent and often still returned the just-merged PR as open.

  • Server now patches the cache synchronously after pulls.merge, close, and toggle-draft succeed.
  • A mutation marker with a 60s TTL is recorded; subsequent resyncs filter their GitHub response through it so a stale fetch can't resurrect the PR.
  • Merge queue handled naturally: pulls.merge fails on queue-enabled repos, no marker recorded, fallback path arms auto-merge as before.

Test plan

  • New tests in routes.test.ts simulate stale GitHub responses for merge/close/toggle-draft and assert the cache stays correct (these failed before the fix).
  • Manually merge a PR via the dashboard — confirm it doesn't reappear after the next 10s poll.
  • Manually mark a draft PR ready — confirm the ready state sticks.

GitHub's search/list APIs are eventually consistent — for several
seconds after a merge or draft-toggle they may still return the old
state. The mutation route's fire-and-forget resync was picking up
that stale data and overwriting the client's optimistic update on
the next 10s poll, so merged PRs reappeared.

Now the route patches the cache directly with the known outcome and
records a mutation marker (60s TTL). Subsequent resyncs filter their
GitHub response through the marker, so a stale fetch can't resurrect
the PR. The marker expires once GitHub has caught up.
@AntonNiklasson AntonNiklasson marked this pull request as ready for review May 6, 2026 06:45
Copilot AI review requested due to automatic review settings May 6, 2026 06:45
@AntonNiklasson AntonNiklasson merged commit a458fc8 into main May 6, 2026
2 checks passed
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR prevents stale dashboard state after PR mutations by synchronously updating server cache on merge/close/toggle-draft and by adding a short-lived mutation marker that filters out eventually-consistent GitHub search results during subsequent resyncs.

Changes:

  • Add an in-memory mutation marker (60s TTL) and apply it during resync so stale GitHub results can’t “resurrect” removed/old PR state.
  • Patch cached prs/reviews immediately after successful merge/close, and patch cached prs after successful toggle-draft.
  • Add tests that simulate stale GitHub responses and assert the cache remains correct.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
packages/server/src/sync.ts Tracks recent mutations and filters resync results to prevent stale GitHub search results from reintroducing old state.
packages/server/src/routes.ts Patches cache immediately after merge/close/toggle-draft and records mutation markers before scheduling background resync.
packages/server/src/routes.test.ts Adds regression tests covering stale GitHub responses after merge/close/toggle-draft.
packages/server/src/cache.ts Introduces patchCache helper for in-place cache updates.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +29 to +39
return (data: CachedListItem[] | null): CachedListItem[] =>
(data ?? []).filter(
(item) => !(item.repo === repo && item.number === number),
);
}

function setDraftInList(repo: string, number: number, draft: boolean) {
return (data: CachedListItem[] | null): CachedListItem[] =>
(data ?? []).map((item) =>
item.repo === repo && item.number === number ? { ...item, draft } : item,
);
Comment on lines +36 to +39
return (data: CachedListItem[] | null): CachedListItem[] =>
(data ?? []).map((item) =>
item.repo === repo && item.number === number ? { ...item, draft } : item,
);
Comment on lines +58 to +60
export function patchCache<T>(key: string, fn: (data: T | null) => T): void {
setCached(key, fn(getCached<T>(key)));
}
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