From e6a861bdaf6d8032a07bfe073007d84e9b0916d7 Mon Sep 17 00:00:00 2001 From: agent_qa Date: Fri, 3 Jul 2026 06:48:54 +0300 Subject: [PATCH] fix(git-sync): a valid-but-nonexistent gitmost_id no longer wedges a space's sync (N1-D1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A vault file whose `gitmost_id` is a WELL-FORMED UUID that matches no page (a stale id from a restored-from-backup file, or a copied/foreign id) fell through importPageMarkdown to writeBody() on a non-existent page, throwing "Page … not found". The push apply recorded that as a per-cycle failure that never cleared — refs never advance, so the whole space's sync looped on the failure indefinitely (observed live: a leftover orphan file kept a space stuck at "1 failure" every ~5s). Same user-visible impact as C9-D1, but the id is a valid uuid so the 22P02 guard does not catch it. Add the missing `currentPage == null` branch in importPageMarkdown: skip the unknown id as an inert no-op so the cycle succeeds and the rest of the space keeps syncing. Verified on the stand: pushing a valid-but-nonexistent gitmost_id now stays at 0 failures (was 1/cycle forever), logs a skip warn, and a concurrent legit edit still syncs. Unit test added; server suite green (2146). NOTE (separate design follow-up, not this commit): the reconcile still cleans the orphan file (it maps to no live page). ADOPTING such a file as a fresh page (the restore-from-backup use case, preserving the git-authored content) needs the title from the filename, which lives in the engine classifier, not this method. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../gitmost-datasource.service.spec.ts | 27 ++++++++++++++++--- .../services/gitmost-datasource.service.ts | 17 ++++++++++++ 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/apps/server/src/integrations/git-sync/services/gitmost-datasource.service.spec.ts b/apps/server/src/integrations/git-sync/services/gitmost-datasource.service.spec.ts index e45be258..9ec190be 100644 --- a/apps/server/src/integrations/git-sync/services/gitmost-datasource.service.spec.ts +++ b/apps/server/src/integrations/git-sync/services/gitmost-datasource.service.spec.ts @@ -268,10 +268,16 @@ describe('GitmostDataSourceService', () => { }); it('returns updatedAt:undefined when the page row is gone after the write (stale-read branch)', async () => { - // writeBody succeeds, but the post-write findById returns nothing (e.g. the - // page was concurrently hard-deleted) -> the optional updatedAt is omitted. + // The page EXISTS at import time (so the unknown-page guard N1-D1 does not + // fire), writeBody succeeds, but the POST-write findById returns nothing (e.g. + // the page was concurrently hard-deleted) -> the optional updatedAt is omitted. const { service, mocks } = build(); - mocks.pageRepo.findById.mockResolvedValue(undefined); + mocks.pageRepo.findById + .mockResolvedValueOnce({ + id: 'p1', + updatedAt: new Date('2026-06-20T11:00:00.000Z'), + }) // currentPage (import-time read) exists + .mockResolvedValue(undefined); // post-write read: page is gone const res = await service .bind(CTX) @@ -281,6 +287,21 @@ describe('GitmostDataSourceService', () => { expect(res.updatedAt).toBeUndefined(); }); + it('skips (no writeBody) when the gitmost_id is a valid UUID matching NO page (bug N1-D1)', async () => { + // A well-formed but stale/foreign id (restore-from-backup, copied file) must + // NOT fall through to writeBody on a non-existent page (which throws "Page not + // found" and wedges the space's sync loop). It is skipped as an inert no-op. + const { service, mocks } = build(); + mocks.pageRepo.findById.mockResolvedValue(undefined); + + const res = await service + .bind(CTX) + .importPageMarkdown('019f2500-face-7000-8000-000000000002', '# orphan'); + + expect(res).toEqual({}); + expect(mocks.collabGateway.writePageBody).not.toHaveBeenCalled(); + }); + // F5 acceptance, criterion (b): guard #2 (docsCanonicallyEqual) must SKIP the // re-ingest when the freshly-parsed doc is canonically equal to the page's // current DB content even though the two are NOT byte-identical — the DB copy diff --git a/apps/server/src/integrations/git-sync/services/gitmost-datasource.service.ts b/apps/server/src/integrations/git-sync/services/gitmost-datasource.service.ts index 12d9214f..29ee0b05 100644 --- a/apps/server/src/integrations/git-sync/services/gitmost-datasource.service.ts +++ b/apps/server/src/integrations/git-sync/services/gitmost-datasource.service.ts @@ -252,6 +252,23 @@ export class GitmostDataSourceService { const currentPage = await this.pageRepo.findById(pageId, { includeContent: true, }); + // Unknown-page guard (bug N1-D1). `importPageMarkdown` is only ever called for a + // vault file that CARRIES a `gitmost_id`, so a null page means the id is a + // well-formed UUID that matches NO page — a stale id from a restored-from-backup + // file, or a copied/foreign id. Left unhandled it falls through to `writeBody()` + // on a non-existent page, which throws "Page … not found"; the push apply records + // that as a per-cycle failure that never clears, wedging the whole space's sync + // loop (same user-visible impact as C9-D1, but the id is a VALID uuid so the + // 22P02 guard does not catch it). Skip it as an inert no-op so the cycle succeeds + // and the rest of the space keeps syncing. (ADOPTING such a file as a fresh page + // — the restore-from-backup use case — is a separate design decision: the title + // lives in the filename, which the engine classifier holds, not this method.) + if (currentPage == null) { + this.logger.warn( + `git-sync[${ctx.spaceId ?? '-'}] skip import of page ${pageId}: no page with that id (stale/foreign gitmost_id; not adopted, no wedge)`, + ); + return {}; + } // Cross-space confused-deputy guard (review S2). The target `pageId` comes // from THIS space's vault file frontmatter, but a file in space A could carry // space B's page id. Without this check that file could resurrect (via