From 70f84fd1e8173f80e7921b73df9c9ad3ea16e1a5 Mon Sep 17 00:00:00 2001 From: Anton Niklasson Date: Fri, 13 Nov 2026 15:01:28 +0100 Subject: [PATCH] Expose persisted sync health and warn when board data is stale --- packages/harness/src/board.e2e.ts | 8 ++ packages/harness/src/github.test.ts | 14 ++++ packages/harness/src/stale.e2e.ts | 24 ++++++ packages/server/src/routes.ts | 7 ++ packages/sync/src/cache/open.test.ts | 21 +++++ packages/sync/src/cache/open.ts | 12 ++- packages/sync/src/cache/schema.ts | 9 ++- packages/sync/src/cache/store.test.ts | 45 +++++++++++ packages/sync/src/cache/store.ts | 90 +++++++++++++++++++--- packages/sync/src/engine.ts | 59 +++++++++++--- packages/sync/src/index.ts | 2 + packages/web/src/App.tsx | 10 +++ packages/web/src/api.ts | 11 +++ packages/web/src/components/SyncHealth.tsx | 68 ++++++++++++++++ 14 files changed, 356 insertions(+), 24 deletions(-) create mode 100644 packages/harness/src/stale.e2e.ts create mode 100644 packages/web/src/components/SyncHealth.tsx diff --git a/packages/harness/src/board.e2e.ts b/packages/harness/src/board.e2e.ts index 8e6acd9..548f56f 100644 --- a/packages/harness/src/board.e2e.ts +++ b/packages/harness/src/board.e2e.ts @@ -64,12 +64,20 @@ test("polls queue entry, queue failure, and recovery through the real sync engin { number: 101, ciStatus: "failure" }, ]); await expect(card.getByText("failing", { exact: true })).toBeVisible(); + await expect( + page.getByLabel("Sync status: Sync failed — data may be stale", { + exact: true, + }), + ).toBeVisible(); scenario.setPhase("in-flight"); await scenario.syncPrs(); await expect(card.getByText("passing", { exact: true })).toBeVisible(); await expect(card).toHaveCount(1); expect(await scenario.persistedPrs()).toHaveLength(1); + await expect( + page.getByLabel("Sync status: Up to date", { exact: true }), + ).toBeVisible(); expect(errors).toEqual([]); expect(apiResponses.length).toBeGreaterThan(3); expect(apiResponses.every((response) => response.status === 200)).toBe( diff --git a/packages/harness/src/github.test.ts b/packages/harness/src/github.test.ts index 4300e53..fdb0f83 100644 --- a/packages/harness/src/github.test.ts +++ b/packages/harness/src/github.test.ts @@ -30,6 +30,11 @@ it("syncs queue transitions into SQLite, preserves the last snapshot on failure, expect(prs).toHaveLength(1); expect(prs[0]).toMatchObject({ number: 101, inMergeQueue, ciStatus }); } + const successful = engine + .getHealth() + .find((health) => health.kind === "prs")!; + expect(successful).toMatchObject({ status: "updated", reason: null }); + expect(successful.lastSuccessAt).toBeTruthy(); github.setPhase("offline"); const failed = await engine.sync(); expect(failed.results[0].fetches.every((f) => f.status === "failed")).toBe( @@ -38,8 +43,17 @@ it("syncs queue transitions into SQLite, preserves the last snapshot on failure, expect(engine.listPullRequests(github.instance.id, "prs")[0].ciStatus).toBe( "failure", ); + expect( + engine.getHealth().find((health) => health.kind === "prs"), + ).toMatchObject({ + status: "failed", + lastSuccessAt: successful.lastSuccessAt, + }); await engine.close(); engine = createSync(options); + expect( + engine.getHealth().find((health) => health.kind === "prs")?.status, + ).toBe("failed"); expect(engine.listPullRequests(github.instance.id, "prs")).toHaveLength(1); expect(engine.listPullRequests(github.instance.id, "prs")[0]).toMatchObject( { inMergeQueue: false, ciStatus: "failure" }, diff --git a/packages/harness/src/stale.e2e.ts b/packages/harness/src/stale.e2e.ts new file mode 100644 index 0000000..1ac6823 --- /dev/null +++ b/packages/harness/src/stale.e2e.ts @@ -0,0 +1,24 @@ +import { test, expect } from "@playwright/test"; +import { startScenario } from "./runtime.js"; + +test("recomputes stale age when unchanged health is polled", async ({ + page, +}) => { + const scenario = await startScenario(); + try { + await scenario.syncPrs(); + await page.goto(scenario.url); + await expect( + page.getByLabel("Sync status: Up to date", { exact: true }), + ).toBeVisible(); + // Advance client time without waiting 90 seconds. The persisted upstream + // timestamp remains unchanged; structural sharing must not hide its age. + await page.clock.install(); + await page.clock.fastForward(120_000); + await expect( + page.getByLabel("Sync status: Data is stale", { exact: true }), + ).toBeVisible(); + } finally { + await scenario.close(); + } +}); diff --git a/packages/server/src/routes.ts b/packages/server/src/routes.ts index c11333d..614f4fc 100644 --- a/packages/server/src/routes.ts +++ b/packages/server/src/routes.ts @@ -68,6 +68,13 @@ api.post("/config/create", (c) => { return c.json(result); }); +api.get("/sync/health", async (c) => { + if (process.env.DEMO === "1") return c.json([]); + const engine = getSync(); + await engine.reconcileConfiguration(); + return c.json(engine.getHealth()); +}); + // Authored PRs with CI + review status api.get("/:instanceId/prs", async (c) => { const { instanceId } = c.req.param(); diff --git a/packages/sync/src/cache/open.test.ts b/packages/sync/src/cache/open.test.ts index 9767be3..ceea009 100644 --- a/packages/sync/src/cache/open.test.ts +++ b/packages/sync/src/cache/open.test.ts @@ -91,6 +91,27 @@ describe("cache open", () => { db.close(); }); + test("adds health to v3 without discarding snapshots", () => { + const first = openCache(); + first.db + .prepare( + "INSERT INTO instances VALUES ('x','X','https://api.github.com','user','key')", + ) + .run(); + first.db + .prepare("UPDATE meta SET value='3' WHERE key='schema_version'") + .run(); + first.db.exec("DROP TABLE sync_health"); + first.db.close(); + const next = openCache(); + expect(next.wiped).toBe(false); + expect(next.db.prepare("SELECT id FROM instances").all()).toEqual([ + { id: "x" }, + ]); + expect(next.db.prepare("SELECT * FROM sync_health").all()).toEqual([]); + next.db.close(); + }); + test("wipeCacheFile removes the file", () => { const { db } = openCache(); db.close(); diff --git a/packages/sync/src/cache/open.ts b/packages/sync/src/cache/open.ts index 6fdfca6..e279ea8 100644 --- a/packages/sync/src/cache/open.ts +++ b/packages/sync/src/cache/open.ts @@ -2,7 +2,7 @@ import { existsSync, mkdirSync, rmSync } from "node:fs"; import { dirname } from "node:path"; import Database from "better-sqlite3"; import { resolveCachePath } from "./path.js"; -import { CACHE_SCHEMA_VERSION, SCHEMA_DDL } from "./schema.js"; +import { CACHE_SCHEMA_VERSION, SCHEMA_DDL, SYNC_HEALTH_DDL } from "./schema.js"; export type Cache = Database.Database; @@ -25,7 +25,15 @@ export function openCache(): OpenCacheResult { "SELECT name FROM sqlite_master WHERE type = 'table' AND name = 'meta'", ) .get(); - const stored = hasMeta ? readSchemaVersion(db) : null; + let stored = hasMeta ? readSchemaVersion(db) : null; + // Health is additive: preserve v3 snapshots and mutation guards. + if (stored === 3 && CACHE_SCHEMA_VERSION === 4) { + db.transaction(() => { + db.exec(SYNC_HEALTH_DDL); + writeSchemaVersion(db, CACHE_SCHEMA_VERSION); + })(); + stored = CACHE_SCHEMA_VERSION; + } if ( (hasTables && stored !== CACHE_SCHEMA_VERSION) || (stored !== null && stored !== CACHE_SCHEMA_VERSION) diff --git a/packages/sync/src/cache/schema.ts b/packages/sync/src/cache/schema.ts index dd23d69..7aa2193 100644 --- a/packages/sync/src/cache/schema.ts +++ b/packages/sync/src/cache/schema.ts @@ -1,7 +1,14 @@ -export const CACHE_SCHEMA_VERSION = 3; +export const CACHE_SCHEMA_VERSION = 4; +export const SYNC_HEALTH_DDL = ` +CREATE TABLE IF NOT EXISTS sync_health ( + instance_id TEXT NOT NULL REFERENCES instances(id) ON DELETE CASCADE, + kind TEXT NOT NULL, attempted_at TEXT NOT NULL, status TEXT NOT NULL, + reason TEXT, retry_at TEXT, PRIMARY KEY(instance_id,kind) +);`; export const SCHEMA_DDL = ` PRAGMA journal_mode = WAL; PRAGMA foreign_keys = ON; +${SYNC_HEALTH_DDL} CREATE TABLE IF NOT EXISTS meta (key TEXT PRIMARY KEY, value TEXT NOT NULL); CREATE TABLE IF NOT EXISTS instances ( id TEXT PRIMARY KEY, label TEXT NOT NULL, base_url TEXT NOT NULL, diff --git a/packages/sync/src/cache/store.test.ts b/packages/sync/src/cache/store.test.ts index 80acafc..fb12cc7 100644 --- a/packages/sync/src/cache/store.test.ts +++ b/packages/sync/src/cache/store.test.ts @@ -143,3 +143,48 @@ test("mutation guards survive reopen, protect all list writes, merge patches and rmSync(dir, { recursive: true, force: true }); } }); + +test("health and rate-budget errors from an old identity cannot overwrite a replacement", () => { + const dir = mkdtempSync(join(tmpdir(), "ghd-health-store-")); + const old = process.env.XDG_CACHE_HOME; + process.env.XDG_CACHE_HOME = dir; + const { db } = openCache(); + try { + const store = createStore(db); + const instance = { + id: "health", + label: "Health", + baseUrl: "https://api.github.com", + username: "u", + credentialKey: "old", + }; + store.reconcile([instance]); + store.recordHealth( + "health", + "prs", + { status: "failed", reason: "old failure", retryAt: null }, + "old", + ); + store.reconcile([{ ...instance, credentialKey: "new" }]); + expect( + store.recordHealth( + "health", + "prs", + { status: "failed", reason: "late failure", retryAt: null }, + "old", + ), + ).toBe(false); + store.recordBudget("health", "graphql", { remaining: 0 }, "old"); + expect(store.health("health", "prs").status).toBe("never"); + expect(store.budget("health", "graphql")).toBeUndefined(); + store.reconcile([]); + expect(() => + store.recordBudget("health", "graphql", { remaining: 0 }, "new"), + ).not.toThrow(); + } finally { + db.close(); + if (old === undefined) delete process.env.XDG_CACHE_HOME; + else process.env.XDG_CACHE_HOME = old; + rmSync(dir, { recursive: true, force: true }); + } +}); diff --git a/packages/sync/src/cache/store.ts b/packages/sync/src/cache/store.ts index 5a62a04..efe0ac8 100644 --- a/packages/sync/src/cache/store.ts +++ b/packages/sync/src/cache/store.ts @@ -25,6 +25,16 @@ export type CacheMutation = | { kind: "patch"; repo: string; number: number; patch: PullRequestPatch } | { kind: "notificationDone"; id: string }; +export interface SyncHealth { + instanceId: string; + kind: Kind; + lastSuccessAt: string | null; + lastAttemptAt: string | null; + status: "never" | "syncing" | "updated" | "unchanged" | "skipped" | "failed"; + reason: string | null; + retryAt: string | null; +} + export interface Metadata { etag?: string | null; remaining?: number | null; @@ -63,16 +73,30 @@ export function createStore(db: Cache) { db.prepare(`INSERT INTO budgets VALUES (?,?,?,?) ON CONFLICT(instance_id,resource) DO UPDATE SET remaining=excluded.remaining,reset_at=excluded.reset_at`); - function recordBudget(id: string, resource: string, metadata: Metadata) { - if (metadata.remaining !== undefined && metadata.remaining !== null) { - saveBudget.run( - id, - resource, - metadata.remaining, - metadata.resetAt ?? null, - ); - } + function recordBudget( + id: string, + resource: string, + metadata: Metadata, + credentialKey?: string, + ) { + db.transaction(() => { + if (credentialKey !== undefined) { + const current = oldInstance.get(id) as + | { credential_key: string } + | undefined; + if (current?.credential_key !== credentialKey) return; + } + if (metadata.remaining !== undefined && metadata.remaining !== null) { + saveBudget.run( + id, + resource, + metadata.remaining, + metadata.resetAt ?? null, + ); + } + })(); } + // Guards live beside snapshots so another process/in-flight fetch cannot // resurrect stale search-index data after a successful write. const clearNotificationEtag = db.prepare( @@ -187,6 +211,54 @@ export function createStore(db: Cache) { if (mutation.kind === "notificationDone") clearNotificationEtag.run(id); })(); }, + health(id: string, kind: Kind): SyncHealth { + const row = db + .prepare("SELECT * FROM sync_health WHERE instance_id=? AND kind=?") + .get(id, kind) as + | { + attempted_at: string; + status: SyncHealth["status"]; + reason: string | null; + retry_at: string | null; + } + | undefined; + const snapshot = state.get(id, kind) as + | { last_run_at: string | null } + | undefined; + return { + instanceId: id, + kind, + lastSuccessAt: snapshot?.last_run_at ?? null, + lastAttemptAt: row?.attempted_at ?? null, + status: row?.status ?? "never", + reason: row?.reason ?? null, + retryAt: row?.retry_at ?? null, + }; + }, + recordHealth( + id: string, + kind: Kind, + outcome: Pick, + credentialKey: string, + ) { + return db.transaction(() => { + const current = oldInstance.get(id) as + | { credential_key: string } + | undefined; + if (current?.credential_key !== credentialKey) return false; + db.prepare( + "INSERT OR REPLACE INTO sync_health VALUES (?,?,?,?,?,?)", + ).run( + id, + kind, + new Date().toISOString(), + outcome.status, + outcome.reason, + outcome.retryAt, + ); + return true; + })(); + }, listInstances: () => listInstances.all() as Instance[], updateAuthenticatedIdentity( id: string, diff --git a/packages/sync/src/engine.ts b/packages/sync/src/engine.ts index c528423..f3a3270 100644 --- a/packages/sync/src/engine.ts +++ b/packages/sync/src/engine.ts @@ -5,6 +5,7 @@ import { type CacheMutation, type Kind, type Metadata, + type SyncHealth, } from "./cache/store.js"; import { authenticate, loadInstances, type GitHubInstance } from "./config.js"; import { fetchPullRequestComments } from "./providers/github/comments.js"; @@ -91,12 +92,25 @@ export function createSync(options: SyncOptions = {}) { )) { const fetches: FetchOutcome[] = []; results.push({ instanceId: instance.id, fetches }); + const finish = (outcome: FetchOutcome, retryAt: string | null = null) => { + store.recordHealth( + instance.id, + outcome.kind, + { + status: outcome.status, + reason: outcome.reason ?? null, + retryAt, + }, + credentialKey(instance), + ); + fetches.push(outcome); + }; let username: string; try { username = await (options.authenticate ?? authenticate)(instance); } catch (err) { for (const kind of kinds) - fetches.push({ + finish({ kind, status: "failed", count: 0, @@ -112,7 +126,7 @@ export function createSync(options: SyncOptions = {}) { ) ) { for (const kind of kinds) - fetches.push({ + finish({ kind, status: "skipped", count: 0, @@ -130,14 +144,23 @@ export function createSync(options: SyncOptions = {}) { budget.reset_at && Date.parse(budget.reset_at) > Date.now() ) { - fetches.push({ - kind, - status: "skipped", - count: 0, - reason: `rate-limit floor (${FLOOR}) until ${budget.reset_at}`, - }); + finish( + { + kind, + status: "skipped", + count: 0, + reason: `rate-limit floor (${FLOOR}) until ${budget.reset_at}`, + }, + budget.reset_at, + ); continue; } + store.recordHealth( + instance.id, + kind, + { status: "syncing", reason: null, retryAt: null }, + credentialKey(instance), + ); try { const result = kind === "prs" @@ -156,7 +179,7 @@ export function createSync(options: SyncOptions = {}) { { credentialKey: credentialKey(instance), username }, ); if (!saved) { - fetches.push({ + finish({ kind, status: "skipped", count: 0, @@ -164,7 +187,7 @@ export function createSync(options: SyncOptions = {}) { }); continue; } - fetches.push({ + finish({ kind, status: result.data === null ? "unchanged" : "updated", count: @@ -175,8 +198,14 @@ export function createSync(options: SyncOptions = {}) { }); } catch (err) { const metadata = errorMetadata(err); - if (metadata) store.recordBudget(instance.id, resource, metadata); - fetches.push({ + if (metadata) + store.recordBudget( + instance.id, + resource, + metadata, + credentialKey(instance), + ); + finish({ kind, status: "failed", count: 0, @@ -282,6 +311,12 @@ export function createSync(options: SyncOptions = {}) { return reconcileConfiguration().then(() => {}); }, getSyncState: store.state, + getHealth: (): SyncHealth[] => + store + .listInstances() + .flatMap((instance) => + KINDS.map((kind) => store.health(instance.id, kind)), + ), recordMutation: ( instanceId: string, mutation: CacheMutation, diff --git a/packages/sync/src/index.ts b/packages/sync/src/index.ts index 8852cd6..ccd4a0e 100644 --- a/packages/sync/src/index.ts +++ b/packages/sync/src/index.ts @@ -20,3 +20,5 @@ export { export type { CacheMutation, PullRequestPatch } from "./cache/store.js"; export { authenticate } from "./config.js"; + +export type { SyncHealth } from "./cache/store.js"; diff --git a/packages/web/src/App.tsx b/packages/web/src/App.tsx index 7411ee6..2abb2c9 100644 --- a/packages/web/src/App.tsx +++ b/packages/web/src/App.tsx @@ -19,6 +19,7 @@ import { NotificationList } from "./components/NotificationList"; import { PrList } from "./components/PrList"; import { ReviewList } from "./components/ReviewList"; import { SectionHeader } from "./components/SectionHeader"; +import { SyncHealth } from "./components/SyncHealth"; import { Welcome } from "./components/Welcome"; import { ShortcutHelp } from "./components/ShortcutHelp"; import { Skeleton } from "./components/Skeleton"; @@ -275,6 +276,15 @@ export function App() { + {!showWelcome && ( + instance.id === activeTab) + } + /> + )}
diff --git a/packages/web/src/api.ts b/packages/web/src/api.ts index 40ebbce..6840b5b 100644 --- a/packages/web/src/api.ts +++ b/packages/web/src/api.ts @@ -33,7 +33,18 @@ export class AutoMergeNotAllowedError extends Error { } } +export interface SyncHealthEntry { + instanceId: string; + kind: "prs" | "reviews" | "notifications"; + lastSuccessAt: string | null; + lastAttemptAt: string | null; + status: "never" | "syncing" | "updated" | "unchanged" | "skipped" | "failed"; + reason: string | null; + retryAt: string | null; +} + export const api = { + syncHealth: () => fetchJson("/api/sync/health"), getConfig: () => fetchJson("/api/config"), createConfig: async () => { const res = await fetch("/api/config/create", { method: "POST" }); diff --git a/packages/web/src/components/SyncHealth.tsx b/packages/web/src/components/SyncHealth.tsx new file mode 100644 index 0000000..88a5c74 --- /dev/null +++ b/packages/web/src/components/SyncHealth.tsx @@ -0,0 +1,68 @@ +import { useQuery } from "@tanstack/react-query"; +import { api } from "../api"; +import type { Instance } from "../types"; + +export function SyncHealth({ instances }: { instances: Instance[] }) { + const { data, error, dataUpdatedAt } = useQuery({ + queryKey: ["sync-health"], + queryFn: api.syncHealth, + refetchInterval: 10_000, + retry: false, + }); + + if (error) + return ( +

+ Cannot reach sync status — data may be stale +

+ ); + if (!data?.length) return null; + const visible = data.filter((entry) => + instances.some((instance) => instance.id === entry.instanceId), + ); + if (!visible.length) return null; + const failed = visible.some((entry) => entry.status === "failed"); + const paused = visible.some((entry) => entry.status === "skipped"); + const waiting = visible.some((entry) => !entry.lastSuccessAt); + const stale = visible.some( + (entry) => + entry.lastSuccessAt && + dataUpdatedAt - Date.parse(entry.lastSuccessAt) > 90_000, + ); + const label = failed + ? "Sync failed — data may be stale" + : paused + ? "Sync paused — data may be stale" + : waiting + ? "Waiting for first sync" + : stale + ? "Data is stale" + : "Up to date"; + return ( +
+ + {label} + +
    + {visible.map((entry) => ( +
  • + { + instances.find((instance) => instance.id === entry.instanceId) + ?.label + }{" "} + · {entry.kind}: {entry.status} · Last successful sync:{" "} + {entry.lastSuccessAt + ? new Date(entry.lastSuccessAt).toLocaleTimeString() + : "never"} + {entry.reason && <> · {entry.reason}} + {entry.retryAt && ( + <> · Retry after {new Date(entry.retryAt).toLocaleTimeString()} + )} +
    • + ))} +
+
+ ); +}