fix: route document asks to comment-first askClaude (plan T6 Step 4); machine-author guard + offer cleanup

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

src/editFlow.ts

@@ -1,13 +1,15 @@
 /**
  * EditFlow — the host edit-turn flow (F11/F12), extracted from the review
  * webview controller ahead of its sunset (native-surfaces migration, spec
- * §6.10). Owns the `cowriting.editDocument` command, the instruction prompt +
- * progress-wrapped "ask Claude" gesture (INV-10 gate, INV-8 host-only LLM
- * surface), and the low-level turn→proposal(s) cut (`runEditAndPropose`,
- * INV-39/40). Never mutates the document directly — every turn lands as one or
- * more F4 proposals via `ProposalController.propose` (INV-10). Reachable from
- * both the review webview (delegates here) and, from Task 6 on, the thread
- * controller — vscode-API-only, no webview state.
+ * §6.10). Owns the instruction prompt + progress-wrapped "ask Claude" gesture
+ * (INV-10 gate, INV-8 host-only LLM surface), and the low-level turn→proposal(s)
+ * cut (`runEditAndPropose`, INV-39/40). Never mutates the document directly —
+ * every turn lands as one or more F4 proposals via `ProposalController.propose`
+ * (INV-10). `runEditAndPropose` is reachable from the review webview (via this
+ * class's `askClaude`) and, from Task 6 on, the thread controller
+ * (`ThreadController.runMakeEdit`) — vscode-API-only, no webview state. The
+ * `cowriting.editDocument` command lives in extension.ts (Finding-1 fix) so it
+ * can hand off to `ThreadController.askClaude()`.
  */
 import * as vscode from "vscode";
 import type { ProposalController } from "./proposalController";
@@ -67,38 +69,29 @@ export class EditFlow implements vscode.Disposable {
     private readonly liveProgressUi: LiveProgressUi,
     private readonly registry: CoeditingRegistry,
   ) {
-    this.disposables.push(
-      // F11: document-scoped Ask-Claude (also reused by #42's reach gateways).
-      // Edits a markdown doc; the rewrite is diffed into F4 proposals.
-      // #42 (INV-38): the editor/title/context (tab) entry passes the clicked
-      // tab's resource Uri — target THAT document, opening it if it isn't already
-      // an open buffer (mirrors showTrackChangesPreview's #41 resolution); the
-      // palette / keybinding / editor/context pass nothing → the active editor.
-      vscode.commands.registerCommand("cowriting.editDocument", async (uri?: vscode.Uri) => {
-        const doc = uri
-          ? vscode.workspace.textDocuments.find((d) => d.uri.toString() === uri.toString()) ??
-            (await vscode.workspace.openTextDocument(uri))
-          : vscode.window.activeTextEditor?.document;
-        if (!doc || doc.languageId !== "markdown") {
-          void vscode.window.showWarningMessage("Cowriting: open a Markdown document to ask Claude to edit it.");
-          return;
-        }
-        void this.askClaude(doc, { kind: "document" });
-      }),
-    );
+    // Code-review fix (Finding 1, session native-surfaces-exec): the
+    // `cowriting.editDocument` command used to be registered here, calling
+    // `this.askClaude(doc, {kind:"document"})` — which prompts via the (now
+    // rejecting-stub) `askEditInstruction`, a silent dead end. That command is now
+    // registered in extension.ts, AFTER ThreadController exists, and hands off to
+    // `ThreadController.askClaude()` (the comments-first ask, D19) instead. This
+    // class's `askClaude`/`askEditInstruction` remain: the review webview's
+    // toolbar ask (`trackChangesPreview.ts`) still delegates to them, slated for
+    // deletion alongside that webview in Task 8.
   }
 
   /**
    * F11 (PUC-3/4): prompt host-side for the instruction (keeps the LLM/secret
    * surface out of the sealed webview, INV-8/35), run the edit turn, and surface
-   * the result as F4 proposal(s). UI wrapper around `runEditAndPropose`. Called
-   * both by the `cowriting.editDocument` command above and (via delegation) by
-   * the review webview's `askClaude` toolbar intent (either scope).
+   * the result as F4 proposal(s). UI wrapper around `runEditAndPropose`. As of the
+   * Finding-1 fix, its only production caller is the review webview's `askClaude`
+   * toolbar intent (either scope) — `cowriting.editDocument`/`cowriting.editSelection`
+   * now delegate to `ThreadController.askClaude()` instead (D19). Both die together
+   * in Task 8 when the webview is sunset.
    */
   async askClaude(document: vscode.TextDocument, target: EditTarget): Promise<void> {
-    // INV-10: the choke point for BOTH the editDocument command and the webview's
-    // askClaude toolbar intent (either scope) — a non-entered doc gets the warning,
-    // not a turn.
+    // INV-10: the choke point for the webview's askClaude toolbar intent (either
+    // scope) — a non-entered doc gets the warning, not a turn.
     if (!this.registry.isCoediting(document.uri)) {
       void vscode.window.showWarningMessage(NOT_COEDITING_WARNING);
       return;

src/extension.ts

@@ -323,11 +323,43 @@ export function activate(context: vscode.ExtensionContext): CowritingApi | undef
     }),
   );
 
+  // Code-review fix (Finding 1, session native-surfaces-exec): `cowriting.editDocument`
+  // used to delegate to `EditFlow.askClaude(doc, {kind:"document"})`, which prompts via
+  // `askEditInstruction` — a rejecting stub since Task 6 sunset the input webview
+  // (spec §6.10). Reached with no selection (the common case for `cowriting.edit`),
+  // that was a SILENT dead end: the reject fired before EditFlow.askClaude's own try/
+  // catch and was `void`'d, so no toast, no thread, nothing. This command now resolves
+  // its target document exactly as before (a tab's clicked URI, falling back to the
+  // active editor), focuses it, and hands off to the SAME comments-first ask as
+  // `cowriting.editSelection` — `ThreadController.askClaude()` top-anchors a
+  // whole-document ask when the (now-focused) editor's selection is empty (D19). Kept
+  // registered + hidden from the palette for the host E2E harness and #42's
+  // tab-targeting behavior; `EditFlow.runEditAndPropose` (untouched) stays the E2E seam
+  // for driving a turn without the native comment UI — see f12Reach.test.ts /
+  // f11Toolbar.test.ts / f12Accept.test.ts / f12Review.test.ts.
+  context.subscriptions.push(
+    vscode.commands.registerCommand("cowriting.editDocument", async (uri?: vscode.Uri) => {
+      const doc = uri
+        ? vscode.workspace.textDocuments.find((d) => d.uri.toString() === uri.toString()) ??
+          (await vscode.workspace.openTextDocument(uri))
+        : vscode.window.activeTextEditor?.document;
+      if (!doc || doc.languageId !== "markdown") {
+        void vscode.window.showWarningMessage("Cowriting: open a Markdown document to ask Claude to edit it.");
+        return;
+      }
+      if (vscode.window.activeTextEditor?.document.uri.toString() !== doc.uri.toString()) {
+        await vscode.window.showTextDocument(doc);
+      }
+      await threadController.askClaude();
+    }),
+  );
+
   // The single user-facing "Ask Claude to Edit" gesture (one command, one
   // keybinding, one menu entry). It routes to the selection flow (editSelection)
   // or the whole-document flow (editDocument) by selection/context — see
-  // routeEdit. The two underlying commands stay registered for the host E2E
-  // harness and the internal seams, but are hidden from the palette.
+  // routeEdit. Both underlying commands now end at ThreadController.askClaude()
+  // (comments-first ask, D19); they stay registered for the host E2E harness and
+  // the internal seams, but are hidden from the palette.
   context.subscriptions.push(
     vscode.commands.registerCommand("cowriting.edit", async (uri?: vscode.Uri) => {
       const editor = vscode.window.activeTextEditor;

src/threadController.ts

@@ -192,12 +192,17 @@ export class ThreadController implements vscode.Disposable {
       end: document.offsetAt(editor.selection.end),
     };
     const fp = buildFingerprint(document.getText(), offsets);
-    const { threadId } = addThread(state.artifact, fp, { author: this.currentAuthor(), body: firstBody });
+    const author = this.currentAuthor();
+    const { threadId } = addThread(state.artifact, fp, { author, body: firstBody });
     this.persist(state);
     const vsThread = this.renderThread(document, state, threadId, offsets, false);
     // D10: the first message of a new thread is also "a comment on a coedited
-    // doc" — run the same respond loop as a reply.
+    // doc" — run the same respond loop as a reply. Finding 2 guard: only for a
+    // human-authored message (never re-trigger the loop off the machine's own
+    // words — see reply() below for the matching guard).
+    if (author.kind !== "agent") {
       void this.respondInThread(vsThread, state, threadId, firstBody);
+    }
     return threadId;
   }
 
@@ -210,9 +215,10 @@ export class ThreadController implements vscode.Disposable {
     if (!document) return;
     const state = this.ensureState(document);
     if (this.guard.isReadOnly(state.docPath)) return;
+    const author = this.currentAuthor();
     let threadId = this.threadIdOf(vsThread);
     if (threadId) {
-      appendMessage(state.artifact, threadId, { author: this.currentAuthor(), body: r.text });
+      appendMessage(state.artifact, threadId, { author, body: r.text });
     } else {
       // D10/D19: a brand-new thread created via the native "+"/comment-widget
       // gesture (not via createThreadOnSelection) — VS Code hands us a real,
@@ -223,7 +229,7 @@ export class ThreadController implements vscode.Disposable {
         end: document.offsetAt(range.end),
       };
       const fp = buildFingerprint(document.getText(), offsets);
-      const created = addThread(state.artifact, fp, { author: this.currentAuthor(), body: r.text });
+      const created = addThread(state.artifact, fp, { author, body: r.text });
       threadId = created.threadId;
       state.vsThreads.set(threadId, vsThread);
       state.live.set(threadId, offsets);
@@ -231,9 +237,15 @@ export class ThreadController implements vscode.Disposable {
     }
     this.persist(state);
     this.refreshComments(vsThread, state, threadId);
-    // D10: every human comment on a coedited doc runs a turn.
+    // D10: every HUMAN comment on a coedited doc runs a turn. Finding 2 guard:
+    // `cowriting.reply` is the ingress for both a genuine human reply and (in
+    // principle) a re-dispatched machine message — never loop the machine's own
+    // reply back through respondInThread (it would double-turn on Claude's words
+    // and re-flip an already-"offer" thread).
+    if (author.kind !== "agent") {
       void this.respondInThread(vsThread, state, threadId, r.text);
     }
+  }
 
   /**
    * D10/PUC-8: run one reply turn for `ask`, then append the machine's reply
@@ -310,6 +322,12 @@ export class ThreadController implements vscode.Disposable {
     const target = this.targetOf(doc, vsThread);
     const ids = await this.editFlow.runEditAndPropose(doc, target, offer.ask);
     vsThread.contextValue = "offer-done";
+    // Finding 3 (seam hygiene): drop the offer once it's spent — otherwise the
+    // `makeThreadEdit(threadId, docPath)` test seam (or a stray second UI click,
+    // now dead per contextValue but still a valid direct call) could re-run the
+    // same edit a second time. A future offer on this thread is a fresh
+    // respondInThread() call, which re-populates the WeakMap entry.
+    this.offers.delete(vsThread);
     if (ids.length) {
       void vscode.window.showInformationMessage(
         `Cowriting: ${ids.length} pending change${ids.length === 1 ? "" : "s"} landed in the buffer — ✓ Keep / ✗ Reject there.`,

test/e2e/suite/commentLoop.test.ts

@@ -44,4 +44,47 @@ suite("comment → reply → offer → proposal (PUC-8, D10/D19)", () => {
     const id = await api.threadController.createThreadOnSelection("hello");
     assert.strictEqual(id, undefined); // gate refuses thread creation entirely
   });
+
+  // Finding 1 (code review, session native-surfaces-exec): `cowriting.edit` with NO
+  // selection used to route through `cowriting.editDocument` → the removed
+  // `askEditInstruction` webview prompt — a rejecting stub `void`'d before any
+  // try/catch, i.e. a completely silent dead end (no toast, no thread). It must now
+  // reach ThreadController.askClaude()'s top-anchored whole-document path (D19) —
+  // the same comments-first ask the selection route already used. There is no API to
+  // drive VS Code's native comment-widget submission from a host E2E test (the actual
+  // comment→reply→offer→proposal round trip is exercised above via the
+  // createThreadOnSelection seam), so this asserts the routing itself: the gesture
+  // reaches ThreadController.askClaude (never the old broken stub) and does not throw.
+  test("cowriting.edit with no selection on a coedited doc reaches the top-anchored ask path, not a silent dead end", async () => {
+    const api = await activateApi();
+    const doc = await vscode.workspace.openTextDocument({
+      language: "markdown",
+      content: "# T\n\nA document-level ask with no selection.\n",
+    });
+    const ed = await vscode.window.showTextDocument(doc);
+    await vscode.commands.executeCommand("cowriting.coeditDocument");
+    // Defensive stubs, mirroring the suite's other test — no real turn should fire
+    // from this gesture alone (no comment text is ever submitted), but guard against
+    // a real @cline/sdk call if the routing regresses.
+    api.threadController.setTurnRunnerForTest(async () => ({ replacement: "stub", model: "stub", sessionId: "s1" }));
+    api.editFlow.setEditTurnForTest(async () => ({ replacement: "stub", model: "stub", sessionId: "s1" }));
+    ed.selection = new vscode.Selection(0, 0, 0, 0); // no selection
+
+    let askClaudeCalls = 0;
+    const origAskClaude = api.threadController.askClaude.bind(api.threadController);
+    api.threadController.askClaude = async () => {
+      askClaudeCalls++;
+      return origAskClaude();
+    };
+    try {
+      await vscode.commands.executeCommand("cowriting.edit");
+    } finally {
+      api.threadController.askClaude = origAskClaude;
+    }
+    assert.strictEqual(
+      askClaudeCalls,
+      1,
+      "cowriting.edit (no selection) reached ThreadController.askClaude — not the removed editDocument prompt stub",
+    );
+  });
 });

test/e2e/suite/f12Reach.test.ts

@@ -72,9 +72,17 @@ suite("F12 SLICE-1 — Ask-Claude reach (#42, INV-38)", () => {
 
   // PUC-3 behavior: editDocument invoked with a tab URI targets THAT document,
   // not whatever editor happens to be active (mirrors #41's clicked-doc resolution).
+  // Finding 1 fix (native-surfaces code review): editDocument no longer prompts via
+  // the removed askEditInstruction webview stub (that path threw a silent, `void`'d,
+  // unhandled rejection — no toast, no thread, no proposal). It now resolves/focuses
+  // its target document and hands off to ThreadController.askClaude() (D19), same as
+  // editSelection. The turn→proposal cut itself (EditFlow.runEditAndPropose against a
+  // {kind:"document"} target) is exercised directly, without the command, in
+  // f11Toolbar.test.ts / f12Accept.test.ts / f12Review.test.ts — this test's job is
+  // the URI-targeting/focus behavior, plus confirming the routing actually reaches
+  // askClaude (proving Finding 1's dead end is gone).
   test("editDocument(uri) targets the clicked tab's document, not the active editor", async () => {
     const api = await getApi();
-    const editFlow = api.editFlow;
 
     // Doc A is the active editor; Doc B is the "clicked tab" we pass by URI.
     const a = await freshDoc("docs/f12-active.md", "# Active\n\nThe active editor paragraph.\n");
@@ -82,55 +90,68 @@ suite("F12 SLICE-1 — Ask-Claude reach (#42, INV-38)", () => {
     await vscode.window.showTextDocument(a.doc);
     await settle();
 
-    // Stub the document instruction prompt (the webview can't run in CI) + the LLM turn.
-    const origPrompt = editFlow.askEditInstruction;
-    editFlow.askEditInstruction = async () => "rewrite it";
-    editFlow.setEditTurnForTest(async () => ({
-      replacement: "# Tab\n\nThe REWRITTEN tab paragraph.\n",
-      model: "sonnet",
-      sessionId: "e2e-f12-tab",
-    }));
+    let askClaudeCalls = 0;
+    // Capture the active-editor doc INSIDE the spy, before delegating to the real
+    // askClaude — askClaude's own `workbench.action.addComment` side effect moves
+    // focus to the ephemeral comment-input widget (a `comment://` URI), which would
+    // make a post-hoc `activeTextEditor` check meaningless. What we're proving here
+    // is editDocument's OWN resolve-and-focus step ran against the right document
+    // before handing off.
+    let focusedDocAtHandoff: string | undefined;
+    const origAskClaude = api.threadController.askClaude.bind(api.threadController);
+    api.threadController.askClaude = async () => {
+      askClaudeCalls++;
+      focusedDocAtHandoff = vscode.window.activeTextEditor?.document.uri.toString();
+      return origAskClaude();
+    };
     try {
       await vscode.commands.executeCommand("cowriting.editDocument", b.doc.uri);
       await settle();
     } finally {
-      editFlow.askEditInstruction = origPrompt;
+      api.threadController.askClaude = origAskClaude;
     }
 
-    // The proposal(s) landed on the TAB doc (B), and the ACTIVE doc (A) has none.
-    assert.ok(api.proposalController.listProposals(b.doc).length >= 1, "tab doc B received the document-edit proposal(s)");
     assert.strictEqual(
-      api.proposalController.listProposals(a.doc).length,
-      0,
-      "active doc A was NOT edited — editDocument honored the tab URI",
+      askClaudeCalls,
+      1,
+      "editDocument(uri) reached ThreadController.askClaude — not the removed prompt stub",
+    );
+    assert.strictEqual(
+      focusedDocAtHandoff,
+      b.doc.uri.toString(),
+      "editDocument(uri) focused the TAB doc B, not the previously-active doc A, before handing off to askClaude",
     );
-    // F12 (INV-48): EditorProposalController optimistically applies proposals into the
-    // buffer, so the proposed text is now in the tab doc B. Confirm one of the proposal
-    // texts is present (the tab doc was edited, not A).
-    assert.ok(b.doc.getText().includes("REWRITTEN"), "F12 optimistic-apply: proposed text in tab doc B");
   });
 
   // No URI arg (palette / keybinding) → fall back to the active editor.
   test("editDocument() with no arg targets the active editor", async () => {
     const api = await getApi();
-    const editFlow = api.editFlow;
     const a = await freshDoc("docs/f12-noarg.md", "# No arg\n\nThe active doc paragraph here.\n");
     await vscode.window.showTextDocument(a.doc);
     await settle();
 
-    const origPrompt = editFlow.askEditInstruction;
-    editFlow.askEditInstruction = async () => "rewrite it";
-    editFlow.setEditTurnForTest(async () => ({
-      replacement: "# No arg\n\nThe REWRITTEN active doc paragraph.\n",
-      model: "sonnet",
-      sessionId: "e2e-f12-noarg",
-    }));
+    let askClaudeCalls = 0;
+    // See the sibling test above for why this is captured inside the spy rather
+    // than after askClaude() returns.
+    let focusedDocAtHandoff: string | undefined;
+    const origAskClaude = api.threadController.askClaude.bind(api.threadController);
+    api.threadController.askClaude = async () => {
+      askClaudeCalls++;
+      focusedDocAtHandoff = vscode.window.activeTextEditor?.document.uri.toString();
+      return origAskClaude();
+    };
     try {
       await vscode.commands.executeCommand("cowriting.editDocument");
       await settle();
     } finally {
-      editFlow.askEditInstruction = origPrompt;
+      api.threadController.askClaude = origAskClaude;
     }
-    assert.ok(api.proposalController.listProposals(a.doc).length >= 1, "active doc received the proposal(s) on no-arg");
+
+    assert.strictEqual(askClaudeCalls, 1, "editDocument() with no arg reached ThreadController.askClaude");
+    assert.strictEqual(
+      focusedDocAtHandoff,
+      a.doc.uri.toString(),
+      "editDocument() with no arg kept the active editor's doc active before handing off to askClaude",
+    );
   });
 });