fix(chat): an abandoned approval must not leave a needs-approval dot

src/main/approval-bridge-rules.test.ts

@@ -1,7 +1,9 @@
-// Unit coverage for the persistent-allowlist short-circuit in
-// ApprovalBridge. Talks to the bridge's unix socket directly with
-// hand-written request frames instead of spawning `claude`, so it runs
-// everywhere (unlike approval-bridge.test.ts, which needs the binary).
+// Unit coverage for the ApprovalBridge paths that don't need a model:
+// the persistent-allowlist short-circuit, and dropping an approval whose
+// tool call Claude already abandoned. Talks to the bridge's unix socket
+// directly with hand-written request frames instead of spawning
+// `claude`, so it runs everywhere (unlike approval-bridge.test.ts,
+// which needs the binary).
 
 import { describe, it, expect, afterEach } from 'vitest'
 import { createConnection, type Socket } from 'node:net'
@@ -46,7 +48,11 @@ afterEach(() => {
 interface Harness {
   store: TestStore
   sessionId: string
+  bridge: ApprovalBridge
   send: (frame: Record<string, unknown>) => Promise<Record<string, unknown> | null>
+  /** Every response frame the bridge has written back, in order —
+   *  including ones that landed after `send` already gave up waiting. */
+  responses: Record<string, unknown>[]
 }
 
 /** Boot a bridge with `rules` pre-loaded and return a `send` that writes one
@@ -65,10 +71,13 @@ async function harness(rules: StoredPermissionRule[]): Promise<Harness> {
   openBridges.push(bridge)
   const sessionId = randomUUID()
   const socketPath = bridge.startSession(sessionId)
+  const responses: Record<string, unknown>[] = []
 
   return {
     store,
     sessionId,
+    bridge,
+    responses,
     send: (frame) =>
       new Promise((resolve, reject) => {
         const socket = createConnection(socketPath)
@@ -81,10 +90,16 @@ async function harness(rules: StoredPermissionRule[]): Promise<Harness> {
         })
         socket.on('data', (chunk: Buffer) => {
           buf += chunk.toString('utf8')
-          const idx = buf.indexOf('\n')
-          if (idx < 0) return
-          clearTimeout(timer)
-          resolve(JSON.parse(buf.slice(0, idx)) as Record<string, unknown>)
+          let idx: number
+          while ((idx = buf.indexOf('\n')) >= 0) {
+            const line = buf.slice(0, idx)
+            buf = buf.slice(idx + 1)
+            if (!line.trim()) continue
+            const parsed = JSON.parse(line) as Record<string, unknown>
+            responses.push(parsed)
+            clearTimeout(timer)
+            resolve(parsed)
+          }
         })
         socket.on('connect', () => {
           socket.write(JSON.stringify({ type: 'request', sessionId, ...frame }) + '\n')
@@ -93,6 +108,10 @@ async function harness(rules: StoredPermissionRule[]): Promise<Harness> {
   }
 }
 
+function tick(ms = 50): Promise<void> {
+  return new Promise((resolve) => setTimeout(resolve, ms))
+}
+
 describe('ApprovalBridge — persistent allowlist', () => {
   it('allows a matching call without surfacing a card', async () => {
     const h = await harness([rule('Bash', 'git status:*')])
@@ -170,3 +189,77 @@ describe('ApprovalBridge — persistent allowlist', () => {
     expect(res).toBeNull()
   })
 })
+
+describe('ApprovalBridge — abandoned tool calls', () => {
+  it('drops a pending approval once a result lands for its tool_use', async () => {
+    // Claude Code's MCP idle timeout aborts our `approve` call and
+    // reports it as an is_error tool_result on the original tool_use.
+    // Nothing reaches the socket, so without this the card and its
+    // needs-approval dot would outlive the turn.
+    const h = await harness([])
+    expect(
+      await h.send({
+        id: 'req-stale',
+        tool_name: 'AskUserQuestion',
+        input: { questions: [] },
+        tool_use_id: 'tu-stale'
+      })
+    ).toBeNull()
+    expect(
+      h.store.getSnapshot().state.jsonClaude.pendingApprovals['req-stale']
+    ).toBeDefined()
+
+    expect(h.bridge.cancelPendingForToolUse(h.sessionId, 'tu-stale')).toBe(true)
+    await tick()
+
+    expect(
+      h.store.getSnapshot().state.jsonClaude.pendingApprovals['req-stale']
+    ).toBeUndefined()
+    // The MCP subprocess is still parked on the socket — answer it so it
+    // unwinds instead of leaking until the session dies.
+    expect(h.responses).toHaveLength(1)
+    expect(h.responses[0]).toMatchObject({ type: 'response', id: 'req-stale' })
+    expect(
+      (h.responses[0] as { result: { behavior: string } }).result.behavior
+    ).toBe('deny')
+  })
+
+  it('leaves approvals for other tool_use ids and other sessions alone', async () => {
+    const h = await harness([])
+    await h.send({
+      id: 'req-keep',
+      tool_name: 'Bash',
+      input: { command: 'rm -rf /' },
+      tool_use_id: 'tu-keep'
+    })
+    expect(h.bridge.cancelPendingForToolUse(h.sessionId, 'tu-other')).toBe(false)
+    expect(h.bridge.cancelPendingForToolUse('some-other-session', 'tu-keep')).toBe(
+      false
+    )
+    await tick()
+    expect(
+      h.store.getSnapshot().state.jsonClaude.pendingApprovals['req-keep']
+    ).toBeDefined()
+    expect(h.responses).toHaveLength(0)
+  })
+
+  it('is a no-op for an approval the user already answered', async () => {
+    // The normal ordering: the user clicks Allow, the tool runs, and its
+    // result arrives. One deny must not be written on top of the allow.
+    const h = await harness([])
+    await h.send({
+      id: 'req-done',
+      tool_name: 'Bash',
+      input: { command: 'ls' },
+      tool_use_id: 'tu-done'
+    })
+    expect(h.bridge.resolveApproval('req-done', { behavior: 'allow' })).toBe(true)
+    await tick()
+    expect(h.bridge.cancelPendingForToolUse(h.sessionId, 'tu-done')).toBe(false)
+    await tick()
+    expect(h.responses).toHaveLength(1)
+    expect(
+      (h.responses[0] as { result: { behavior: string } }).result.behavior
+    ).toBe('allow')
+  })
+})

src/main/approval-bridge.ts

@@ -178,6 +178,44 @@ export class ApprovalBridge {
     }
   }
 
+  /** Claude Code abandoned the tool call we were asking about, so the
+   *  answer can no longer land anywhere. The usual cause is its MCP
+   *  idle timeout firing on our `approve` call after 1800s of silence
+   *  ("sent no response or progress for 1800s; aborting") — by design,
+   *  so an agent nobody answered works around the question instead of
+   *  blocking forever. It reports that as an is_error tool_result on
+   *  the *original* tool_use; the approval socket itself hears nothing.
+   *  So without this reconciliation the pending entry outlives the
+   *  turn, and the sidebar keeps a needs-approval dot for a tool the
+   *  chat already shows as skipped.
+   *
+   *  Called from the json-claude stream reader for every tool_result:
+   *  a result means the call was decided, so any approval still pending
+   *  for that tool_use id is by definition stale. Answers the socket so
+   *  the MCP subprocess unwinds, then drops the card. */
+  cancelPendingForToolUse(
+    sessionId: string,
+    toolUseId: string,
+    reason = 'tool call abandoned before approval resolved'
+  ): boolean {
+    for (const [id, pending] of Array.from(this.pendingResponses.entries())) {
+      if (pending.sessionId !== sessionId) continue
+      if (pending.payload.toolUseId !== toolUseId) continue
+      this.writeResponse(pending.socket, id, { behavior: 'deny', message: reason })
+      this.pendingResponses.delete(id)
+      this.store.dispatch({
+        type: 'jsonClaude/approvalResolved',
+        payload: { requestId: id }
+      })
+      log(
+        'approval-bridge',
+        `stale approval dropped session=${sessionId} id=${id} tool=${pending.payload.toolName} toolUseId=${toolUseId} reason="${reason}"`
+      )
+      return true
+    }
+    return false
+  }
+
   /** Dispatched from the renderer-facing IPC handler. Push the user's
    *  chosen PermissionResult back out over the waiting socket. */
   resolveApproval(requestId: string, result: ApprovalResult): boolean {

src/main/index.ts

@@ -431,6 +431,8 @@ const jsonClaudeManager = new JsonClaudeManager(store, {
     store.getSnapshot().state.settings.useSystemClaudeForJsonMode === true,
   getApprovalSocketPath: (sessionId) => approvalBridge.startSession(sessionId),
   closeApprovalSession: (sessionId) => approvalBridge.stopSession(sessionId),
+  cancelApprovalForToolUse: (sessionId, toolUseId) =>
+    approvalBridge.cancelPendingForToolUse(sessionId, toolUseId),
   getClaudeEnvVars: () =>
     store.getSnapshot().state.settings.claudeEnvVars || {},
   getControlServer: () => getControlServerInfo(),

src/main/json-claude-manager-fork.test.ts

@@ -51,6 +51,7 @@ function makeManager(store: Store): JsonClaudeManager {
     getUseSystemClaude: () => false,
     getApprovalSocketPath: (sid) => `/tmp/sock-${sid}`,
     closeApprovalSession: vi.fn(),
+    cancelApprovalForToolUse: vi.fn(),
     getClaudeEnvVars: () => ({}),
     getControlServer: () => null,
     getControlBridgeScriptPath: () => '/tmp/bridge.js',

src/main/json-claude-manager.test.ts

@@ -81,13 +81,15 @@ describe('JsonClaudeManager', () => {
   function makeManager(
     store: Store,
     launchSettings: ClaudeLaunchSettings = { tuiFullscreen: true },
-    useSystemClaude = false
+    useSystemClaude = false,
+    cancelApprovalForToolUse: (sessionId: string, toolUseId: string) => void = vi.fn()
   ): JsonClaudeManager {
     return new JsonClaudeManager(store, {
       getClaudeCommand: () => 'claude',
       getUseSystemClaude: () => useSystemClaude,
       getApprovalSocketPath: (sid) => `/tmp/sock-${sid}`,
       closeApprovalSession: vi.fn(),
+      cancelApprovalForToolUse,
       getClaudeEnvVars: () => ({}),
       getControlServer: () => null,
       getControlBridgeScriptPath: () => '/tmp/bridge.js',
@@ -293,6 +295,67 @@ describe('JsonClaudeManager', () => {
     expect(call.args).toContain('r/b')
   })
 
+  // Load-bearing absence: with no per-server `timeout`, Claude's default
+  // MCP idle timeout (1800s) aborts an approval nobody answered, and the
+  // agent works around the question instead of parking forever. Setting
+  // one here to keep an approval card alive longer would trade a
+  // self-healing agent for a blocked one — Ness reconciles the abort
+  // instead (see the stale-approval test below).
+  it('sets no timeout on the ness-permissions MCP server', () => {
+    const store = new Store()
+    const mgr = makeManager(store)
+    store.dispatch({
+      type: 'jsonClaude/sessionStarted',
+      payload: { sessionId: 'sess-mcp-timeout', worktreePath: '/tmp/wt' }
+    })
+    mgr.create('sess-mcp-timeout', '/tmp/wt')
+    const call = spawnCalls
+      .filter((c) => c.args.some((a) => a.includes('--permission-prompt-tool')))
+      .pop()
+    const configIdx = call!.args.indexOf('--mcp-config')
+    const config = JSON.parse(call!.args[configIdx + 1]) as {
+      mcpServers: Record<string, { timeout?: number }>
+    }
+    expect(config.mcpServers['ness-permissions']).toBeDefined()
+    expect(config.mcpServers['ness-permissions'].timeout).toBeUndefined()
+  })
+
+  it('drops a stale approval when a tool_result lands for its tool_use', () => {
+    // Claude Code aborting its `approve` MCP call is reported as an
+    // is_error tool_result on the original tool_use — the only signal we
+    // get that the approval window closed.
+    const store = new Store()
+    const cancel = vi.fn()
+    const mgr = makeManager(store, { tuiFullscreen: true }, false, cancel)
+    const sessionId = 'sess-stale-approval'
+    store.dispatch({
+      type: 'jsonClaude/sessionStarted',
+      payload: { sessionId, worktreePath: '/tmp/wt' }
+    })
+    mgr.create(sessionId, '/tmp/wt')
+    sessionProcs()[0].stdout.emit(
+      'data',
+      Buffer.from(
+        JSON.stringify({
+          type: 'user',
+          message: {
+            role: 'user',
+            content: [
+              {
+                type: 'tool_result',
+                tool_use_id: 'toolu_timeout',
+                is_error: true,
+                content:
+                  '<tool_use_error>Error calling tool (AskUserQuestion): MCP server "ness-permissions" tool "approve" sent no response or progress for 1800s; aborting.</tool_use_error>'
+              }
+            ]
+          }
+        }) + '\n'
+      )
+    )
+    expect(cancel).toHaveBeenCalledWith(sessionId, 'toolu_timeout')
+  })
+
   it('omits --append-system-prompt, --model, --name when launch settings are unset', () => {
     const store = new Store()
     const mgr = makeManager(store, { tuiFullscreen: true })

src/main/json-claude-manager.ts

@@ -101,6 +101,11 @@ export interface JsonClaudeManagerOptions {
   getUseSystemClaude: () => boolean
   getApprovalSocketPath: (sessionId: string) => string
   closeApprovalSession: (sessionId: string) => void
+  /** Called for every tool_result we see so an approval still pending
+   *  for that tool_use id can be dropped — Claude Code aborting its
+   *  `approve` MCP call (idle timeout) is otherwise invisible to us.
+   *  See ApprovalBridge.cancelPendingForToolUse. */
+  cancelApprovalForToolUse: (sessionId: string, toolUseId: string) => void
   getClaudeEnvVars: () => Record<string, string>
   /** Looked up from main when building the inline MCP config. Returning
    *  null means the ness-control bridge isn't injected (settings flag
@@ -594,6 +599,14 @@ export class JsonClaudeManager {
     // need a separate Node install in packaged builds.
     //   * ness-permissions: receives the per-tool approval requests
     //     Claude raises via --permission-prompt-tool. Tool is 'approve'.
+    //     Deliberately carries no per-server `timeout`, so Claude's
+    //     default MCP idle timeout (1800s) still aborts an approval
+    //     nobody answered. That's wanted: the agent gets an error
+    //     tool_result and can work around the question or move to
+    //     something else instead of parking forever. Ness reconciles
+    //     the abort via cancelApprovalForToolUse — setting a timeout
+    //     here to "fix" a vanishing approval card would trade a
+    //     self-healing agent for a blocked one.
     //   * ness-control: the same MCP bridge used by xterm-backed
     //     Claude tabs, exposing ness-control tools (worktree mgmt,
     //     browser tabs, shell tabs, etc.). Skipped when settings has
@@ -1711,6 +1724,10 @@ export class JsonClaudeManager {
     if (type === 'user') {
       const results = extractToolResults(parsed)
       for (const r of results) {
+        // A result means this tool call is decided, so an approval still
+        // pending for it can never be answered — drop it before the
+        // card and its needs-approval dot outlive the turn.
+        this.opts.cancelApprovalForToolUse(instance.sessionId, r.toolUseId)
         this.store.dispatch({
           type: 'jsonClaude/toolResultAttached',
           payload: {