refactor(windows): shrink the job-membership path

Elegance pass. No behaviour change -- all three mutation checks still bind
(restoring the size>1 veto, stamping only on name change, dropping the root
requirement each turn their tests red).

- windows-pty-job-membership.ts 54 -> 31 lines. A deps object carrying one
  optional function became a defaulted parameter, the accumulate loop became a
  filter, and the docblock lost two thirds of its bulk.

  It also lost a claim that was simply false: it said a widened set "is the
  conservative direction for every caller: it keeps a live agent rather than
  retiring it early". For the retire caller, never retiring IS the failure --
  that is the bug this stack just fixed, still being described as a feature
  three commits later.

- One local `identityOlderThan(ms)` in the tracker replaces two hand-rolled
  `Date.now() - refreshedAt` comparisons, one of which I had added.

- The provider's two parallel maps collapse into one Map<id, {name, at}>.
  Parallel maps meant every delete site had to remember its sibling, in three
  places; the reviewer flagged the leak risk and I fixed it by pairing them,
  which leaves the hazard for the next person. One map removes the class.

Comments trimmed to the load-bearing sentence throughout, per AGENTS.md.

src/main/daemon/pty-subprocess/foreground-process-tracker.ts

@@ -123,22 +123,21 @@ export function createPtyForegroundProcessTracker(args: {
     }
     foregroundRefreshInFlight = true
     lastForegroundRefreshStartedAt = now
+    const identityOlderThan = (ms: number): boolean =>
+      cachedAgentForeground !== null && Date.now() - cachedAgentForeground.refreshedAt > ms
     const retireStaleForegroundIdentity = ({ onlyWhenAged = false } = {}): void => {
       const currentFallbackProcess = getFallbackProcess()
-      const identityAgeMs =
-        cachedAgentForeground === null ? 0 : Date.now() - cachedAgentForeground.refreshedAt
       if (
         fallbackIsShell &&
         !getActiveStartupAgent() &&
         currentFallbackProcess !== null &&
         isShellProcess(currentFallbackProcess) &&
-        (!onlyWhenAged || identityAgeMs > WINDOWS_DETACHED_DESCENDANT_IDENTITY_MAX_AGE_MS)
+        (!onlyWhenAged || identityOlderThan(WINDOWS_DETACHED_DESCENDANT_IDENTITY_MAX_AGE_MS))
       ) {
         cachedAgentForeground = null
         startupAgentForeground = null
       } else if (
-        cachedAgentForeground !== null &&
-        Date.now() - cachedAgentForeground.refreshedAt > FOREGROUND_AGENT_CACHE_TTL_MS &&
+        identityOlderThan(FOREGROUND_AGENT_CACHE_TTL_MS) &&
         currentFallbackProcess !== null &&
         isAgentForegroundWrapperProcess(currentFallbackProcess)
       ) {
@@ -154,19 +153,14 @@ export function createPtyForegroundProcessTracker(args: {
         }
         if (!processName || !recognizeAgentProcess(processName)) {
           if (process.platform === 'win32' && fallbackIsShell && cachedAgentForeground !== null) {
-            // Job, not console: "is anything besides the shell alive?" needs no
-            // console attachment, so it needs no forked helper (#10857).
+            // Job, not console: needs no console attachment, so no fork (#10857).
             const paneProcessIds = readWindowsPtyJobProcessIds(proc)
             // Unverifiable is never exit proof (ssh-execution-boundary.md): hold.
             if (paneProcessIds === null) {
               return
             }
-            // A superset answer is not proof of life -- a WSL pane keeps
-            // console-detached plumbing in its job, so `size > 1` stays true
-            // forever and used to veto retirement outright. That pinned a dead
-            // agent's name (#9258's bug, new mechanism) and, because a non-null
-            // cache makes idleNoEvidenceShell false, pinned the refresh at 1s
-            // and defeated the 15s idle backoff. Let age settle it instead.
+            // `size > 1` is a superset, not proof of life. Vetoing on it pinned a
+            // dead agent forever wherever a detached descendant lives; age settles it.
             retireStaleForegroundIdentity({ onlyWhenAged: paneProcessIds.size > 1 })
             return
           }

src/main/providers/local-pty-provider.ts

@@ -124,10 +124,8 @@ const pendingLocalPtySpawns = new Map<string, Set<PendingLocalPtySpawn>>()
 const ptyShellName = new Map<string, string>()
 const ptyAgentForegroundContextPaths = new Map<string, string[]>()
 // Why: remember the last recognized agent foreground so a degraded scan doesn't report the shell and look like an exit.
-const ptyLastRecognizedForeground = new Map<string, string>()
-// When each entry above was last confirmed by a scan, so job evidence -- which is
-// only a superset -- cannot keep it alive indefinitely.
-const ptyLastRecognizedForegroundAt = new Map<string, number>()
+// `at` is when a scan last confirmed it, so job evidence -- only a superset -- cannot hold it forever.
+const ptyLastRecognizedForeground = new Map<string, { name: string; at: number }>()
 const ptyTerminalHandle = new Map<string, string>()
 const ptyWorktreeId = new Map<string, string>()
 const ptyInitialCwd = new Map<string, string>()
@@ -287,7 +285,6 @@ function clearPtyState(id: string): void {
   ptyShellName.delete(id)
   ptyAgentForegroundContextPaths.delete(id)
   ptyLastRecognizedForeground.delete(id)
-  ptyLastRecognizedForegroundAt.delete(id)
   ptyTerminalHandle.delete(id)
   ptyWorktreeId.delete(id)
   ptyInitialCwd.delete(id)
@@ -1408,14 +1405,13 @@ export class LocalPtyProvider implements IPtyProvider {
     const proc = ptyProcesses.get(id)
     if (!proc) {
       ptyLastRecognizedForeground.delete(id)
-      ptyLastRecognizedForegroundAt.delete(id)
       return null
     }
     const fallbackProcess = resolveForegroundFallbackProcess(
       proc.process || null,
       ptyShellName.get(id)
     )
-    const cachedAgent = ptyLastRecognizedForeground.get(id) ?? null
+    const cachedAgent = ptyLastRecognizedForeground.get(id)?.name ?? null
     let paneMembershipUnavailable = false
     // Why: job membership preserves a live cached agent without the whole-table
     // scan (incomplete under Windows load). Job, not console: this asks "is
@@ -1430,16 +1426,15 @@ export class LocalPtyProvider implements IPtyProvider {
         if (ptyProcesses.get(id) !== proc) {
           return null
         }
-        // Why the age bound: `size > 1` is a superset answer, not proof of life.
-        // A WSL pane keeps console-detached plumbing in its job, so this
-        // short-circuit was permanent there and the scan below -- the only path
-        // that can clear the cache -- never ran again. Fall through once stale.
-        const cachedAgentAgeMs = Date.now() - (ptyLastRecognizedForegroundAt.get(id) ?? 0)
+        // Why the age bound: `size > 1` is a superset, not proof of life, so this
+        // short-circuit was permanent wherever a detached descendant lives -- and it
+        // skips the scan below, the only path that can clear the cache.
+        const confirmedAt = ptyLastRecognizedForeground.get(id)?.at ?? 0
         if (
           paneProcessIds !== null &&
           paneProcessIds.size > 1 &&
           cachedAgent !== null &&
-          cachedAgentAgeMs <= WINDOWS_DETACHED_DESCENDANT_IDENTITY_MAX_AGE_MS
+          Date.now() - confirmedAt <= WINDOWS_DETACHED_DESCENDANT_IDENTITY_MAX_AGE_MS
         ) {
           return cachedAgent
         }
@@ -1461,7 +1456,7 @@ export class LocalPtyProvider implements IPtyProvider {
         return null
       }
       // Why: a degraded scan reporting shell-as-foreground fires a false "agent done"; keep last recognized agent instead.
-      const lastRecognizedAgent = ptyLastRecognizedForeground.get(id) ?? null
+      const lastRecognizedAgent = ptyLastRecognizedForeground.get(id)?.name ?? null
       const resolvedAgent = resolution.processName
         ? recognizeAgentProcessFromCommandLine(resolution.processName)
         : null
@@ -1473,14 +1468,12 @@ export class LocalPtyProvider implements IPtyProvider {
         lastRecognizedAgent
       )
       if (stable.lastRecognizedAgent) {
-        // Every confirmation restarts the clock: the bound means "time since we
-        // last saw the agent", not "how long it has run". Stamping only on a
-        // change would age out a live agent that keeps being recognized.
-        ptyLastRecognizedForegroundAt.set(id, Date.now())
-        ptyLastRecognizedForeground.set(id, stable.lastRecognizedAgent)
+        // Every confirmation restarts the clock: the bound is "time since we last
+        // saw the agent", not "how long it ran". Stamping only on a name change
+        // would expire a live agent that keeps being recognized.
+        ptyLastRecognizedForeground.set(id, { name: stable.lastRecognizedAgent, at: Date.now() })
       } else {
         ptyLastRecognizedForeground.delete(id)
-        ptyLastRecognizedForegroundAt.delete(id)
       }
       return stable.processName
     } catch {
@@ -1488,7 +1481,7 @@ export class LocalPtyProvider implements IPtyProvider {
         return null
       }
       // Why: an inspection error is a degraded read; fall back to last recognized agent (null reads as an exit).
-      return ptyLastRecognizedForeground.get(id) ?? null
+      return ptyLastRecognizedForeground.get(id)?.name ?? null
     }
   }
 

src/main/providers/windows-pty-job-membership.test.ts

@@ -27,7 +27,7 @@ describe('readWindowsPtyJobProcessIds', () => {
     forkMock.mockClear()
 
     for (let read = 0; read < 50; read += 1) {
-      readWindowsPtyJobProcessIds(pty(), { listJobProcessIds })
+      readWindowsPtyJobProcessIds(pty(), listJobProcessIds)
     }
 
     expect(forkMock).not.toHaveBeenCalled()
@@ -35,16 +35,14 @@ describe('readWindowsPtyJobProcessIds', () => {
   })
 
   it('reports the shell alone, which is what lets a stale agent be retired', () => {
-    const membership = readWindowsPtyJobProcessIds(pty(), { listJobProcessIds: () => [100] })
+    const membership = readWindowsPtyJobProcessIds(pty(), () => [100])
 
     expect(membership).toEqual(new Set([100]))
     expect(membership?.size).toBe(1)
   })
 
   it('reports descendants, which is what keeps a live agent cached', () => {
-    const membership = readWindowsPtyJobProcessIds(pty(), {
-      listJobProcessIds: () => [100, 200, 300]
-    })
+    const membership = readWindowsPtyJobProcessIds(pty(), () => [100, 200, 300])
 
     expect(membership?.size).toBe(3)
   })
@@ -56,13 +54,11 @@ describe('readWindowsPtyJobProcessIds', () => {
     // null is never evidence that processes died
     // (docs/reference/ssh-execution-boundary.md). An empty list means the tree
     // is gone, which this function has never been the one to report.
-    expect(readWindowsPtyJobProcessIds(pty(), { listJobProcessIds: () => pids })).toBeNull()
+    expect(readWindowsPtyJobProcessIds(pty(), () => pids)).toBeNull()
   })
 
   it('drops nonsense pids rather than trusting the whole answer', () => {
-    const membership = readWindowsPtyJobProcessIds(pty(), {
-      listJobProcessIds: () => [100, 0, -1, 1.5, 200]
-    })
+    const membership = readWindowsPtyJobProcessIds(pty(), () => [100, 0, -1, 1.5, 200])
 
     expect(membership).toEqual(new Set([100, 200]))
   })
@@ -73,9 +69,7 @@ describe('why the filter does NOT use this', () => {
     // Shell exited, a descendant is still up. Size 1 -- but reading that as
     // "the shell is alone, retire the agent" inverts the truth. The forked
     // probe this replaced required the root in the raw list; so does this.
-    const membership = readWindowsPtyJobProcessIds(pty(100), {
-      listJobProcessIds: () => [200]
-    })
+    const membership = readWindowsPtyJobProcessIds(pty(100), () => [200])
 
     expect(membership).toBeNull()
   })
@@ -88,7 +82,7 @@ describe('why the filter does NOT use this', () => {
     const listJobProcessIds = vi.fn(() => [100])
     const proc = pty(100)
 
-    readWindowsPtyJobProcessIds(proc, { listJobProcessIds })
+    readWindowsPtyJobProcessIds(proc, listJobProcessIds)
 
     expect(listJobProcessIds).toHaveBeenCalledWith(proc)
     expect(listJobProcessIds).not.toHaveBeenCalledWith(100)
@@ -106,9 +100,7 @@ describe('why the filter does NOT use this', () => {
     // vs console [69908,40980] -- the job is a superset. Harmless for the
     // `size > 1` callers, wrong for the filter.
     const detachedChild = 104068
-    const membership = readWindowsPtyJobProcessIds(pty(40980), {
-      listJobProcessIds: () => [40980, detachedChild]
-    })
+    const membership = readWindowsPtyJobProcessIds(pty(40980), () => [40980, detachedChild])
 
     expect(membership?.has(detachedChild)).toBe(true)
   })

src/main/providers/windows-pty-job-membership.ts

@@ -1,54 +1,31 @@
 import type { IPty } from 'node-pty'
 import { listPtyJobProcessIds } from '../windows/windows-pty-job'
 
-type WindowsPtyJobMembershipDeps = {
-  listJobProcessIds?: (proc: IPty) => readonly number[] | null
-}
-
 /**
  * Processes still running under a pane, or null when there is no answer.
  *
- * Read from the pane's Win32 job object, not from `GetConsoleProcessList`.
- * That API must be called from a process attached to the console, which is why
- * node-pty answered it by forking a helper — and why asking on a foreground
- * poll, per pane, accumulated hundreds of hidden `conpty_console_list_agent`
- * processes until the machine ran out of memory (#10857). Each read spawned a
- * process; killing them changed nothing because the next poll spawned more.
+ * Read from the pane's Win32 job object. `GetConsoleProcessList` would need
+ * console attachment, which is why node-pty answers it by forking a helper, and
+ * why asking on a foreground poll exhausted memory (#10857).
+ * `QueryInformationJobObject` has no such constraint: one syscall, no children.
  *
- * `QueryInformationJobObject` has no such constraint: any process holding the
- * job handle can ask, so this is one syscall and zero children. Orca already
- * creates the job per PTY.
+ * The job is a SUPERSET of the console -- it keeps console-detached descendants
+ * -- so `size > 1` is not proof of life, only absence of proof of absence.
+ * `size === 1` (the shell alone) is decisive; callers must bound the rest.
  *
- * Semantics callers rely on, unchanged by the switch:
- * - a root-only set proves the shell is alone, so a stale agent can be retired;
- * - `size > 1` proves something is still running under the shell.
- *
- * The one difference is that a descendant which detached from the console stays
- * in the job. That widens the set, which is the conservative direction for
- * every caller: it keeps a live agent rather than retiring it early.
- *
- * Null still means unverifiable per docs/reference/ssh-execution-boundary.md --
- * an unpatched node-pty, a non-ConPTY terminal, or a tree no longer tracked.
- * It is never evidence that processes died.
+ * Null means unverifiable per docs/reference/ssh-execution-boundary.md, never
+ * that processes died.
  */
 export function readWindowsPtyJobProcessIds(
   proc: IPty,
-  deps: WindowsPtyJobMembershipDeps = {}
+  listJobProcessIds: (proc: IPty) => readonly number[] | null = listPtyJobProcessIds
 ): ReadonlySet<number> | null {
-  const pids = (deps.listJobProcessIds ?? listPtyJobProcessIds)(proc)
+  const pids = listJobProcessIds(proc)
   if (!pids) {
     return null
   }
-  const membership = new Set<number>()
-  for (const pid of pids) {
-    if (Number.isSafeInteger(pid) && pid > 0) {
-      membership.add(pid)
-    }
-  }
-  // Why require the shell: callers read a size-1 set as "the shell is alone, so
-  // retire the agent". A set of one pid that is NOT the shell means the shell
-  // already exited with a descendant still up -- the opposite state, and one the
-  // forked probe this replaced refused outright (it required the root in the raw
-  // list). Reporting unverifiable keeps that refusal.
+  const membership = new Set(pids.filter((pid) => Number.isSafeInteger(pid) && pid > 0))
+  // Without the shell, a size-1 set would read as "shell alone, retire" when it
+  // means the opposite. The forked probe this replaced refused the same way.
   return membership.has(proc.pid) ? membership : null
 }