From fe23ffa100e9be5bd270d990ef7564c860df2e3b Mon Sep 17 00:00:00 2001 From: Ben Stull Date: Wed, 10 Jun 2026 10:35:16 -0700 Subject: [PATCH] F3 SLICE-5 review fixes: fence-aware replacement extraction + editSelection guards (#6) Co-Authored-By: Claude Opus 4.8 (1M context) --- esbuild.mjs | 2 +- scripts/smoke-live-turn.mjs | 2 +- src/extension.ts | 19 +++++++++++++++++-- src/liveTurn.ts | 13 ++++++++++--- test/liveTurn.test.ts | 16 ++++++++++++---- 5 files changed, 41 insertions(+), 11 deletions(-) diff --git a/esbuild.mjs b/esbuild.mjs index 907c23e..35be90b 100644 --- a/esbuild.mjs +++ b/esbuild.mjs @@ -9,7 +9,7 @@ const options = { bundle: true, platform: "node", format: "cjs", - target: "node20", + target: "node22", sourcemap: true, // vscode is provided by the host; @cline/sdk is ESM-only and is loaded at // runtime via dynamic import() from node_modules, so keep both external. diff --git a/scripts/smoke-live-turn.mjs b/scripts/smoke-live-turn.mjs index 6a230b1..1a5f17d 100644 --- a/scripts/smoke-live-turn.mjs +++ b/scripts/smoke-live-turn.mjs @@ -17,6 +17,6 @@ try { console.log(`elapsed: ${((Date.now() - t0) / 1000).toFixed(1)}s`); process.exit(0); } catch (err) { - console.error(`live turn failed (expected when Claude Code is absent/signed out): ${err.message}`); + console.error(`live turn failed (expected when Claude Code is absent/signed out): ${err instanceof Error ? err.message : String(err)}`); process.exit(1); } diff --git a/src/extension.ts b/src/extension.ts index de4ac71..b3b5ede 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -86,7 +86,12 @@ export function activate(context: vscode.ExtensionContext): CowritingApi | undef context.subscriptions.push( vscode.commands.registerCommand("cowriting.editSelection", async () => { const editor = vscode.window.activeTextEditor; - if (!editor || editor.selection.isEmpty || !editor.document.uri.fsPath.startsWith(root)) { + if ( + !editor || + editor.selection.isEmpty || + editor.document.uri.scheme !== "file" || + !editor.document.uri.fsPath.startsWith(root) + ) { void vscode.window.showWarningMessage("Cowriting: select some text in a workspace document first."); return; } @@ -95,6 +100,10 @@ export function activate(context: vscode.ExtensionContext): CowritingApi | undef placeHolder: "e.g. tighten this paragraph", }); if (!instruction) return; + if (editor.selection.isEmpty) { + void vscode.window.showWarningMessage("Cowriting: select some text in a workspace document first."); + return; + } const document = editor.document; const selection = editor.selection; const expectedVersion = document.version; @@ -106,6 +115,12 @@ export function activate(context: vscode.ExtensionContext): CowritingApi | undef async () => { const { runEditTurn } = await import("./liveTurn"); const turn = await runEditTurn(instruction, selectedText); + if (turn.replacement === "") { + void vscode.window.showWarningMessage( + "Cowriting: Claude returned an empty replacement — nothing was applied.", + ); + return; + } const ok = await attributionController.applyAgentEdit( document, new vscode.Range(selection.start, selection.end), @@ -119,7 +134,7 @@ export function activate(context: vscode.ExtensionContext): CowritingApi | undef ); if (!ok) { void vscode.window.showWarningMessage( - "Cowriting: the document changed while Claude was editing — nothing was applied.", + "Cowriting: the edit could not be applied (the document changed during the turn, or the editor rejected the edit) — nothing was changed.", ); } }, diff --git a/src/liveTurn.ts b/src/liveTurn.ts index 9d952d7..9a4d7ed 100644 --- a/src/liveTurn.ts +++ b/src/liveTurn.ts @@ -24,8 +24,15 @@ const SYSTEM_PROMPT = [ "unless the instruction says otherwise.", ].join(" "); -/** Strip a wrapping markdown code fence if the model added one anyway. */ -export function extractReplacement(outputText: string): string { +/** + * Strip a wrapping markdown code fence if the model added one anyway. + * If `selectedText` itself was fence-wrapped (i.e. the user's selection was a + * fenced block), the model mirroring that format is correct — return + * `outputText` unchanged so the fence is preserved. + */ +export function extractReplacement(outputText: string, selectedText: string): string { + const fencePattern = /^\s*```[^\n]*\n[\s\S]*?\n?```\s*$/; + if (fencePattern.test(selectedText)) return outputText; const m = outputText.match(/^\s*```[^\n]*\n([\s\S]*?)\n?```\s*$/); return m ? m[1] : outputText; } @@ -53,5 +60,5 @@ export async function runEditTurn( "(is Claude Code installed and signed in?)", ); } - return { replacement: extractReplacement(result.outputText), model: modelId, sessionId: result.runId }; + return { replacement: extractReplacement(result.outputText, selectedText), model: modelId, sessionId: result.runId }; } diff --git a/test/liveTurn.test.ts b/test/liveTurn.test.ts index 6c1d13d..da8596e 100644 --- a/test/liveTurn.test.ts +++ b/test/liveTurn.test.ts @@ -3,16 +3,24 @@ import { extractReplacement } from "../src/liveTurn"; describe("extractReplacement", () => { it("returns plain text untouched", () => { - expect(extractReplacement("Hello world.")).toBe("Hello world."); + expect(extractReplacement("Hello world.", "x")).toBe("Hello world."); }); it("strips a wrapping fence", () => { - expect(extractReplacement("```\nHello world.\n```")).toBe("Hello world."); + expect(extractReplacement("```\nHello world.\n```", "x")).toBe("Hello world."); }); it("strips a language-tagged fence", () => { - expect(extractReplacement("```markdown\nHello.\n```\n")).toBe("Hello."); + expect(extractReplacement("```markdown\nHello.\n```\n", "x")).toBe("Hello."); }); it("leaves inner fences alone", () => { const inner = "before\n```js\ncode\n```\nafter"; - expect(extractReplacement(inner)).toBe(inner); + expect(extractReplacement(inner, "x")).toBe(inner); + }); + it("keeps the fence when the original selection was itself fence-wrapped", () => { + const sel = "```python\nold()\n```"; + const out = "```python\nnew()\n```"; + expect(extractReplacement(out, sel)).toBe(out); + }); + it("still strips when the selection was plain but the model added a fence", () => { + expect(extractReplacement("```\nplain\n```", "plain old text")).toBe("plain"); }); });