fix: address Codex review on PR #3448

electron/bridges/ai/ptyExec.cjs

@@ -95,6 +95,26 @@ function startPtyJob(ptyStream, command, options) {
   const CANCEL_RETRY_MS = 5000;
   const CANCEL_WALL_TIMEOUT_MS = 30000;
 
+  // Bounded fallback for the live shell probe: if the probe's _Q marker never
+  // comes back (hosts that swallow or never echo the typed probe, shells that
+  // block on $PPID inspection, …), waiting for the full command timeout
+  // starves the session — the real command is never even typed (#3446). After
+  // this window, give up on the probe and deliver the wrapper with the
+  // already-resolved shell kind so the command still reaches the remote shell.
+  // The probe runs in the foreground of the interactive shell, so a hung probe
+  // (blocked ps, stuck /proc read) leaves the shell busy: typing the wrapper
+  // right away would only queue it in the PTY. The fallback therefore
+  // interrupts the probe first and waits for the shell to return to idle
+  // before typing the wrapper.
+  const PROBE_FALLBACK_TIMEOUT_MS = 10000;
+  // Bounded settle window after the interrupt: the wrapper is delivered as
+  // soon as a fresh idle prompt is observed, or unconditionally after this
+  // delay (sessions without a known expectedPrompt can never match one).
+  const PROBE_INTERRUPT_SETTLE_MS = 2000;
+  let probeFallbackTimer = null;
+  let probeInterruptTimer = null;
+  let probeInterruptBase = -1;
+
   const usesLiveShellProbe = probeLiveShell && ["posix", "fish"].includes(resolvedShellKind);
   let probingShell = usesLiveShellProbe;
   let deliveringInput = false;
@@ -159,6 +179,44 @@ function startPtyJob(ptyStream, command, options) {
     }
   }
 
+  function clearProbeFallback() {
+    if (probeFallbackTimer) {
+      clearTimeout(probeFallbackTimer);
+      probeFallbackTimer = null;
+    }
+    if (probeInterruptTimer) {
+      clearTimeout(probeInterruptTimer);
+      probeInterruptTimer = null;
+    }
+    probeInterruptBase = -1;
+  }
+
+  function deliverWrapperAfterProbeInterrupt() {
+    probeInterruptTimer = null;
+    probeInterruptBase = -1;
+    if (finished || cancelRequested || !probingShell) return;
+    probingShell = false;
+    probeOutput = "";
+    writeWrappedCommand();
+  }
+
+  function armProbeFallback() {
+    clearProbeFallback();
+    probeFallbackTimer = setTimeout(() => {
+      probeFallbackTimer = null;
+      if (finished || cancelRequested || !probingShell) return;
+      // The probe may itself be the stuck foreground job. Interrupt it so the
+      // shell regains its input loop — otherwise the wrapper written below
+      // merely queues behind the hung probe and can start much later.
+      probeInterruptBase = probeOutput.length;
+      sendInterrupt();
+      probeInterruptTimer = setTimeout(
+        deliverWrapperAfterProbeInterrupt,
+        PROBE_INTERRUPT_SETTLE_MS,
+      );
+    }, PROBE_FALLBACK_TIMEOUT_MS);
+  }
+
   function finishWithoutReturnedPrompt() {
     if (!pendingEnd || finished) return;
     promptRecoveryPendingPtys.add(ptyStream);
@@ -261,6 +319,7 @@ function startPtyJob(ptyStream, command, options) {
     }
     cancelRequested = true;
     clearPromptFallback();
+    clearProbeFallback();
     clearCancelRetryTimer();
     // Cancel the startup timer too — otherwise a pre-start cancel resolves
     // as "Background job startup timed out" instead of "Cancelled".
@@ -442,6 +501,7 @@ function startPtyJob(ptyStream, command, options) {
     clearTimeout(wallTimeoutId);
     clearStartupTimeout();
     clearPromptFallback();
+    clearProbeFallback();
     clearEndMarkerWait();
     clearCancelRetryTimer();
     // Clear any pending one-shot cancel timers so they do not keep the
@@ -553,8 +613,21 @@ function startPtyJob(ptyStream, command, options) {
         return;
       }
       const probe = parseLiveShellProbe(stripAnsi(probeOutput), marker);
-      if (!probe) return;
+      if (!probe) {
+        // After the fallback interrupted a hung probe, a fresh idle prompt in
+        // the post-interrupt output means the shell is reading input again —
+        // deliver the wrapper immediately instead of waiting out the settle
+        // window. Only output received after the interrupt counts, so a prompt
+        // that was already in the buffer before it cannot fake an idle shell.
+        if (probeInterruptTimer !== null
+          && hasExpectedPromptSuffix(probeOutput.slice(probeInterruptBase), expectedPrompt)) {
+          clearProbeFallback();
+          deliverWrapperAfterProbeInterrupt();
+        }
+        return;
+      }
       probingShell = false;
+      clearProbeFallback();
       probeOutput = "";
       if (probe.kind) resolvedShellKind = probe.kind;
       if (finished || cancelRequested) return;
@@ -765,6 +838,7 @@ function startPtyJob(ptyStream, command, options) {
       deliveringInput = false;
       if (!pendingEnd) armOutputTimeout();
       if (!foundStart) armStartupTimeout();
+      if (probingShell) armProbeFallback();
     }
   }
 

electron/bridges/ai/ptyExecPacedTimeout.test.cjs

@@ -125,3 +125,120 @@ for (const shellKind of ['powershell', 'cmd']) {
     assert.match((await job.resultPromise).error, /Cancelled/);
   });
 }
+
+test('a missing live probe reply falls back to delivering the command (#3446)', async (t) => {
+  t.mock.timers.enable({ apis: ['setTimeout'] });
+  const pty = new EventEmitter();
+  const writes = [];
+  const interrupts = [];
+  pty.write = (data) => {
+    if (data === '\x03') {
+      interrupts.push(data);
+      return;
+    }
+    writes.push(String(data));
+  };
+  const command = 'mkdir /tmp/nc_silent_probe_fallback';
+  const job = startPtyJob(pty, command, {
+    shellKind: 'posix', probeLiveShell: true, timeoutMs: 60000,
+  });
+  const probeLength = 2 + buildLiveShellProbe(job.marker).length;
+  while (writes.join('').length < probeLength) t.mock.timers.tick(30);
+  const writesBeforeFallback = writes.length;
+  // The probe reply never arrives. The probe runs in the foreground of the
+  // interactive shell, so the fallback must first interrupt it — otherwise
+  // the wrapper would only queue behind the hung probe — and then type the
+  // wrapper once the shell has settled back to idle.
+  t.mock.timers.tick(10000);
+  assert.equal(interrupts.length, 1, 'the stalled probe is interrupted');
+  assert.ok(
+    !writes.slice(writesBeforeFallback).join('').includes(command),
+    'the wrapper is not typed while the probe still owns the foreground',
+  );
+  const wrapped = buildWrappedCommand(command, 'posix', job.marker, true);
+  let guard = 0;
+  while (!writes.slice(writesBeforeFallback).join('').includes(wrapped) && guard++ < 500) {
+    t.mock.timers.tick(30);
+  }
+  assert.ok(
+    writes.slice(writesBeforeFallback).join('').includes(wrapped),
+    'wrapper is typed after the probe interrupt settles',
+  );
+  pty.emit('data', `${job.marker}_S\n${job.marker}_E:0\n`);
+  const result = await job.resultPromise;
+  assert.equal(result.ok, true);
+  assert.equal(result.exitCode, 0);
+  t.mock.timers.reset();
+  pty.emit('close');
+});
+
+test('the probe fallback delivers the wrapper once the shell is idle again (#3446)', async (t) => {
+  t.mock.timers.enable({ apis: ['setTimeout'] });
+  const pty = new EventEmitter();
+  const writes = [];
+  const interrupts = [];
+  pty.write = (data) => {
+    if (data === '\x03') {
+      interrupts.push(data);
+      return;
+    }
+    writes.push(String(data));
+  };
+  const command = 'echo after-idle';
+  const job = startPtyJob(pty, command, {
+    shellKind: 'posix', probeLiveShell: true, timeoutMs: 60000,
+    expectedPrompt: 'host:~$ ',
+  });
+  const probeLength = 2 + buildLiveShellProbe(job.marker).length;
+  while (writes.join('').length < probeLength) t.mock.timers.tick(30);
+  // A stale idle prompt from before the probe reply went missing must not
+  // satisfy the fallback's idle check on its own.
+  pty.emit('data', 'host:~$ ');
+  const writesBeforeFallback = writes.length;
+  t.mock.timers.tick(10000);
+  assert.equal(interrupts.length, 1, 'the stalled probe is interrupted');
+  t.mock.timers.tick(500);
+  assert.ok(
+    !writes.slice(writesBeforeFallback).join('').includes(command),
+    'the wrapper waits for a fresh idle prompt after the interrupt',
+  );
+  // The shell recovered from the interrupted probe and shows its prompt.
+  pty.emit('data', '^Chost:~$ ');
+  const wrapped = buildWrappedCommand(command, 'posix', job.marker, true);
+  let guard = 0;
+  while (!writes.slice(writesBeforeFallback).join('').includes(wrapped) && guard++ < 200) {
+    t.mock.timers.tick(30);
+  }
+  assert.ok(
+    writes.slice(writesBeforeFallback).join('').includes(wrapped),
+    'wrapper is typed as soon as the shell is idle again',
+  );
+  pty.emit('data', `${job.marker}_S\n${job.marker}_E:0\n`);
+  const result = await job.resultPromise;
+  assert.equal(result.ok, true);
+  t.mock.timers.reset();
+  pty.emit('close');
+});
+
+test('cancelling during a stalled probe never types the command (#3446)', async (t) => {
+  t.mock.timers.enable({ apis: ['setTimeout'] });
+  const pty = new EventEmitter();
+  const writes = [];
+  pty.write = (data) => {
+    if (data === '\x03') return;
+    writes.push(String(data));
+  };
+  const command = 'echo should-not-run';
+  const job = startPtyJob(pty, command, {
+    shellKind: 'posix', probeLiveShell: true, timeoutMs: 60000,
+  });
+  const probeLength = 2 + buildLiveShellProbe(job.marker).length;
+  while (writes.join('').length < probeLength) t.mock.timers.tick(30);
+  job.cancel();
+  t.mock.timers.tick(10000);
+  assert.ok(writes.every((data) => !data.includes('echo should-not-run')));
+  const result = await job.resultPromise;
+  assert.match(result.error, /Cancelled/);
+  t.mock.timers.reset();
+  pty.emit('close');
+});