fix(agent-edit-v2): detect live ref drift against persisted base not live fallback

[devYRPauli]

Problem

POST /api/agent/:slug/edit/v2 could silently clobber a concurrent human edit and apply an agent's replace_block to the wrong block, returning 200 success instead of rejecting.

Scenario (covered by the existing agent-edit-v2-live-structural-drift-regression test):

1. An agent snapshots a document and captures a block ref (e.g. b3 = "Second paragraph.").
2. A live collaborator inserts a block before that block, so the live fragment shifts ("Second paragraph." moves to b4), without bumping the persisted revision.
3. The agent sends replace_block on its captured b3 with baseRevision = the snapshot revision.

Expected: 409 FRAGMENT_DIVERGENCE (the block topology drifted under the agent's ref).
Actual: 200 - the ref resolved positionally to the now-shifted b3 (the human's new block) and replaced it, discarding the concurrent insert.

Root cause

The ref-drift guard findLiveRefDrift(persistedDescriptors, liveDescriptors, ...) is meant to compare the persisted base (what the agent's refs were captured against) with the live authoritative fragment. But both sides were sourced from the same value:

• liveDescriptors come from authoritativeBase.base.markdown (the live fragment).
• persistedDescriptors come from doc.markdown, where doc = await getCanonicalReadableDocument(slug, 'state') ?? getDocumentBySlug(slug).

Under live presence, getCanonicalReadableDocument(..., 'state') returns the live (yjs) fallback, so doc.markdown is the live fragment too. The guard therefore compared the live fragment against itself and never detected drift - exactly when it is needed.

Instrumented confirmation: with a live client connected, persistedDescriptors and liveDescriptors were identical (both the 5-block live fragment, including the human insert), while the real persisted row (getDocumentBySlug) still held the 4-block base the agent's b3 was captured against.

Fix

Source the drift baseline from the real persisted document row for baseRevision-only edits (the frame the agent's refs were actually captured against). When a baseToken is supplied, the agent explicitly opted into the live authoritative base (already validated above), so that base remains the comparison frame and behavior is unchanged.

const persistedDriftBaseline = baseToken ? null : getDocumentBySlug(slug);
const driftBaselineMarkdown = baseToken
  ? stripEphemeralCollabSpans(doc.markdown ?? '')
  : stripEphemeralCollabSpans(
    persistedDriftBaseline?.revision === baseRevision
      ? persistedDriftBaseline.markdown
      : doc.markdown ?? '',
  );
const persistedBase = parseMarkdownWithHtmlFallback(parser, driftBaselineMarkdown);

The persisted row is used only when its revision still equals the declared baseRevision. Otherwise the read falls back to doc.markdown.

Verification

• src/tests/agent-edit-v2-live-structural-drift-regression.test.ts: fails on main (200), passes with this change (409, the human insert is preserved and the persisted revision is unchanged).
• No new failures across the related agent-edit / edit-v2 regression suites; baseToken-based edits are unaffected (no spurious FRAGMENT_DIVERGENCE).
• No new type errors (tsc -p tsconfig.server.json unchanged).

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 48a7abde2d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Do not reject freshly refreshed live refs

This makes every baseRevision-only edit compare refs against the persisted row whenever the authoritative base is live. However /snapshot//state can build the advertised bN refs from the live Yjs fallback while still returning the row revision, so a client that refreshes after a live insert and posts that fresh snapshot.revision (e.g. persisted A/B/C, live A/X/B/C, then replace fresh b3) will now compare live b3 to persisted b3 and get FRAGMENT_DIVERGENCE forever unless it switches to baseToken. Since the endpoint still accepts baseRevision and the snapshot exposes it, this rejects valid fresh live-state edits rather than only stale refs.

Useful? React with 👍 / 👎.

[devYRPauli]

The scenario needs a client to hold live-derived bN refs and a postable baseRevision at the same time. Both read surfaces stop that.

/state and /snapshot gate the revision on mutationReady:

server/document-engine.ts:1251   revision: mutationReady ? doc.revision : null,
server/document-engine.ts:1295   revision: mutationReady ? doc.revision : null,
server/agent-snapshot.ts:146     revision: mutationReady ? document.revision : null,

mutationReady is always false on those two surfaces when the read serves the live Yjs fallback. buildFallbackReadOptions returns { mutationReady: false, repairPending: true } for state and snapshot (server/collab.ts:6527). All six call sites of buildYjsFallbackReadableDocument pass either that helper or the same literal, so no live-fallback read reaches those payloads mutation-ready.

The refresh in the scenario therefore returns revision: null, not the row revision. The client cannot post snapshot.revision as baseRevision.

main already covers this end to end, in src/tests/canonical-read-freshness-regression.test.ts:

line	assertion
139	state.readSource === 'yjs_fallback'
142	state.revision === null
146	/state withholds the editV2 link
171	snapshotBody.readSource === 'yjs_fallback'
174	snapshotBody.revision === null
176	/snapshot withholds the editV2 link

The b1..bN refs come from one place, buildSnapshotPayload at server/agent-snapshot.ts:125. No other read surface emits them, so there is no second route to a fresh live ref set.

The 409 does not loop. buildSnapshot at server/agent-edit-v2.ts:266 calls the same buildAgentSnapshot, so the embedded snapshot also carries revision: null and a mutationBase.token. baseToken is the only forward path the API offers once live is ahead of the row, which is the intended protocol.

The case this PR does reject is the stale one. An agent posts a numeric baseRevision that still matches the row, while a live insert has already shifted the blocks under its refs. server/agent-edit-v2.ts:831 admits that request, because the row revision has not moved. Comparing the live fragment against itself hides the shift and applies the edit to the wrong block.

I made no change, because the fresh path cannot reach this check and the stale path is the defect the PR fixes.

[devYRPauli]

The bot's P2 on the drift baseline was a real find - fixed in d7dadc8.

The baseline re-read the row with a second getDocumentBySlug(slug) call several awaits after doc was fetched and revision-validated against baseRevision. That second read wasn't itself revision-checked, so a concurrent write landing in the window returned a newer row than the frame the agent's refs were validated against, and that newer content could then be compared as if it were the base - a spurious FRAGMENT_DIVERGENCE on an otherwise valid edit.

Now the re-read row is only used when its revision still matches baseRevision, falling back to the already-validated doc otherwise. The baseToken path is unchanged.

One note on the bot's framing: "permanently reject" overstates it - a retry would pick up the new revision and succeed. It's a spurious rejection, not a permanent one.

I didn't add a regression test: reproducing this deterministically needs a timing seam to inject a concurrent write between the two reads, and the current harness has no such hook - a test without it would assert on shape rather than behavior. The existing agent-edit-v2-live-structural-drift-regression test still passes, and 7 of the 8 agent-edit-v2* suites pass locally (agent-edit-v2-live-viewer-regression fails identically on a clean tree without this change, so it's pre-existing and unrelated). Happy to add a test if you'd like the seam introduced.