diff --git a/plans/2026-07-02-71-surface-polish.md b/plans/2026-07-02-71-surface-polish.md new file mode 100644 index 0000000..a5edfc3 --- /dev/null +++ b/plans/2026-07-02-71-surface-polish.md @@ -0,0 +1,342 @@ +# #71 Surface Polish Batch Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Land the six batched Minor findings from the native-surfaces migration's final whole-branch review (issue #71) — misleading `pin()` warning, orphaned command declarations, a wrong when-clause key, README F3/banner drift, an ungated palette entry, and an uncleared debounce timer. + +**Architecture:** Six independent point fixes across `src/diffViewController.ts`, `src/gitBaseline.ts`, `package.json`, and `README.md`. No new modules, no model/persistence changes, no new invariants. Verification is the existing suites (all six items are copy/declaration/cleanup changes with no unit-testable pure surface — the unit suite is vscode-free and these all live at the vscode boundary; E2E cannot observe warning-message text or palette visibility). + +**Tech Stack:** VS Code extension (TypeScript, esbuild CJS bundle), vitest unit suite (`npm test`), `@vscode/test-electron` host E2E (`npm run test:e2e`). + +## Global Constraints + +- Work happens on branch `worktree-s71-surface-polish` in the worktree at `.claude/worktrees/s71-surface-polish` (canonical checkout is occupied by concurrent session 0062 — never touch it). +- Commits cite #71, read like surrounding history, and carry the `Co-Authored-By: Claude Fable 5 ` trailer. +- Warning copy for the not-coediting case is exactly `"Run ✦ Coedit this Document with Claude first."` — the string `src/threadController.ts:181` already uses. Do not coin a variant. +- No inline trailing comments on shell commands (wgl `no-inline-cli-comments`). +- Final gate before PR: `npm test` (unit), `npm run typecheck`, `npm run build`, `npm run test:e2e` all green. + +--- + +### Task 1: `pin()` explicit not-coediting branch (item 1) + +**Files:** +- Modify: `src/diffViewController.ts:145-155` + +**Interfaces:** +- Consumes: `this.modes: Map` (set only by `establish()`). +- Produces: nothing new — same `pin(document): void` signature; only the warning copy forks. + +`pin()` currently emits "this document is git-tracked — commit to advance the baseline" for **any** non-snapshot mode, including `undefined` (a document that never established, i.e. is not being coedited) — a wrong message for that case. + +- [ ] **Step 1: Fork the guard on `undefined`** + +Replace lines 145–155 (the whole `pin` method) with: + +```ts + pin(document: vscode.TextDocument): void { + if (!this.isDiffable(document)) return; + const key = this.uriKey(document); + const mode = this.modes.get(key); + if (mode === undefined) { + // never established — the document is not being coedited (#71 item 1) + void vscode.window.showWarningMessage("Run ✦ Coedit this Document with Claude first."); + return; + } + if (mode !== "snapshot") { + void vscode.window.showWarningMessage( + "Cowriting: this document is git-tracked — commit to advance the baseline.", + ); + return; + } + this.capture(document, "pinned"); + } +``` + +Keep the existing doc comment above the method unchanged. + +- [ ] **Step 2: Verify** + +Run: `npm run typecheck` +Expected: clean exit. + +- [ ] **Step 3: Commit** + +```bash +git add src/diffViewController.ts +git commit -m "fix: markReviewed on a never-established doc says 'coedit first', not 'git-tracked' (#71 item 1)" +``` + +--- + +### Task 2: delete orphaned `acceptProposal` / `rejectProposal` declarations (item 2) + +**Files:** +- Modify: `package.json` (two `contributes.commands` entries ~85-93, two `contributes.menus.commandPalette` entries ~180-187) + +**Interfaces:** none — pure declaration removal. `test/e2e/suite-no-workspace/noWorkspace.test.ts:42-43` already asserts these commands are **not registered** at runtime; nothing registers them in `src/`. + +- [ ] **Step 1: Delete the two command declarations** + +In `contributes.commands`, delete both objects: + +```json + { + "command": "cowriting.acceptProposal", + "title": "✓ Accept Proposal", + "category": "Cowriting" + }, + { + "command": "cowriting.rejectProposal", + "title": "✗ Reject Proposal", + "category": "Cowriting" + }, +``` + +- [ ] **Step 2: Delete their palette-hiding menu entries** + +In `contributes.menus.commandPalette`, delete both objects (they exist only to hide the now-deleted declarations): + +```json + { + "command": "cowriting.acceptProposal", + "when": "false" + }, + { + "command": "cowriting.rejectProposal", + "when": "false" + }, +``` + +- [ ] **Step 3: Verify no dangling references** + +Run: `grep -rn "acceptProposal\|rejectProposal" package.json src/` +Expected: no matches (the only remaining references are the retirement assertions in `test/e2e/suite-no-workspace/noWorkspace.test.ts`). + +Run: `node -e "JSON.parse(require('fs').readFileSync('package.json','utf8')); console.log('valid json')"` +Expected: `valid json` + +- [ ] **Step 4: Commit** + +```bash +git add package.json +git commit -m "chore: delete orphaned acceptProposal/rejectProposal declarations (#71 item 2)" +``` + +--- + +### Task 3: `openReviewPreview` editor/title when-key (item 3) + +**Files:** +- Modify: `package.json:260` (the `editor/title` menu entry for `cowriting.openReviewPreview`) + +**Interfaces:** none — one-token when-clause fix. Title menus key off the resource, so `resourceLangId` is correct (every sibling `editor/title` entry already uses it); `editorLangId` is the odd one out. + +- [ ] **Step 1: Fix the key** + +In `contributes.menus."editor/title"`, change: + +```json + { + "command": "cowriting.openReviewPreview", + "when": "editorLangId == markdown", + "group": "navigation@9" + } +``` + +to: + +```json + { + "command": "cowriting.openReviewPreview", + "when": "resourceLangId == markdown", + "group": "navigation@9" + } +``` + +(Only the `editor/title` entry. The `commandPalette` entries elsewhere legitimately use `editorLangId`; the `editor/title/context` and `explorer/context` entries already use `resourceLangId`.) + +- [ ] **Step 2: Commit** + +```bash +git add package.json +git commit -m "fix: openReviewPreview title-menu when uses resourceLangId like its siblings (#71 item 3)" +``` + +--- + +### Task 4: gate the `cowriting.createThread` palette entry (item 5) + +**Files:** +- Modify: `package.json` (`contributes.menus.commandPalette`) + +**Interfaces:** none — `createThreadOnSelection` (`src/threadController.ts:197-201`) already returns `undefined` silently without a selection / coediting / authorable scheme; the palette entry is currently unconditionally visible, so the command silently no-ops from the palette. Fix = gate the palette entry with the **same when-clause its `editor/context` entry uses** (`package.json:290-293`), so the palette only offers it when it can act — consistent with how `markReviewed`'s palette gate was tightened at merge. + +- [ ] **Step 1: Add the palette gate** + +In `contributes.menus.commandPalette`, insert after the `cowriting.proposeAgentEdit` entry: + +```json + { + "command": "cowriting.createThread", + "when": "editorHasSelection && (resourceScheme == file || resourceScheme == untitled) && cowriting.isCoediting" + }, +``` + +- [ ] **Step 2: Verify JSON** + +Run: `node -e "JSON.parse(require('fs').readFileSync('package.json','utf8')); console.log('valid json')"` +Expected: `valid json` + +- [ ] **Step 3: Commit** + +```bash +git add package.json +git commit -m "fix: gate createThread palette entry on selection+coediting like its context-menu entry (#71 item 5)" +``` + +--- + +### Task 5: clear the `gitBaseline` reflog debounce timer in `dispose()` (item 6) + +**Files:** +- Modify: `src/gitBaseline.ts:113-126` (`watchReflog`) + +**Interfaces:** none — same disposable contract; the pushed disposable additionally clears the pending debounce timeout so no one-shot `repo.status()` fires post-dispose. + +- [ ] **Step 1: Clear the timer in the pushed disposable** + +In `watchReflog`, change: + +```ts + const watcher = fs.watch(reflog, () => { + if (pending) clearTimeout(pending); + pending = setTimeout(() => void repo.status().catch(() => {}), 150); + }); + this.disposables.push({ dispose: () => watcher.close() }); +``` + +to: + +```ts + const watcher = fs.watch(reflog, () => { + if (pending) clearTimeout(pending); + pending = setTimeout(() => void repo.status().catch(() => {}), 150); + }); + this.disposables.push({ + dispose: () => { + watcher.close(); + if (pending) clearTimeout(pending); + }, + }); +``` + +- [ ] **Step 2: Verify** + +Run: `npm run typecheck` +Expected: clean exit. + +- [ ] **Step 3: Commit** + +```bash +git add src/gitBaseline.ts +git commit -m "fix: clear reflog debounce timer on GitBaselineWatcher dispose (#71 item 6)" +``` + +--- + +### Task 6: README F3 drift + stale chord notes in superseded banners (item 4) + +**Files:** +- Modify: `README.md` (F3 section ~106, F7 banner ~230-233, F10 banner ~313-317) + +**Interfaces:** none — docs only. Current reality the banner must state: `Cowriting: Toggle Attribution` no longer exists (annotations toggle via **Toggle Annotations**, How it works §6); "Ask Claude to Edit Selection" is palette-hidden (`package.json:201-203` — `"when": "false"`), superseded by **Ask Claude to Edit** (`Ctrl+Alt+E` / `Cmd+Alt+E`) and the comments-first **Ask Claude** (§3). `Ctrl+Alt+R` today is **Review Changes** (native diff), not any preview. + +- [ ] **Step 1: Add the F3 banner** + +Immediately after the `## F3 — Live human/Claude attribution (Feature #6)` heading (before the "As you and Claude coauthor…" paragraph), insert: + +```markdown +> **Commands superseded (native-surfaces migration).** The attribution **data +> layer** below is current, but the Commands block is historical: +> `Cowriting: Toggle Attribution` was retired — authorship/change coloring now +> toggles via **Toggle Annotations** on a coedited document (How it works §6) — +> and "Ask Claude to Edit Selection" is palette-hidden; use **Ask Claude to +> Edit** (`Ctrl+Alt+E` / `Cmd+Alt+E`) or the comments-first **Ask Claude** (How +> it works §3). Kept below as a historical record. +``` + +- [ ] **Step 2: Chord note in the F7 banner** + +Extend the F7 superseded banner's last sentence. Change: + +```markdown +> **Superseded (Task 8, native-surfaces migration).** The bespoke webview this +> section describes was deleted; its authorship/change coloring lives on +> **inside VS Code's own built-in Markdown preview** — see **How it works** §6 +> above. Kept below as a historical record of the pre-migration design. +``` + +to: + +```markdown +> **Superseded (Task 8, native-surfaces migration).** The bespoke webview this +> section describes was deleted; its authorship/change coloring lives on +> **inside VS Code's own built-in Markdown preview** — see **How it works** §6 +> above. Kept below as a historical record of the pre-migration design. (The +> `Ctrl+Alt+R` chord below now opens the native **Review Changes** diff; the +> annotated preview opens via **Open Cowriting Review Preview**, no chord.) +``` + +- [ ] **Step 3: Chord note in the F10 banner** + +Change the F10 banner's last sentence from: + +```markdown +> **How it works** above. Kept below as a historical record. +``` + +to: + +```markdown +> **How it works** above. Kept below as a historical record. (The `Ctrl+Alt+R` +> chord below now opens the native **Review Changes** diff, not this panel.) +``` + +- [ ] **Step 4: Commit** + +```bash +git add README.md +git commit -m "docs: F3 superseded-commands banner + stale-chord notes in F7/F10 banners (#71 item 4)" +``` + +--- + +### Task 7: full verification + PR + merge + +**Files:** none new. + +- [ ] **Step 1: Full local gate** + +Run each; all must be green: + +```bash +npm test +npm run typecheck +npm run build +npm run test:e2e +``` + +Expected: unit suite passes (~250 tests), typecheck clean, build succeeds, host E2E passes (undo suites may self-skip via the #54 runtime probe — a loud warning, not a failure). + +- [ ] **Step 2: Push branch + open PR** + +```bash +git push -u origin worktree-s71-surface-polish +``` + +Open a PR against `main` on `benstull/vscode-cowriting-plugin` (Gitea, SSH remote) titled `Surface polish: batched Minor findings from the native-surfaces final review (#71)`; body lists the six items and cites the issue. + +- [ ] **Step 3: Merge (squash) and confirm #71 auto-close/close** + +Merge the PR; verify `main` carries the change and close #71 if the merge didn't.