From d0cd0e6878d980a8c6aeebcd129fb22f1f6ae882 Mon Sep 17 00:00:00 2001 From: Harlan Wilton Date: Fri, 9 Oct 2026 12:58:35 +1100 Subject: [PATCH] fix(sync): read changed trees before trusting push timestamps --- layers/registry/server/utils/sync-repo.ts | 6 ++++-- test/unit/sync-prefetch.test.ts | 12 ++++++++++++ test/unit/sync-repo-freshness.test.ts | 23 +++++++++++++++++++++++ 3 files changed, 39 insertions(+), 2 deletions(-) diff --git a/layers/registry/server/utils/sync-repo.ts b/layers/registry/server/utils/sync-repo.ts index c0b197f7..b22db39e 100644 --- a/layers/registry/server/utils/sync-repo.ts +++ b/layers/registry/server/utils/sync-repo.ts @@ -567,8 +567,10 @@ export function unchangedRepoStatus(input: { const { existing, hasAdmittedSkills, headTreeSha, repoPushedAt } = input if (!hasAdmittedSkills || !existing?.last_tree_sha) return null - if (headTreeSha && existing.last_tree_sha === headTreeSha) - return 'skipped-tree-sha' + // A known tree SHA is the content identity. Use pushed_at only when + // GitHub did not provide it. + if (headTreeSha) + return existing.last_tree_sha === headTreeSha ? 'skipped-tree-sha' : null if (existing.pushed_at != null && repoPushedAt != null && existing.pushed_at >= repoPushedAt) return 'skipped-pushed-at' return null diff --git a/test/unit/sync-prefetch.test.ts b/test/unit/sync-prefetch.test.ts index 9c170215..a0b6b4b6 100644 --- a/test/unit/sync-prefetch.test.ts +++ b/test/unit/sync-prefetch.test.ts @@ -116,6 +116,18 @@ describe('prefetchUnchangedRepos', () => { .toEqual({ repo_meta_synced_at: null }) }) + it('queues a changed tree even when the push timestamp has not advanced', async () => { + harness.raw.prepare(`UPDATE repos SET pushed_at = ? WHERE owner = 'acme' AND repo = 'changed'`) + .run(Date.parse('2026-10-01T00:00:00Z') / 1000) + vi.stubGlobal('fetch', vi.fn(graphqlAnswer)) + + const result = await prefetchUnchangedRepos({ db: harness.db, bindings, now: NOW }, [candidate('changed')]) + + expect(result.queue).toEqual([candidate('changed')]) + expect(harness.raw.prepare(`SELECT repo_meta_synced_at FROM repos WHERE owner = 'acme' AND repo = 'changed'`).get()) + .toEqual({ repo_meta_synced_at: null }) + }) + it('queues every candidate when GitHub does not answer the batch', async () => { vi.stubGlobal('fetch', vi.fn(async () => new Response('bad gateway', { status: 502 }))) diff --git a/test/unit/sync-repo-freshness.test.ts b/test/unit/sync-repo-freshness.test.ts index 6d7995d4..dd9cd692 100644 --- a/test/unit/sync-repo-freshness.test.ts +++ b/test/unit/sync-repo-freshness.test.ts @@ -229,6 +229,29 @@ describe('syncRepo freshness cursor', () => { expect(statements.some(sql => sql.includes('SELECT name, current_sha'))).toBe(false) }) + it.each([1783857600, 1783857601])('reads a changed tree when the stored push timestamp is %s', async (pushedAt) => { + sqlite.prepare(`UPDATE repos SET pushed_at = ?`).run(pushedAt) + github.getRepoSummary.mockResolvedValue(repoSummary('changed-tree')) + github.getTree.mockResolvedValue({ status: 503, data: null, rateLimit: null, notModified: false }) + + const result = await syncRepo('acme', 'skills', {}, db) + + expect(result).toMatchObject({ status: 'failed', reason: 'tree_fetch_failed:503' }) + expect(github.getTree.mock.calls[0]?.[2]).toBe('changed-tree') + expect(sqlite.prepare(`SELECT repo_meta_synced_at FROM repos`).pluck().get()).toBe(75) + }) + + it('uses the push timestamp when GitHub gives no head tree', async () => { + sqlite.prepare(`UPDATE repos SET pushed_at = 1783857600`).run() + github.getRepoSummary.mockResolvedValue(repoSummary(null)) + + const result = await syncRepo('acme', 'skills', {}, db) + + expect(result.status).toBe('skipped-pushed-at') + expect(github.getTree).not.toHaveBeenCalled() + expect(sqlite.prepare(`SELECT repo_meta_synced_at FROM repos`).pluck().get()).toBe(1783900800) + }) + it('clears a stale too-large verdict once the repo syncs clean', async () => { sqlite.prepare(`UPDATE repos SET tree_truncated_at = 123`).run()