fix(git-sync): hold refs on suppressed deletes + stamp delete/restore provenance (PR #119 review)
Two stability warnings from the #119 review: 1. delete-cap no longer drops deletions forever. When planned deletes exceed GIT_SYNC_MAX_DELETES_PER_CYCLE the apply client's deletePage now THROWS instead of resolving to a no-op. A throw is recorded by the engine as a per-page failure, so `refs/docmost/last-pushed` is NOT advanced past the commit that dropped the files — the next cycle re-diffs from the un-advanced ref and re-plans the same deletes (a transient over-cap is retried, not silently dropped and then recreated by the next pull). Previously a resolving no-op let the engine count `deleted++` with no failure, advance the ref, and never replay the deletions. 2. git-sync soft-delete and restore now stamp provenance. deletePage routes GIT_SYNC_PROVENANCE through pageService.removePage, and restorePage stamps lastUpdatedSource='git-sync' on the restore update — so the page-change listener's loop-guard (skip when lastUpdatedSource==='git-sync') recognizes both as its own writes instead of scheduling a wasted echo cycle. Done via a backward-compatible optional `lastUpdatedSource` param on pageRepo.removePage/restorePage (omitted for ordinary user deletes/restores). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -298,6 +298,11 @@ export class PageRepo {
|
||||
pageId: string,
|
||||
deletedById: string,
|
||||
workspaceId: string,
|
||||
// Optional provenance marker. When the soft-delete is driven by an automated
|
||||
// data plane (e.g. git-sync), stamp `lastUpdatedSource` so the change-listener
|
||||
// loop-guard recognizes it as its own write and does not schedule an echo
|
||||
// cycle. Omitted for ordinary user deletes (column keeps its prior value).
|
||||
lastUpdatedSource?: string,
|
||||
): Promise<void> {
|
||||
const currentDate = new Date();
|
||||
|
||||
@@ -348,6 +353,7 @@ export class PageRepo {
|
||||
.set({
|
||||
deletedById: deletedById,
|
||||
deletedAt: currentDate,
|
||||
...(lastUpdatedSource ? { lastUpdatedSource } : {}),
|
||||
})
|
||||
.where('id', 'in', pageIds)
|
||||
.where('deletedAt', 'is', null)
|
||||
@@ -378,7 +384,14 @@ export class PageRepo {
|
||||
}
|
||||
}
|
||||
|
||||
async restorePage(pageId: string, workspaceId: string): Promise<void> {
|
||||
async restorePage(
|
||||
pageId: string,
|
||||
workspaceId: string,
|
||||
// See removePage: stamp `lastUpdatedSource` for automated (git-sync) restores
|
||||
// so the change-listener loop-guard skips the echo cycle. Omitted for
|
||||
// ordinary user restores.
|
||||
lastUpdatedSource?: string,
|
||||
): Promise<void> {
|
||||
// First, check if the page being restored has a deleted parent
|
||||
const pageToRestore = await this.db
|
||||
.selectFrom('pages')
|
||||
@@ -429,7 +442,12 @@ export class PageRepo {
|
||||
// On restore, disarm the death timer: pulling a note out of trash means
|
||||
// "keep it". Otherwise a deadline now in the past would re-trash it on the
|
||||
// next cleanup sweep.
|
||||
.set({ deletedById: null, deletedAt: null, temporaryExpiresAt: null })
|
||||
.set({
|
||||
deletedById: null,
|
||||
deletedAt: null,
|
||||
temporaryExpiresAt: null,
|
||||
...(lastUpdatedSource ? { lastUpdatedSource } : {}),
|
||||
})
|
||||
.where('id', 'in', pageIds)
|
||||
.execute();
|
||||
|
||||
|
||||
Reference in New Issue
Block a user