INV-5 gap: rejecting a proposal after typing inside its pending range silently skips the revert #70

New Issue

2026-07-02T22:43:38Z

benstull commented

2026-07-02 22:43:38 +00:00

Surfaced by session 0065's Task 9 review (native-surfaces migration, full-loop E2E).

Behavior (pre-existing F12 code, src/proposalController.ts revertInPlace): revertInPlace() only restores proposal.original when the anchor resolve() succeeds. When the writer has typed INSIDE the pending proposal's range first (which orphans the exact-substring anchor per INV-1/INV-11), revertInPlace skips the WorkspaceEdit entirely, removes the proposal from the store, and still returns true — so ✗ Reject silently leaves the (edited) proposed text in the buffer instead of restoring the retained original. INV-5 ("Reject restores the retained original exactly") does not hold on this path, and the reported success masks it.

Contrast: ✓ Keep after the same interior tweak is fine by design — finalizeInPlace looks up by id and bypasses resolution.

Repro sketch (host E2E): propose → optimistic-apply → type inside the pending range → rejectById → buffer keeps the edited proposed text; command reports success.

Fix directions to evaluate:

(a) fall back to the proposal's tracked current range (F12 keeps applied-range bookkeeping) instead of the raw anchor resolve;
(b) treat orphaned-reject as a hard failure surfaced to the user (honest, minimal);
(c) fuzzy/expanded resolve for the reject path only.

Found during: plan 2026-07-01-native-surfaces-migration.md Task 9; the full-loop E2E deliberately rejects only an un-tweaked proposal because of this gap.

Surfaced by session 0065's Task 9 review (native-surfaces migration, full-loop E2E). **Behavior (pre-existing F12 code, `src/proposalController.ts` `revertInPlace`):** `revertInPlace()` only restores `proposal.original` when the anchor `resolve()` succeeds. When the writer has typed INSIDE the pending proposal's range first (which orphans the exact-substring anchor per INV-1/INV-11), `revertInPlace` skips the WorkspaceEdit entirely, removes the proposal from the store, and still returns `true` — so ✗ Reject silently leaves the (edited) proposed text in the buffer instead of restoring the retained original. INV-5 ("Reject restores the retained original exactly") does not hold on this path, and the reported success masks it. **Contrast:** ✓ Keep after the same interior tweak is fine by design — `finalizeInPlace` looks up by id and bypasses resolution. **Repro sketch (host E2E):** propose → optimistic-apply → type inside the pending range → `rejectById` → buffer keeps the edited proposed text; command reports success. **Fix directions to evaluate:** - (a) fall back to the proposal's tracked current range (F12 keeps applied-range bookkeeping) instead of the raw anchor resolve; - (b) treat orphaned-reject as a hard failure surfaced to the user (honest, minimal); - (c) fuzzy/expanded resolve for the reject path only. Found during: plan `2026-07-01-native-surfaces-migration.md` Task 9; the full-loop E2E deliberately rejects only an un-tweaked proposal because of this gap.

benstull added the priority/P2 type/bug labels 2026-07-02 22:43:38 +00:00

benstull referenced this issue from a commit

2026-07-02 23:08:52 +00:00

docs(plan): correct Task 9 interior-tweak wording to the shipped anchoring contract; note INV-5 reject gap (#70)

benstull referenced this issue

2026-07-02 23:09:27 +00:00

Native surfaces migration — evolve the extension onto VS Code's native review surfaces (9-task plan, spec v0.2.1) #72

benstull referenced this issue from a commit

2026-07-03 01:18:24 +00:00

fix(#70): reject after an interior tweak restores the original via tracked applied span (INV-5)

benstull referenced this issue from a commit

2026-07-03 01:18:24 +00:00