fix(git-sync): a valid-but-nonexistent gitmost_id no longer wedges a space's sync (N1-D1)
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) <noreply@anthropic.com>
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user