fix(planreview): stop comments dirtying the plan, and gate decisions on saving

[tusharbhardwaj-bk]

Three follow-ups from the first release on expbkt3.

1. Commenting re-sent the whole plan as a reviewer edit

Reported: sending annotations attached a <plan_edit mode="full"> block containing the entire document, even though nothing had been edited. The attached copy was Plate-normalized (***, \ hard breaks, * bullets), which was the clue.

Root cause, reproduced before changing anything: Plate serializes a commented leaf as an MDX JSX element, and mdast-util-to-markdown throws on it inside a table cell —

Error: Cannot handle unknown node `mdxJsxTextElement`
  ❯ handleTableCell mdast-util-gfm-table/lib/index.js:216

handleChange treats a failed serialize as a reviewer edit ("Treat an editor tree that cannot be serialized as changed"), so isDirty flipped, the client sent editedMarkdown, the server cut a version, and the diff against the agent baseline was non-empty — producing the full-plan resend. Any plan containing a table hit this the moment you commented on it.

The earlier trap test missed it because its fixture had no table.

Fix: stripPlanReviewCommentMarks removes comment properties and re-merges the leaves split: true fragmented; getMarkdown() and the change comparison both serialize through it. Comments are review state and can no longer influence what the agent is told changed. A second test asserts the raw serializer still throws, so the strip cannot be quietly deleted later on the assumption Plate handles it now.

2. Unsaved hand edits did not gate the decision

Deciding on a plan whose edits are not yet a version leaves it ambiguous which text was approved. Unsaved edits now replace the decision row with a single Save the plan, with a one-line explanation beneath.

3. Approving with open comments contradicted itself

The prompt said "implement the plan you proposed above, exactly as written" and then attached a list of requested changes. It now says to start implementing and treat the comments as amendments to apply along the way, explicitly not to return to planning. The button reads Approve with comments in that state.

Verification

151 tests passed across 10 scoped files. tsgo --noEmit clean for apps/web, packages/shared and apps/server; lint and check-fork-markers.ts pass.

One existing server test asserted the old approval wording for exactly this case and was updated to assert the new intent, including not.toContain("exactly as written").

Reviewer check on expbkt3: comment on a plan containing a table and send — the prompt should carry only the <review_comment> blocks and no <plan_edit>.

🤖 Generated with Claude Code

---

^{Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.}

[github-actions]

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ℹ️ No successful main baseline artifact is available yet. This run establishes the initial measurement.

Provider	Metric	Main baseline	This PR	Impact	PR ceiling
Codex	Total thread wire	—	11.4 KiB	—	15.1 KiB	✅
Codex	Thread snapshot wire	—	5.7 KiB	—	7.3 KiB	✅
Codex	Live turn WebSocket wire	—	5.7 KiB	—	7.8 KiB	✅
Codex	Live turn WebSocket decoded	—	50.4 KiB	—	66.4 KiB	✅
Codex	Live turn messages	—	11	—	21	✅
Claude	Total thread wire	—	11.5 KiB	—	15.1 KiB	✅
Claude	Thread snapshot wire	—	5.7 KiB	—	7.3 KiB	✅
Claude	Live turn WebSocket wire	—	5.7 KiB	—	7.8 KiB	✅
Claude	Live turn WebSocket decoded	—	51.2 KiB	—	66.4 KiB	✅
Claude	Live turn messages	—	11	—	21	✅

Baseline: unavailable · PR result: 89c9f14 · 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: 95.6 KiB
• Claude decoded thread snapshot: 96.3 KiB

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