fix(desktop): preview markup survives page reloads

[kalvenschraut]

When you mark up a page in the desktop preview browser, a dev-server full reload (Vite full-reload, Next.js refresh) or a manual refresh ended the annotation and threw away the comment, selections, regions, strokes, and style edits.

Fix

• The picker preload keeps a draft in the main process (preview:annotation-draft IPC), sent on each edit and on pagehide. Main validates it (isPreviewAnnotationDraft) before storing it.
• pickElement keeps the session open across a main-frame reload of the same page, ignoring the fragment. On dom-ready it re-sends START_PICK with the draft. Navigating anywhere else, or a reload that redirects away, still cancels as before.
• The preload restores the comment, tool, regions, strokes, and style edits. It finds selected elements again by CSS path. An element the app renders late keeps its old spot as a region and is swapped back when it appears, polling for up to 5s. Unresolved elements stay in the draft, so a quick second reload doesn't degrade them.
• If a reload fails, for example while the dev server restarts, the pick stays active and the annotate button stays clickable so it can be cancelled. The next successful refresh restores the markup.

Desktop only, since the markup overlay runs in the Electron preview.

Verification

• vp test run apps/desktop/src/preview/Manager.test.ts apps/desktop/src/preview/PickedElementPayload.test.ts (119 passed). The new tests cover the reload-keeps-session path, the redirect-cancels path, and the draft validator.
• Desktop typecheck clean. Lint and format clean on touched files; the remaining warnings were already there before this change.
• Reviewed by GPT 6.1 Sol (xhigh) over two rounds. Round 1 found two issues: style edits were dropped for a restored element the user re-selected by hand, and the pick couldn't be cancelled after a failed reload. Both are fixed in 7fe996c, and round 2 approved.
• Not exercised in a real Electron reload.

Model: Claude Opus 5.5 via Claude Code in T3 Code.

🤖 Generated with Claude Code

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: RTVision/t3code/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: fc56d9f6-2959-48c4-a5ae-00cc208a2c7b

📥 Commits

Reviewing files that changed from the base of the PR and between b544a37 and 7fe996c.

📒 Files selected for processing (7)

• apps/desktop/src/preview/GuestProtocol.ts
• apps/desktop/src/preview/Manager.test.ts
• apps/desktop/src/preview/Manager.ts
• apps/desktop/src/preview/PickPreload.ts
• apps/desktop/src/preview/PickedElementPayload.test.ts
• apps/desktop/src/preview/PickedElementPayload.ts
• apps/web/src/components/preview/PreviewView.tsx

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

The preview now validates, captures, and restores annotation drafts across same-page reloads. Pick sessions end when the reloaded page has a different URL. An active pick remains cancellable after a failed page load.

Changes

Preview annotation draft restoration

Layer / File(s)	Summary
Define and validate annotation drafts apps/desktop/src/preview/GuestProtocol.ts, apps/desktop/src/preview/PickedElementPayload.ts, apps/desktop/src/preview/PickedElementPayload.test.ts	The guest protocol adds a draft channel. Shared types and validators define accepted draft data, with tests for valid and invalid drafts.
Capture and restore drafts in the preview apps/desktop/src/preview/PickPreload.ts	PickPreload synchronizes annotation state and restores comments, tools, regions, strokes, selected elements, and style changes. It retries unresolved selectors for up to five seconds.
Resume pick sessions after reload apps/desktop/src/preview/Manager.ts, apps/desktop/src/preview/Manager.test.ts, apps/web/src/components/preview/PreviewView.tsx	PreviewManager resumes picking with the saved draft after a same-page reload and ends the pick when navigation reaches a different page. The web preview keeps an active pick cancellable after a failed load.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant PreviewManager
  participant PickPreload
  participant PreviewPage
  PickPreload->>PreviewManager: Send validated annotation draft
  PreviewPage->>PreviewManager: Report main-frame navigation
  PreviewManager->>PreviewManager: Compare page URLs without fragments
  PreviewPage->>PreviewManager: Report dom-ready
  PreviewManager->>PickPreload: Restart picking with saved draft
  PickPreload->>PreviewPage: Restore annotation and retry selectors

Loading

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to 7fe99

The reviewed reload-restoration paths show no actionable issue. The change is mergeable after normal checks; a real Electron failed-load reload remains untested.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 7fe99

Draft restoration is limited to the active preview session, validates incoming data, and checks the page URL before replay. No introduced security vulnerability was established. Some overlapping reload and cancellation behavior remains unverified, so the assessment retains limited uncertainty.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The evidenced new exposure is retention and replay of annotation content and DOM edits within a preview pick. Storage is scoped to one pick closure and reception to its WebContents; the inspected draft path does not invoke filesystem, credential, or deployment operations. This bounds the demonstrated path, not every possible effect of a compromised guest.

Trust Boundaries and Controls

• observed — Main validates drafts before retention, and the preload validates again before restoration. Replay is gated by the original page URL. The draft handler does not inspect sender-frame identity, and the payload contains no session or navigation generation; structural validation should therefore not be described as provenance authentication.

Resilience and Maintainability Implications

• inferred — Ownership under overlapping reloads and cancellation remains uncertain. Restore dispatch has no final settlement or generation check, but its inspected operations contain no explicit asynchronous wait. The sequential reload/redirect test and stale-capture replacement test do not establish a stale-replay attack or resolve these interleavings.

Hardening Proposals

• proposed — Bind replay to an explicit pick and navigation generation, and recheck ownership immediately before dispatch. This would make stale-replay containment explicit rather than dependent on scheduling assumptions; it is a hardening proposal, not a verified vulnerability.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 17.65% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 17 functions across 7 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: preserving desktop preview markup across page reloads.
Description check	✅ Passed	The description explains the problem, the fix, and verification results. It does not include the required “Scope and approval” section or explain why prior approval was not needed, but the other main …
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider	Metric	Main baseline	This PR	Impact	PR ceiling
Codex	Total thread wire	5.0 KiB	5.0 KiB	0 B (0.0%)	6.8 KiB	✅
Codex	Thread snapshot wire	3.8 KiB	3.8 KiB	0 B (0.0%)	4.9 KiB	✅
Codex	Live turn WebSocket wire	1.2 KiB	1.2 KiB	0 B (0.0%)	2.0 KiB	✅
Codex	Live turn WebSocket decoded	20.9 KiB	20.9 KiB	0 B (0.0%)	29.3 KiB	✅
Codex	Live turn messages	2	2	0 (0.0%)	8	✅
Claude	Total thread wire	5.0 KiB	5.0 KiB	0 B (0.0%)	6.8 KiB	✅
Claude	Thread snapshot wire	3.8 KiB	3.8 KiB	0 B (0.0%)	4.9 KiB	✅
Claude	Live turn WebSocket wire	1.2 KiB	1.2 KiB	0 B (0.0%)	2.0 KiB	✅
Claude	Live turn WebSocket decoded	21.2 KiB	21.2 KiB	0 B (0.0%)	29.3 KiB	✅
Claude	Live turn messages	2	2	0 (0.0%)	8	✅

Baseline: b544a37 · PR result: 7fe996c · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

• Codex decoded thread snapshot: 108.5 KiB
• Claude decoded thread snapshot: 108.8 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.