From 2a3053eb752aa62182211dfb875c445e4de34400 Mon Sep 17 00:00:00 2001
From: OrcaWin <alpha-eng@stably.ai>
Date: Sat, 25 Jul 2026 04:25:59 -0700
Subject: [PATCH] fix(relay): use a monotonic deadline for the DefaultShell
 probe cooldown

A wall-clock deadline let an NTP correction or VM resume stretch the 30s
cooldown into an arbitrarily long window of not honoring the configured
shell - a milder version of the bug this branch fixes. performance.now()
matches the elapsed-window idiom already used in pty-handler.

Also pin the spawn-failure branch: reg.exe failing to spawn carries a
string code, so the numeric-exit-code guard is the only thing keeping it
retryable, and document that access-denied shares the cached path.
---
 src/relay/openssh-default-shell.test.ts | 21 +++++++++++++++++++++
 src/relay/openssh-default-shell.ts      | 16 ++++++++++++++--
 2 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/src/relay/openssh-default-shell.test.ts b/src/relay/openssh-default-shell.test.ts
index 69f0d1f36ea..ccaa2c4e5d7 100644
--- a/src/relay/openssh-default-shell.test.ts
+++ b/src/relay/openssh-default-shell.test.ts
@@ -32,6 +32,13 @@ function timeoutError(): Error {
   })
 }
 
+/** The rejection Node surfaces when reg.exe cannot be spawned at all. */
+function spawnFailureError(): Error {
+  // Why: spawn failures report killed/signal as `undefined` and a *string* code,
+  // unlike the numeric exit code of a probe that actually ran.
+  return Object.assign(new Error('spawn reg.exe ENOENT'), { code: 'ENOENT' })
+}
+
 /** Drive the promisified execFile callback with a per-attempt outcome. */
 function mockAttempts(outcomes: (string | Error)[]): { attempts: () => number } {
   let attempt = 0
@@ -108,6 +115,20 @@ describe('readOpenSshDefaultShell', () => {
     expect(attempts()).toBe(1)
   })
 
+  it('retries when reg.exe could not be spawned at all', async () => {
+    // Why pinned: a spawn failure carries a string `code`, so `typeof code === 'number'`
+    // is the only thing keeping it retryable rather than cached for the process lifetime.
+    vi.useFakeTimers()
+    const { attempts } = mockAttempts([spawnFailureError(), regOutput(POWERSHELL_7)])
+    const { readOpenSshDefaultShell, OPENSSH_DEFAULT_SHELL_RETRY_COOLDOWN_MS } = await loadModule()
+
+    expect(await readOpenSshDefaultShell()).toBe('')
+    vi.advanceTimersByTime(OPENSSH_DEFAULT_SHELL_RETRY_COOLDOWN_MS)
+
+    expect(await readOpenSshDefaultShell()).toBe(POWERSHELL_7)
+    expect(attempts()).toBe(2)
+  })
+
   it('retries after a transient failure and then returns the real DefaultShell', async () => {
     // REGRESSION: a single transient reg.exe error used to memoize '' for the
     // lifetime of the relay process, so the host's configured OpenSSH shell was
diff --git a/src/relay/openssh-default-shell.ts b/src/relay/openssh-default-shell.ts
index c1b646e0c96..ce79093bb84 100644
--- a/src/relay/openssh-default-shell.ts
+++ b/src/relay/openssh-default-shell.ts
@@ -32,8 +32,15 @@ let inFlight: Promise<string> | undefined
  * install, where the value simply does not exist — which is authoritative, not a
  * failure. Only a probe killed by the timeout or that failed to spawn is retryable.
  * Why exit code and not stderr text: reg.exe messages are localized.
+ *
+ * "Access is denied" also exits non-zero and so caches for the process lifetime.
+ * That matches the pre-async behavior, and a registry ACL does not flap the way a
+ * timeout does.
  */
 function probeCompleted(error: unknown): boolean {
+  // Why loose `== null`: a spawn failure (reg.exe missing) reports killed/signal as
+  // `undefined` rather than false/null, and carries a string `code` like 'ENOENT'.
+  // The `typeof code === 'number'` guard is what keeps those retryable.
   const { killed, signal, code } = (error ?? {}) as {
     killed?: boolean
     signal?: NodeJS.Signals | null
@@ -61,7 +68,7 @@ export async function readOpenSshDefaultShell(): Promise<string> {
   if (probe?.kind === 'resolved') {
     return probe.shell
   }
-  if (probe?.kind === 'failed' && Date.now() < probe.retryAtMs) {
+  if (probe?.kind === 'failed' && performance.now() < probe.retryAtMs) {
     return ''
   }
   // Why: share one in-flight probe so concurrent spawns don't each launch reg.exe.
@@ -73,7 +80,12 @@ export async function readOpenSshDefaultShell(): Promise<string> {
     .catch((error: unknown) => {
       probe = probeCompleted(error)
         ? { kind: 'resolved', shell: '' }
-        : { kind: 'failed', retryAtMs: Date.now() + OPENSSH_DEFAULT_SHELL_RETRY_COOLDOWN_MS }
+        : {
+            kind: 'failed',
+            // Why monotonic: an NTP correction or VM resume must not stretch a 30s
+            // cooldown into an hour of not honoring the configured shell.
+            retryAtMs: performance.now() + OPENSSH_DEFAULT_SHELL_RETRY_COOLDOWN_MS
+          }
       return ''
     })
     .finally(() => {