cmux NEXT composer: a failed upload tries again on the next key or click, never refused for lack of one

webviews/src/agent-session/acpmux/composer/uploadRetry.ts

@@ -1,17 +1,21 @@
 // A file whose upload failed tries again on its own before its chip shows Retry (Leo,
 // 2026-11-01, after a pasted mp4 failed until he clicked Retry: "i had to retry to get it to
 // load???"). The host starts an upload only on the person's fresh key or click (its begin spends
-// that gesture, AgentPaneModel `uploadStage`), so the first try again goes a moment after the
-// failure, and the last waits for the person's next key or click in the page, as the chip's Retry
-// click would. Return never starts one: its send keeps that gesture for the prompt.
+// that gesture, AgentPaneModel `uploadStage`), so every try again waits for the person's next key
+// or click in the page, as the chip's Retry click would. One sent on a timer with no gesture was
+// always refused ("The app refused the agent connection."), and that refusal took the place of
+// the upload's own error on its tile (Leo, 2026-10-31: "idk what's going on with video
+// attachments"). Return never starts one: its send keeps that gesture for the prompt.
 
 /// How many times a failed upload is tried again on its own before its chip shows Retry.
 export const UPLOAD_AUTO_RETRIES = 2;
-/// How long after the failure the first try again goes.
+/// How long after the failure a try again goes where there is no page to hear input (a test host).
 export const UPLOAD_RETRY_DELAY_MS = 1_000;
-/// How long the last try again waits for the person's next key or click before the chip shows Retry.
+/// How long a try again waits for the person's next key or click before the chip shows Retry.
 export const UPLOAD_RETRY_INPUT_WAIT_MS = 15_000;
 
+const MODIFIER_KEYS = new Set(["Shift", "Control", "Alt", "Meta", "CapsLock", "Fn"]);
+
 /// Runs `again` for try `tries` (0 is the first try again) of a failed upload, or `giveUp` once the
 /// tries are spent or the wait for input ends. Answers what stops a try still waiting (the chip's x).
 export function retryUploadLater(
@@ -24,7 +28,7 @@ export function retryUploadLater(
     giveUp();
     return () => {};
   }
-  if (tries < UPLOAD_AUTO_RETRIES - 1 || !doc) {
+  if (!doc) {
     const timer = setTimeout(again, UPLOAD_RETRY_DELAY_MS);
     return () => clearTimeout(timer);
   }
@@ -37,8 +41,9 @@ export function retryUploadLater(
   };
   function input(event: Event) {
     if (done || !event.isTrusted) return;
-    if (event.type === "keydown" && ((event as KeyboardEvent).key === "Enter" || (event as KeyboardEvent).repeat))
-      return;
+    const key = event.type === "keydown" ? (event as KeyboardEvent) : undefined;
+    // A lone modifier reaches AppKit as flagsChanged, not keyDown: the host records no gesture for it.
+    if (key && (key.key === "Enter" || key.repeat || MODIFIER_KEYS.has(key.key))) return;
     stop();
     // After the host recorded this key or click: the try begins with it.
     setTimeout(again, 0);

webviews/test/composer-upload-first-send.test.tsx

@@ -42,6 +42,7 @@ beforeAll(() => {
 });
 
 afterEach(() => {
+  restoreInput?.();
   act(() => root?.unmount());
   root = undefined;
   host.replaceChildren();
@@ -99,7 +100,8 @@ test("a send made while a pasted video uploads waits for it, then goes once with
   expect(sent).toEqual([["Sunbringer Menu Animation.mp4"]]);
 });
 
-test("a failed upload is tried again on its own; its chip never shows Retry when that works", async () => {
+test("a failed upload is tried again on the person's next key; its chip never shows Retry when that works", async () => {
+  const input = personInput();
   const answers: UploadResult[] = [
     { error: "Couldn't send the file to gcp-codex-lane-2." },
     { paths: ["/remote/clip.mp4"] },
@@ -111,6 +113,9 @@ test("a failed upload is tried again on its own; its chip never shows Retry when
   });
   let shownFailed = false;
   await act(() => void handle.current!.attach([mp4()], true));
+  await until(() => calls === 1);
+  act(() => input.key());
+  input.restore();
   await until(() => {
     if (handle.current!.pending.some((chip) => chip.failed)) shownFailed = true;
     return handle.current!.attachments.length === 1;
@@ -122,6 +127,63 @@ test("a failed upload is tried again on its own; its chip never shows Retry when
   expect(handle.current!.loading).toBe(0);
 });
 
+/// The person's key or click as the page's retry hears it: jsdom cannot dispatch a trusted event, so
+/// the listener retryUploadLater put on `document` is called with one.
+let restoreInput: (() => void) | undefined;
+function personInput() {
+  const listeners: ((event: Event) => void)[] = [];
+  const add = document.addEventListener.bind(document);
+  document.addEventListener = ((type: string, run: (event: Event) => void, options?: unknown) => {
+    if (type === "keydown" || type === "pointerdown") listeners.push(run);
+    add(type, run as EventListener, options as AddEventListenerOptions);
+  }) as typeof document.addEventListener;
+  const restore = () => {
+    document.addEventListener = add as typeof document.addEventListener;
+    restoreInput = undefined;
+  };
+  restoreInput = restore;
+  return {
+    key: () => {
+      for (const run of listeners.splice(0)) run({ type: "keydown", key: "h", isTrusted: true } as unknown as Event);
+    },
+    restore,
+  };
+}
+
+test("a failed upload never tries again without the person's gesture, and keeps its own words", async () => {
+  // The host starts an upload only on a fresh key or click (AgentPaneModel `uploadStage` spends it
+  // at begin); one sent without that is refused as "The app refused the agent connection.".
+  // Leo, 2026-10-31: "idk what's going on with video attachments" (that refusal beside a recording).
+  const input = personInput();
+  try {
+    let gesture = true;
+    const refused: number[] = [];
+    let calls = 0;
+    const handle = mount(async () => {
+      calls += 1;
+      if (!gesture) {
+        refused.push(calls);
+        return { error: "The app refused the agent connection." };
+      }
+      gesture = false;
+      return calls === 1 ? { error: "The upload timed out: the app did not answer." } : { paths: ["/u/clip.mp4"] };
+    });
+    await act(() => void handle.current!.attach([mp4()], true));
+    await until(() => calls === 1);
+    const quiet = Date.now();
+    await until(() => Date.now() - quiet > UPLOAD_RETRY_DELAY_MS + 200, UPLOAD_RETRY_DELAY_MS * 3);
+    expect(refused).toEqual([]);
+    // The person types: that key is the try's gesture, and the try goes with it.
+    gesture = true;
+    act(() => input.key());
+    await until(() => handle.current!.attachments.length === 1);
+    expect(calls).toBe(2);
+    expect(refused).toEqual([]);
+  } finally {
+    input.restore();
+  }
+});
+
 test("a failed upload that keeps failing shows Retry, and still holds a send until Retry or x", async () => {
   // No input arrives in this page, so after its first try again the wait ends at the input wait;
   // a chip already past its tries shows Retry at once.
@@ -160,6 +222,7 @@ test("an upload the host refused outright shows Retry at once, with no try again
 });
 
 test("an automatic retry stays in its original chat after a plain chat switch", async () => {
+  const input = personInput();
   // The composer instance survives a chat switch. Uploading A's file with B's current upload
   // handler would leak it to B's machine, even though A's pending chip stayed scoped to A.
   const handle: { current: Probe | null } = { current: null };
@@ -186,8 +249,9 @@ test("an automatic retry stays in its original chat after a plain chat switch",
   // An ordinary chat selection, with no explicit draft/attachment carry.
   act(() => root!.render(<Harness options={b} handle={handle} />));
   expect(handle.current!.pending).toEqual([]);
-  const retryWindowStarted = Date.now();
-  await until(() => Date.now() - retryWindowStarted > UPLOAD_RETRY_DELAY_MS + 100);
+  // The person's next key in chat B: the try it starts must not go through B.
+  act(() => input.key());
+  await act(turn);
   expect(bCalls).toBe(0);
   expect(handle.current!.attachments).toEqual([]);
 
@@ -199,9 +263,11 @@ test("an automatic retry stays in its original chat after a plain chat switch",
   await until(() => handle.current!.attachments.some((attached) => attached.name === "Sunbringer Menu Animation.mp4"));
   expect(aCalls).toBe(2);
   expect(bCalls).toBe(0);
+  input.restore();
 });
 
 test("an intentionally carried upload retries through its destination chat", async () => {
+  const input = personInput();
   const handle: { current: Probe | null } = { current: null };
   let aCalls = 0;
   let bCalls = 0;
@@ -225,6 +291,8 @@ test("an intentionally carried upload retries through its destination chat", asy
   await act(() => void handle.current!.attach([mp4()], true));
   await until(() => aCalls === 1 && handle.current!.pending.length === 1);
   act(() => root!.render(<Harness options={b} handle={handle} />));
+  act(() => input.key());
+  input.restore();
   await until(() => bCalls === 1 && handle.current!.attachments.length === 1);
   expect(aCalls).toBe(1);
   expect(handle.current!.pending).toEqual([]);
@@ -259,7 +327,7 @@ function fakeDocument() {
   };
 }
 
-test("the first try again goes on its own; the last waits for the person's key or click, never Return", async () => {
+test("every try again waits for the person's key or click, never Return", async () => {
   const { doc, fire, count } = fakeDocument();
   let again = 0;
   let gaveUp = 0;
@@ -269,17 +337,21 @@ test("the first try again goes on its own; the last waits for the person's key o
     () => (gaveUp += 1),
     doc,
   );
-  // fixed-wait: the 1 s before the first try again is the behavior under test.
+  // fixed-wait: no try goes on a timer, without the host's gesture.
   await new Promise((resolve) => setTimeout(resolve, UPLOAD_RETRY_DELAY_MS + 50));
+  expect(again).toBe(0);
+  fire("pointerdown");
+  await turn();
   expect(again).toBe(1);
   retryUploadLater(
     UPLOAD_AUTO_RETRIES - 1,
     () => (again += 1),
     () => (gaveUp += 1),
     doc,
   );
-  // Return is the send's own gesture: it never starts an upload.
+  // Return is the send's own gesture, and a lone modifier is none: neither starts an upload.
   fire("keydown", { key: "Enter" });
+  fire("keydown", { key: "Shift" });
   await turn();
   expect(again).toBe(1);
   fire("keydown", { key: "h" });