diff --git a/docs/superpowers/plans/2026-07-02-70-inv5-reject-after-interior-edit.md b/docs/superpowers/plans/2026-07-02-70-inv5-reject-after-interior-edit.md new file mode 100644 index 0000000..9232a87 --- /dev/null +++ b/docs/superpowers/plans/2026-07-02-70-inv5-reject-after-interior-edit.md @@ -0,0 +1,526 @@ +# #70 — INV-5 Reject-After-Interior-Edit Gap 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:** `✗ Reject` on an optimistically-applied proposal restores the retained original (INV-5) even after the writer typed inside the pending range — and when it genuinely cannot locate the applied span, it fails loudly instead of reporting success while leaving the proposed text behind. + +**Architecture:** Issue #70's fix direction **(a)** with an honest **(b)** fallback. A new per-doc `appliedSpans: Map` becomes the real F12 applied-range bookkeeping: written at optimistic-apply time, re-synced whenever the exact anchor resolves at `renderAll`, shifted through interior-safe buffer edits (`shift()`), and **deleted (distrusted)** when an edit straddles a span boundary (e.g. a whole-buffer external replace — a shifted range clamps to garbage there, and restoring at a guessed offset would corrupt the doc, INV-11's "never applied by guess"). `revertInPlace` then reverts via exact resolve → `appliedSpans` fallback → hard failure (warning, proposal kept, `false` returned). Direction (c) (fuzzy resolve) is rejected: it guesses. `rejectAll` orders by the same fallback and reports skips like `acceptAllProposals` does. + +**Tech Stack:** TypeScript VS Code extension; `npm test` (vitest unit), `npm run test:e2e` (@vscode/test-electron host suite — `ProposalController` is vscode-coupled, so coverage is host E2E, matching every existing F12 test). + +## Global Constraints + +- INV-5: "Reject restores the retained original exactly" — the contract this fix enforces; `revertInPlace`'s doc comment already promises "reverts the whole block regardless of any in-place edits the human made". +- INV-11: anchors are immutable for a proposal's life; stale text is flagged, **never applied by guess** — the fallback must only use a span whose tracking is provably continuous (no fuzzy resolve, no boundary-straddled ranges). +- INV-12: accept/reject stay human-only gestures; no new mutation paths. +- Keep `✓ Keep` (`finalizeInPlace`) behavior byte-identical — its lookup-by-id bypass of resolution is correct by design (issue "Contrast" note). +- Existing test suites must stay green: 265+ unit, full host E2E (notably `proposals.test.ts` INV-11 stale-accept, `f12InlineDiff.test.ts`, `fullLoop.test.ts`). +- Wiggleverse hygiene: no inline trailing comments in CLI blocks; commits cite the issue and carry the `Co-Authored-By` trailer. + +--- + +### Task 1: `appliedSpans` bookkeeping + `revertInPlace` tracked-range fallback (the INV-5 repro) + +**Files:** +- Modify: `src/proposalController.ts` (DocState ~line 35, `ensureState` ~127, `optimisticApply` ~336, `finalizeInPlace` ~417, `revertInPlace` ~434, `renderAll` ~495, `onDidChange` ~529) +- Test: `test/e2e/suite/f12InlineDiff.test.ts` (new suite appended) + +**Interfaces:** +- Consumes: `shift(range, edit)` / `resolve(text, fp)` from `src/anchorer.ts` (existing); `DocState.applied: Set` (existing). +- Produces: `DocState.appliedSpans: Map` (private bookkeeping; Tasks 2–3 rely on its distrust + ordering semantics); `revertInPlace(docPath, proposalId, opts?: { silent?: boolean }): Promise` — signature gains the optional `opts` used by Task 3. + +- [ ] **Step 1: Write the failing E2E test — the issue's repro sketch** + +Append to `test/e2e/suite/f12InlineDiff.test.ts`: + +```typescript +// #70 (INV-5): rejecting a proposal AFTER typing inside its pending range must +// still restore the retained original. The exact-substring anchor is orphaned by +// the interior tweak (INV-1/INV-11) — the revert falls back to the F12 +// applied-range bookkeeping (appliedSpans), which shift-tracks the span through +// interior edits. +suite("F12 inline diff — #70 reject after interior edit (INV-5)", () => { + test("revertInPlace restores the original after a tweak inside the pending range", async () => { + const ORIGINAL = "The quick brown fox jumps."; + const { doc } = await freshDoc("docs/s70-reject-tweaked.md", `# T\n\n${ORIGINAL}\n`); + const api = await getApi(); + const p = api.proposalController; + const docKey = p.keyFor(doc); + const fp = { text: ORIGINAL, before: "", after: "", lineHint: 2 }; + const id = await p.propose(doc, fp, "The rapid brown fox jumps.", + { kind: "agent", id: "claude", agent: { sdk: "x", model: "m", sessionId: "s" } }, { granularity: "block" }); + await p.optimisticApply(doc, id!); + await settle(); + assert.ok(doc.getText().includes("The rapid brown fox jumps."), "optimistically applied"); + // Human types INSIDE the pending range → orphans the exact anchor (INV-11). + const at = doc.getText().indexOf("rapid"); + const we = new vscode.WorkspaceEdit(); + we.replace(doc.uri, new vscode.Range(doc.positionAt(at), doc.positionAt(at + "rapid".length)), "RAPID-ish"); + assert.ok(await vscode.workspace.applyEdit(we), "interior tweak applied"); + await settle(); + assert.strictEqual(p.listProposals(doc).find((v) => v.id === id)!.anchorStart, null, "anchor orphaned by the tweak"); + // ✗ Reject: must restore the retained original EXACTLY (INV-5), not skip. + assert.ok(await p.revertInPlace(docKey, id!), "reject reports success"); + await settle(); + assert.strictEqual(doc.getText(), `# T\n\n${ORIGINAL}\n`, "original restored exactly (INV-5)"); + assert.strictEqual(p.listProposals(doc).length, 0, "proposal cleared"); + }); + + test("rejectByIdInPlace routes the tweaked-applied case the same way", async () => { + const ORIGINAL = "A steady closing line."; + const { doc } = await freshDoc("docs/s70-reject-route.md", `# T\n\n${ORIGINAL}\n`); + const api = await getApi(); + const p = api.proposalController; + const docKey = p.keyFor(doc); + const fp = { text: ORIGINAL, before: "", after: "", lineHint: 2 }; + const id = await p.propose(doc, fp, "A wobbly closing line.", + { kind: "agent", id: "claude", agent: { sdk: "x", model: "m", sessionId: "s" } }, { granularity: "block" }); + await p.optimisticApply(doc, id!); + await settle(); + const at = doc.getText().indexOf("wobbly"); + const we = new vscode.WorkspaceEdit(); + we.replace(doc.uri, new vscode.Range(doc.positionAt(at), doc.positionAt(at)), "very-"); + assert.ok(await vscode.workspace.applyEdit(we), "interior insertion applied"); + await settle(); + assert.ok(await p.rejectByIdInPlace(docKey, id!), "gesture-level reject succeeds"); + await settle(); + assert.strictEqual(doc.getText(), `# T\n\n${ORIGINAL}\n`, "original restored exactly (INV-5)"); + }); +}); +``` + +- [ ] **Step 2: Run the new suite to verify it fails** + +```bash +npm run test:e2e 2>&1 | grep -A 3 "#70" +``` + +Expected: both tests FAIL — first on `doc.getText()` still holding the tweaked replacement (`RAPID-ish`) after a `true` return (the silent-skip bug), i.e. the "original restored exactly" assertion. + +- [ ] **Step 3: Implement `appliedSpans` + the fallback** + +In `src/proposalController.ts`: + +(1) `DocState` gains the map (after `applied`): + +```typescript + /** ids already optimistically applied to the buffer (so the trigger doesn't re-apply). */ + applied: Set; + /** + * #70 (INV-5): the F12 applied-range bookkeeping — proposal id -> the applied + * span's CURRENT buffer range. Written at optimistic-apply, re-synced whenever + * the exact anchor resolves (renderAll), shifted through interior-safe edits, + * and DELETED (distrusted) when an edit straddles a span boundary — a clamped + * range is a guess, and stale text is never applied by guess (INV-11). Lets + * Reject restore the original after the human types INSIDE the pending range + * (which orphans the exact-substring anchor). + */ + appliedSpans: Map; +``` + +(2) `ensureState` initializes it: `appliedSpans: new Map(),` after `applied: new Set(),`. + +(3) `optimisticApply` records the span where it already re-anchors (after `state.applied.add(proposalId);` and the `appliedStart`/`appliedEnd` computation — move the `add` down beside it or record right after the existing lines): + +```typescript + state.applied.add(proposalId); + // Re-anchor to the applied text now in the buffer (its start is unchanged; its + // end shifts by the net length delta of the replacement). + const appliedStart = resolved.start; + const appliedEnd = appliedStart + proposal.replacement.length; + state.appliedSpans.set(proposalId, { start: appliedStart, end: appliedEnd }); +``` + +(4) `renderAll`'s resolved branch re-syncs (covers the prior-session-applied reload, where the in-memory map starts empty): + +```typescript + } else { + this.recordProposal(state, proposal, resolved, true); + if (state.applied.has(proposal.id)) state.appliedSpans.set(proposal.id, resolved); + } +``` + +(5) `onDidChange` maintains it — interior-safe edits shift, boundary-straddling edits distrust. Replace the method body: + +```typescript + private onDidChange(e: vscode.TextDocumentChangeEvent): void { + const state = this.docs.get(this.keyOf(e.document)); + if (!state || (state.live.size === 0 && state.appliedSpans.size === 0)) return; + for (const change of e.contentChanges) { + const edit = { start: change.rangeOffset, end: change.rangeOffset + change.rangeLength, newLength: change.text.length }; + for (const [id, range] of state.live) state.live.set(id, shift(range, edit)); + // #70: an edit fully inside a tracked applied span (or fully outside it) + // keeps the span meaningful; one straddling a boundary — e.g. a whole-buffer + // external replace — makes the shifted range a clamped guess, so the span + // is distrusted (deleted) rather than reverted-to by guess (INV-11). + for (const [id, range] of state.appliedSpans) { + const outside = edit.end <= range.start || edit.start >= range.end; + const inside = edit.start >= range.start && edit.end <= range.end; + if (outside || inside) state.appliedSpans.set(id, shift(range, edit)); + else state.appliedSpans.delete(id); + } + } + } +``` + +(6) `revertInPlace` — the fix itself. Replace the method (keep its position; doc comment updated to describe the layered lookup; `opts` is consumed by Task 3): + +```typescript + /** + * F12/#64 (INV-51): REJECT an optimistically-applied proposal — replace its live + * applied span with the stored `original`, then clear it. Reverts the whole block + * regardless of any in-place edits the human made to the inserted text: when an + * interior tweak has orphaned the exact anchor (INV-11), the revert falls back to + * the shift-tracked applied span (#70, INV-5). When neither locates the span + * (e.g. a boundary-straddling external rewrite distrusted it), the reject FAILS + * loudly — warning shown, proposal kept, `false` returned — instead of silently + * leaving the proposed text in the buffer. Undo (or ✓ Keep) remains the way out. + */ + async revertInPlace(docPath: string, proposalId: string, opts?: { silent?: boolean }): Promise { + const hit = this.byId(docPath, proposalId); + if (!hit) return false; + const document = this.openDoc(hit.state); + if (!document) return false; + // Never optimistically applied → nothing of ours is in the buffer; clearing + // the record IS the reject (the legacy pending-only path). + if (hit.proposal.original === undefined) { + this.store.update(docPath, (a) => removeProposal(a, proposalId)); + hit.state.applied.delete(proposalId); + hit.state.appliedSpans.delete(proposalId); + this.renderAll(document); + return true; + } + const fp = hit.state.artifact.anchors[hit.proposal.anchorId]?.fingerprint; + const resolved = fp ? resolve(document.getText(), fp) : "orphaned"; + const span = resolved !== "orphaned" ? resolved : hit.state.appliedSpans.get(proposalId); + if (!span) { + if (!opts?.silent) { + void vscode.window.showWarningMessage( + "Cowriting: this proposal's applied text can't be located (it changed or moved) — undo your edits, or Keep it to leave the buffer as-is (it is never reverted by guess).", + ); + } + return false; + } + const we = new vscode.WorkspaceEdit(); + we.replace( + document.uri, + new vscode.Range(document.positionAt(span.start), document.positionAt(span.end)), + hit.proposal.original, + ); + if (!(await vscode.workspace.applyEdit(we))) return false; + this.store.update(docPath, (a) => removeProposal(a, proposalId)); + hit.state.applied.delete(proposalId); + hit.state.appliedSpans.delete(proposalId); + this.renderAll(document); + return true; + } +``` + +(7) `finalizeInPlace` cleans the map beside its existing `applied.delete`: + +```typescript + hit.state.applied.delete(proposalId); + hit.state.appliedSpans.delete(proposalId); +``` + +- [ ] **Step 4: Typecheck + unit tests + the new E2E** + +```bash +npm run typecheck && npm test +npm run test:e2e +``` + +Expected: typecheck clean; all unit tests PASS (no unit test touches `ProposalController`); host E2E PASS including the two new #70 tests and all pre-existing F12/proposals/fullLoop suites. + +- [ ] **Step 5: Commit** + +```bash +git add src/proposalController.ts test/e2e/suite/f12InlineDiff.test.ts +git commit -m "fix(#70): reject after an interior tweak restores the original via tracked applied span (INV-5) + +Co-Authored-By: Claude Fable 5 " +``` + +--- + +### Task 2: The honest hard-failure path (fix direction (b) for the residual orphan) + +**Files:** +- Modify: `src/proposalController.ts` (only if Step 2 exposes a gap — the Task 1 code already implements the path) +- Test: `test/e2e/suite/f12InlineDiff.test.ts` (extend the #70 suite) + +**Interfaces:** +- Consumes: Task 1's `appliedSpans` distrust semantics and `revertInPlace` failure contract. +- Produces: nothing new — locks the contract with a regression test. + +- [ ] **Step 1: Write the failing/locking E2E test** + +Append inside the `#70` suite from Task 1: + +> **Execution note:** the first cut of this test used a whole-buffer rewrite as the +> distrusting edit and FAILED — VS Code minimizes workspace edits before change +> events fire, so a rewrite sharing a prefix/suffix with the old text decomposes +> into interior hunks the span legitimately survives (and the revert then correctly +> restores the original in place). The distrusting edit must straddle a span +> boundary even after minimization — a deletion from outside the span into its +> interior, as below. + +```typescript + // Fix direction (b): when the tracked span itself is distrusted (an edit + // straddled its boundary — here a deletion running from the heading into the + // span's interior), the reject FAILS honestly: warning path, proposal kept, + // buffer untouched. Silent-skip-and-report-success (the #70 bug) must not come + // back; reverting at a clamped guessed offset (INV-11) must not either. + // (A boundary-straddling DELETION is used because VS Code minimizes workspace + // edits before change events fire — a wholesale rewrite sharing a prefix/suffix + // decomposes into interior hunks the span legitimately survives.) + test("reject fails loudly — proposal kept, buffer untouched — when the span is distrusted", async () => { + const ORIGINAL = "Sentence one stands here."; + const { doc } = await freshDoc("docs/s70-reject-distrust.md", `# T\n\n${ORIGINAL}\n`); + const api = await getApi(); + const p = api.proposalController; + const docKey = p.keyFor(doc); + const fp = { text: ORIGINAL, before: "", after: "", lineHint: 2 }; + const id = await p.propose(doc, fp, "Sentence ONE stands here.", + { kind: "agent", id: "claude", agent: { sdk: "x", model: "m", sessionId: "s" } }, { granularity: "block" }); + await p.optimisticApply(doc, id!); + await settle(); + // Delete from inside the heading through the middle of the applied span: + // the edit straddles the span's start boundary → the tracked span is + // distrusted AND the exact anchor no longer resolves. + const mid = doc.getText().indexOf("ONE"); + const we = new vscode.WorkspaceEdit(); + we.replace(doc.uri, new vscode.Range(doc.positionAt(2), doc.positionAt(mid + 1)), ""); + assert.ok(await vscode.workspace.applyEdit(we), "boundary-straddling deletion applied"); + await settle(); + const before = doc.getText(); + assert.strictEqual(await p.revertInPlace(docKey, id!), false, "reject reports failure (no silent success)"); + await settle(); + assert.strictEqual(doc.getText(), before, "buffer untouched — never reverted by guess (INV-11)"); + assert.ok(p.listProposals(doc).some((v) => v.id === id), "proposal kept (not silently dropped)"); + // Cleanup for later tests: the never-locatable husk is discarded via the + // plain record-only reject. + assert.strictEqual(p.rejectById(docKey, id!), true); + }); +``` + +- [ ] **Step 2: Run it** + +```bash +npm run test:e2e 2>&1 | grep -B 2 -A 5 "distrusted" +``` + +Expected: PASS if Task 1's implementation is complete (this is the locking regression); if it FAILS, the failure pinpoints which leg (distrust bookkeeping vs. failure return vs. proposal retention) is wrong — fix `src/proposalController.ts` accordingly, not the test. + +- [ ] **Step 3: Commit** + +```bash +git add test/e2e/suite/f12InlineDiff.test.ts src/proposalController.ts +git commit -m "test(#70): lock the honest hard-failure reject path (distrusted span → warn, keep, false) + +Co-Authored-By: Claude Fable 5 " +``` + +--- + +### Task 3: `rejectAll` parity — tweaked proposals revert too, skips are reported + +**Files:** +- Modify: `src/proposalController.ts` (`rejectAll` ~line 461), `src/extension.ts` (`cowriting.rejectAllProposals` handler ~line 220) +- Test: `test/e2e/suite/f12InlineDiff.test.ts` (extend the #70 suite) + +**Interfaces:** +- Consumes: Task 1's `revertInPlace(docPath, id, opts?: { silent?: boolean })` and `appliedSpans`. +- Produces: `rejectAll(document): Promise<{ reverted: number; skipped: number }>` — additive `skipped` field (existing `{ reverted }` destructurings stay valid). + +- [ ] **Step 1: Write the failing E2E test** + +Append inside the `#70` suite: + +```typescript + // rejectAll must revert a tweaked (orphaned-anchor) proposal via the same + // tracked-span fallback, ordered descending by that span so earlier reverts + // never shift later ones — and must count what it could not revert. + test("rejectAll reverts tweaked proposals via the tracked span and reports skips", async () => { + const { doc } = await freshDoc("docs/s70-rejall-tweak.md", "# T\n\nOne aaa.\n\nTwo bbb.\n"); + const api = await getApi(); + const p = api.proposalController; + const editFlow = api.editFlow; + editFlow.setEditTurnForTest(async () => ({ replacement: "# T\n\nOne AAA.\n\nTwo BBB.\n", model: "m", sessionId: "s" })); + const ids = await editFlow.runEditAndPropose(doc, { kind: "document" }, "up"); + await settle(); + for (const id of ids) await p.optimisticApply(doc, id); + await settle(); + // Tweak INSIDE the first applied block → its exact anchor orphans. + const at = doc.getText().indexOf("AAA"); + const we = new vscode.WorkspaceEdit(); + we.replace(doc.uri, new vscode.Range(doc.positionAt(at), doc.positionAt(at + 3)), "AAAZ"); + assert.ok(await vscode.workspace.applyEdit(we), "interior tweak applied"); + await settle(); + const { reverted, skipped } = await p.rejectAll(doc); + await settle(); + assert.strictEqual(reverted, ids.length, "ALL proposals reverted, tweaked one included"); + assert.strictEqual(skipped, 0, "nothing skipped"); + assert.strictEqual(doc.getText(), "# T\n\nOne aaa.\n\nTwo bbb.\n", "document fully restored (INV-5)"); + assert.strictEqual(p.listProposals(doc).length, 0, "all cleared"); + }); +``` + +- [ ] **Step 2: Run it to verify the current gap** + +```bash +npm run test:e2e 2>&1 | grep -B 2 -A 5 "rejectAll reverts tweaked" +``` + +Expected: FAIL — today the tweaked proposal sorts as an orphan (`start: -1`, reverted last), and after the first successful revert's `renderAll` the old code silently removes it without restoring (`document fully restored` assertion fails), and `skipped` does not exist on the return type (typecheck catches first — that is the same failure). + +- [ ] **Step 3: Implement — order by the fallback span, count skips, report them** + +Replace `rejectAll` in `src/proposalController.ts`: + +```typescript + /** + * F12/#64 (INV-53): reject EVERY pending proposal on a document — revert each in + * DESCENDING span order (so an earlier revert never shifts a later one's + * offsets), symmetric with #46's accept-all. #70: a tweaked proposal whose exact + * anchor is orphaned orders (and reverts) by its shift-tracked applied span; + * one with no locatable span is SKIPPED — counted, never guessed (INV-11) — + * and the per-proposal warning is suppressed in favor of the batch report. + */ + async rejectAll(document: vscode.TextDocument): Promise<{ reverted: number; skipped: number }> { + if (!this.isTracked(document)) return { reverted: 0, skipped: 0 }; + const docPath = this.keyOf(document); + const state = this.ensureState(document); + state.artifact = this.store.load(docPath) ?? emptyArtifact(docPath); + const text = document.getText(); + const ordered = state.artifact.proposals + .map((p) => { + const fp = state.artifact.anchors[p.anchorId]?.fingerprint; + const r = fp ? resolve(text, fp) : "orphaned"; + const start = r !== "orphaned" ? r.start : state.appliedSpans.get(p.id)?.start ?? -1; + return { id: p.id, start }; + }) + .sort((a, b) => b.start - a.start); + let reverted = 0; + let skipped = 0; + for (const it of ordered) { + if (await this.revertInPlace(docPath, it.id, { silent: true })) reverted++; + else skipped++; + } + return { reverted, skipped }; + } +``` + +And in `src/extension.ts`, the `cowriting.rejectAllProposals` handler mirrors accept-all's skip note: + +```typescript + const { reverted, skipped } = await proposalController.rejectAll(doc); + if (reverted === 0 && skipped === 0) return; + const skipNote = skipped > 0 ? `, ${skipped} skipped (applied text not locatable — undo your edits or Keep)` : ""; + void vscode.window.showInformationMessage( + `Cowriting: rejected ${reverted} proposal${reverted === 1 ? "" : "s"}${skipNote}.`, + ); +``` + +- [ ] **Step 4: Typecheck + full test run** + +```bash +npm run typecheck && npm test +npm run test:e2e +``` + +Expected: all PASS — including the pre-existing `rejectAll` E2E tests (`f12InlineDiff` "rejectAll reverts every pending proposal", "control parity", `proposals.test.ts` cleanup calls), whose `{ reverted }` destructuring is unaffected by the additive field. + +- [ ] **Step 5: Commit** + +```bash +git add src/proposalController.ts src/extension.ts test/e2e/suite/f12InlineDiff.test.ts +git commit -m "fix(#70): rejectAll reverts tweaked proposals via the tracked span; skips are counted and reported + +Co-Authored-By: Claude Fable 5 " +``` + +--- + +### Task 4: Close the loop — fullLoop E2E rejects a tweaked proposal (the deliberate avoidance falls) + +**Files:** +- Modify: `test/e2e/suite/fullLoop.test.ts` (the PUC-2 reject section, ~line 144) + +**Interfaces:** +- Consumes: the shipped Task 1 behavior; existing fullLoop fixtures (`rejectId`, `REJECT_REPLACEMENT`, `ORIG_REJECT`). +- Produces: nothing — upgrades the flagship E2E so the gap can't silently regress. + +- [ ] **Step 1: Tweak inside the reject proposal's range before rejecting** + +In `test/e2e/suite/fullLoop.test.ts`, immediately before the existing line +`assert.ok(await api.proposalController.revertInPlace(docKey, rejectId), "reject reverts in place");`, +insert: + +```typescript + // #70 (INV-5): the reject leg now ALSO takes an interior tweak first — the + // original deliberately rejected only an un-tweaked proposal because the + // pre-fix revert silently skipped an orphaned anchor. The tweak orphans the + // exact anchor; the revert must restore ORIG_REJECT via the tracked span. + const rejAt = doc.getText().indexOf(REJECT_REPLACEMENT); + assert.ok(rejAt >= 0, "reject proposal's applied text present"); + const rejTweak = new vscode.WorkspaceEdit(); + rejTweak.replace(doc.uri, new vscode.Range(doc.positionAt(rejAt + 2), doc.positionAt(rejAt + 2)), "x"); + assert.ok(await vscode.workspace.applyEdit(rejTweak), "human tweak inside the reject proposal's range"); + await settle(); + assert.strictEqual( + api.proposalController.listProposals(doc).find((v) => v.id === rejectId)!.anchorStart, + null, + "tweak orphans the reject proposal's exact anchor (INV-11)", + ); +``` + +(The existing assertions right after — proposal cleared, `ORIG_REJECT` restored, `REJECT_REPLACEMENT` gone — now verify the #70 path. If `REJECT_REPLACEMENT` is a multi-word string whose slice at `+2` splits a word the later `!doc.getText().includes(REJECT_REPLACEMENT)` assertion still holds; the `includes(ORIG_REJECT)` restore assertion is the INV-5 check.) + +- [ ] **Step 2: Run the fullLoop suite** + +```bash +npm run test:e2e 2>&1 | grep -B 2 -A 8 "fullLoop\|full-loop\|full loop" +``` + +Expected: PASS end-to-end (would FAIL on unfixed code: the buffer would keep the tweaked `REJECT_REPLACEMENT`, so `includes(ORIG_REJECT)` fails). + +- [ ] **Step 3: Full verification sweep** + +```bash +npm run typecheck && npm test && npm run test:e2e +``` + +Expected: everything green. + +- [ ] **Step 4: Commit** + +```bash +git add test/e2e/suite/fullLoop.test.ts +git commit -m "test(#70): fullLoop reject leg now tweaks inside the pending range first (INV-5 end-to-end) + +Co-Authored-By: Claude Fable 5 " +``` + +--- + +## Post-execution review wave (not in the original tasks) + +A high-effort multi-agent branch review after Task 4 confirmed 10 findings; all +but one were fixed in a follow-up commit on this branch ("harden the +tracked-span revert per branch review"): pure `shiftTracked` in `anchorer.ts` +(+11 unit tests; insertion-at-span-start lands before the span), tracked spans +cleared on document close, rebuild-only resync at `renderAll`, `guard.isReadOnly` +on `revertInPlace`/`finalizeInPlace`, a "Discard proposal (leave text)" action on +the hard-fail warning, CodeLens pair anchored at the tracked span (+1 E2E), +QuickPick batch menu routed through the reporting commands, and one +`clearProposal` helper (also fixes `reject()`'s stale `applied`/`appliedSpans`). +The remaining finding — `resolve()` accepts a single exact occurrence without a +context check, so a duplicated block can hijack any anchor consumer — pre-dates +this fix and is filed as its own issue. + +## Self-review notes + +- **Issue coverage:** direction (a) → Task 1 (`appliedSpans` fallback); direction (b) → Task 2 (hard failure, warn, keep); direction (c) explicitly rejected (guessing violates INV-11). The issue's repro sketch is Task 1's test verbatim; the "full-loop E2E deliberately rejects only an un-tweaked proposal" note is retired by Task 4; `rejectAll` (same `revertInPlace` seam) is covered by Task 3. +- **Type consistency:** `revertInPlace(docPath, proposalId, opts?: { silent?: boolean })` defined in Task 1, consumed in Task 3; `rejectAll` returns `{ reverted, skipped }` (additive); `appliedSpans: Map` used in Tasks 1 and 3. +- **Contrast guard:** `finalizeInPlace` untouched except symmetric `appliedSpans.delete` cleanup — the Keep bypass stays. diff --git a/src/anchorer.ts b/src/anchorer.ts index e8f9162..664f544 100644 --- a/src/anchorer.ts +++ b/src/anchorer.ts @@ -101,6 +101,30 @@ export function resolve(docText: string, fp: Fingerprint): OffsetRange | "orphan return "orphaned"; } +/** + * #70 (INV-5/INV-11): maintain a TRACKED applied span across an in-session edit. + * Unlike `shift`, which clamps every point into a best-effort range, a tracked + * span must stay *provably* meaningful — it is a revert target. So: + * - an edit fully OUTSIDE or fully INSIDE the span shifts it (the span is + * still "the applied block, as tweaked"); + * - an insertion exactly AT the span start lands BEFORE the span (both ends + * shift right) — matching exact-resolve semantics, where text typed just + * before the applied block is never part of it; + * - any edit STRADDLING a span boundary returns "distrusted": the shifted + * result would be a clamped guess, and stale text is never reverted by + * guess (INV-11). + */ +export function shiftTracked(range: OffsetRange, edit: TextEdit): OffsetRange | "distrusted" { + const delta = edit.newLength - (edit.end - edit.start); + if (edit.start === edit.end && edit.start === range.start) { + return { start: range.start + delta, end: range.end + delta }; + } + const outside = edit.end <= range.start || edit.start >= range.end; + const inside = edit.start >= range.start && edit.end <= range.end; + if (!outside && !inside) return "distrusted"; + return shift(range, edit); +} + /** Maintain a live range across an in-session edit (spec §6.4 `shift`). */ export function shift(range: OffsetRange, edit: TextEdit): OffsetRange { const delta = edit.newLength - (edit.end - edit.start); diff --git a/src/editorProposalController.ts b/src/editorProposalController.ts index 379f4d4..cdf4bf1 100644 --- a/src/editorProposalController.ts +++ b/src/editorProposalController.ts @@ -226,9 +226,16 @@ export class EditorProposalController implements vscode.Disposable, vscode.CodeL if (document.languageId !== "markdown") return []; if (!this.registry.isCoediting(document.uri)) return []; const key = this.proposals.keyFor(document); - const applied = this.proposals - .listProposals(document) - .filter((v) => v.anchorStart !== null && this.proposals.isApplied(key, v.id)); + // #70: an interior tweak orphans the exact anchor (anchorStart null) but the + // proposal stays fully decidable — Keep by id, Reject via the tracked span — + // so the lens pair anchors at the tracked span instead of vanishing (which + // made the fixed reject path unreachable from the primary gesture surface). + const applied: { id: string; at: number }[] = []; + for (const v of this.proposals.listProposals(document)) { + if (!this.proposals.isApplied(key, v.id)) continue; + const at = v.anchorStart ?? this.proposals.trackedSpan(key, v.id)?.start; + if (at !== undefined) applied.push({ id: v.id, at }); + } const lenses: vscode.CodeLens[] = []; if (applied.length >= 2) { const top = new vscode.Range(0, 0, 0, 0); @@ -238,7 +245,7 @@ export class EditorProposalController implements vscode.Disposable, vscode.CodeL ); } for (const v of applied) { - const pos = document.positionAt(v.anchorStart!); + const pos = document.positionAt(v.at); const line = new vscode.Range(pos.line, 0, pos.line, 0); lenses.push( new vscode.CodeLens(line, { title: "✓ Keep", command: "cowriting.proposalAcceptMenu", arguments: [v.id] }), @@ -261,10 +268,12 @@ export class EditorProposalController implements vscode.Disposable, vscode.CodeL if (!pick) return; const all = pick.includes("ALL"); if (kind === "accept") { - if (all) await this.proposals.acceptAllProposals(doc); + // The batch commands own the applied/skipped reporting (#70: a silent:true + // batch with a discarded tally re-creates the silent-skip failure mode). + if (all) await vscode.commands.executeCommand("cowriting.acceptAllProposals"); else await this.proposals.finalizeInPlace(key, id); } else { - if (all) await this.proposals.rejectAll(doc); + if (all) await vscode.commands.executeCommand("cowriting.rejectAllProposals"); else await this.proposals.revertInPlace(key, id); } } diff --git a/src/extension.ts b/src/extension.ts index 91916c3..37d17f2 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -223,12 +223,12 @@ export function activate(context: vscode.ExtensionContext): CowritingApi | undef void vscode.window.showWarningMessage("Cowriting: open a Markdown document to reject its proposals."); return; } - const { reverted } = await proposalController.rejectAll(doc); - if (reverted > 0) { - void vscode.window.showInformationMessage( - `Cowriting: rejected ${reverted} proposal${reverted === 1 ? "" : "s"}.`, - ); - } + const { reverted, skipped } = await proposalController.rejectAll(doc); + if (reverted === 0 && skipped === 0) return; + const skipNote = skipped > 0 ? `, ${skipped} skipped (applied text not locatable — undo your edits or Keep)` : ""; + void vscode.window.showInformationMessage( + `Cowriting: rejected ${reverted} proposal${reverted === 1 ? "" : "s"}${skipNote}.`, + ); }), ); diff --git a/src/proposalController.ts b/src/proposalController.ts index 30372e7..1435729 100644 --- a/src/proposalController.ts +++ b/src/proposalController.ts @@ -13,7 +13,7 @@ import * as vscode from "vscode"; import { SidecarRouter, docIdentity } from "./sidecarRouter"; import { emptyArtifact, type Artifact, type Fingerprint, type Proposal, type Provenance } from "./model"; -import { resolve, shift, buildFingerprint, type OffsetRange } from "./anchorer"; +import { resolve, shift, shiftTracked, buildFingerprint, type OffsetRange } from "./anchorer"; import { addProposal, removeProposal, setProposalApplied } from "./proposalModel"; import type { AttributionController } from "./attributionController"; import type { VersionGuard } from "./versionGuard"; @@ -42,6 +42,16 @@ interface DocState { unresolved: Set; /** ids already optimistically applied to the buffer (so the trigger doesn't re-apply). */ applied: Set; + /** + * #70 (INV-5): the F12 applied-range bookkeeping — proposal id -> the applied + * span's CURRENT buffer range. Written at optimistic-apply, re-synced whenever + * the exact anchor resolves (renderAll), shifted through interior-safe edits, + * and DELETED (distrusted) when an edit straddles a span boundary — a clamped + * range is a guess, and stale text is never applied by guess (INV-11). Lets + * Reject restore the original after the human types INSIDE the pending range + * (which orphans the exact-substring anchor). + */ + appliedSpans: Map; } export class ProposalController implements vscode.Disposable { @@ -63,6 +73,13 @@ export class ProposalController implements vscode.Disposable { this.disposables.push(this.onDidChangeProposalsEmitter); this.disposables.push( vscode.workspace.onDidChangeTextDocument((e) => this.onDidChange(e)), + // #70: a tracked applied span is only meaningful while its buffer is live — + // a closed doc can change on disk with no change events, so a span kept + // across close/reopen could revert over unrelated text (INV-11: never by + // guess). Cleared here; a resolving anchor rebuilds it at the next renderAll. + vscode.workspace.onDidCloseTextDocument((doc) => { + this.docs.get(this.keyOf(doc))?.appliedSpans.clear(); + }), // PUC-7 restore-on-enter (mirrors ThreadController/AttributionController): // `renderAll` is the only path that recomputes state.live/unresolved AND // fires onDidChangeProposals — the event EditorProposalController listens @@ -135,6 +152,7 @@ export class ProposalController implements vscode.Disposable { live: new Map(), unresolved: new Set(), applied: new Set(), + appliedSpans: new Map(), }; this.docs.set(docPath, state); } @@ -324,6 +342,22 @@ export class ProposalController implements vscode.Disposable { return this.docs.get(docPath)?.applied.has(proposalId) ?? false; } + /** #70: the shift-tracked applied span (the revert-fallback bookkeeping) — + * exposed so the editor surface can keep the decision gestures reachable + * for an applied proposal whose exact anchor an interior tweak orphaned. */ + trackedSpan(docPath: string, proposalId: string): OffsetRange | undefined { + return this.docs.get(docPath)?.appliedSpans.get(proposalId); + } + + /** The one proposal-clear sequence (record + in-memory bookkeeping + re-render) + * shared by finalize/revert/reject so no path leaves a stale entry behind. */ + private clearProposal(state: DocState, proposalId: string, document?: vscode.TextDocument): void { + this.store.update(state.docPath, (a) => removeProposal(a, proposalId)); + state.applied.delete(proposalId); + state.appliedSpans.delete(proposalId); + if (document) this.renderAll(document); + } + /** * F12/#64 (INV-48): optimistically apply a pending proposal INTO the buffer so * the editor shows the would-be-accepted result (editable). Reuses the F4 @@ -374,6 +408,7 @@ export class ProposalController implements vscode.Disposable { // end shifts by the net length delta of the replacement). const appliedStart = resolved.start; const appliedEnd = appliedStart + proposal.replacement.length; + state.appliedSpans.set(proposalId, { start: appliedStart, end: appliedEnd }); const appliedFp = buildFingerprint(document.getText(), { start: appliedStart, end: appliedEnd }); this.store.update(docPath, (a) => setProposalApplied(a, proposalId, appliedFp, original)); this.renderAll(document); @@ -415,51 +450,79 @@ export class ProposalController implements vscode.Disposable { * purposes anymore (the `onDidApplyAgentEdit → advance` wire was removed). */ async finalizeInPlace(docPath: string, proposalId: string): Promise { + if (this.guard.isReadOnly(docPath)) return false; const hit = this.byId(docPath, proposalId); if (!hit) return false; const document = this.openDoc(hit.state); if (!document) return false; this.attribution.signalLanded(document); - this.store.update(docPath, (a) => removeProposal(a, proposalId)); - hit.state.applied.delete(proposalId); - this.renderAll(document); + this.clearProposal(hit.state, proposalId, document); return true; } /** * F12/#64 (INV-51): REJECT an optimistically-applied proposal — replace its live * applied span with the stored `original`, then clear it. Reverts the whole block - * regardless of any in-place edits the human made to the inserted text. + * regardless of any in-place edits the human made to the inserted text: when an + * interior tweak has orphaned the exact anchor (INV-11), the revert falls back to + * the shift-tracked applied span (#70, INV-5). When neither locates the span + * (e.g. a boundary-straddling external rewrite distrusted it), the reject FAILS + * loudly — warning shown, proposal kept, `false` returned — instead of silently + * leaving the proposed text in the buffer. Undo (or ✓ Keep) remains the way out. */ - async revertInPlace(docPath: string, proposalId: string): Promise { + async revertInPlace(docPath: string, proposalId: string, opts?: { silent?: boolean }): Promise { + if (this.guard.isReadOnly(docPath)) return false; const hit = this.byId(docPath, proposalId); if (!hit) return false; const document = this.openDoc(hit.state); if (!document) return false; + // Never optimistically applied → nothing of ours is in the buffer; clearing + // the record IS the reject (the legacy pending-only path). + if (hit.proposal.original === undefined) { + this.clearProposal(hit.state, proposalId, document); + return true; + } const fp = hit.state.artifact.anchors[hit.proposal.anchorId]?.fingerprint; const resolved = fp ? resolve(document.getText(), fp) : "orphaned"; - if (resolved !== "orphaned" && hit.proposal.original !== undefined) { - const we = new vscode.WorkspaceEdit(); - we.replace( - document.uri, - new vscode.Range(document.positionAt(resolved.start), document.positionAt(resolved.end)), - hit.proposal.original, - ); - if (!(await vscode.workspace.applyEdit(we))) return false; + const span = resolved !== "orphaned" ? resolved : hit.state.appliedSpans.get(proposalId); + if (!span) { + if (!opts?.silent) { + // The revert is refused, but the record must stay dismissable (a window + // reload empties both the tracked spans and the undo stack): offer a + // record-only discard that leaves the buffer exactly as it stands. + const DISCARD = "Discard proposal (leave text)"; + void vscode.window + .showWarningMessage( + "Cowriting: this proposal's applied text can't be located (it changed or moved) — it is never reverted by guess. Undo your edits, Keep it, or discard the record as-is.", + DISCARD, + ) + .then((choice) => { + if (choice === DISCARD) this.clearProposal(hit.state, proposalId, document); + }); + } + return false; } - this.store.update(docPath, (a) => removeProposal(a, proposalId)); - hit.state.applied.delete(proposalId); - this.renderAll(document); + const we = new vscode.WorkspaceEdit(); + we.replace( + document.uri, + new vscode.Range(document.positionAt(span.start), document.positionAt(span.end)), + hit.proposal.original, + ); + if (!(await vscode.workspace.applyEdit(we))) return false; + this.clearProposal(hit.state, proposalId, document); return true; } /** * F12/#64 (INV-53): reject EVERY pending proposal on a document — revert each in - * DESCENDING anchor order (so an earlier revert never shifts a later one's - * offsets), symmetric with #46's accept-all. Returns the reverted count. + * DESCENDING span order (so an earlier revert never shifts a later one's + * offsets), symmetric with #46's accept-all. #70: a tweaked proposal whose exact + * anchor is orphaned orders (and reverts) by its shift-tracked applied span; + * one with no locatable span is SKIPPED — counted, never guessed (INV-11) — + * and the per-proposal warning is suppressed in favor of the batch report. */ - async rejectAll(document: vscode.TextDocument): Promise<{ reverted: number }> { - if (!this.isTracked(document)) return { reverted: 0 }; + async rejectAll(document: vscode.TextDocument): Promise<{ reverted: number; skipped: number }> { + if (!this.isTracked(document)) return { reverted: 0, skipped: 0 }; const docPath = this.keyOf(document); const state = this.ensureState(document); state.artifact = this.store.load(docPath) ?? emptyArtifact(docPath); @@ -468,19 +531,22 @@ export class ProposalController implements vscode.Disposable { .map((p) => { const fp = state.artifact.anchors[p.anchorId]?.fingerprint; const r = fp ? resolve(text, fp) : "orphaned"; - return { id: p.id, start: r === "orphaned" ? -1 : r.start }; + const start = r !== "orphaned" ? r.start : state.appliedSpans.get(p.id)?.start ?? -1; + return { id: p.id, start }; }) .sort((a, b) => b.start - a.start); let reverted = 0; - for (const it of ordered) if (await this.revertInPlace(docPath, it.id)) reverted++; - return { reverted }; + let skipped = 0; + for (const it of ordered) { + if (await this.revertInPlace(docPath, it.id, { silent: true })) reverted++; + else skipped++; + } + return { reverted, skipped }; } private reject(state: DocState, proposal: Proposal): void { if (this.guard.isReadOnly(state.docPath)) return; - this.store.update(state.docPath, (a) => removeProposal(a, proposal.id)); - const document = this.openDoc(state); - if (document) this.renderAll(document); + this.clearProposal(state, proposal.id, this.openDoc(state)); } // ---- PUC-4: load / external change / resolve-or-flag -------------------------------- @@ -510,6 +576,14 @@ export class ProposalController implements vscode.Disposable { this.recordProposal(state, proposal, { start: off, end: off }, false); } else { this.recordProposal(state, proposal, resolved, true); + // #70: REBUILD the applied-range bookkeeping from an exact resolve when + // the map has no entry (a prior-session apply, or after a close/reopen + // clear). An entry already present has been shift-maintained continuously + // since apply and is ground truth — never clobbered by a resolve, which + // can land on an exact duplicate of the applied text elsewhere. + if (state.applied.has(proposal.id) && !state.appliedSpans.has(proposal.id)) { + state.appliedSpans.set(proposal.id, resolved); + } } } this.renderStatus(state); @@ -528,10 +602,19 @@ export class ProposalController implements vscode.Disposable { private onDidChange(e: vscode.TextDocumentChangeEvent): void { const state = this.docs.get(this.keyOf(e.document)); - if (!state || state.live.size === 0) return; + if (!state || (state.live.size === 0 && state.appliedSpans.size === 0)) return; for (const change of e.contentChanges) { const edit = { start: change.rangeOffset, end: change.rangeOffset + change.rangeLength, newLength: change.text.length }; for (const [id, range] of state.live) state.live.set(id, shift(range, edit)); + // #70: an edit fully inside a tracked applied span (or fully outside it) + // keeps the span meaningful; one straddling a boundary — e.g. a whole-buffer + // external replace — makes the shifted range a clamped guess, so the span + // is distrusted (deleted) rather than reverted-to by guess (INV-11). + for (const [id, range] of state.appliedSpans) { + const shifted = shiftTracked(range, edit); + if (shifted === "distrusted") state.appliedSpans.delete(id); + else state.appliedSpans.set(id, shifted); + } } } diff --git a/test/anchorer.test.ts b/test/anchorer.test.ts index 852ddff..5dea86f 100644 --- a/test/anchorer.test.ts +++ b/test/anchorer.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from "vitest"; -import { buildFingerprint, resolve, shift, type OffsetRange } from "../src/anchorer"; +import { buildFingerprint, resolve, shift, shiftTracked, type OffsetRange } from "../src/anchorer"; const DOC = "line0\nline1 needle here\nline2\nneedle again on line3\n"; @@ -67,3 +67,54 @@ describe("shift", () => { expect(shift(r, { start: 3, end: 7, newLength: 1 })).toEqual({ start: 3, end: 7 }); }); }); + +describe("shiftTracked (#70 — tracked applied spans)", () => { + const r: OffsetRange = { start: 10, end: 20 }; + + it("edit entirely before the span shifts both endpoints by the delta", () => { + expect(shiftTracked(r, { start: 0, end: 0, newLength: 3 })).toEqual({ start: 13, end: 23 }); + }); + + it("edit entirely after the span leaves it unchanged", () => { + expect(shiftTracked(r, { start: 25, end: 28, newLength: 0 })).toEqual({ start: 10, end: 20 }); + }); + + it("edit fully inside the span keeps the start and grows/shrinks the end", () => { + // replace [12,15) with 5 chars (delta = +2) + expect(shiftTracked(r, { start: 12, end: 15, newLength: 5 })).toEqual({ start: 10, end: 22 }); + }); + + it("replacing the span's exact full contents is an interior edit", () => { + expect(shiftTracked(r, { start: 10, end: 20, newLength: 4 })).toEqual({ start: 10, end: 14 }); + }); + + it("an insertion exactly at the span start lands BEFORE the span (never absorbed)", () => { + expect(shiftTracked(r, { start: 10, end: 10, newLength: 6 })).toEqual({ start: 16, end: 26 }); + }); + + it("an insertion exactly at the span end lands AFTER the span (never absorbed)", () => { + expect(shiftTracked(r, { start: 20, end: 20, newLength: 6 })).toEqual({ start: 10, end: 20 }); + }); + + it("an insertion at an EMPTY span's point lands before it (deletion-proposal span)", () => { + const empty: OffsetRange = { start: 10, end: 10 }; + expect(shiftTracked(empty, { start: 10, end: 10, newLength: 3 })).toEqual({ start: 13, end: 13 }); + }); + + it("an edit straddling the span's start boundary is distrusted (INV-11)", () => { + expect(shiftTracked(r, { start: 8, end: 12, newLength: 0 })).toBe("distrusted"); + }); + + it("an edit straddling the span's end boundary is distrusted (INV-11)", () => { + expect(shiftTracked(r, { start: 18, end: 22, newLength: 1 })).toBe("distrusted"); + }); + + it("an edit fully containing the span (e.g. a wholesale replace) is distrusted (INV-11)", () => { + expect(shiftTracked(r, { start: 0, end: 30, newLength: 12 })).toBe("distrusted"); + }); + + it("a deletion consuming an EMPTY span's neighborhood is distrusted (INV-11)", () => { + const empty: OffsetRange = { start: 10, end: 10 }; + expect(shiftTracked(empty, { start: 8, end: 12, newLength: 0 })).toBe("distrusted"); + }); +}); diff --git a/test/e2e/suite/f12InlineDiff.test.ts b/test/e2e/suite/f12InlineDiff.test.ts index 0f03fd6..4b35097 100644 --- a/test/e2e/suite/f12InlineDiff.test.ts +++ b/test/e2e/suite/f12InlineDiff.test.ts @@ -204,6 +204,153 @@ suite("F12 inline diff — control parity (#64, INV-53)", () => { }); }); +// #70 (INV-5): rejecting a proposal AFTER typing inside its pending range must +// still restore the retained original. The exact-substring anchor is orphaned by +// the interior tweak (INV-1/INV-11) — the revert falls back to the F12 +// applied-range bookkeeping (appliedSpans), which shift-tracks the span through +// interior edits. +suite("F12 inline diff — #70 reject after interior edit (INV-5)", () => { + test("revertInPlace restores the original after a tweak inside the pending range", async () => { + const ORIGINAL = "The quick brown fox jumps."; + const { doc } = await freshDoc("docs/s70-reject-tweaked.md", `# T\n\n${ORIGINAL}\n`); + const api = await getApi(); + const p = api.proposalController; + const docKey = p.keyFor(doc); + const fp = { text: ORIGINAL, before: "", after: "", lineHint: 2 }; + const id = await p.propose(doc, fp, "The rapid brown fox jumps.", + { kind: "agent", id: "claude", agent: { sdk: "x", model: "m", sessionId: "s" } }, { granularity: "block" }); + await p.optimisticApply(doc, id!); + await settle(); + assert.ok(doc.getText().includes("The rapid brown fox jumps."), "optimistically applied"); + // Human types INSIDE the pending range → orphans the exact anchor (INV-11). + const at = doc.getText().indexOf("rapid"); + const we = new vscode.WorkspaceEdit(); + we.replace(doc.uri, new vscode.Range(doc.positionAt(at), doc.positionAt(at + "rapid".length)), "RAPID-ish"); + assert.ok(await vscode.workspace.applyEdit(we), "interior tweak applied"); + await settle(); + assert.strictEqual(p.listProposals(doc).find((v) => v.id === id)!.anchorStart, null, "anchor orphaned by the tweak"); + // ✗ Reject: must restore the retained original EXACTLY (INV-5), not skip. + assert.ok(await p.revertInPlace(docKey, id!), "reject reports success"); + await settle(); + assert.strictEqual(doc.getText(), `# T\n\n${ORIGINAL}\n`, "original restored exactly (INV-5)"); + assert.strictEqual(p.listProposals(doc).length, 0, "proposal cleared"); + }); + + test("rejectByIdInPlace routes the tweaked-applied case the same way", async () => { + const ORIGINAL = "A steady closing line."; + const { doc } = await freshDoc("docs/s70-reject-route.md", `# T\n\n${ORIGINAL}\n`); + const api = await getApi(); + const p = api.proposalController; + const docKey = p.keyFor(doc); + const fp = { text: ORIGINAL, before: "", after: "", lineHint: 2 }; + const id = await p.propose(doc, fp, "A wobbly closing line.", + { kind: "agent", id: "claude", agent: { sdk: "x", model: "m", sessionId: "s" } }, { granularity: "block" }); + await p.optimisticApply(doc, id!); + await settle(); + const at = doc.getText().indexOf("wobbly"); + const we = new vscode.WorkspaceEdit(); + we.replace(doc.uri, new vscode.Range(doc.positionAt(at), doc.positionAt(at)), "very-"); + assert.ok(await vscode.workspace.applyEdit(we), "interior insertion applied"); + await settle(); + assert.ok(await p.rejectByIdInPlace(docKey, id!), "gesture-level reject succeeds"); + await settle(); + assert.strictEqual(doc.getText(), `# T\n\n${ORIGINAL}\n`, "original restored exactly (INV-5)"); + }); + + // Fix direction (b): when the tracked span itself is distrusted (an edit + // straddled its boundary — here a deletion running from the heading into the + // span's interior), the reject FAILS honestly: warning path, proposal kept, + // buffer untouched. Silent-skip-and-report-success (the #70 bug) must not come + // back; reverting at a clamped guessed offset (INV-11) must not either. + // (A boundary-straddling DELETION is used because VS Code minimizes workspace + // edits before change events fire — a wholesale rewrite sharing a prefix/suffix + // decomposes into interior hunks the span legitimately survives.) + test("reject fails loudly — proposal kept, buffer untouched — when the span is distrusted", async () => { + const ORIGINAL = "Sentence one stands here."; + const { doc } = await freshDoc("docs/s70-reject-distrust.md", `# T\n\n${ORIGINAL}\n`); + const api = await getApi(); + const p = api.proposalController; + const docKey = p.keyFor(doc); + const fp = { text: ORIGINAL, before: "", after: "", lineHint: 2 }; + const id = await p.propose(doc, fp, "Sentence ONE stands here.", + { kind: "agent", id: "claude", agent: { sdk: "x", model: "m", sessionId: "s" } }, { granularity: "block" }); + await p.optimisticApply(doc, id!); + await settle(); + // Delete from inside the heading through the middle of the applied span: + // the edit straddles the span's start boundary → the tracked span is + // distrusted AND the exact anchor no longer resolves. + const mid = doc.getText().indexOf("ONE"); + const we = new vscode.WorkspaceEdit(); + we.replace(doc.uri, new vscode.Range(doc.positionAt(2), doc.positionAt(mid + 1)), ""); + assert.ok(await vscode.workspace.applyEdit(we), "boundary-straddling deletion applied"); + await settle(); + const before = doc.getText(); + assert.strictEqual(await p.revertInPlace(docKey, id!), false, "reject reports failure (no silent success)"); + await settle(); + assert.strictEqual(doc.getText(), before, "buffer untouched — never reverted by guess (INV-11)"); + assert.ok(p.listProposals(doc).some((v) => v.id === id), "proposal kept (not silently dropped)"); + // Cleanup for later tests: the never-locatable husk is discarded via the + // plain record-only reject. + assert.strictEqual(p.rejectById(docKey, id!), true); + }); + + // rejectAll must revert a tweaked (orphaned-anchor) proposal via the same + // tracked-span fallback, ordered descending by that span so earlier reverts + // never shift later ones — and must count what it could not revert. + test("rejectAll reverts tweaked proposals via the tracked span and reports skips", async () => { + const { doc } = await freshDoc("docs/s70-rejall-tweak.md", "# T\n\nOne aaa.\n\nTwo bbb.\n"); + const api = await getApi(); + const p = api.proposalController; + const editFlow = api.editFlow; + editFlow.setEditTurnForTest(async () => ({ replacement: "# T\n\nOne AAA.\n\nTwo BBB.\n", model: "m", sessionId: "s" })); + const ids = await editFlow.runEditAndPropose(doc, { kind: "document" }, "up"); + await settle(); + for (const id of ids) await p.optimisticApply(doc, id); + await settle(); + // Tweak INSIDE the first applied block → its exact anchor orphans. + const at = doc.getText().indexOf("AAA"); + const we = new vscode.WorkspaceEdit(); + we.replace(doc.uri, new vscode.Range(doc.positionAt(at), doc.positionAt(at + 3)), "AAAZ"); + assert.ok(await vscode.workspace.applyEdit(we), "interior tweak applied"); + await settle(); + const { reverted, skipped } = await p.rejectAll(doc); + await settle(); + assert.strictEqual(reverted, ids.length, "ALL proposals reverted, tweaked one included"); + assert.strictEqual(skipped, 0, "nothing skipped"); + assert.strictEqual(doc.getText(), "# T\n\nOne aaa.\n\nTwo bbb.\n", "document fully restored (INV-5)"); + assert.strictEqual(p.listProposals(doc).length, 0, "all cleared"); + }); + + // The decision gestures must stay REACHABLE for a tweaked proposal: the ✓/✗ + // CodeLens pair anchors at the tracked span when the interior tweak orphans + // the exact anchor — otherwise the fixed reject path exists only at the API. + test("the ✓ Keep / ✗ Reject CodeLens pair survives an interior tweak (tracked-span anchor)", async () => { + const ORIGINAL = "Lens target sentence here."; + const { doc } = await freshDoc("docs/s70-lens-tweak.md", `# T\n\n${ORIGINAL}\n`); + const api = await getApi(); + const p = api.proposalController; + const fp = { text: ORIGINAL, before: "", after: "", lineHint: 2 }; + const id = await p.propose(doc, fp, "Lens TARGET sentence here.", + { kind: "agent", id: "claude", agent: { sdk: "x", model: "m", sessionId: "s" } }, { granularity: "block" }); + await p.optimisticApply(doc, id!); + await settle(); + const at = doc.getText().indexOf("TARGET"); + const we = new vscode.WorkspaceEdit(); + we.replace(doc.uri, new vscode.Range(doc.positionAt(at), doc.positionAt(at + 6)), "TARGET-tweaked"); + assert.ok(await vscode.workspace.applyEdit(we), "interior tweak applied"); + await settle(); + assert.strictEqual(p.listProposals(doc).find((v) => v.id === id)!.anchorStart, null, "anchor orphaned"); + const lenses = api.editorProposalController.provideCodeLenses(doc); + const titles = lenses.map((l) => l.command?.title); + assert.deepStrictEqual(titles, ["✓ Keep", "✗ Reject"], "the pair survives the tweak"); + assert.deepStrictEqual(lenses[1].command!.arguments, [id], "reject lens still targets the tweaked proposal"); + // The reject it routes at still restores the original (the Task-1 contract). + assert.ok(await p.revertInPlace(p.keyFor(doc), id!), "reject via the surviving gesture succeeds"); + await settle(); + assert.strictEqual(doc.getText(), `# T\n\n${ORIGINAL}\n`, "original restored exactly (INV-5)"); + }); +}); + // Reload-safety: a proposal that was optimistically applied in a PRIOR session // persists with `original` set and its fp re-anchored to the applied text. A fresh // controller (empty in-memory `applied` set) must NOT re-capture `original` from the diff --git a/test/e2e/suite/fullLoop.test.ts b/test/e2e/suite/fullLoop.test.ts index a2ee4d7..5dc8858 100644 --- a/test/e2e/suite/fullLoop.test.ts +++ b/test/e2e/suite/fullLoop.test.ts @@ -140,7 +140,22 @@ suite("full native loop — PUC-7 → PUC-8 → PUC-2 → PUC-1 (§6.8, §7.1 ru assert.ok(claudeSpan, "claude's words are attributed"); assert.ok(humanSpan, "the human tweak is attributed separately (char-honest split)"); - // ---- PUC-2, reject: revert the untouched second proposal in place ---------------- + // ---- PUC-2, reject: revert the second proposal in place -------------------------- + // #70 (INV-5): the reject leg now ALSO takes an interior tweak first — the + // original deliberately rejected only an un-tweaked proposal because the + // pre-fix revert silently skipped an orphaned anchor. The tweak orphans the + // exact anchor; the revert must restore ORIG_REJECT via the tracked span. + const rejAt = doc.getText().indexOf(REJECT_REPLACEMENT); + assert.ok(rejAt >= 0, "reject proposal's applied text present"); + const rejTweak = new vscode.WorkspaceEdit(); + rejTweak.replace(doc.uri, new vscode.Range(doc.positionAt(rejAt + 2), doc.positionAt(rejAt + 2)), "x"); + assert.ok(await vscode.workspace.applyEdit(rejTweak), "human tweak inside the reject proposal's range"); + await settle(); + assert.strictEqual( + api.proposalController.listProposals(doc).find((v) => v.id === rejectId)!.anchorStart, + null, + "tweak orphans the reject proposal's exact anchor (INV-11)", + ); assert.ok(await api.proposalController.revertInPlace(docKey, rejectId), "reject reverts in place"); await settle(); assert.strictEqual(