Repository navigation
Plannotator Inbox: guided reviews from agents (step 5) - #1760
Merged
Merged
Conversation
backnotprop
force-pushed
the
feat/inbox-guides
branch
4 times, most recently
from
October 7, 2026 16:11
2487929 to
4ffeaa3
Compare
get_guide_brief and submit_guide on the Inbox MCP; guides validated by parseGuideSnapshot and buildAuthoredGuideSnapshot's strict placement checks, stored as inbox/blobs/<sha256> after the message lands, carried on the message (idempotency on the input as sent); the row's Guided review mark, the card in the thread, and Plannotator's guide viewer over the window (GuideView with the Inbox as GuideHost, AllFilesCodeView read-only, mounted through a dynamic import); reviewed ticks kept on the message. Proofs: tests/e2e/inbox-guides.spec.ts. Also two timing fixes in the window's e2e.
backnotprop
force-pushed
the
feat/inbox-guides
branch
from
October 7, 2026 16:34
4ffeaa3 to
11519c7
Compare
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Plannotator Inbox, step 5: guided reviews from agents
An agent sends the person a guided review of a code change through two new MCP tools,
get_guide_briefandsubmit_guide. The Inbox checks it with Plannotator's own guide checks, keeps the snapshot as a blob on the message, marks the thread's row "Guided review" and shows the guide at the foot of its message. Open renders it in Plannotator's guide viewer over the whole window, read-only, with the Inbox as itsGuideHost.Base:
mainwith the window (PR 1755), notifications (1756), decisions (1757) and attachments (1759) and the Claude Code connection (1758, 4bbc34c) merged. The branch is rebased onto 4bbc34c with both sides kept in every conflict, and is one squashed commit.git merge-tree --write-tree origin/main HEADshows no conflict.Review fixes (REVIEW-PLANNOTATOR-INBOX-guides-2026-10-07)
buildAuthoredGuideSnapshotstamps the build time, so a retried guide plus patch built a different snapshot. Idempotency now comparesguide.input_sha256, the hash of what the agent sent as given, so a retry answers the first message and its guide. The blob is written only after the message lands, so a refused send leaves no orphan (item I4) and a retry writes nothing. Proved through the SDK client in the guides e2e: the second call returnsreplayed: truewith the firstmessage_idand the first guide'ssha256.guide_reviewed) and are saved throughPOST /api/inbox/messages/:id/guide/reviewed. The route is same-origin andserverSessionguarded like the other writing routes;422means not a list of booleans,404means no message or no guide. Proved end to end: tick, then read the stored record. Then the Inbox is stopped, its last port is held by another listener, and it restarts on another port, which is a new origin. The guide opens there with both ticks. The localStorage copy is gone.inbox.htmland its modules (the guide chain, review-editor's renderer) run at page load. Only the mount, the diff render and the highlighter wait for Open. A truly lazy load is not cheap here, becauseapps/inboxbuilds withinlineDynamicImports: truelike every Plannotator single-file app. The +390 kB stands as the cost.writeBlob(exported frompackages/shared/inbox/attachments.ts) and read throughinboxBlobPath.packages/shared/inbox/blobs.tsis deleted. The blob is still written only after the message lands. A guide's blob joins the thread's attachment blobs inthreadBlobsand in the delete paths' used-set, so delete thread or delete project never removes a blob a guide still uses, and Settings counts it once. "In use" means: some message still in the store names the hash, as an attachment'ssent_sha256or a guide'ssha256. The delete census collects those from every remaining message and removes only blobs outside that set. Proved in the guides e2e: two threads with the same guide blob; deleting one keeps the blob, deleting the second removes it, and the other guides' blobs stay. Removing the guide from the used-set makes that proof fail.sendAgentMessagepath; submit_guide takes none.What changed
packages/server/inbox-guides.ts, new; registered frompackages/server/inbox-mcp.ts):get_guide_briefreturns the same shape as Workspaces' tool of that name:methodology(GUIDE_REVIEW_PROMPT),output_schema(GUIDE_SCHEMA_JSON),diff_steps,rules, andexample. The example is a submit_guide call that works as it is: the AUTHORED fixture ofguide-cli.test.tsand its patch.submit_guidetakes eitherguidepluspatch, orsnapshot(an object or its JSON). It also takesbody, which defaults to the guide's intent, and send_message's routing fields.parseGuideSnapshot/parseGuideSnapshotJsonfor a snapshot.buildAuthoredGuideSnapshot's strict placement checks for both forms. They are called, never re-implemented, with the project root as the cwd.isErrorinvalid_guide: …and names the file.SEND_ROUTING_FIELDS,sendProjectandsendAgentMessage. Both tools use them, and send_message behaves as before.inbox/blobs/<sha256>through the attachments'writeBlob.guide: { sha256, input_sha256, bytes, title, sections, files, additions, deletions }andguide_reviewed. Rows gainguide: boolean(packages/core/inbox-types.ts,packages/shared/inbox/store.ts).GET /api/inbox/messages/:id/guidereturns the stored snapshot. It serves by message only, never a blob by hash. The handler is ininbox-guides.tsand is one block ininbox.ts.packages/inbox):InboxList.tsx).GuideCard.tsxis the card at the foot of the message.GuidePane.tsxis the guide over the window: header "← Thread · Guided review · from in , sent " and a close button. Route:#thread=…&guide=<message id>. Escape and "Thread" go back to the thread.guide/GuideReader.tsxholdsGuideViewbehindGuideHostProvider, over review-editor'sAllFilesCodeViewinreadOnlymode. That is the renderer guides.show uses.lazy(() => import('#guide-reader'))(package.jsonimports). Its bytes are inline and its modules run at page load (I1).inbox.csshas its own block.apps/inbox/index.cssscanspackages/guide-viewerandpackages/review-editor/components.apps/inbox/vite.config.tsgains apps/review's worker block: Pierre's inline worker needs the ES format.Record sections this makes real
.product/approved/plannotator-inbox-window-2026-10-07/:Proofs
tests/e2e/inbox-guides.spec.tsruns withbun run test:e2e:inboxin the existinginbox-e2e.ymljob, after the binary is built. It is Playwright against the compiled binary under a temp data dir and HOME. Agents go throughplannotator inbox mcpwith the SDK's stdio client (scripts/inbox-sim.tsgainsguideBriefandsubmitGuide). It proves:get_guide_briefreturns the brief.FIXTURE_V1_LOCALas a snapshot (from Claude Code) land as two rows with the mark.idempotency_keyanswers the first message.src/nope.ts);src/auth.ts is placed twice);guidewithoutpatch..local/proof/guides/with a contact sheet:1.1-guide-rows,4.1-guide-in-thread,4.2-guide-open,4.2-guide-reviewed-tick,4.2-authored-guide-open. CI uploads them with the window's artifact.tests/entry-assets.test.tsgains one test. No Inbox module except the reader may statically import@plannotator/guide-viewer,@plannotator/review-editoror#guide-reader, and GuidePane must load the reader lazily. I checked that it fails when App.tsx imports the guide viewer statically.packages/server/inbox.test.tsnow also checks that both guide tool descriptions stay under 2,048 characters. The tool list check covers the two new tools throughINBOX_MCP_TOOLS.Gates run
bun run typecheck: green. It includespackages/inbox/tsconfig.jsonandapps/inbox/tsconfig.json.bun test packages apps/hook tests scriptsunder a tempPLANNOTATOR_DATA_DIRand HOME: packages 4,956 pass / 0 fail; tests 61 / 0; scripts 198 / 0; apps/hook 566 pass / 1 fail. The one failure isannotate-cli.test.ts"exits 1 with one line naming the port", and it fails the same way on a cleanorigin/mainworktree on this machine, so it is not from this change. The shim testinbox-cli.test.tsnow reads the tool list fromINBOX_MCP_TOOLSinstead of a literal four-tool list.bun test tests, which includestests/entry-assets.test.tsandtests/favicon-surfaces.test.ts: green.bun run test:e2e:inboxafter the rebase onto 4bbc34c: 51 of 51 green (window, notifications, decisions, attachments, guides).bun run --cwd apps/guides-show build:viewer && bun run --cwd apps/guides-show check:manifest: "guide-viewer-manifest.ts is in sync".bun run --cwd packages/ui smoke:package: green (ui 0.52.1 with core 0.25.12). Core changed, but onlyinbox-types.ts, which ui does not import.bun run check:release-version: consistent.DOM_TESTS=1allowlist is unchanged.What it leaves out
Deviations from the record and the plan, with reasons
body, the message body is the guide's intent and a new thread's subject is "Guided review: <title>".sendMessageneeds a body, and the card needs a message to sit under.FIXTURE_V1_WORKSPACEuses, is checked as""instead of refused.@plannotator/review-editor, forAllFilesCodeViewonly. plannotator-ops's binary-size note said to avoid importing review-editor. The guide viewer has no diff renderer of its own, and guides.show uses exactly this one inreadOnlymode. Without it the guide is not Plannotator's guide viewer. Cost:inbox.htmlgrows from 20,145 kB to 20,535 kB (+390 kB; Pierre was already in it). Only the mount is behind the dynamic import; the bytes and the module evaluation are not (I1).apps/guides-show/tsconfig.viewer.jsonsays. The window's strict typecheck reads the reader's shape fromguide/reader-module.d.tsthrough apathsentry. The reader itself is checked bypackages/inbox/tsconfig.guide.jsonfor local use, and its own files report no errors.engineLabelis now exported frompackages/guide-viewer/GuideViewer.tsx, so the reader names the generator exactly as the viewer does (for exampleclaude-codebecomes "Claude Code") instead of copying its table. This is JS only: no class was added, and the guides.show manifest check is unchanged.text-xsof 13px and Pierre's theme variables ondiffs-container. guides.show and the in-app guide get both frompackages/review-editor/index.css. Importing that file whole would restyle the Inbox..ib-shellgetsisolation: isolateso the message body's own layers never draw over the open guide. Without it, a frame showed the thread's text through the guide.inbox/blobs/<sha256>, as the step brief names it and as attachments (1759) writes it, through the same writer. RESEARCH.md section 8 had sketchedblobs/sha256/<2 hex>/<rest>.tests/e2e/inbox.spec.ts, "agents write three threads"). It failed once in my run. When the first send's refresh lands before the other two, the list correctly holds them behind "N new", and the test assumed it never would. The test now clicks the notice when it shows. A second race in the same spec hit after the rebase: the Settings test readconfig.jsonright after the switch moved, but the switch moves before its POST answers. The test now polls the file. The window spec also clears only its own PNGs, so the guides' frames inproof/guides/survive.inbox-e2e.ymlgains paths (packages/guide-viewer/**,packages/review-editor/**,packages/server/guide/**,packages/core/guide*), and the guides spec adds about 10 s to the existing job. There is no new job.Depends on: nothing beyond main.