fix(git-sync,converter): 3 HIGH round-trip/data-loss fixes found by QA (#359)

Implements the 3 HIGH fixes agent_qa found via the RALPH test cycle (analysis theirs,
implementation here), fixtures-first.

- T2E-1 [converter]: a multi-paragraph callout collapsed to one paragraph on round-trip.
  markdown-converter `case "callout"` now joins block children with "\n>\n" (a blank ">"
  separator), byte-identical to the blockquote serializer, so two paragraphs survive
  re-import. Fixture 12-callout-multiblock; two existing tests updated (their old
  expectations encoded the collapse bug).
- T6-listnest [converter]: a callout/blockquote nested in a list item corrupted on
  round-trip (callout type lost, `[!type]` leaked into text — confirmed DB corruption).
  Add bridgeNestedCallouts: a post-marked, nesting-agnostic JSDOM pass that reconstructs
  callouts from `[!type]`-opening blockquotes at ANY depth (the indented `  > [!type]`
  form the column-0-anchored preprocessor misses), emitting the same callout div as the
  top-level path (disjoint inputs, no double-processing). Strict `^[!type]` guard, so a
  real blockquote — or one with `[!` mid-line — is NOT converted. Fixture 13-callout-in-list
  + a no-lead control + false-positive controls (proven non-vacuous).
- D-P3-1 [git-sync]: a "ghost" file (a tracked id that was never a page) was silently
  absence-deleted on pull-reconcile -> data loss. Add GitSyncClient.pageIdsExist (datasource:
  workspace-scoped SELECT over any space incl. trashed/moved, non-UUID ids filtered first);
  cycle.ts computes the candidate-delete set and passes existing ids as `deletableIds` into
  planReconciliation, which now absence-deletes ONLY real page rows — a ghost is preserved.
  A cycle-level e2e test proves the ghost survives runCycle end-to-end.

Suites green: prosemirror-markdown 687, git-sync 272 (no type errors), datasource 36,
orchestrator 26; both packages tsc clean. Scope: the 3 fixes + tests/fixtures only, no
develop re-merge. T4E-esc (LOW escaping) left for the maintainer, per agent_qa.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-07-05 21:27:21 +03:00
parent 6956563961
commit 5ac08b8f4b
17 changed files with 657 additions and 23 deletions
@@ -51,6 +51,23 @@ export interface GitSyncClient {
rootPageId?: string,
): Promise<{ pages: GitSyncPageNodeLite[]; complete: boolean }>;
/**
* Existence probe for the pull-side ghost guard (D-P3-1). Given a set of
* candidate pageIds, return the subset that corresponds to a REAL page ROW —
* INCLUDING soft-deleted (trashed) and pages in ANY OTHER space, i.e. any
* `id` that has ever been a page (`SELECT id FROM pages WHERE id = ANY(...)`,
* workspace-scoped). Malformed (non-UUID) ids are filtered out before the
* query and are never returned.
*
* The pull reconciler only absence-deletes a tracked file whose pageId is
* returned here: a deleted/moved/trashed page HAS a row -> its vault file is
* cleaned up; a GHOST id (a git file whose id was NEVER a page) has NO row ->
* it is preserved rather than silently deleted (the data-loss bug). Called on
* only the small candidate-delete set (tracked ids absent from the live tree),
* so the query is bounded and usually empty.
*/
pageIdsExist(pageIds: string[]): Promise<string[]>;
/**
* One page WITH its ProseMirror body content. `applyPullActions` reads
* `id`, `slugId`, `title`, `parentPageId`, `spaceId` (for the file meta) and
+28
View File
@@ -170,10 +170,38 @@ export async function runCycle(deps: RunCycleDeps): Promise<RunCycleResult> {
});
const tree = await client.listSpaceTree(spaceId);
// D-P3-1 ghost guard: an absence-delete must not silently remove a git file
// whose pageId was NEVER a page (a hand-authored file with an unknown id).
// Compute the candidate-delete set (tracked ids absent from the live tree)
// and ask the datasource which of them are REAL page rows (incl. trashed /
// other spaces). Only those may be absence-deleted; a ghost id is preserved
// (adopted/skipped by the push side).
//
// Size note: on a COMPLETE fetch this set is usually small/empty (only
// genuinely removed pages look absent), so the `id IN (...)` probe is cheap.
// On an INCOMPLETE fetch (`tree.complete === false`) MANY live pages look
// absent, so the set — and the probe — can be large. That is only a perf
// consideration, not a correctness one: `decideAbsenceDeletions` (inside
// `computePullActions`) still SUPPRESSES every absence delete on an
// incomplete fetch, so no ghost-guarded deletion is applied that cycle
// regardless of what the probe returns.
const livePageIds = new Set(
tree.pages.filter((p) => p && p.id).map((p) => p.id),
);
const candidateDeleteIds = existing
.map((e) => e.pageId)
.filter((id) => !livePageIds.has(id));
const deletableIds =
candidateDeleteIds.length > 0
? await client.pageIdsExist(candidateDeleteIds)
: [];
const pullActions = computePullActions({
pages: tree.pages,
treeComplete: tree.complete,
existing,
deletableIds,
});
// Bail before the first destructive write phase if the lock was lost.
+17 -3
View File
@@ -148,6 +148,14 @@ export interface PullActionsInput {
treeComplete: boolean;
/** Parsed tracked files: `{ pageId, relPath }` (from `readExisting`). */
existing: { pageId: string; relPath: string }[];
/**
* The subset of tracked pageIds that correspond to a REAL page row (D-P3-1
* ghost guard, from `client.pageIdsExist`). Only ids in this set may be
* absence-deleted; a ghost id (never a page) is preserved. When omitted, all
* absent ids are deletable (the historical behavior; pure unit callers that
* do not model ghosts).
*/
deletableIds?: string[];
}
/**
@@ -191,7 +199,7 @@ export interface PullActions {
* thin `applyPullActions`.
*/
export function computePullActions(input: PullActionsInput): PullActions {
const { pages, treeComplete, existing } = input;
const { pages, treeComplete, existing, deletableIds } = input;
const layout = buildVaultLayout(pages);
const live: LiveEntry[] = [];
@@ -206,8 +214,14 @@ export function computePullActions(input: PullActionsInput): PullActions {
}
// Plan reconciliation (pure). `plan.toDelete` is ABSENCE-based only;
// `plan.moved` carries move old-path removals separately.
const plan = planReconciliation(live, existing);
// `plan.moved` carries move old-path removals separately. The ghost guard
// (D-P3-1) gates absence-deletes to ids that are a real page row; when
// `deletableIds` is omitted, all absent ids are deletable (historical).
const plan = planReconciliation(
live,
existing,
deletableIds === undefined ? undefined : new Set(deletableIds),
);
// Decide whether the ABSENCE-based deletions may be applied this cycle
// (SPEC §8): incomplete-fetch suppression + empty-live + mass-delete guard.
+20 -2
View File
@@ -96,6 +96,16 @@ export interface ReconciliationPlan {
export function planReconciliation(
live: LiveEntry[],
existing: ExistingEntry[],
/**
* The subset of tracked pageIds that correspond to a REAL page row (D-P3-1
* ghost guard). When provided, a tracked file whose pageId is ABSENT from
* `live` is absence-deleted ONLY if its id is in this set — a deleted/moved/
* trashed page has a row (delete its stale vault file), while a GHOST id (a
* git file whose id was never a page) has NO row and is PRESERVED. When
* `undefined` (pure unit callers that do not model ghosts), every absent id is
* treated as deletable — the historical behavior.
*/
deletableIds?: ReadonlySet<string>,
): ReconciliationPlan {
// Desired path for each live pageId.
const liveByPageId = new Map<string, string>();
@@ -119,9 +129,17 @@ export function planReconciliation(
for (const ex of existing) {
const liveRel = liveByPageId.get(ex.pageId);
if (liveRel === undefined) {
// Tracked page is gone from the live tree -> absence delete.
// Tracked page is gone from the live tree -> candidate absence delete.
// D-P3-1: a candidate is only deleted when its id corresponds to a real
// page row (deleted/moved/trashed). A GHOST id (never a page) is NOT in
// `deletableIds` and is preserved rather than silently deleted. When the
// gate is not supplied (pure unit callers), fall back to the historical
// "all absent ids deletable" behavior.
const deletable = deletableIds === undefined || deletableIds.has(ex.pageId);
// Never queue a path a live page will (re)write (path reuse -> no loss).
if (!liveTargetPaths.has(ex.relPath)) toDeleteSet.add(ex.relPath);
if (deletable && !liveTargetPaths.has(ex.relPath)) {
toDeleteSet.add(ex.relPath);
}
continue;
}
if (liveRel !== ex.relPath) {