Skip to content

Annotate drafts follow the file's path (0.29 PR 2) - #1710

Merged
backnotprop merged 5 commits into
mainfrom
feat/annotate-drafts-by-path
Oct 5, 2026
Merged

Annotate drafts follow the file's path (0.29 PR 2)#1710
backnotprop merged 5 commits into
mainfrom
feat/annotate-drafts-by-path

Conversation

@backnotprop

@backnotprop backnotprop commented Oct 5, 2026

Copy link
Copy Markdown
Owner

PR 2 of the 0.29 agent-sessions design (.product/drafts/agent-sessions-0.29/DESIGN.md, "Drafts survive an edit and a reopen").

Problem

Annotate drafts were keyed only by a hash of the document text. When an agent edited the file and the review was reopened, the reviewer's saved comments did not come back.

What changes

Single local file (mode annotate, not a URL). The draft is now saved under the content key and also under a path key, annotateFileDraftKey(<abs path>). This reuses the #1590 dual-key logic in review-draft.ts, with the content key in the role of the patch key:

  • On load, the content copy wins unless the path copy is strictly newer by draftGeneration.
  • The path copy remembers every content key it was saved on. These stamps are written by the server: they are stripped from client bodies and from load responses.
  • The path key's tombstone guards both keys. A stale save gets 409 with the current generation.
  • Restored comments re-anchor by text. A comment whose text is gone shows the existing Unanchored chip and still exports.

Per-document copies. This covers every document a local-file or folder session comments on, other than the session's own root: a folder session's files and a single-file session's linked documents.

  • New endpoint /api/draft/document. /api/plan advertises it with documentDrafts: true.
  • Each document is saved under its own path key. Its saved comments are merged in the first time it opens in a session: ids the session already holds are skipped, and a toast says how many came back.
  • A file's comments therefore follow it between a folder session and a session on the file alone. Bundles (PR 4) will get this for free.
  • The server sets the generation for these writes: one above anything the key has seen. A single-file session that loads the copy later resumes above it.
  • The client writes a document only after it has read that document's copy. It sends a document empty only if it sent that document with content before.
  • With per-document copies on, the session draft carries the root document's comments, even while another document is open. Before this change, the session draft carried whatever document was open.

Clearing:

  • A decision clears the session's own keys plus the path copy of every document the session wrote or restored from. After that, document writes are refused with 409. The decisions are /api/feedback, /api/approve, and the reviewer's Close (/api/exit).
  • A client DELETE (everything removed, or the restore banner dismissed) clears only the session draft's keys.
  • The client-lease auto-dismiss still keeps everything.

Out of scope: URL, live-app and annotate-last sessions. PLANNOTATOR_ANNOTATE_HISTORY does not govern drafts. They are crash recovery, and the design ties that flag to history copies only.

Both runtimes are covered. Shared logic is in packages/shared/annotate-draft.ts, vendored to Pi. The Bun server and the Pi mirror route through it.

Tests

  • packages/server/annotate-draft.scenarios.ts runs against both runtimes through annotate-draft.test.ts (Bun) and apps/pi-extension/server/serverAnnotate-drafts.test.ts (Pi). It covers:
    • edit + reopen restores
    • content-vs-path precedence ("strictly newer")
    • a decision clears both keys, and the tombstone refuses a stale save
    • Close clears the path copy
    • the advert, and a URL session without it
    • folder → single file, and single file → folder
    • a folder decision clears covered files only, and later writes get 409
    • an empty write clears a copy
    • paths outside the session, and a single-file session's own file, are refused
  • packages/editor/documentDrafts.test.ts: write planning and merge rules.
  • packages/editor/App.documentDrafts.test.tsx (DOM, added to DOM_TESTS): opening a linked document merges its saved comment, and the session draft never carries it.
  • Passed: bun test packages/server apps/pi-extension packages/shared packages/editor, DOM_TESTS=1 bun test --isolate packages/editor plus the draft/linked-doc DOM suites, and bun run typecheck.

Notes for review

  • The reviewer's Close still deletes the draft, and now the path copy as well. Open question 1 in the design (should Close keep it?) is not decided, so this keeps today's rule.
  • Known edge, as the design states: two open sessions on the same file write the same path key, and the last save wins.

backnotprop and others added 5 commits October 5, 2026 08:26
@backnotprop @claude
feat(annotate): drafts follow the file's path as well as its text
606db99 
Single-file annotate drafts are also saved under a path key (the #1590
dual-key pattern via review-draft.ts), so comments come back after an
agent edits the file and the review is reopened. Every other document a
local-file or folder session comments on is saved under its own path and
merged in when it is opened in any session. A decision clears the
session's keys and every document it covered. Bun and Pi servers.

Co-Authored-By: Claude <noreply@anthropic.com>
@backnotprop @claude
fix(annotate): drafts retry a stale save, keep unbacked documents, ca…
0786840 
…nonical paths

- useAnnotationDraft: a save refused as stale (409 with draftGeneration)
  adopts the server's generation and saves once more; DraftTransport.save
  may resolve { staleGeneration } (optional, additive).
- Documents the server keeps no path copy for (outside the session roots,
  a symlink alias of the root, a self-link copy of the root) ride the
  session draft again (composeSessionDraft).
- Document paths must name an existing regular file and are canonicalized
  with realpath, so an alias of the root is the root.
- Tests: stale retry against real draft sessions, unbacked documents,
  linked-document edits, symlink/missing/directory paths; docs.

Co-Authored-By: Claude <noreply@anthropic.com>
@backnotprop @claude
fix(annotate): refuse session-draft saves after the decision
0267422 
A second tab's save after a decision got a stale 409, retried above the
tombstone, and was accepted, so the next session restored comments that
were already sent. Once settled, every save answers 409 { decided: true }
in both runtimes (saveRequest shapes the response), and useAnnotationDraft
never retries a decided refusal.

Co-Authored-By: Claude <noreply@anthropic.com>
@backnotprop
Merge remote-tracking branch 'origin/main' into feat/annotate-drafts-…
6759a2a 
…by-path

# Conflicts:
#	apps/pi-extension/server/serverAnnotate.ts
#	apps/pi-extension/vendor.sh
#	packages/server/annotate.ts
@backnotprop @claude
fix(annotate): an agent close keeps late saves; closed-tab copy per case
7eafcbc 
An agent close deletes nothing, so session-draft saves and document-copy
writes that arrive after it now land (a comment typed in the last
debounce window was dropped); the tab also flushes its pending saves on
the session-closed event. A reviewer decision still refuses later saves,
and the closed tab's DELETE stays a no-op. The closed-tab copy now says
what brings the draft back: a local file or folder even after edits, a
URL or agent message only unchanged, a live app by its address.

Co-Authored-By: Claude <noreply@anthropic.com>
@backnotprop
backnotprop merged commit 7fecb02 into main Oct 5, 2026
28 checks passed
This was referenced Oct 5, 2026
Merged
Merged
backnotprop added a commit that referenced this pull request Oct 5, 2026
@backnotprop @claude
Several files in one annotate review (0.29 PR 4, bundles) (#1718)
88d7b4f 
* feat(annotate): several files in one review (0.29 PR 4, bundles)

plannotator annotate spec.md mock.html notes.md (every argument an existing
file named by its path) opens ONE review of the files, in the typed order,
with one decision. No flag, no file-count limit.

- Shared rule (annotate-target.ts, vendored to Pi): probeAnnotateBundlePath,
  the bundle selection, resolveAnnotateBundleFiles (type and 2MB checks that
  name the file). Prose, URLs, folders and searched names keep the #1483
  tiers; the ambiguity error gains a hint on hosts that open bundles. Strict
  gates take only a single target or only file paths (else exit 2).
- Server mode annotate-bundle in Bun and Pi: /api/plan carries bundle, /api/doc
  follows the single-file rule per file, history saved per text file at open,
  per-file draft copies (#1710), host status documents, archive surface
  annotate-bundle with target.documents.
- CLI: host result record gains documents; the agent message names every file.
- Editor: first file opens by itself, header switcher (2 of 3 · name), the
  Files tab and All files panel in bundle order, export one section per file in
  bundle order, Ask AI surface line names file N of M.
- Tool contract: description line for lists, plannotatorBundleSubject,
  isOlderCliBundleRefusal; Bash take-over maps several file paths to a list.
  Mod launches bundles and maps an older CLI's refusal to an update message.
- Pi and OpenCode (embedded and CLI bridge) slash commands open bundles too.

Co-Authored-By: Claude <noreply@anthropic.com>

* fix(annotate): bundle review follow-ups

- Move annotate-bundle out of @plannotator/core into packages/shared (no new
  core export, so the published-UI smoke passes without a core publish).
- A list of file paths with a missing file fails naming it (shared 'missing'
  selection) on the CLI, Pi and OpenCode; tool list entries are passed as paths
  (bare word -> ./word) so the tool and Bash take-over get the same rule.
- Strict refusal decided by probe only (no fetch, live probe or Resolved: line).
- A symlinked bundle file is served: its real directory is a root for it.
- The CLI uses the shared selection end to end (a.md ./a.md opens a.md).
- .env named as refused; bundle files have no Close pill and leaving a linked
  document returns to the last bundle file; docs; engine-harness bundle case.

Co-Authored-By: Claude <noreply@anthropic.com>

* fix(annotate): an existing path is never reported missing

A stray . or .. (or any existing directory) beside file paths was read as a
missing file ('File not found: .'), where main opened the file. The missing
selection now needs the new pathExists option (annotatePathExists): only a
token with nothing at its path, and nothing found by search, is missing.
Wired on the CLI, Pi and OpenCode; test rows on the shared rule and the CLI.
Symlink-root comment corrected; AGENTS.md notes the dotted-word edge.

Co-Authored-By: Claude <noreply@anthropic.com>

---------

Co-authored-by: Claude <noreply@anthropic.com>
This was referenced Oct 6, 2026
Merged
Merged
@scottames-github-bot scottames-github-bot Bot mentioned this pull request Oct 11, 2026
Merged
1 task
@konradmichalik konradmichalik mentioned this pull request Oct 14, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@backnotprop