Skip to content

Revision-only moves from collaborator comments and target-less handle writes still report a conflict until re-read #145

Description

@ElliotDrel

Follow-up from #144

#144 fixed the case where this server's own comment tools (addComment, replyToComment, updateComment, resolveComment, deleteComment) moved the Docs revisionId and the next guarded body write failed with "This document (...) changed since you last read it" despite no outside edit. The comment tools now re-arm the tracked revision after they write.

Two neighbouring cases are still open. Both are correct (they fail closed) and both cost the user exactly one unnecessary re-read.

1. A collaborator's comment made in the Docs UI

Measured live on 2026-09-02: a comment write advances the Docs revisionId while Drive modifiedTime stays byte-identical (Drive version bumps, modifiedTime does not). The read tracker (dist/readTracker.js guardMutation) compares only modifiedTime, so it sees nothing, and the write goes out with the pre-comment writeControl.requiredRevisionId. Google returns 400 and the tool reports a revision conflict. Reading the document again clears it.

When the comment came from this process, #144 handles it. When a collaborator comments from the UI, nothing in-process can see it, so the conflict still fires.

Options, in the order I would try them:

  • Probe revisionId in guardMutation alongside modifiedTime. One extra documents.get({fields: 'revisionId'}) per guarded write. If modifiedTime is unchanged and only the revision moved, that is provably not a body edit (a body edit always moves modifiedTime), so the guard can re-arm to the new revision and let the write proceed. Fails closed if the probe fails. This is the same rule the handle path already applies (see 2).
  • Keep the conflict but say what happened: "the body did not change, only comments did; read again to continue". Cheaper, still one round trip.

Rejected during #144: treating any revision-only move as safe without the modifiedTime check, and retrying the write on a 400. Both weaken the guard.

2. Handle-based writes with no targets

dist/docsHandles.js guardTargets re-arms effectiveRevisionId on a revision-only move when the caller supplies targets (the #108 range-precise path). With no targets it returns at the normalized.length === 0 early exit without re-arming, so a target-less handle write (for example replaceDocumentWithMarkdown through a readHandle) stays pinned to the handle's original revisionId and conflicts after any comment activity, including this server's own, because #144 only re-arms the read tracker, not the handle record.

Fix shape: on the no-targets path, if a fetchRevisionId probe is available and modifiedTime is unchanged, re-arm the same way the targeted path does; otherwise keep the current behaviour.

Acceptance

  • A comment made outside this process, followed by a guarded body write with no re-read, either succeeds (option 1) or fails with a message that names comments as the cause and says a re-read is enough.
  • A target-less handle write after this server's own comment tool succeeds without a re-read.
  • Unit tests for both, plus an assertion in live/missions/verify-comment-collateral.mjs for the handle case (the collaborator case cannot be driven live from one account).
  • No change to what happens when modifiedTime moved: that is still a conflict.

Not a release blocker: the guard is correct today, and the cost is one extra read.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions