From 62b65d68771da3ec00c2f732620971b09d0178e8 Mon Sep 17 00:00:00 2001 From: Destin Date: Fri, 31 Jul 2026 12:14:55 -0700 Subject: [PATCH 01/20] fix(android): PermissionRequest hook timeout now updates on existing installs Bootstrap only appended the PermissionRequest blocking-relay hook entry when none existed, so every pre-existing install kept timeout: 300 forever. Extracted the logic into Bootstrap.ensurePermissionRequestHook, a find-and-replace helper that overwrites command + timeout on a matching existing entry (mirrors desktop install-hooks.js semantics). Task 1 of the permission-ask timeout fix: prerequisite for Task 2's relay-asset + timeout-value bump, which would otherwise regress existing Android installs from a 120s auto-deny into a permanent wedge. Co-Authored-By: Claude Opus 5 --- .../com/youcoded/app/runtime/Bootstrap.kt | 73 +++++++++++-------- .../app/runtime/BootstrapHooksTest.kt | 48 ++++++++++++ 2 files changed, 91 insertions(+), 30 deletions(-) create mode 100644 app/src/test/kotlin/com/youcoded/app/runtime/BootstrapHooksTest.kt diff --git a/app/src/main/kotlin/com/youcoded/app/runtime/Bootstrap.kt b/app/src/main/kotlin/com/youcoded/app/runtime/Bootstrap.kt index 3041340ea..a57a4c9bd 100644 --- a/app/src/main/kotlin/com/youcoded/app/runtime/Bootstrap.kt +++ b/app/src/main/kotlin/com/youcoded/app/runtime/Bootstrap.kt @@ -25,6 +25,48 @@ class Bootstrap(internal val context: Context) { // Last Claude Code release shipping cli.js. See the comment on // isFullySetup and installClaudeCode() for why this is pinned. private const val PINNED_CLAUDE_CODE_VERSION = "2.1.112" + + /** Ensure the PermissionRequest blocking-relay hook entry exists AND + * carries the current command + timeout. WHY: earlier versions only + * appended when missing, so every existing install kept timeout 300 + * forever — and the relay asset DOES redeploy on every launch, so a + * relay-only change would put relay-2h30m against CC-300s: CC kills the + * hook with no decision and AskUserQuestion wedges permanently + * (2026-07-30 spec §Constraints). Mirrors desktop install-hooks.js + * find-and-replace semantics. */ + fun ensurePermissionRequestHook( + hooksObj: org.json.JSONObject, + blockingHookCommand: String, + timeoutSeconds: Int, + ) { + val prEvent = "PermissionRequest" + val prArray = hooksObj.optJSONArray(prEvent) ?: org.json.JSONArray() + var updated = false + for (i in 0 until prArray.length()) { + val hooks = prArray.optJSONObject(i)?.optJSONArray("hooks") ?: continue + for (j in 0 until hooks.length()) { + val h = hooks.optJSONObject(j) + if (h?.optString("command")?.contains("hook-relay-blocking.js") == true) { + h.put("command", blockingHookCommand) + h.put("timeout", timeoutSeconds) + updated = true + } + } + } + if (!updated) { + val hookEntry = org.json.JSONObject() + hookEntry.put("matcher", ".*") + val hooksList = org.json.JSONArray() + val hookDef = org.json.JSONObject() + hookDef.put("type", "command") + hookDef.put("command", blockingHookCommand) + hookDef.put("timeout", timeoutSeconds) + hooksList.put(hookDef) + hookEntry.put("hooks", hooksList) + prArray.put(hookEntry) + } + hooksObj.put(prEvent, prArray) + } } val usrDir: File get() = File(context.filesDir, "usr") @@ -987,36 +1029,7 @@ class Bootstrap(internal val context: Context) { } // Register PermissionRequest with blocking relay (long timeout for user approval) - val prEvent = "PermissionRequest" - val prArray = hooksObj.optJSONArray(prEvent) ?: org.json.JSONArray() - var prRegistered = false - for (i in 0 until prArray.length()) { - val entry = prArray.optJSONObject(i) - val hooks = entry?.optJSONArray("hooks") - if (hooks != null) { - for (j in 0 until hooks.length()) { - val h = hooks.optJSONObject(j) - if (h?.optString("command")?.contains("hook-relay-blocking.js") == true) { - prRegistered = true - break - } - } - } - if (prRegistered) break - } - if (!prRegistered) { - val hookEntry = org.json.JSONObject() - hookEntry.put("matcher", ".*") - val hooksList = org.json.JSONArray() - val hookDef = org.json.JSONObject() - hookDef.put("type", "command") - hookDef.put("command", blockingHookCommand) - hookDef.put("timeout", 300) - hooksList.put(hookDef) - hookEntry.put("hooks", hooksList) - prArray.put(hookEntry) - } - hooksObj.put(prEvent, prArray) + ensurePermissionRequestHook(hooksObj, blockingHookCommand, 300) // Auto-title hook: always deploy the bundled asset. Post-decomposition, // title-update.sh is app-owned (not a toolkit hook) on both platforms — diff --git a/app/src/test/kotlin/com/youcoded/app/runtime/BootstrapHooksTest.kt b/app/src/test/kotlin/com/youcoded/app/runtime/BootstrapHooksTest.kt new file mode 100644 index 000000000..48b899111 --- /dev/null +++ b/app/src/test/kotlin/com/youcoded/app/runtime/BootstrapHooksTest.kt @@ -0,0 +1,48 @@ +package com.youcoded.app.runtime + +import org.json.JSONArray +import org.json.JSONObject +import org.junit.Assert.assertEquals +import org.junit.Test + +/** Guards the 2026-07-30 spec §Constraints inversion: an install that already + * has the hook must still receive a changed timeout on the next launch. */ +class BootstrapHooksTest { + + private fun existingHooks(timeout: Int): JSONObject { + val h = JSONObject().put("type", "command") + .put("command", "node /old/path/hook-relay-blocking.js").put("timeout", timeout) + val entry = JSONObject().put("matcher", ".*") + .put("hooks", JSONArray().put(h)) + return JSONObject().put("PermissionRequest", JSONArray().put(entry)) + } + + @Test + fun `overwrites timeout and command on an existing entry`() { + val hooksObj = existingHooks(300) + Bootstrap.ensurePermissionRequestHook(hooksObj, "node /new/path/hook-relay-blocking.js", 10800) + val h = hooksObj.getJSONArray("PermissionRequest") + .getJSONObject(0).getJSONArray("hooks").getJSONObject(0) + assertEquals(10800, h.getInt("timeout")) + assertEquals("node /new/path/hook-relay-blocking.js", h.getString("command")) + } + + @Test + fun `appends a new entry when none exists`() { + val hooksObj = JSONObject() + Bootstrap.ensurePermissionRequestHook(hooksObj, "node /p/hook-relay-blocking.js", 10800) + val arr = hooksObj.getJSONArray("PermissionRequest") + assertEquals(1, arr.length()) + val h = arr.getJSONObject(0).getJSONArray("hooks").getJSONObject(0) + assertEquals(10800, h.getInt("timeout")) + assertEquals("command", h.getString("type")) + } + + @Test + fun `does not duplicate on repeat runs`() { + val hooksObj = existingHooks(300) + Bootstrap.ensurePermissionRequestHook(hooksObj, "node /p/hook-relay-blocking.js", 10800) + Bootstrap.ensurePermissionRequestHook(hooksObj, "node /p/hook-relay-blocking.js", 10800) + assertEquals(1, hooksObj.getJSONArray("PermissionRequest").length()) + } +} From 12e46b78c763a7f902d9cd0452ae4ba721aee913 Mon Sep 17 00:00:00 2001 From: Destin Date: Fri, 31 Jul 2026 12:22:09 -0700 Subject: [PATCH 02/20] feat(permissions): staggered 2h/2h30m/3h timeout tiers, both platforms + stale-site sweep Sets the new timeout tier values across all six sites (desktop relay, desktop CC hook entry, Android relay asset, Android CC hook entry via new Bootstrap.PERMISSION_HOOK_TIMEOUT_SECONDS constant) and rewrites every comment/doc/test-harness assertion that still asserted the old design (300s/120s/fail-open-on-timeout). New pinning test desktop/tests/permission-timeout-margins.test.ts reads the literal values out of source (not process.env-resolved) so it can't pass vacuously, and asserts relay < CC with a real margin plus the 32-bit setTimeout ceiling. Co-Authored-By: Claude Opus 5 --- app/src/main/assets/hook-relay-blocking.js | 5 +- .../com/youcoded/app/parser/EventBridge.kt | 2 +- .../com/youcoded/app/parser/HookEvent.kt | 2 +- .../com/youcoded/app/runtime/Bootstrap.kt | 10 +++- desktop/docs/blocking-relay-handoff.md | 9 ++- desktop/docs/test-blocking-relay.js | 10 +++- desktop/hook-scripts/relay-blocking.js | 14 +++-- desktop/scripts/install-hooks.js | 6 +- .../tests/permission-timeout-margins.test.ts | 56 +++++++++++++++++++ 9 files changed, 95 insertions(+), 19 deletions(-) create mode 100644 desktop/tests/permission-timeout-margins.test.ts diff --git a/app/src/main/assets/hook-relay-blocking.js b/app/src/main/assets/hook-relay-blocking.js index 89eb28825..ec134aeeb 100644 --- a/app/src/main/assets/hook-relay-blocking.js +++ b/app/src/main/assets/hook-relay-blocking.js @@ -13,7 +13,10 @@ var net = require('net'); var socket = process.env.CLAUDE_MOBILE_SOCKET; if (!socket) process.exit(0); -var TIMEOUT_MS = parseInt(process.env.CLAUDE_RELAY_TIMEOUT || '120000', 10); +// Tier-2 backstop: 2h30m — above EventBridge's 2h hold, below Bootstrap's 3h +// CC hook timeout. Relay-wins = exit 2 (clean deny); CC-wins = hook killed +// with no decision = AskUserQuestion wedges forever. Do NOT equalize (spec §1). +var TIMEOUT_MS = parseInt(process.env.CLAUDE_RELAY_TIMEOUT || '9000000', 10); var input = ''; process.stdin.setEncoding('utf8'); diff --git a/app/src/main/kotlin/com/youcoded/app/parser/EventBridge.kt b/app/src/main/kotlin/com/youcoded/app/parser/EventBridge.kt index b2611337d..89c2c7ada 100644 --- a/app/src/main/kotlin/com/youcoded/app/parser/EventBridge.kt +++ b/app/src/main/kotlin/com/youcoded/app/parser/EventBridge.kt @@ -143,7 +143,7 @@ class EventBridge(private val socketName: String) { /** * Monitor a held PermissionRequest socket for remote closure. - * When hook-relay-blocking.js times out (120s) or Claude Code kills the hook + * When hook-relay-blocking.js times out (its 2h30m tier-2 backstop) or Claude Code kills the hook * process, the socket closes. We detect this and emit PermissionExpired so * the React UI can clear the stale approval card. * diff --git a/app/src/main/kotlin/com/youcoded/app/parser/HookEvent.kt b/app/src/main/kotlin/com/youcoded/app/parser/HookEvent.kt index 060b46653..072a59d27 100644 --- a/app/src/main/kotlin/com/youcoded/app/parser/HookEvent.kt +++ b/app/src/main/kotlin/com/youcoded/app/parser/HookEvent.kt @@ -57,7 +57,7 @@ sealed class HookEvent { ) : HookEvent() /** Emitted when a held PermissionRequest socket closes before a response - * was sent — e.g., hook-relay-blocking.js timed out (120s) or Claude Code + * was sent — e.g., hook-relay-blocking.js timed out (2h30m backstop) or Claude Code * killed the hook process. React uses this to clear stale approval cards. * Desktop equivalent: hook-relay.ts socket.on('close') → 'permission-expired'. */ data class PermissionExpired( diff --git a/app/src/main/kotlin/com/youcoded/app/runtime/Bootstrap.kt b/app/src/main/kotlin/com/youcoded/app/runtime/Bootstrap.kt index a57a4c9bd..617cf4af6 100644 --- a/app/src/main/kotlin/com/youcoded/app/runtime/Bootstrap.kt +++ b/app/src/main/kotlin/com/youcoded/app/runtime/Bootstrap.kt @@ -26,6 +26,11 @@ class Bootstrap(internal val context: Context) { // isFullySetup and installClaudeCode() for why this is pinned. private const val PINNED_CLAUDE_CODE_VERSION = "2.1.112" + /** Tier-3 CC hook timeout (3h) — 30m above the relay asset's 2h30m so CC + * never kills the hook first (no decision = AskUserQuestion wedges + * forever, spec §1). Pinned by desktop/tests/permission-timeout-margins. */ + const val PERMISSION_HOOK_TIMEOUT_SECONDS = 10800 + /** Ensure the PermissionRequest blocking-relay hook entry exists AND * carries the current command + timeout. WHY: earlier versions only * appended when missing, so every existing install kept timeout 300 @@ -1028,8 +1033,9 @@ class Bootstrap(internal val context: Context) { hooksObj.put(event, eventArray) } - // Register PermissionRequest with blocking relay (long timeout for user approval) - ensurePermissionRequestHook(hooksObj, blockingHookCommand, 300) + // Register PermissionRequest with blocking relay (tier-3 CC timeout, see + // PERMISSION_HOOK_TIMEOUT_SECONDS doc comment for the margin rationale) + ensurePermissionRequestHook(hooksObj, blockingHookCommand, PERMISSION_HOOK_TIMEOUT_SECONDS) // Auto-title hook: always deploy the bundled asset. Post-decomposition, // title-update.sh is app-owned (not a toolkit hook) on both platforms — diff --git a/desktop/docs/blocking-relay-handoff.md b/desktop/docs/blocking-relay-handoff.md index 258dcf78c..73a09bac4 100644 --- a/desktop/docs/blocking-relay-handoff.md +++ b/desktop/docs/blocking-relay-handoff.md @@ -39,15 +39,14 @@ Keep PTY detection as the catch-all (for trust gates, non-hook menus), but add s The relay writes its payload and **waits**. The server decides what happens: - **Fire-and-forget:** Server closes socket without writing → relay sees `end` → exits 0 (backward compatible) -- **Blocking allow:** Server holds socket, writes `{"allow":true}\n` → relay exits 0 -- **Blocking deny:** Server holds socket, writes `{"allow":false}\n` → relay exits 2 -- **Timeout safety:** Relay has a configurable timeout (default 30s, via `CLAUDE_RELAY_TIMEOUT` env var). If server goes silent → relay exits 0 (fail-open) +- **Blocking decision:** Server holds socket, writes a decision (`{"decision":"allow"}` or `{"decision":"deny"}`) → relay wraps it in `hookSpecificOutput` and exits 0 either way; Claude Code reads the `decision` field, not the exit code. Exit 2 is the timeout path only (see below) — there is no deny-specific exit code. +- **Timeout safety:** Relay has a configurable timeout (default 2h30m, via `CLAUDE_RELAY_TIMEOUT` env var). If the server goes silent past it → relay exits 2 (fail-closed deny) Key property: **relay doesn't need to know which hooks are blocking.** The server decides. This means adding new blocking hook types only requires server-side changes. ### Spike Test Results (VALIDATED, 4/4 PASS) -We created `hook-scripts/relay-blocking.js` (the new relay) and `scripts/test-blocking-relay.js` (test harness). Results: +We created `hook-scripts/relay-blocking.js` (the new relay) and `docs/test-blocking-relay.js` (test harness). Results: ``` === Blocking Relay Protocol Spike Test === @@ -120,6 +119,6 @@ The pipe protocol works on Windows. Backward compatibility confirmed. ## Design Constraints - PTY-based Ink detection (`usePromptDetector`, `PromptCard`, `TrustGate`) must remain as the fallback for non-hook interactive prompts. Do not remove or break it. -- The protocol must fail-open (exit 0) on timeout or error — Claude Code must never deadlock waiting for a response that will never come. +- The protocol fails CLOSED (exit 2) on timeout, and OPEN (exit 0) on connection error — no listener means a terminal session, which gets CC's own prompt. Claude Code must never deadlock waiting for a response that will never come; the staggered timeout tiers (2026-07-30 spec §1) are what guarantee that. - `relay-blocking.js` must remain backward-compatible with the current fire-and-forget server behavior. - The spec is at `~/.claude/specs/claude-desktop-ui-spec.md` — update the "Planned updates" section when done to reflect that blocking approval flow is implemented. diff --git a/desktop/docs/test-blocking-relay.js b/desktop/docs/test-blocking-relay.js index c1eb0ac6e..10655795f 100644 --- a/desktop/docs/test-blocking-relay.js +++ b/desktop/docs/test-blocking-relay.js @@ -138,10 +138,14 @@ async function main() { }, { expectedCode: 2 }); // Test 4: Timeout — server holds socket open, never responds - // Override relay timeout to 3s so this test doesn't take 30s - await runTest('Timeout (server holds, relay fails open)', (socket, _payload) => { + // Override relay timeout to 3s so this test doesn't take 2h30m (shipped + // default). Fails CLOSED (exit 2) — the pre-2026 fail-open (exit 0) + // contract this used to pin was replaced by the 2026-07-30 staggered + // timeout tiers; a silent auto-allow on timeout is the wrong failure mode + // for a permission gate. + await runTest('Timeout (server holds, relay fails CLOSED — exit 2 deny)', (socket, _payload) => { // Deliberately do nothing — hold the socket open - }, { expectedCode: 0, timeout: 10000, env: { CLAUDE_RELAY_TIMEOUT: '3000' } }); + }, { expectedCode: 2, timeout: 10000, env: { CLAUDE_RELAY_TIMEOUT: '3000' } }); // Summary console.log('\n=== Summary ==='); diff --git a/desktop/hook-scripts/relay-blocking.js b/desktop/hook-scripts/relay-blocking.js index d848ac9d3..7be76d033 100644 --- a/desktop/hook-scripts/relay-blocking.js +++ b/desktop/hook-scripts/relay-blocking.js @@ -16,10 +16,16 @@ const net = require('net'); const os = require('os'); const path = require('path'); const PIPE_NAME = process.env.CLAUDE_DESKTOP_PIPE || (process.platform === 'win32' ? '\\\\.\\pipe\\claude-desktop-hooks' : path.join(os.tmpdir(), 'claude-desktop-hooks.sock')); -// Default 300s to match the Claude Code hook timeout in settings.json. -// If the relay times out before Claude Code's hook timeout, it exits with -// code 2 (deny), causing an auto-deny before the user can respond. -const TIMEOUT_MS = parseInt(process.env.CLAUDE_RELAY_TIMEOUT || '300000', 10); +// Tier-2 backstop: 2h30m — deliberately 30m LONGER than the app's own 2h hold +// (hook-relay.ts APP_HOLD_MS: the app must answer first, it's the only party +// that can label the card accurately) and 30m SHORTER than the Claude Code +// hook timeout in settings.json (install-hooks.js, 10800s): if the app hangs, +// this relay must fire before CC does. Relay timeout = exit 2 = clean deny +// that unblocks the turn; a CC hook-kill delivers NO decision, and +// AskUserQuestion then waits forever on CC's default-"never" question +// timeout. Do NOT tidy these back to equal — that restores the silent +// 5-minute wedge this replaced (2026-07-30 spec §1). +const TIMEOUT_MS = parseInt(process.env.CLAUDE_RELAY_TIMEOUT || '9000000', 10); let input = ''; process.stdin.setEncoding('utf8'); diff --git a/desktop/scripts/install-hooks.js b/desktop/scripts/install-hooks.js index 40ff91aee..7217deac3 100644 --- a/desktop/scripts/install-hooks.js +++ b/desktop/scripts/install-hooks.js @@ -111,7 +111,9 @@ function installHooks() { } } - // Register PermissionRequest with blocking relay (longer timeout for user response) + // Register PermissionRequest with blocking relay. Tier-3 backstop: 3h — + // 30m ABOVE the relay's 2h30m so CC never wins (CC winning kills the hook + // with no decision; see relay-blocking.js header). Margins are load-bearing. if (!settings.hooks['PermissionRequest']) { settings.hooks['PermissionRequest'] = []; } @@ -123,7 +125,7 @@ function installHooks() { const blockingEntry = { matcher: '', - hooks: [{ type: 'command', command: expectedBlockingCmd, timeout: 300 }], + hooks: [{ type: 'command', command: expectedBlockingCmd, timeout: 10800 }], }; const existingBlockingIdx = settings.hooks['PermissionRequest'].findIndex((matcher) => diff --git a/desktop/tests/permission-timeout-margins.test.ts b/desktop/tests/permission-timeout-margins.test.ts new file mode 100644 index 000000000..ac68b66de --- /dev/null +++ b/desktop/tests/permission-timeout-margins.test.ts @@ -0,0 +1,56 @@ +import { describe, it, expect } from 'vitest'; +import fs from 'fs'; +import path from 'path'; + +// Pins the §1 tier margins by reading the LITERALS, not process.env-resolved +// values (the env override would make an env-based test pass vacuously — +// spec §Constraints). All six sites live in this one repo. +// +// Structure note (for tasks 7/8, which EXTEND this file): the file-reading +// helpers below (repoRoot, read, literal) are module-scope and generic — +// Task 7 adds an APP_HOLD_MS assertion reading desktop/src/main/hook-relay.ts, +// Task 8 adds a PERMISSION_HOLD_MS assertion reading EventBridge.kt. Both can +// reuse `literal()` as-is. +const repoRoot = path.resolve(__dirname, '..', '..'); +const read = (p: string) => fs.readFileSync(path.join(repoRoot, p), 'utf8'); + +function literal(file: string, re: RegExp): number { + const m = read(file).match(re); + if (!m) throw new Error(`pattern ${re} not found in ${file}`); + return parseInt(m[1].replace(/_/g, ''), 10); +} + +const RELAY_RE = /CLAUDE_RELAY_TIMEOUT \|\| '(\d+)'/; + +describe('permission timeout tier margins (2026-07-30 spec §1)', () => { + const desktopRelay = () => literal('desktop/hook-scripts/relay-blocking.js', RELAY_RE); + const androidRelay = () => literal('app/src/main/assets/hook-relay-blocking.js', RELAY_RE); + const desktopCcSeconds = () => + literal('desktop/scripts/install-hooks.js', /command: expectedBlockingCmd, timeout: (\d+)/); + const androidCcSeconds = () => literal( + 'app/src/main/kotlin/com/youcoded/app/runtime/Bootstrap.kt', + /PERMISSION_HOOK_TIMEOUT_SECONDS = ([\d_]+)/); + + it('relay backstop is 2h30m on both platforms', () => { + expect(desktopRelay()).toBe(9000000); + expect(androidRelay()).toBe(9000000); + }); + + it('CC hook entry is 3h on both platforms', () => { + expect(desktopCcSeconds()).toBe(10800); + expect(androidCcSeconds()).toBe(10800); + }); + + it('relay fires strictly BEFORE CC, with a real margin', () => { + // CC winning is the bad outcome: hook killed with no decision → + // AskUserQuestion waits forever on its default-"never" question timeout. + expect(desktopRelay()).toBeLessThanOrEqual(desktopCcSeconds() * 1000 - 15 * 60 * 1000); + expect(androidRelay()).toBeLessThanOrEqual(androidCcSeconds() * 1000 - 15 * 60 * 1000); + }); + + it('every value is under the 32-bit setTimeout ceiling', () => { + for (const v of [desktopRelay(), androidRelay(), desktopCcSeconds() * 1000]) { + expect(v).toBeLessThan(2147483647); // overflow fires IMMEDIATELY — the bug, disguised + } + }); +}); From 6aa2928953d242a6f650990c6019afc1abdde9a4 Mon Sep 17 00:00:00 2001 From: Destin Date: Fri, 31 Jul 2026 12:27:06 -0700 Subject: [PATCH 03/20] fix(permission-timeout): correct stale fail-open transcript line, add missing overflow check Two code-review findings on Task 2 (12e46b78): - blocking-relay-handoff.md's validated spike-test transcript still showed the OLD fail-open timeout result, contradicting the corrected design bullet above it and the relabeled test-blocking-relay.js Test 4. Now reads fail-closed, exit=2. - permission-timeout-margins.test.ts's 32-bit setTimeout ceiling check omitted androidCcSeconds() * 1000, leaving the Android CC-hook tier's overflow case unverified. No timeout values changed. --- desktop/docs/blocking-relay-handoff.md | 2 +- desktop/tests/permission-timeout-margins.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/desktop/docs/blocking-relay-handoff.md b/desktop/docs/blocking-relay-handoff.md index 73a09bac4..259ba9f33 100644 --- a/desktop/docs/blocking-relay-handoff.md +++ b/desktop/docs/blocking-relay-handoff.md @@ -53,7 +53,7 @@ We created `hook-scripts/relay-blocking.js` (the new relay) and `docs/test-block [TEST] Fire-and-forget (server closes immediately)... PASS (exit=0, expected=0, 413ms) [TEST] Blocking allow (server sends allow=true)... PASS (exit=0, expected=0, 853ms) [TEST] Blocking deny (server sends allow=false)... PASS (exit=2, expected=2, 853ms) -[TEST] Timeout (server holds, relay fails open)... PASS (exit=0, expected=0, 3365ms) +[TEST] Timeout (server holds, relay fails closed)... PASS (exit=2, expected=2, 3365ms) ``` The pipe protocol works on Windows. Backward compatibility confirmed. diff --git a/desktop/tests/permission-timeout-margins.test.ts b/desktop/tests/permission-timeout-margins.test.ts index ac68b66de..29d1eb5f4 100644 --- a/desktop/tests/permission-timeout-margins.test.ts +++ b/desktop/tests/permission-timeout-margins.test.ts @@ -49,7 +49,7 @@ describe('permission timeout tier margins (2026-07-30 spec §1)', () => { }); it('every value is under the 32-bit setTimeout ceiling', () => { - for (const v of [desktopRelay(), androidRelay(), desktopCcSeconds() * 1000]) { + for (const v of [desktopRelay(), androidRelay(), desktopCcSeconds() * 1000, androidCcSeconds() * 1000]) { expect(v).toBeLessThan(2147483647); // overflow fires IMMEDIATELY — the bug, disguised } }); From f9fbdb574f6ec1dd2c31bb622b92095e6776ccab Mon Sep 17 00:00:00 2001 From: Destin Date: Fri, 31 Jul 2026 12:36:12 -0700 Subject: [PATCH 04/20] fix(permission-timeout): correct deny-exit-code fiction in harness + handoff doc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit test-blocking-relay.js asserted a protocol that never shipped: sending {"allow": false} and expecting exit 2. relay-blocking.js has no such path — it doesn't read .allow, and only ever exits 2 on relay timeout. A delivered decision (allow or deny) always exits 0, with the decision riding in stdout's hookSpecificOutput. Fixed the test to send a real deny decision ({"decision":{"behavior":"deny"}}) and assert exit 0, and relabeled it to say what it verifies. blocking-relay-handoff.md's "Spike Test Results" block still quoted the old fabricated 4/4 run (deny at exit=2), contradicting the "there is no deny-specific exit code" line a previous fix pass had already corrected 13 lines above it. Replaced with a real re-run of the fixed harness (measured today, not carried forward). Co-Authored-By: Claude Opus 5 --- desktop/docs/blocking-relay-handoff.md | 14 +++++++------- desktop/docs/test-blocking-relay.js | 19 ++++++++++++------- 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/desktop/docs/blocking-relay-handoff.md b/desktop/docs/blocking-relay-handoff.md index 259ba9f33..183ce3543 100644 --- a/desktop/docs/blocking-relay-handoff.md +++ b/desktop/docs/blocking-relay-handoff.md @@ -44,19 +44,19 @@ The relay writes its payload and **waits**. The server decides what happens: Key property: **relay doesn't need to know which hooks are blocking.** The server decides. This means adding new blocking hook types only requires server-side changes. -### Spike Test Results (VALIDATED, 4/4 PASS) +### Spike Test Results (re-run 2026-07-31, 4/4 PASS) -We created `hook-scripts/relay-blocking.js` (the new relay) and `docs/test-blocking-relay.js` (test harness). Results: +We created `hook-scripts/relay-blocking.js` (the new relay) and `docs/test-blocking-relay.js` (test harness). The harness previously asserted a deny-specific exit code (`exit 2`) that never matched shipped behavior — the relay has no such path; a delivered deny exits 0 like any other delivered decision, with the decision riding in stdout's `hookSpecificOutput`. Test 3 and this results block were corrected to match `relay-blocking.js` as written, then re-run for real numbers (`node desktop/docs/test-blocking-relay.js` from the repo root; Linux, not Windows — see note below): ``` === Blocking Relay Protocol Spike Test === -[TEST] Fire-and-forget (server closes immediately)... PASS (exit=0, expected=0, 413ms) -[TEST] Blocking allow (server sends allow=true)... PASS (exit=0, expected=0, 853ms) -[TEST] Blocking deny (server sends allow=false)... PASS (exit=2, expected=2, 853ms) -[TEST] Timeout (server holds, relay fails closed)... PASS (exit=2, expected=2, 3365ms) +[TEST] Fire-and-forget (server closes immediately)... PASS (exit=0, expected=0, 331ms) +[TEST] Blocking allow (server sends allow=true)... PASS (exit=0, expected=0, 835ms) +[TEST] Blocking deny (server sends deny decision, exits 0 — decision rides in stdout)... PASS (exit=0, expected=0, 838ms) +[TEST] Timeout (server holds, relay fails CLOSED — exit 2 deny)... PASS (exit=2, expected=2, 3337ms) ``` -The pipe protocol works on Windows. Backward compatibility confirmed. +The pipe protocol works. Backward compatibility confirmed. (This re-run was executed on Linux, where `net.createServer().listen(PIPE_NAME)` binds the Windows-style pipe string as a plain Unix socket file — the harness doesn't branch on platform. Timings are illustrative for this machine/run, not a perf contract.) ## Implementation Plan diff --git a/desktop/docs/test-blocking-relay.js b/desktop/docs/test-blocking-relay.js index 10655795f..96281908c 100644 --- a/desktop/docs/test-blocking-relay.js +++ b/desktop/docs/test-blocking-relay.js @@ -4,9 +4,11 @@ * * Runs four scenarios against relay-blocking.js: * 1. Fire-and-forget: server closes immediately → relay exits 0 - * 2. Blocking allow: server sends {"allow":true} → relay exits 0 - * 3. Blocking deny: server sends {"allow":false} → relay exits 2 - * 4. Timeout: server holds forever → relay exits 0 after timeout + * 2. Blocking allow: server sends {"decision":{"behavior":"allow"}} → relay exits 0 + * 3. Blocking deny: server sends {"decision":{"behavior":"deny"}} → relay exits 0 + * (the decision rides in stdout's hookSpecificOutput, not the exit code — + * there is no deny-specific exit code; see relay-blocking.js) + * 4. Timeout: server holds forever → relay exits 2 (fail-closed) after timeout * * Usage: node scripts/test-blocking-relay.js */ @@ -130,12 +132,15 @@ async function main() { }, 500); }, { expectedCode: 0 }); - // Test 3: Blocking deny — server sends deny response - await runTest('Blocking deny (server sends allow=false)', (socket, payload) => { + // Test 3: Blocking deny — server sends a deny decision. The relay does not + // special-case deny: it wraps whatever `decision` the server sent into + // hookSpecificOutput on stdout and exits 0, same as allow. Exit 2 is + // reserved for the timeout path (test 4), never for a delivered decision. + await runTest('Blocking deny (server sends deny decision, exits 0 — decision rides in stdout)', (socket, payload) => { setTimeout(() => { - socket.end(JSON.stringify({ allow: false }) + '\n'); + socket.end(JSON.stringify({ decision: { behavior: 'deny' } }) + '\n'); }, 500); - }, { expectedCode: 2 }); + }, { expectedCode: 0 }); // Test 4: Timeout — server holds socket open, never responds // Override relay timeout to 3s so this test doesn't take 2h30m (shipped From 26935d88dfff9ebdf5c00bf17069e215012fac44 Mon Sep 17 00:00:00 2001 From: Destin Date: Fri, 31 Jul 2026 12:47:06 -0700 Subject: [PATCH 05/20] fix(permission-timeout): correct flat decision shape in harness + handoff doc Test 2 of test-blocking-relay.js still sent the legacy flat {allow:true} response while its docstring claimed the nested {decision:{behavior:allow}} shape - fixed the handler and relabeled the test to match. The handoff doc's "Blocking decision" bullet documented flat-string decisions ({"decision":"allow"}), which is wrong: main.ts's hookRelay.respond() calls and relay-blocking.js both use the nested shape, and a flat or bare-string decision would ship decision:undefined to Claude Code. Re-ran the harness for real numbers since Test 2's label changed; timings are noise-level unchanged from the prior run. Co-Authored-By: Claude Opus 5 --- desktop/docs/blocking-relay-handoff.md | 12 ++++++------ desktop/docs/test-blocking-relay.js | 10 +++++++--- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/desktop/docs/blocking-relay-handoff.md b/desktop/docs/blocking-relay-handoff.md index 183ce3543..f5e4d10e7 100644 --- a/desktop/docs/blocking-relay-handoff.md +++ b/desktop/docs/blocking-relay-handoff.md @@ -39,21 +39,21 @@ Keep PTY detection as the catch-all (for trust gates, non-hook menus), but add s The relay writes its payload and **waits**. The server decides what happens: - **Fire-and-forget:** Server closes socket without writing → relay sees `end` → exits 0 (backward compatible) -- **Blocking decision:** Server holds socket, writes a decision (`{"decision":"allow"}` or `{"decision":"deny"}`) → relay wraps it in `hookSpecificOutput` and exits 0 either way; Claude Code reads the `decision` field, not the exit code. Exit 2 is the timeout path only (see below) — there is no deny-specific exit code. +- **Blocking decision:** Server holds socket, writes a decision in the real nested shape — `{"decision":{"behavior":"allow"}}` or `{"decision":{"behavior":"deny"}}` (see `main.ts`'s `hookRelay.respond()` calls and `relay-blocking.js`, which reads `appDecision.decision` and re-wraps it as `hookSpecificOutput.decision`) → relay wraps it in `hookSpecificOutput` and exits 0 either way; Claude Code reads the `decision` field, not the exit code. The nesting matters: a flat `{"decision":"allow"}` or bare-string decision reaches Claude Code as `decision: undefined`, silently dropping the answer. Exit 2 is the timeout path only (see below) — there is no deny-specific exit code. - **Timeout safety:** Relay has a configurable timeout (default 2h30m, via `CLAUDE_RELAY_TIMEOUT` env var). If the server goes silent past it → relay exits 2 (fail-closed deny) Key property: **relay doesn't need to know which hooks are blocking.** The server decides. This means adding new blocking hook types only requires server-side changes. ### Spike Test Results (re-run 2026-07-31, 4/4 PASS) -We created `hook-scripts/relay-blocking.js` (the new relay) and `docs/test-blocking-relay.js` (test harness). The harness previously asserted a deny-specific exit code (`exit 2`) that never matched shipped behavior — the relay has no such path; a delivered deny exits 0 like any other delivered decision, with the decision riding in stdout's `hookSpecificOutput`. Test 3 and this results block were corrected to match `relay-blocking.js` as written, then re-run for real numbers (`node desktop/docs/test-blocking-relay.js` from the repo root; Linux, not Windows — see note below): +We created `hook-scripts/relay-blocking.js` (the new relay) and `docs/test-blocking-relay.js` (test harness). The harness previously asserted a deny-specific exit code (`exit 2`) that never matched shipped behavior — the relay has no such path; a delivered deny exits 0 like any other delivered decision, with the decision riding in stdout's `hookSpecificOutput`. Test 2's handler also previously sent the legacy flat `{"allow":true}` shape instead of the real nested `{"decision":{"behavior":"allow"}}` shape. Both were corrected to match `relay-blocking.js` as written, then re-run for real numbers (`node desktop/docs/test-blocking-relay.js` from the repo root; Linux, not Windows — see note below): ``` === Blocking Relay Protocol Spike Test === -[TEST] Fire-and-forget (server closes immediately)... PASS (exit=0, expected=0, 331ms) -[TEST] Blocking allow (server sends allow=true)... PASS (exit=0, expected=0, 835ms) -[TEST] Blocking deny (server sends deny decision, exits 0 — decision rides in stdout)... PASS (exit=0, expected=0, 838ms) -[TEST] Timeout (server holds, relay fails CLOSED — exit 2 deny)... PASS (exit=2, expected=2, 3337ms) +[TEST] Fire-and-forget (server closes immediately)... PASS (exit=0, expected=0, 333ms) +[TEST] Blocking allow (server sends allow decision)... PASS (exit=0, expected=0, 839ms) +[TEST] Blocking deny (server sends deny decision, exits 0 — decision rides in stdout)... PASS (exit=0, expected=0, 831ms) +[TEST] Timeout (server holds, relay fails CLOSED — exit 2 deny)... PASS (exit=2, expected=2, 3348ms) ``` The pipe protocol works. Backward compatibility confirmed. (This re-run was executed on Linux, where `net.createServer().listen(PIPE_NAME)` binds the Windows-style pipe string as a plain Unix socket file — the harness doesn't branch on platform. Timings are illustrative for this machine/run, not a perf contract.) diff --git a/desktop/docs/test-blocking-relay.js b/desktop/docs/test-blocking-relay.js index 96281908c..c75778b17 100644 --- a/desktop/docs/test-blocking-relay.js +++ b/desktop/docs/test-blocking-relay.js @@ -122,13 +122,17 @@ async function main() { socket.end(); }, { expectedCode: 0 }); - // Test 2: Blocking allow — server sends allow response - await runTest('Blocking allow (server sends allow=true)', (socket, payload) => { + // Test 2: Blocking allow — server sends an allow decision in the real + // nested wire shape ({ decision: { behavior: 'allow' } }). A flat or + // bare-string decision would ship `decision: undefined` to Claude Code — + // see relay-blocking.js, which reads `appDecision.decision` and re-wraps + // it as `hookSpecificOutput.decision`. + await runTest('Blocking allow (server sends allow decision)', (socket, payload) => { // Simulate a brief "thinking" delay, then approve const parsed = JSON.parse(payload); log(` Received: ${parsed.hook_event_name} tool=${parsed.tool_name}`); setTimeout(() => { - socket.end(JSON.stringify({ allow: true }) + '\n'); + socket.end(JSON.stringify({ decision: { behavior: 'allow' } }) + '\n'); }, 500); }, { expectedCode: 0 }); From 2b2dd46adf3d860f8baccfaee1ca329f7a96f7f5 Mon Sep 17 00:00:00 2001 From: Destin Date: Fri, 31 Jul 2026 12:48:02 -0700 Subject: [PATCH 06/20] docs(relay): mark the spike's implementation plan as historical MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Steps 2/3/5 sketch an allow:boolean API and PreToolUse blocking; what shipped is PermissionRequest with a nested decision object. Annotate rather than rewrite — it is a record of the spike, not current API docs. Co-Authored-By: Claude Fable 5 --- desktop/docs/blocking-relay-handoff.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/desktop/docs/blocking-relay-handoff.md b/desktop/docs/blocking-relay-handoff.md index f5e4d10e7..eac29c4ff 100644 --- a/desktop/docs/blocking-relay-handoff.md +++ b/desktop/docs/blocking-relay-handoff.md @@ -60,6 +60,15 @@ The pipe protocol works. Backward compatibility confirmed. (This re-run was exec ## Implementation Plan +> **Historical — this plan shipped, with two deliberate divergences.** Read it as +> the original spike's proposal, not as current API docs. What actually shipped: +> (1) the blocking hook is **`PermissionRequest`**, not `PreToolUse` (Step 5 below +> guessed this might become possible — it did); (2) `respond()` takes the nested +> **decision object** `{ decision: { behavior: 'allow' | 'deny' } }`, not the +> `allow: boolean` / `{"allow": …}` shape sketched in Steps 2, 3 and 5 — see the +> corrected protocol bullet near the top of this file. For current behavior, +> trust `hook-relay.ts` and `relay-blocking.js`, not this section. + ### Step 1: Replace relay.js with relay-blocking.js - `relay-blocking.js` is a drop-in replacement — when server closes without writing back, behavior is identical to current `relay.js` - Rename or replace; update any references in `scripts/install-hooks.js` or hook config From 5273c9111fe24e996c2c053030b8c2ef108ae307 Mon Sep 17 00:00:00 2001 From: Destin Date: Fri, 31 Jul 2026 12:52:56 -0700 Subject: [PATCH 07/20] =?UTF-8?q?feat(chat):=20PERMISSION=5FEXPIRED=20reas?= =?UTF-8?q?ons=20=E2=80=94=20retain=20on=20hook-closed,=20quiet=20PERMISSI?= =?UTF-8?q?ON=5FCARD=5FRESOLVED?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An expired permission ask used to always flip to 'failed', which turned the session's attention dot green even when Claude Code's terminal menu was still live and blocked waiting for input. Add ToolCallState.expired and a `reason` on PERMISSION_EXPIRED: only 'hook-closed' (far end died, menu may still be on screen) retains the card as awaiting-approval + expired so the red dot and pty-input gates keep holding; every other reason (or an absent one, covering the native broker and older remote shims) resolves as before. New PERMISSION_CARD_RESOLVED action quietly completes an expired card once the menu is confirmed gone or the user dismisses it — it only ever touches cards already marked expired, so a live ask still has to go through its buttons. Task 3 of the permission-ask-timeout plan. --- .../state/__tests__/chat-reducer.test.ts | 86 +++++++++++++++++++ desktop/src/renderer/state/chat-reducer.ts | 54 ++++++++++-- desktop/src/renderer/state/chat-types.ts | 14 +++ desktop/src/shared/types.ts | 7 ++ 4 files changed, 155 insertions(+), 6 deletions(-) diff --git a/desktop/src/renderer/state/__tests__/chat-reducer.test.ts b/desktop/src/renderer/state/__tests__/chat-reducer.test.ts index f18e66a29..43510ce15 100644 --- a/desktop/src/renderer/state/__tests__/chat-reducer.test.ts +++ b/desktop/src/renderer/state/__tests__/chat-reducer.test.ts @@ -3,6 +3,7 @@ import { chatReducer } from '../chat-reducer'; import { createSessionChatState, serializeChatState } from '../chat-types'; import type { ChatState } from '../chat-types'; import type { ToolCallState } from '../../../shared/types'; +import { hasPendingInteraction, canRetrySubmit } from '../pty-input-gate'; function stateWithInFlightTurn(sessionId = 'sess-1', turnId = 'turn-1'): ChatState { const session = createSessionChatState(); @@ -652,3 +653,88 @@ describe('chatReducer COMPACTION_COMPLETE — native auto-compaction (I2b)', () expect(next).toBe(state); // untouched — no spurious marker }); }); + +describe('PERMISSION_EXPIRED reasons (2026-07-30 spec §2/§2a/§2c/§2d)', () => { + // Fresh two-session-map state, mirroring initState() in the tool-card- + // duplication block above. A named helper (rather than inlining) makes the + // "fresh state, no pending ask" case in the last test read clearly. + function emptySession(sessionId = 's1'): ChatState { + return new Map([[sessionId, createSessionChatState()]]); + } + + // Arrange: PERMISSION_REQUEST with no matching running tool takes the + // synthetic-card path in chat-reducer.ts (no TRANSCRIPT_TOOL_USE has landed + // yet), minting toolCalls.get('perm-r1') with status 'awaiting-approval' + // and requestId 'r1' — the exact shape a live permission ask has in prod + // right before it expires. + function withPendingAsk(sessionId = 's1'): ChatState { + return chatReducer(emptySession(sessionId), { + type: 'PERMISSION_REQUEST', + sessionId, + toolName: 'Bash', + input: {}, + requestId: 'r1', + } as any); + } + + function expire( + state: ChatState, + opts: { reason?: 'app-timeout' | 'unroutable' | 'delivery-failed' | 'hook-closed' }, + sessionId = 's1', + ): ChatState { + return chatReducer(state, { + type: 'PERMISSION_EXPIRED', + sessionId, + requestId: 'r1', + ...opts, + } as any); + } + + it("'hook-closed' retains: awaiting-approval + expired, requestId cleared, no error", () => { + const state = expire(withPendingAsk(), { reason: 'hook-closed' }); + const tool = state.get('s1')!.toolCalls.get('perm-r1')!; + expect(tool.status).toBe('awaiting-approval'); // red dot + input gates keep holding + expect(tool.expired).toBe(true); + expect(tool.requestId).toBeUndefined(); + expect(tool.error).toBeUndefined(); + }); + + it("'hook-closed' still counts as pending for both pty gates", () => { + const state = expire(withPendingAsk(), { reason: 'hook-closed' }); + const session = state.get('s1')!; + expect(hasPendingInteraction(session)).toBe(true); + expect(canRetrySubmit(session)).toBe(false); + }); + + it("'app-timeout' resolves as failed with accurate copy, never retains", () => { + const state = expire(withPendingAsk(), { reason: 'app-timeout' }); + const tool = state.get('s1')!.toolCalls.get('perm-r1')!; + expect(tool.status).toBe('failed'); + expect(tool.expired).toBeUndefined(); + expect(tool.error).toContain('auto-denied'); + }); + + it('absent reason resolves — the native-broker / old-shim default', () => { + const state = expire(withPendingAsk(), {}); + expect(state.get('s1')!.toolCalls.get('perm-r1')!.status).toBe('failed'); + }); + + it("'delivery-failed' resolves", () => { + const state = expire(withPendingAsk(), { reason: 'delivery-failed' }); + expect(state.get('s1')!.toolCalls.get('perm-r1')!.status).toBe('failed'); + }); + + it('PERMISSION_CARD_RESOLVED quietly completes an expired card only', () => { + let state = expire(withPendingAsk(), { reason: 'hook-closed' }); + state = chatReducer(state, { type: 'PERMISSION_CARD_RESOLVED', sessionId: 's1', toolUseId: 'perm-r1' }); + const tool = state.get('s1')!.toolCalls.get('perm-r1')!; + expect(tool.status).toBe('complete'); + expect(tool.error).toBeUndefined(); + expect(tool.expired).toBeUndefined(); + // a NON-expired awaiting card must be untouched (only the §2 resolver and + // Dismiss use this action, and both only ever see expired cards) + const fresh = withPendingAsk(); + const untouched = chatReducer(fresh, { type: 'PERMISSION_CARD_RESOLVED', sessionId: 's1', toolUseId: 'perm-r1' }); + expect(untouched.get('s1')!.toolCalls.get('perm-r1')!.status).toBe('awaiting-approval'); + }); +}); diff --git a/desktop/src/renderer/state/chat-reducer.ts b/desktop/src/renderer/state/chat-reducer.ts index 3d09d3897..0a31227a4 100644 --- a/desktop/src/renderer/state/chat-reducer.ts +++ b/desktop/src/renderer/state/chat-reducer.ts @@ -1238,12 +1238,37 @@ export function chatReducer(state: ChatState, action: ChatAction): ChatState { const toolCalls = new Map(session.toolCalls); for (const [id, tool] of toolCalls) { if (tool.status === 'awaiting-approval' && tool.requestId === action.requestId) { - toolCalls.set(id, { - ...tool, - status: 'failed', - requestId: undefined, - error: 'Permission request expired — socket closed before a response was sent', - }); + if (action.reason === 'hook-closed') { + // Far end died (relay timeout/death, CC killed the hook) but the + // Ink menu may STILL be on screen. Retain provisionally: keep + // awaiting-approval so useSessionAttention stays red and the + // pty-input gates keep blocking sends into the live menu. + // usePromptDetector's menu-absence rule or the Dismiss button + // resolves it (spec §2/§2a). The reducer never reads the buffer. + toolCalls.set(id, { ...tool, requestId: undefined, expired: true }); + } else if (action.reason === 'app-timeout' || action.reason === 'unroutable') { + // The app itself delivered a deny — accurate copy, no retention. + toolCalls.set(id, { + ...tool, + status: 'failed', + requestId: undefined, + error: action.reason === 'unroutable' + ? 'No open session could show this request — YouCoded auto-denied it' + : 'No response in time — YouCoded auto-denied this request so Claude could continue', + }); + } else { + // 'delivery-failed' or absent (native PermissionBroker cancel, + // renderer delivery-failure recovery, older remote shim). DEFAULT + // IS RESOLVE, never retain: native sessions have no PTY (nothing + // to rebind), and delivery-failure means the socket is provably + // gone. Do not flip this default (spec §2c/§2d). + toolCalls.set(id, { + ...tool, + status: 'failed', + requestId: undefined, + error: 'Permission request expired — socket closed before a response was sent', + }); + } break; } } @@ -1252,6 +1277,23 @@ export function chatReducer(state: ChatState, action: ChatAction): ChatState { return next; } + case 'PERMISSION_CARD_RESOLVED': { + const session = next.get(action.sessionId); + if (!session) return state; + const tool = session.toolCalls.get(action.toolUseId); + // Only expired cards resolve this way — a live ask must go through its + // buttons (which deliver a real decision through the socket). + if (!tool || !tool.expired) return state; + const toolCalls = new Map(session.toolCalls); + // 'complete' with no error: nothing failed — the ask was answered in + // the terminal or dismissed. If the tool really runs, the transcript's + // TOOL_RESULT overwrites this with the true outcome. + const { expired: _resolved, ...rest } = tool; + toolCalls.set(action.toolUseId, { ...rest, status: 'complete' }); + next.set(action.sessionId, { ...session, toolCalls }); + return next; + } + case 'HISTORY_LOADED': { const session = next.get(action.sessionId); if (!session) return state; diff --git a/desktop/src/renderer/state/chat-types.ts b/desktop/src/renderer/state/chat-types.ts index db4424b0d..bcec28407 100644 --- a/desktop/src/renderer/state/chat-types.ts +++ b/desktop/src/renderer/state/chat-types.ts @@ -460,6 +460,20 @@ export type ChatAction = type: 'PERMISSION_EXPIRED'; sessionId: string; requestId: string; + /** Why the ask ended. ONLY 'hook-closed' (far end died, menu may still + * be live) retains the card. Absent = resolve: retention is the + * riskier behavior, and the native PermissionBroker + older remote + * shims never send a reason — defaulting to retain would wedge them + * (spec §2c/§2d). Optional so older serialized actions deserialize. */ + reason?: 'app-timeout' | 'unroutable' | 'delivery-failed' | 'hook-closed'; + } + | { + /** Quiet local resolve of an EXPIRED card: the user answered in the + * terminal (menu left the buffer) or clicked Dismiss. No error text — + * nothing failed. */ + type: 'PERMISSION_CARD_RESOLVED'; + sessionId: string; + toolUseId: string; } | { type: 'PERMISSION_RESPONDED'; diff --git a/desktop/src/shared/types.ts b/desktop/src/shared/types.ts index 7c0af59ac..5d94ef504 100644 --- a/desktop/src/shared/types.ts +++ b/desktop/src/shared/types.ts @@ -288,6 +288,13 @@ export interface ToolCallState { /** Native broker only: winning rule came from the destructive deny-list → * the "Always allow" button shows a consequence-gated confirm. Task 13. */ denyListed?: boolean; + /** Permission ask whose hook socket died with the terminal menu possibly + * still live ('hook-closed' expiry). The card STAYS awaiting-approval so + * the red strip dot and pty-input gates keep holding (the 2026-07-30 bug + * was this card flipping 'failed' — session looked fine while CC stayed + * blocked). requestId is cleared: the socket is gone, respond() can never + * work. Resolved by usePromptDetector's menu-absence rule or Dismiss. */ + expired?: true; response?: string; error?: string; /** Set when the tool result carries a structuredPatch (Edit/MultiEdit). */ From 18616c482e77fb99602dcefddefa76072b2d3fac Mon Sep 17 00:00:00 2001 From: Destin Date: Fri, 31 Jul 2026 13:06:37 -0700 Subject: [PATCH 08/20] fix(chat): guard PERMISSION_CARD_RESOLVED against endTurn's stale expired marker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit endTurn() force-fails any still-awaiting-approval tool but never cleared `expired`, so a retained hook-closed card whose session then died became {status: 'failed', expired: true}. A later quiet PERMISSION_CARD_RESOLVED (Dismiss button or stale-detector callback) would pass the expired-only guard and silently flip a real failure to 'complete' with no error text — the exact "session looks fine but isn't" bug this branch exists to fix. Fix both sides: endTurn() clears `expired` when it force-fails a tool, and PERMISSION_CARD_RESOLVED's guard now also requires status === 'awaiting-approval'. Adds a regression test that ends a retained hook-closed card's turn via a real SESSION_PROCESS_EXITED dispatch and confirms a follow-up PERMISSION_CARD_RESOLVED cannot resurrect it. Co-Authored-By: Claude Opus 5 --- .../state/__tests__/chat-reducer.test.ts | 33 +++++++++++++++++++ desktop/src/renderer/state/chat-reducer.ts | 23 ++++++++++--- 2 files changed, 52 insertions(+), 4 deletions(-) diff --git a/desktop/src/renderer/state/__tests__/chat-reducer.test.ts b/desktop/src/renderer/state/__tests__/chat-reducer.test.ts index 43510ce15..37749d44a 100644 --- a/desktop/src/renderer/state/__tests__/chat-reducer.test.ts +++ b/desktop/src/renderer/state/__tests__/chat-reducer.test.ts @@ -737,4 +737,37 @@ describe('PERMISSION_EXPIRED reasons (2026-07-30 spec §2/§2a/§2c/§2d)', () = const untouched = chatReducer(fresh, { type: 'PERMISSION_CARD_RESOLVED', sessionId: 's1', toolUseId: 'perm-r1' }); expect(untouched.get('s1')!.toolCalls.get('perm-r1')!.status).toBe('awaiting-approval'); }); + + it('endTurn force-failing a retained hook-closed card clears `expired`, and a later PERMISSION_CARD_RESOLVED cannot erase the failure', () => { + // Retain via 'hook-closed': awaiting-approval + expired: true. + let state = expire(withPendingAsk(), { reason: 'hook-closed' }); + + // The session then actually dies (endTurn is spread by + // SESSION_PROCESS_EXITED). This force-fails the still-awaiting tool. + state = chatReducer(state, { + type: 'SESSION_PROCESS_EXITED', + sessionId: 's1', + exitCode: 1, + } as any); + + const failed = state.get('s1')!.toolCalls.get('perm-r1')!; + expect(failed.status).toBe('failed'); + expect(failed.error).toBe('Turn ended'); + // Regression guard: endTurn() must clear the stale `expired` marker once + // it force-fails the card, or a later quiet PERMISSION_CARD_RESOLVED can + // still pass the (buggy) `!tool.expired`-only guard and erase this + // failure with no error text. + expect(failed.expired).toBeUndefined(); + + // A stray PERMISSION_CARD_RESOLVED (Dismiss button, stale-detector + // callback) must NOT resurrect the failed card as a quiet 'complete'. + state = chatReducer(state, { + type: 'PERMISSION_CARD_RESOLVED', + sessionId: 's1', + toolUseId: 'perm-r1', + }); + const stillFailed = state.get('s1')!.toolCalls.get('perm-r1')!; + expect(stillFailed.status).toBe('failed'); + expect(stillFailed.error).toBe('Turn ended'); + }); }); diff --git a/desktop/src/renderer/state/chat-reducer.ts b/desktop/src/renderer/state/chat-reducer.ts index 0a31227a4..534b0d753 100644 --- a/desktop/src/renderer/state/chat-reducer.ts +++ b/desktop/src/renderer/state/chat-reducer.ts @@ -176,7 +176,15 @@ function endTurn( for (const id of session.activeTurnToolIds) { const tool = toolCalls.get(id); if (tool && (tool.status === 'running' || tool.status === 'awaiting-approval')) { - toolCalls.set(id, { ...tool, status: 'failed', error: errorMessage }); + // Fix: also clear `expired` when force-failing. A retained hook-closed + // card (PERMISSION_EXPIRED reason 'hook-closed') carries `expired: true` + // while still 'awaiting-approval'. If the session then dies here, the + // card is now genuinely settled ('failed') — leaving `expired` set is + // stale state that lets a later quiet PERMISSION_CARD_RESOLVED (Dismiss + // button, stale-detector callback) pass its `!tool.expired` guard and + // silently overwrite this real failure with 'complete' and no error. + const { expired: _expired, ...rest } = tool; + toolCalls.set(id, { ...rest, status: 'failed', error: errorMessage }); } } return { @@ -1281,9 +1289,16 @@ export function chatReducer(state: ChatState, action: ChatAction): ChatState { const session = next.get(action.sessionId); if (!session) return state; const tool = session.toolCalls.get(action.toolUseId); - // Only expired cards resolve this way — a live ask must go through its - // buttons (which deliver a real decision through the socket). - if (!tool || !tool.expired) return state; + // Only a still-awaiting expired card resolves this way — a live ask + // must go through its buttons (which deliver a real decision through + // the socket). Fix: also require status === 'awaiting-approval', not + // just `expired`. endTurn() force-fails a retained hook-closed card + // ('failed' + real error) if the session dies before it's resolved, + // but historically left `expired` set — without this status check a + // late PERMISSION_CARD_RESOLVED (Dismiss button, stale-detector + // callback) would pass on that stale marker and silently flip a real + // failure to 'complete' with no error, erasing it. + if (!tool || !tool.expired || tool.status !== 'awaiting-approval') return state; const toolCalls = new Map(session.toolCalls); // 'complete' with no error: nothing failed — the ask was answered in // the terminal or dismissed. If the tool really runs, the transcript's From 0aa8b279c53cb946fb05a937d4ab05581c3c27a8 Mon Sep 17 00:00:00 2001 From: Destin Date: Fri, 31 Jul 2026 13:16:37 -0700 Subject: [PATCH 09/20] fix(chat): preserve `expired` across the synthetic-permission-card merge MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TRANSCRIPT_TOOL_USE's synthetic-merge branch rebuilds the tool card from a hand-picked field list that omitted `expired`. A hook-closed retained card (awaiting-approval + expired: true + no requestId) merging with its real tool_use event lost `expired`, leaving an orphan that was neither answerable (no requestId) nor resolvable (PERMISSION_CARD_RESOLVED requires `expired`) — stuck holding the red attention dot and pty-input gates forever. Also adds a test pinning PERMISSION_CARD_RESOLVED's `status === 'awaiting-approval'` guard clause independently: the existing regression test didn't discriminate it, since endTurn()'s expired-stripping already blocked that path before the guard mattered. Co-Authored-By: Claude Opus 5 --- .../state/__tests__/chat-reducer.test.ts | 70 +++++++++++++++++++ desktop/src/renderer/state/chat-reducer.ts | 12 +++- 2 files changed, 81 insertions(+), 1 deletion(-) diff --git a/desktop/src/renderer/state/__tests__/chat-reducer.test.ts b/desktop/src/renderer/state/__tests__/chat-reducer.test.ts index 37749d44a..be97b1554 100644 --- a/desktop/src/renderer/state/__tests__/chat-reducer.test.ts +++ b/desktop/src/renderer/state/__tests__/chat-reducer.test.ts @@ -770,4 +770,74 @@ describe('PERMISSION_EXPIRED reasons (2026-07-30 spec §2/§2a/§2c/§2d)', () = expect(stillFailed.status).toBe('failed'); expect(stillFailed.error).toBe('Turn ended'); }); + + it('TRANSCRIPT_TOOL_USE merging a retained hook-closed synthetic card preserves `expired`', () => { + // Retain via 'hook-closed': synthetic perm-r1 stays awaiting-approval, + // expired: true, requestId cleared (the exact shape the merge in + // TRANSCRIPT_TOOL_USE hand-picks fields off of). + let state = expire(withPendingAsk(), { reason: 'hook-closed' }); + + // The transcript watcher's real tool_use event arrives after the hook + // already expired — this is the merge branch that promotes perm-r1 to + // the real toolUseId. + state = chatReducer(state, { + type: 'TRANSCRIPT_TOOL_USE', + sessionId: 's1', + uuid: 'line-uuid-1', + toolUseId: 'toolu_real_1', + toolName: 'Bash', + toolInput: {}, + } as any); + + const synthetic = [...state.get('s1')!.toolCalls.keys()].filter((k) => k.startsWith('perm-')); + expect(synthetic).toHaveLength(0); // synthetic replaced, not duplicated + + const merged = state.get('s1')!.toolCalls.get('toolu_real_1')!; + // Regression guard: the merge builds a NEW object off a hand-picked field + // list. `expired` was missing from that list, so a merged retained card + // used to end up awaiting-approval with no requestId (unanswerable) and + // no `expired` flag (unresolvable — PERMISSION_CARD_RESOLVED's guard + // requires it) — an orphan that keeps the red dot and pty gates stuck. + expect(merged.status).toBe('awaiting-approval'); + expect(merged.expired).toBe(true); + expect(merged.requestId).toBeUndefined(); + + // And it must now actually be resolvable via PERMISSION_CARD_RESOLVED. + state = chatReducer(state, { + type: 'PERMISSION_CARD_RESOLVED', + sessionId: 's1', + toolUseId: 'toolu_real_1', + }); + const resolved = state.get('s1')!.toolCalls.get('toolu_real_1')!; + expect(resolved.status).toBe('complete'); + expect(resolved.expired).toBeUndefined(); + }); + + it('PERMISSION_CARD_RESOLVED guard independently requires status === awaiting-approval, not just `expired`', () => { + // Discrimination test: a reviewer confirmed that reverting ONLY the + // guard's `status !== 'awaiting-approval'` clause still left the older + // regression test (above) passing, because endTurn()'s `expired`- + // stripping already blocks that particular path. This test builds a + // settled card that STILL carries `expired: true` (bypassing endTurn + // entirely) so it pins the guard clause on its own. + const state = withPendingAsk(); + const toolCalls = new Map(state.get('s1')!.toolCalls); + toolCalls.set('perm-r1', { + ...toolCalls.get('perm-r1')!, + status: 'failed', + error: 'Some real failure', + expired: true, // settled, but the stale marker was never cleared + }); + const settled = new Map(state); + settled.set('s1', { ...state.get('s1')!, toolCalls }); + + const next = chatReducer(settled, { + type: 'PERMISSION_CARD_RESOLVED', + sessionId: 's1', + toolUseId: 'perm-r1', + }); + const tool = next.get('s1')!.toolCalls.get('perm-r1')!; + expect(tool.status).toBe('failed'); + expect(tool.error).toBe('Some real failure'); + }); }); diff --git a/desktop/src/renderer/state/chat-reducer.ts b/desktop/src/renderer/state/chat-reducer.ts index 534b0d753..da93814a8 100644 --- a/desktop/src/renderer/state/chat-reducer.ts +++ b/desktop/src/renderer/state/chat-reducer.ts @@ -832,7 +832,16 @@ export function chatReducer(state: ChatState, action: ChatAction): ChatState { for (const [synId, synTool] of toolCalls) { if (synId.startsWith('perm-') && synTool.toolName === action.toolName && synTool.status === 'awaiting-approval') { - // Replace synthetic with real tool, preserving permission state + // Replace synthetic with real tool, preserving permission state. + // Fix: this field list is HAND-PICKED, not a spread — any ToolCallState + // field a synthetic card can carry that isn't listed here is silently + // dropped on merge. `expired` was missing: a hook-closed retained card + // (status stays 'awaiting-approval', expired: true, requestId cleared) + // that merges with its real TRANSCRIPT_TOOL_USE used to lose `expired`, + // leaving a card with no requestId (unanswerable) and no expired flag + // (unresolvable — PERMISSION_CARD_RESOLVED's guard requires it) — an + // orphan stuck awaiting-approval forever. If you add a new field to + // ToolCallState that a synthetic card can carry, add it here too. toolCalls.delete(synId); toolCalls.set(action.toolUseId, { toolUseId: action.toolUseId, @@ -842,6 +851,7 @@ export function chatReducer(state: ChatState, action: ChatAction): ChatState { requestId: synTool.requestId, permissionSuggestions: synTool.permissionSuggestions, denyListed: synTool.denyListed, + expired: synTool.expired, }); // Update the tool group to reference the real ID const toolGroups = new Map(session.toolGroups); From fdfd1bee596fda080911aa78b386997b7e1df3d9 Mon Sep 17 00:00:00 2001 From: Destin Date: Fri, 31 Jul 2026 13:29:48 -0700 Subject: [PATCH 10/20] fix(chat): clear `expired` on tool-result settle, make TOOL_USE re-emit idempotent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two more instances of the retained-vs-settled incoherence bug class: - TRANSCRIPT_TOOL_RESULT settled a card (`failed`/`complete`) without clearing `expired`, so the primary success path for a hook-closed retained card (user answers the live terminal menu, tool runs, the result lands on the same toolUseId) left an ordinary successful tool permanently flagged expired. - TRANSCRIPT_TOOL_USE's non-merge branch unconditionally stamped a bare `{ status: 'running' }` object for any toolUseId with no matching perm-* synthetic, with no check for an already-progressed entry. transcript-watcher.ts re-emits tool-use on a repeated line uuid by design (CC rewrites the same JSONL line as the assistant message grows), so a real production race — hook progresses a toolUseId past 'running', then the watcher re-emits tool-use for that same id — erased retention and flipped the pty-input gates back open while the terminal's Ink menu could still be live. Both fixes keep every card in one of the two coherent end states: retained (awaiting-approval + expired, no requestId) or settled (failed/complete, expired cleared). Co-Authored-By: Claude Opus 5 --- .../state/__tests__/chat-reducer.test.ts | 88 +++++++++++++++++++ desktop/src/renderer/state/chat-reducer.ts | 41 +++++++-- 2 files changed, 121 insertions(+), 8 deletions(-) diff --git a/desktop/src/renderer/state/__tests__/chat-reducer.test.ts b/desktop/src/renderer/state/__tests__/chat-reducer.test.ts index be97b1554..45394c991 100644 --- a/desktop/src/renderer/state/__tests__/chat-reducer.test.ts +++ b/desktop/src/renderer/state/__tests__/chat-reducer.test.ts @@ -813,6 +813,94 @@ describe('PERMISSION_EXPIRED reasons (2026-07-30 spec §2/§2a/§2c/§2d)', () = expect(resolved.expired).toBeUndefined(); }); + it('TRANSCRIPT_TOOL_RESULT settling a retained hook-closed card clears `expired` (success path: user answers the live terminal menu, tool runs, result lands on the same toolUseId)', () => { + // Retain via 'hook-closed', then merge the real toolUseId in via + // TRANSCRIPT_TOOL_USE (mirrors the prior regression test) so the card + // under test has the exact shape a merged retained card has in prod. + let state = expire(withPendingAsk(), { reason: 'hook-closed' }); + state = chatReducer(state, { + type: 'TRANSCRIPT_TOOL_USE', + sessionId: 's1', + uuid: 'line-uuid-1', + toolUseId: 'toolu_real_1', + toolName: 'Bash', + toolInput: {}, + } as any); + expect(state.get('s1')!.toolCalls.get('toolu_real_1')!.expired).toBe(true); + + // The user answered the still-live Ink menu directly in the terminal; + // Claude Code ran the tool and the transcript watcher's tool_result lands + // on the same toolUseId. This is the PRIMARY success path for a retained + // card — PERMISSION_CARD_RESOLVED never fires here. + const complete = chatReducer(state, { + type: 'TRANSCRIPT_TOOL_RESULT', + sessionId: 's1', + uuid: 'result-uuid-1', + toolUseId: 'toolu_real_1', + result: 'ok', + isError: false, + } as any); + const completeTool = complete.get('s1')!.toolCalls.get('toolu_real_1')!; + // Regression guard: settling this path used to spread `...existing` over + // status/response only, leaving a stale `expired: true` on an ordinary + // successful tool — UI that renders on `expired` would wrongly show it + // as expired forever. + expect(completeTool.status).toBe('complete'); + expect(completeTool.expired).toBeUndefined(); + + // Same bug, the failed branch. + const failed = chatReducer(state, { + type: 'TRANSCRIPT_TOOL_RESULT', + sessionId: 's1', + uuid: 'result-uuid-2', + toolUseId: 'toolu_real_1', + result: 'boom', + isError: true, + } as any); + const failedTool = failed.get('s1')!.toolCalls.get('toolu_real_1')!; + expect(failedTool.status).toBe('failed'); + expect(failedTool.expired).toBeUndefined(); + }); + + it('TRANSCRIPT_TOOL_USE re-emit for an already-progressed real toolUseId is idempotent — does not erase retention or flip status back to running', () => { + // Retain via 'hook-closed', then merge the real toolUseId in — the card is + // now { status: 'awaiting-approval', expired: true, requestId: undefined }. + let state = expire(withPendingAsk(), { reason: 'hook-closed' }); + state = chatReducer(state, { + type: 'TRANSCRIPT_TOOL_USE', + sessionId: 's1', + uuid: 'line-uuid-1', + toolUseId: 'toolu_real_1', + toolName: 'Bash', + toolInput: {}, + } as any); + const merged = state.get('s1')!.toolCalls.get('toolu_real_1')!; + expect(merged.status).toBe('awaiting-approval'); + expect(merged.expired).toBe(true); + + // CC rewrites the same JSONL line again (assistant message still growing) + // and the watcher re-emits tool-use for the SAME real toolUseId — by + // design, per transcript-watcher.ts's dedup-by-uuid comment. No perm-* + // synthetic entry exists anymore (already merged above), so this used to + // fall through to the unconditional toolCalls.set(...) that stamps a + // bare { status: 'running' } object, erasing retention entirely and + // flipping the pty gates open while the terminal menu may still be live. + const reEmitted = chatReducer(state, { + type: 'TRANSCRIPT_TOOL_USE', + sessionId: 's1', + uuid: 'line-uuid-1', + toolUseId: 'toolu_real_1', + toolName: 'Bash', + toolInput: {}, + } as any); + const card = reEmitted.get('s1')!.toolCalls.get('toolu_real_1')!; + expect(card.status).toBe('awaiting-approval'); + expect(card.expired).toBe(true); + expect(card.requestId).toBeUndefined(); + expect(hasPendingInteraction(reEmitted.get('s1')!)).toBe(true); + expect(canRetrySubmit(reEmitted.get('s1')!)).toBe(false); + }); + it('PERMISSION_CARD_RESOLVED guard independently requires status === awaiting-approval, not just `expired`', () => { // Discrimination test: a reviewer confirmed that reverting ONLY the // guard's `status !== 'awaiting-approval'` clause still left the older diff --git a/desktop/src/renderer/state/chat-reducer.ts b/desktop/src/renderer/state/chat-reducer.ts index da93814a8..83ede2ac1 100644 --- a/desktop/src/renderer/state/chat-reducer.ts +++ b/desktop/src/renderer/state/chat-reducer.ts @@ -899,12 +899,28 @@ export function chatReducer(state: ChatState, action: ChatAction): ChatState { } if (mergedSynthetic) return next; - toolCalls.set(action.toolUseId, { - toolUseId: action.toolUseId, - toolName: action.toolName, - input: action.toolInput, - status: 'running', - }); + // Fix: this write must be IDEMPOTENT for a toolUseId that already has a + // progressed entry. transcript-watcher.ts's readNewLines re-emits + // tool-use on a repeated line uuid by design (CC rewrites the same + // JSONL line as the assistant message grows, and the earlier tool_use + // block is still present in the rewrite) — see its dedup-by-uuid + // comment. If the hook relay already carried this same toolUseId past + // 'running' (awaiting-approval, or retained via PERMISSION_EXPIRED + // 'hook-closed': awaiting-approval + expired: true), a later re-emit + // used to unconditionally stamp a bare { status: 'running' } object + // here, erasing that progress — and since the pty-input gates key off + // `status`, a retained card's gates would flip back open while the + // terminal's Ink menu may still be live. Only a genuinely new + // toolUseId gets a fresh 'running' card. + const existingCall = toolCalls.get(action.toolUseId); + toolCalls.set(action.toolUseId, existingCall + ? { ...existingCall, toolName: action.toolName, input: action.toolInput } + : { + toolUseId: action.toolUseId, + toolName: action.toolName, + input: action.toolInput, + status: 'running', + }); let { assistantTurns, timeline, currentTurnId } = getOrCreateTurn(session); const toolGroups = new Map(session.toolGroups); @@ -984,14 +1000,23 @@ export function chatReducer(state: ChatState, action: ChatAction): ChatState { // Carry structuredPatch onto the tool state so DiffView can render // with absolute file line numbers (Claude Code ships it pre-computed). const patch = action.structuredPatch; + // Fix: a tool_result settling a card must clear `expired`, or a card + // retained through PERMISSION_EXPIRED('hook-closed') stays flagged + // expired forever even though it just settled normally. This IS the + // retention feature's primary success path (not an edge case): the + // user answers the still-live terminal menu, Claude Code runs the + // tool, and this tool_result lands on the same toolUseId — + // PERMISSION_CARD_RESOLVED never fires for it. A card must always end + // up either retained (awaiting-approval + expired) or settled + // (failed/complete, expired cleared) — never both. if (action.isError) { toolCalls.set(action.toolUseId, { - ...existing, status: 'failed', error: action.result, + ...existing, status: 'failed', error: action.result, expired: undefined, ...(patch ? { structuredPatch: patch } : {}), }); } else { toolCalls.set(action.toolUseId, { - ...existing, status: 'complete', response: action.result, + ...existing, status: 'complete', response: action.result, expired: undefined, ...(patch ? { structuredPatch: patch } : {}), }); } From 0f90dc9c8ae6bf797eaf2524248dd34e4527d253 Mon Sep 17 00:00:00 2001 From: Destin Date: Fri, 31 Jul 2026 13:39:57 -0700 Subject: [PATCH 11/20] feat(chat): expired cards resolve when the terminal menu leaves the buffer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Task 4 of the permission-ask-timeout plan. Adds the §2 standing rule: a card retained by PERMISSION_EXPIRED('hook-closed') only resolves once the Ink menu has been absent from the visible terminal buffer for two consecutive flushes (expired-card-resolver.ts). A one-shot check races in both directions — CC's fallback menu can render a beat after the hook dies (false resolve, the original bug), and a terminal answer's socket-close often lands before the buffer flush removes the menu (false retain) — requiring two consecutive absent flushes self-corrects both. Also fixes usePromptDetector's awaiting-approval bail, which previously returned early on ANY awaiting-approval tool including retained (expired) ones. Since a retained card never leaves awaiting-approval on its own, that bail would have switched prompt detection off for the session permanently — silencing this resolver, the later digit-rebind feature, and every unrelated setup PromptCard (trust gate, usage limit, resume). The bail and the debounced re-check both now exempt expired cards. Co-Authored-By: Claude Opus 5 --- .../src/renderer/hooks/usePromptDetector.ts | 50 +++++++++++++++++-- .../__tests__/expired-card-resolver.test.ts | 43 ++++++++++++++++ .../renderer/state/expired-card-resolver.ts | 38 ++++++++++++++ 3 files changed, 128 insertions(+), 3 deletions(-) create mode 100644 desktop/src/renderer/state/__tests__/expired-card-resolver.test.ts create mode 100644 desktop/src/renderer/state/expired-card-resolver.ts diff --git a/desktop/src/renderer/hooks/usePromptDetector.ts b/desktop/src/renderer/hooks/usePromptDetector.ts index 8fd9f17f7..4d78faecd 100644 --- a/desktop/src/renderer/hooks/usePromptDetector.ts +++ b/desktop/src/renderer/hooks/usePromptDetector.ts @@ -2,6 +2,7 @@ import { useEffect, useRef } from 'react'; import { parseInkSelect, menuToButtons } from '../parser/ink-select-parser'; import { useChatDispatch, useChatStore } from '../state/chat-context'; import { getVisibleScreenText, onBufferReady } from './terminal-registry'; +import { expiredToolIds, nextAbsentCount } from '../state/expired-card-resolver'; // How long to wait before showing a parser-detected prompt, giving the hook // system time to deliver a PermissionRequest via the named pipe relay. @@ -67,6 +68,10 @@ export function usePromptDetector() { const lastPermissionClearedRef = useRef>(new Map()); const prevAwaitingRef = useRef>(new Map()); + // §2 standing rule: per-session count of consecutive buffer flushes with no + // Ink menu while an expired card is retained. At 2, resolve the card(s). + const expiredAbsentRef = useRef>(new Map()); + // Perf: detect awaiting-approval transitions in an effect (off the render // path) and iterate activeTurnToolIds (current-turn only, per chat-reducer // rule #2) rather than the session-lifetime toolCalls Map. With many @@ -100,8 +105,44 @@ export function usePromptDetector() { // (the hook-based UI is handling the permission flow) const sessionState = store.getState().get(sid); if (sessionState) { + // §2 resolver runs BEFORE the awaiting-approval bail below — and that + // bail must ignore expired cards, or retention switches this whole + // detector off for the session, taking the resolver itself, the + // digit-rebind feature, and every unrelated setup PromptCard (trust + // gate, usage limit, resume) down with it (spec §2b). A retained card + // stays awaiting-approval indefinitely, so without this exemption the + // bail below would never fire again for this session. + const expired = expiredToolIds(sessionState); + if (expired.length > 0) { + // Menu-absence standing rule (spec §2): resolve only after the Ink + // menu has been absent for two consecutive buffer flushes — see + // expired-card-resolver.ts for why a one-shot check races both ways. + const expiredScreen = getVisibleScreenText(sid); + const menuPresent = !!(expiredScreen && parseInkSelect(expiredScreen)); + const prev = expiredAbsentRef.current.get(sid) ?? 0; + const { count, resolve } = nextAbsentCount(menuPresent, prev); + expiredAbsentRef.current.set(sid, count); + if (resolve) { + expiredAbsentRef.current.delete(sid); + for (const toolUseId of expired) { + const action = { type: 'PERMISSION_CARD_RESOLVED' as const, sessionId: sid, toolUseId }; + dispatch(action); + (window as any).claude?.remote?.broadcastAction?.(action); + } + } + } else { + // Nothing expired for this session — drop any stale counter so a + // FUTURE expiry starts its own count from zero instead of leaking + // state across unrelated permission asks. + expiredAbsentRef.current.delete(sid); + } for (const [, tool] of sessionState.toolCalls) { - if (tool.status === 'awaiting-approval') return; + // Live asks silence the parser below — the hook-based ToolCard + // owns that menu and a PromptCard must not double-render it. + // EXPIRED asks must NOT silence it — their menu has no live socket + // behind it, and the resolver above needs this function to keep + // running every flush to ever see two consecutive absences. + if (tool.status === 'awaiting-approval' && !tool.expired) return; } } @@ -161,11 +202,14 @@ export function usePromptDetector() { pendingTimerRef.current.delete(sid); // Re-check: if a PermissionRequest arrived during the debounce, - // a tool will be in awaiting-approval — don't show the prompt + // a tool will be in awaiting-approval — don't show the prompt. + // Same expired exemption as the top-of-flush bail above: an + // expired card must not block this setup-prompt PromptCard from + // showing (spec §2b). const currentSession = store.getState().get(sid); if (currentSession) { for (const [, tool] of currentSession.toolCalls) { - if (tool.status === 'awaiting-approval') return; + if (tool.status === 'awaiting-approval' && !tool.expired) return; } } diff --git a/desktop/src/renderer/state/__tests__/expired-card-resolver.test.ts b/desktop/src/renderer/state/__tests__/expired-card-resolver.test.ts new file mode 100644 index 000000000..d33b0adde --- /dev/null +++ b/desktop/src/renderer/state/__tests__/expired-card-resolver.test.ts @@ -0,0 +1,43 @@ +import { describe, it, expect } from 'vitest'; +import { nextAbsentCount, MENU_ABSENT_FLUSHES_TO_RESOLVE, expiredToolIds } from '../expired-card-resolver'; +import { createSessionChatState } from '../chat-types'; +import type { ToolCallState } from '../../../shared/types'; + +describe('expired-card menu-absence rule (spec §2)', () => { + it('menu present resets the counter — false retain self-heals later', () => { + expect(nextAbsentCount(true, 1)).toEqual({ count: 0, resolve: false }); + }); + it('one absent flush is NOT enough — socket close races the buffer flush', () => { + expect(nextAbsentCount(false, 0)).toEqual({ count: 1, resolve: false }); + }); + it('two consecutive absent flushes resolve', () => { + expect(nextAbsentCount(false, 1)).toEqual({ count: 2, resolve: true }); + expect(MENU_ABSENT_FLUSHES_TO_RESOLVE).toBe(2); + }); +}); + +describe('expiredToolIds', () => { + function makeTool(toolUseId: string, overrides: Partial): ToolCallState { + return { + toolUseId, + toolName: 'Bash', + status: 'awaiting-approval', + input: {}, + ...overrides, + } as ToolCallState; + } + + it('returns ids of tools that are both awaiting-approval AND expired', () => { + const session = createSessionChatState(); + session.toolCalls.set('t1', makeTool('t1', { expired: true })); + session.toolCalls.set('t2', makeTool('t2', {})); // live ask, not expired + session.toolCalls.set('t3', makeTool('t3', { status: 'complete', expired: true })); // resolved already + expect(expiredToolIds(session)).toEqual(['t1']); + }); + + it('returns an empty array when no cards are expired', () => { + const session = createSessionChatState(); + session.toolCalls.set('t1', makeTool('t1', {})); + expect(expiredToolIds(session)).toEqual([]); + }); +}); diff --git a/desktop/src/renderer/state/expired-card-resolver.ts b/desktop/src/renderer/state/expired-card-resolver.ts new file mode 100644 index 000000000..39e3c72cd --- /dev/null +++ b/desktop/src/renderer/state/expired-card-resolver.ts @@ -0,0 +1,38 @@ +import type { SessionChatState } from './chat-types'; + +/** §2 standing rule (2026-07-30 spec): a card retained by a 'hook-closed' + * expiry resolves only after the Ink menu has been ABSENT from the visible + * buffer for TWO consecutive flushes. One flush is a race in both + * directions: a terminal answer's socket-close often lands BEFORE the flush + * that removes the menu (would false-retain, self-heals here), and CC's own + * fallback menu renders a beat AFTER a hook kill (a one-shot parse would + * false-RESOLVE — clearing the red dot while the session is still blocked + * on a menu chat view never renders; that is the original reported bug). */ +export const MENU_ABSENT_FLUSHES_TO_RESOLVE = 2; + +/** Ids of tool calls that are a still-live retained card: awaiting-approval + * AND expired. Scans the session-lifetime `toolCalls` map (not + * `activeTurnToolIds`) because a retained card can outlive the turn it was + * asked in — the whole point of retention is that it survives past the + * point a normal ask would have resolved. */ +export function expiredToolIds(session: SessionChatState): string[] { + const ids: string[] = []; + for (const [id, tool] of session.toolCalls) { + if (tool.status === 'awaiting-approval' && tool.expired) ids.push(id); + } + return ids; +} + +/** Pure transition for the per-session consecutive-absence counter. Menu + * present resets to 0 (self-heals a false retain); menu absent increments, + * and resolves once it reaches MENU_ABSENT_FLUSHES_TO_RESOLVE consecutive + * absent flushes (guards against a one-shot false resolve — see the + * module-level comment above). */ +export function nextAbsentCount( + menuPresent: boolean, + prevCount: number, +): { count: number; resolve: boolean } { + if (menuPresent) return { count: 0, resolve: false }; + const count = prevCount + 1; + return { count, resolve: count >= MENU_ABSENT_FLUSHES_TO_RESOLVE }; +} From fe92566a615a57bc57eeb787ba1119ef3fd20735 Mon Sep 17 00:00:00 2001 From: Destin Date: Fri, 31 Jul 2026 13:49:50 -0700 Subject: [PATCH 12/20] feat(chat): expired approval cards render Dismiss; delivery failures tagged delivery-failed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ToolCard and CompactToolStrip gated their approval UI on `tool.requestId`, but retained expired cards (Task 3) deliberately clear it — so a retained card rendered header-only, with no explanation and no way out, on a session whose red attention dot stayed lit. Widen both gates to `tool.requestId || tool.expired` and add an expired branch: explanatory copy that says what's known (buttons stopped working) rather than assumed (the ask failed), plus a Dismiss button that quietly dispatches PERMISSION_CARD_RESOLVED. AskUserQuestion gets the same branch — Dismiss is its only recovery path, since replaying its multi-select TUI blind risks a wrong answer. Also tag the three renderer sites that dispatch PERMISSION_EXPIRED as a generic "unstick this card" action (ToolCard's onFailedCb, and CompactToolStrip's delivered===false and catch branches) with reason: 'delivery-failed'. Under the new retention rules an untagged dispatch would PIN these cards forever instead of resolving them — the socket is provably gone in all three cases, so retention would leave dead buttons on screen with no recovery. --- .../src/renderer/components/ToolCard.test.tsx | 141 +++++++++++++++++- desktop/src/renderer/components/ToolCard.tsx | 36 ++++- .../buddy/CompactToolStrip.test.tsx | 131 ++++++++++++++++ .../components/buddy/CompactToolStrip.tsx | 99 +++++++----- 4 files changed, 369 insertions(+), 38 deletions(-) create mode 100644 desktop/src/renderer/components/buddy/CompactToolStrip.test.tsx diff --git a/desktop/src/renderer/components/ToolCard.test.tsx b/desktop/src/renderer/components/ToolCard.test.tsx index 031a48e04..e7743e1f3 100644 --- a/desktop/src/renderer/components/ToolCard.test.tsx +++ b/desktop/src/renderer/components/ToolCard.test.tsx @@ -1,8 +1,8 @@ // @vitest-environment jsdom import '@testing-library/jest-dom/vitest'; import React from 'react'; -import { describe, it, expect, beforeEach } from 'vitest'; -import { render, screen, cleanup } from '@testing-library/react'; +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { render, screen, cleanup, fireEvent } from '@testing-library/react'; import { ChatProvider } from '../state/chat-context'; import ToolCard from './ToolCard'; import type { ToolCallState } from '../../shared/types'; @@ -62,3 +62,140 @@ describe('ToolCard — Skill compact variant', () => { expect(screen.queryByTestId('tool-card-chevron')).not.toBeNull(); }); }); + +describe('ToolCard — expired approval card', () => { + const originalClaude = (window as any).claude; + + beforeEach(() => { + cleanup(); + (window as any).claude = { remote: { broadcastAction: () => {} } }; + }); + + afterEach(() => { + (window as any).claude = originalClaude; + }); + + it('renders a normal (non-expired) awaiting-approval card with Yes/No, not header-only', () => { + const tool = makeTool({ + toolName: 'Bash', + input: { command: 'ls' }, + status: 'awaiting-approval', + requestId: 'req-1', + }); + render( + + + + ); + expect(screen.getByText('Yes')).toBeInTheDocument(); + expect(screen.getByText('No')).toBeInTheDocument(); + }); + + // The bug this task fixes: retention (Task 3) clears requestId on a + // retained card, but the old gate (`tool.requestId &&`) required it — so a + // retained card rendered header-only, with no explanation and no way out, + // on a session whose red attention dot stayed lit. + it('widens the gate so an expired card (requestId cleared) still renders UI', () => { + const tool = makeTool({ + toolName: 'Bash', + input: { command: 'rm -rf /tmp/x' }, + status: 'awaiting-approval', + requestId: undefined, + expired: true, + }); + render( + + + + ); + expect( + screen.getByText(/The buttons on this card timed out, but Claude may still be/) + ).toBeInTheDocument(); + expect(screen.getByRole('button', { name: /Dismiss — I answered in the terminal/ })).toBeInTheDocument(); + // The old Yes/No/Always Allow row must NOT render — requestId is gone, + // so those buttons could never deliver a response anyway. + expect(screen.queryByText('Yes')).toBeNull(); + expect(screen.queryByText('No')).toBeNull(); + }); + + it('Dismiss on an expired card dispatches PERMISSION_CARD_RESOLVED and mirrors to remote', () => { + const broadcastAction = vi.fn(); + (window as any).claude = { remote: { broadcastAction } }; + const tool = makeTool({ + toolName: 'Bash', + input: { command: 'ls' }, + status: 'awaiting-approval', + requestId: undefined, + expired: true, + toolUseId: 'toolu_expired_1', + }); + render( + + + + ); + fireEvent.click(screen.getByRole('button', { name: /Dismiss — I answered in the terminal/ })); + expect(broadcastAction).toHaveBeenCalledWith({ + type: 'PERMISSION_CARD_RESOLVED', + sessionId: 's1', + toolUseId: 'toolu_expired_1', + }); + }); + + it('AskUserQuestion also gets the expired branch with Dismiss as its only out', () => { + const tool = makeTool({ + toolName: 'AskUserQuestion', + input: { + questions: [ + { question: 'Pick one', header: 'Choice', multiSelect: false, options: [{ label: 'A' }, { label: 'B' }] }, + ], + }, + status: 'awaiting-approval', + requestId: undefined, + expired: true, + }); + render( + + + + ); + expect(screen.getByRole('button', { name: /Dismiss — I answered in the terminal/ })).toBeInTheDocument(); + // The question options themselves must not render — there's no live + // socket to submit answers through. + expect(screen.queryByText('Pick one')).toBeNull(); + }); + + // A delivered===false response means the socket is provably gone — under + // the new retention rules (Task 3), a bare PERMISSION_EXPIRED with no + // reason would still resolve (reason absent defaults to resolve), but this + // pins the intent explicitly so a future reducer default change can't + // silently start retaining these. + it('tags delivery failure (delivered === false) with reason: delivery-failed', () => { + const broadcastAction = vi.fn(); + (window as any).claude = { + remote: { broadcastAction }, + session: { respondToPermission: vi.fn().mockResolvedValue(false) }, + }; + const tool = makeTool({ + toolName: 'Bash', + input: { command: 'ls' }, + status: 'awaiting-approval', + requestId: 'req-1', + toolUseId: 'toolu_del_fail', + }); + render( + + + + ); + fireEvent.click(screen.getByText('Yes')); + return Promise.resolve().then(() => { + expect(broadcastAction).toHaveBeenCalledWith({ + type: 'PERMISSION_EXPIRED', + sessionId: 's1', + requestId: 'req-1', + reason: 'delivery-failed', + }); + }); + }); +}); diff --git a/desktop/src/renderer/components/ToolCard.tsx b/desktop/src/renderer/components/ToolCard.tsx index 48e04c279..94dedcb7f 100644 --- a/desktop/src/renderer/components/ToolCard.tsx +++ b/desktop/src/renderer/components/ToolCard.tsx @@ -782,7 +782,33 @@ export default React.memo(function ToolCard({ tool, sessionId, inGroup = false } {/* Permission / AskUserQuestion / ExitPlanMode UI */} - {tool.status === 'awaiting-approval' && tool.requestId && (() => { + {tool.status === 'awaiting-approval' && (tool.requestId || tool.expired) && (() => { + // Expired: the hook socket is dead (requestId cleared) but CC's Ink + // menu may still be live in the terminal. Task 9 adds digit-rebind + // buttons here; Dismiss is the universal out (only out for + // AskUserQuestion, which never rebinds — spec §3/§1b). Copy says what + // is known (buttons stopped working), not what is assumed (the ask + // failed) — the user may have already answered in the terminal. + if (tool.expired || !tool.requestId) { + const resolveLocally = () => { + if (!sessionId) return; + const action = { type: 'PERMISSION_CARD_RESOLVED' as const, sessionId, toolUseId: tool.toolUseId }; + dispatch(action); + (window as any).claude?.remote?.broadcastAction(action); + }; + return ( +
+

+ The buttons on this card timed out, but Claude may still be + waiting in the terminal. Answer it there — or dismiss this if + you already did. +

+ +
+ ); + } // AskUserQuestion needs its own UI with option selection instead of Yes/No const isAskUser = tool.toolName === 'AskUserQuestion' && isValidQuestions(tool.input); // ExitPlanMode has a 4-option Ink menu in the CLI (bypass/manual/refine/feedback), @@ -797,7 +823,13 @@ export default React.memo(function ToolCard({ tool, sessionId, inGroup = false } }; const onFailedCb = () => { if (sessionId && tool.requestId) { - const action = { type: 'PERMISSION_EXPIRED' as const, sessionId, requestId: tool.requestId }; + // 'delivery-failed': the respond() write returned false or threw — + // the socket is provably gone, so this must RESOLVE the card, never + // retain it (retention would pin a card whose buttons cannot work). + const action = { + type: 'PERMISSION_EXPIRED' as const, sessionId, + requestId: tool.requestId, reason: 'delivery-failed' as const, + }; dispatch(action); (window as any).claude?.remote?.broadcastAction(action); } diff --git a/desktop/src/renderer/components/buddy/CompactToolStrip.test.tsx b/desktop/src/renderer/components/buddy/CompactToolStrip.test.tsx new file mode 100644 index 000000000..cd509892f --- /dev/null +++ b/desktop/src/renderer/components/buddy/CompactToolStrip.test.tsx @@ -0,0 +1,131 @@ +// @vitest-environment jsdom +import '@testing-library/jest-dom/vitest'; +import React from 'react'; +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { render, screen, cleanup, fireEvent } from '@testing-library/react'; +import { ChatProvider } from '../../state/chat-context'; +import { CompactToolStrip } from './CompactToolStrip'; +import type { ToolCallState } from '../../../shared/types'; + +function makeTool(overrides: Partial): ToolCallState { + return { + toolUseId: 'toolu_test', + toolName: 'Bash', + input: { command: 'ls' }, + status: 'complete', + ...overrides, + }; +} + +describe('CompactToolStrip — expired approval card', () => { + const originalClaude = (window as any).claude; + + beforeEach(() => { + cleanup(); + (window as any).claude = { remote: { broadcastAction: () => {} } }; + }); + + afterEach(() => { + (window as any).claude = originalClaude; + }); + + it('renders Allow/Deny/Always for a normal (non-expired) awaiting-approval tool', () => { + const tool = makeTool({ status: 'awaiting-approval', requestId: 'req-1' }); + render( + + + + ); + expect(screen.getByText('✓ Allow')).toBeInTheDocument(); + expect(screen.getByText('✕ Deny')).toBeInTheDocument(); + expect(screen.getByText('∞ Always')).toBeInTheDocument(); + }); + + // Same bug as ToolCard: the old gate (`tool.requestId &&`) required a live + // requestId, but a retained card clears it — so a retained card in the + // buddy strip showed an amber dot with no Allow/Deny/Always row AND no + // Dismiss, leaving the user with no in-app way to act on it. + it('widens the gate so an expired card (requestId cleared) shows a Dismiss button instead of nothing', () => { + const tool = makeTool({ status: 'awaiting-approval', requestId: undefined, expired: true }); + render( + + + + ); + expect(screen.getByText('Dismiss')).toBeInTheDocument(); + expect(screen.queryByText('✓ Allow')).toBeNull(); + expect(screen.queryByText('✕ Deny')).toBeNull(); + expect(screen.queryByText('∞ Always')).toBeNull(); + }); + + it('Dismiss on an expired card dispatches PERMISSION_CARD_RESOLVED and mirrors to remote', () => { + const broadcastAction = vi.fn(); + (window as any).claude = { remote: { broadcastAction } }; + const tool = makeTool({ + status: 'awaiting-approval', + requestId: undefined, + expired: true, + toolUseId: 'toolu_expired_1', + }); + render( + + + + ); + fireEvent.click(screen.getByText('Dismiss')); + expect(broadcastAction).toHaveBeenCalledWith({ + type: 'PERMISSION_CARD_RESOLVED', + sessionId: 's1', + toolUseId: 'toolu_expired_1', + }); + }); + + // Same rationale as ToolCard's delivery-failed test: delivered===false + // means the socket is provably gone, so this dispatch must RESOLVE (never + // retain) — tag it explicitly rather than relying on the reducer's default. + it('tags delivery failure (delivered === false) with reason: delivery-failed', async () => { + const broadcastAction = vi.fn(); + (window as any).claude = { + remote: { broadcastAction }, + session: { respondToPermission: vi.fn().mockResolvedValue(false) }, + }; + const tool = makeTool({ status: 'awaiting-approval', requestId: 'req-1' }); + render( + + + + ); + fireEvent.click(screen.getByText('✓ Allow')); + await Promise.resolve(); + await Promise.resolve(); + expect(broadcastAction).toHaveBeenCalledWith({ + type: 'PERMISSION_EXPIRED', + sessionId: 's1', + requestId: 'req-1', + reason: 'delivery-failed', + }); + }); + + it('tags a thrown respond() error with reason: delivery-failed', async () => { + const broadcastAction = vi.fn(); + (window as any).claude = { + remote: { broadcastAction }, + session: { respondToPermission: vi.fn().mockRejectedValue(new Error('socket closed')) }, + }; + const tool = makeTool({ status: 'awaiting-approval', requestId: 'req-1' }); + render( + + + + ); + fireEvent.click(screen.getByText('✓ Allow')); + await Promise.resolve(); + await Promise.resolve(); + expect(broadcastAction).toHaveBeenCalledWith({ + type: 'PERMISSION_EXPIRED', + sessionId: 's1', + requestId: 'req-1', + reason: 'delivery-failed', + }); + }); +}); diff --git a/desktop/src/renderer/components/buddy/CompactToolStrip.tsx b/desktop/src/renderer/components/buddy/CompactToolStrip.tsx index d26cfc5a6..c14020903 100644 --- a/desktop/src/renderer/components/buddy/CompactToolStrip.tsx +++ b/desktop/src/renderer/components/buddy/CompactToolStrip.tsx @@ -168,13 +168,17 @@ function ToolRow({ decision, ); if (delivered === false) { - // Fix: Socket already closed — mark expired so the UI unsticks. + // Fix: Socket already closed — resolve the card so the UI unsticks. + // reason: 'delivery-failed' — the write returned false, so the + // socket is provably gone; this must RESOLVE, never retain (spec + // §2c/§2d — retention would pin a card whose buttons cannot work). // Reset responding so user can retry if needed. setResponding(false); const action = { type: 'PERMISSION_EXPIRED' as const, sessionId, requestId: tool.requestId, + reason: 'delivery-failed' as const, }; dispatch(action); (window as any).claude?.remote?.broadcastAction(action); @@ -190,12 +194,15 @@ function ToolRow({ (window as any).claude?.remote?.broadcastAction(action); } catch (err) { console.error('CompactToolStrip: failed to respond to permission:', err); - // Treat as expired so the card doesn't get stuck + // Treat as expired so the card doesn't get stuck. reason: + // 'delivery-failed' — same rationale as the delivered===false branch + // above: the throw means the socket is gone, so this must RESOLVE. if (tool.requestId) { const action = { type: 'PERMISSION_EXPIRED' as const, sessionId, requestId: tool.requestId, + reason: 'delivery-failed' as const, }; dispatch(action); (window as any).claude?.remote?.broadcastAction(action); @@ -248,38 +255,62 @@ function ToolRow({ > {target} - {/* Inline Allow / Deny / Always buttons only for awaiting-approval tools */} - {tool.status === 'awaiting-approval' && tool.requestId ? ( - - - - - + {/* Inline Allow / Deny / Always buttons only for awaiting-approval tools. + An expired card (requestId cleared, socket dead) gets a single + Dismiss instead — same quiet local resolve as ToolCard's, since + there's nothing left to respond to over the dead socket. */} + {tool.status === 'awaiting-approval' && (tool.requestId || tool.expired) ? ( + tool.expired || !tool.requestId ? ( + + + + ) : ( + + + + + + ) ) : null} ); From b6369c03102aa569334c0ec959b135540b114f40 Mon Sep 17 00:00:00 2001 From: Destin Date: Fri, 31 Jul 2026 14:02:38 -0700 Subject: [PATCH 13/20] feat(workbench): expired permission-card fixture MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tasks 3-5 added the retained awaiting-approval card (expired:true, Dismiss control) that PERMISSION_EXPIRED('hook-closed') produces, but nothing in the workbench could reproduce it — Destin had no way to review it visually. Adds a `permission_expired` JSONL line type to fixture-loader.ts, mirroring the existing `permission_request` branch: dispatches PERMISSION_EXPIRED through the real reducer, then swaps the block pushed by the prior permission_request line (blocks are snapshots, so without this the gallery would keep showing the stale pre-expiry Yes/No card). New fixture bash-awaiting-approval-expired.jsonl exercises it in the Tool Gallery, alongside the existing awaiting-approval and denylisted variants. Co-Authored-By: Claude Opus 5 --- .../dev/workbench/fixture-loader.test.ts | 25 ++++++++++++++++ .../renderer/dev/workbench/fixture-loader.ts | 29 +++++++++++++++++++ .../bash-awaiting-approval-expired.jsonl | 3 ++ 3 files changed, 57 insertions(+) create mode 100644 desktop/src/renderer/dev/workbench/fixtures/tools/bash-awaiting-approval-expired.jsonl diff --git a/desktop/src/renderer/dev/workbench/fixture-loader.test.ts b/desktop/src/renderer/dev/workbench/fixture-loader.test.ts index c1d94d39e..84c6534fd 100644 --- a/desktop/src/renderer/dev/workbench/fixture-loader.test.ts +++ b/desktop/src/renderer/dev/workbench/fixture-loader.test.ts @@ -79,6 +79,31 @@ describe('loadFixture', () => { } }); + it('retains an awaiting-approval card with expired:true on a permission_expired line', () => { + const raw = [ + '{"type":"tool_use","id":"toolu_01BashExpired","name":"Bash","input":{"command":"rm -rf node_modules && npm ci"}}', + '{"type":"permission_request","tool_use_id":"toolu_01BashExpired","requestId":"wb-expired-1","denyListed":false}', + '{"type":"permission_expired","tool_use_id":"toolu_01BashExpired","requestId":"wb-expired-1","reason":"hook-closed"}', + ].join('\n'); + + const result = loadFixture('bash-awaiting-approval-expired', raw); + + // permission_expired swaps the block pushed by permission_request in + // place — it must not add a second block for the same tool. + expect(result.blocks).toHaveLength(1); + expect(result.blocks[0].kind).toBe('tool'); + if (result.blocks[0].kind === 'tool') { + expect(result.blocks[0].tool).toMatchObject({ + toolUseId: 'toolu_01BashExpired', + toolName: 'Bash', + status: 'awaiting-approval', + expired: true, + requestId: undefined, + }); + } + expect(result.error).toBeUndefined(); + }); + it('returns an error field when the fixture is malformed', () => { const result = loadFixture('broken', 'not valid json\n'); diff --git a/desktop/src/renderer/dev/workbench/fixture-loader.ts b/desktop/src/renderer/dev/workbench/fixture-loader.ts index 40dca65fd..f63985356 100644 --- a/desktop/src/renderer/dev/workbench/fixture-loader.ts +++ b/desktop/src/renderer/dev/workbench/fixture-loader.ts @@ -157,6 +157,35 @@ export function loadFixture(name: string, raw: string, sessionId: string = SANDB const after = state.get(sessionId); const tool = after?.toolCalls.get(parsed.tool_use_id); if (tool) blocks.push({ kind: 'tool', tool }); + } else if (parsed.type === 'permission_expired') { + // WHY: reproduces the RETAINED card state from PERMISSION_EXPIRED + // ('hook-closed') — spec 2026-07-30 §2a. The hook socket died but + // Claude's menu may still be live in the terminal, so the card must + // NOT resolve to failed: it stays awaiting-approval, gains + // `expired: true`, loses its requestId, and swaps Yes/No for a + // "Dismiss — I answered in the terminal" control. This is the one + // card state Tasks 3-5 added that nothing else in the workbench can + // produce, so a fixture line must exist to review it (task-6-brief). + // Always follows a permission_request line for the same tool_use_id. + const action: ChatAction = { + type: 'PERMISSION_EXPIRED', + sessionId, + requestId: parsed.requestId, + reason: parsed.reason ?? 'hook-closed', + }; + state = chatReducer(state, action); + actions.push(action); + // The permission_request line above already pushed a block for this + // tool — but that block is a snapshot object, and PERMISSION_EXPIRED + // produces a NEW tool object (reducer spreads, never mutates). Swap + // the existing block in place so the gallery renders the post-expiry + // state instead of the stale awaiting-approval-with-buttons one. + const after = state.get(sessionId); + const tool = after?.toolCalls.get(parsed.tool_use_id); + if (tool) { + const idx = blocks.findIndex((b) => b.kind === 'tool' && b.tool.toolUseId === parsed.tool_use_id); + if (idx !== -1) blocks[idx] = { kind: 'tool', tool }; + } } // Unknown types are silently skipped (same policy as before). } diff --git a/desktop/src/renderer/dev/workbench/fixtures/tools/bash-awaiting-approval-expired.jsonl b/desktop/src/renderer/dev/workbench/fixtures/tools/bash-awaiting-approval-expired.jsonl new file mode 100644 index 000000000..3a37b4b2b --- /dev/null +++ b/desktop/src/renderer/dev/workbench/fixtures/tools/bash-awaiting-approval-expired.jsonl @@ -0,0 +1,3 @@ +{"type":"tool_use","id":"toolu_01BashExpired","name":"Bash","input":{"command":"rm -rf node_modules && npm ci","description":"Reinstall dependencies"}} +{"type":"permission_request","tool_use_id":"toolu_01BashExpired","requestId":"wb-expired-1","denyListed":false} +{"type":"permission_expired","tool_use_id":"toolu_01BashExpired","requestId":"wb-expired-1","reason":"hook-closed"} From 9ed6f0584de82d6d929a9af962a4f600cbbdb448 Mon Sep 17 00:00:00 2001 From: Destin Date: Fri, 31 Jul 2026 14:10:18 -0700 Subject: [PATCH 14/20] feat(main): app-owned 2h permission hold, 60s unroutable cap, reasoned expiry emits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The app now ends the permission-ask wait itself instead of relying on the far end (relay/Claude Code) to close the socket. HookRelay starts a per-request hold timer on every PermissionRequest: 2h (APP_HOLD_MS) for asks routable to a live session, 60s (UNROUTABLE_HOLD_MS) dead-man cap for asks whose session no longer exists. On fire it auto-denies with the nested { decision: { behavior: 'deny', message } } shape relay-blocking.js expects and emits 'permission-expired' with an explicit reason ('app-timeout' / 'unroutable') BEFORE the socket's 'close' handler can run — respond()/ closeSocket() delete the pending entry synchronously, so 'close' never fires for app-initiated endings. The 'close' handler now emits 'hook-closed' only when it still finds the entry, i.e. only when the far end went away first (relay timeout, Claude Code killing the hook). That asymmetry is exactly the discrimination the renderer's retain-vs-resolve logic (Task 3) depends on. - HookRelay: holdTimers map, setSessionGate(), clearHold() called from respond()/closeSocket()/socket-close/stop() so a leaked 2h timer never holds a socket reference or fires into a torn-down connection. - SessionManager.hasSession() exposes the same `sessions` map the class already uses for lookup, wired as the app's routability gate in main.ts. - main.ts forwards `reason` inside the PermissionExpired payload as `_reason` (no IPC channel shape change) to hook-dispatcher.ts, which already threads it into the PERMISSION_EXPIRED action (Task 3 contract). - Extended tests/hook-relay.test.ts with the tier-1 hold/cap/hook-closed cases and tests/permission-timeout-margins.test.ts with the APP_HOLD_MS literal check (2h, strictly under the 2h30m relay backstop). Co-Authored-By: Claude Opus 5 --- desktop/src/main/hook-relay.ts | 76 ++++++++++++++++++- desktop/src/main/main.ts | 12 ++- desktop/src/main/session-manager.ts | 5 ++ desktop/src/renderer/state/hook-dispatcher.ts | 4 +- desktop/tests/hook-relay.test.ts | 73 ++++++++++++++++++ .../tests/permission-timeout-margins.test.ts | 6 ++ 6 files changed, 170 insertions(+), 6 deletions(-) diff --git a/desktop/src/main/hook-relay.ts b/desktop/src/main/hook-relay.ts index 9f1bc09e6..d9b257d4b 100644 --- a/desktop/src/main/hook-relay.ts +++ b/desktop/src/main/hook-relay.ts @@ -11,6 +11,18 @@ const DEFAULT_PIPE_NAME = process.platform === 'win32' ? '\\\\.\\pipe\\claude-desktop-hooks' : path.join(os.tmpdir(), 'claude-desktop-hooks.sock'); +// §1 tier-1 hold (2026-07-30 spec): the APP owns the permission-ask clock — +// 2h here < 2h30m relay backstop (relay-blocking.js) < 3h CC hook entry +// (install-hooks.js). Margins are load-bearing: if CC ever wins it kills the +// hook with NO decision and AskUserQuestion waits forever on CC's +// default-"never" question timeout. Do not equalize. NOTE: setTimeout does +// not advance during system suspend, so the hold can stretch past 2h of +// wall-clock on a laptop that slept — expected, not a bug. +export const APP_HOLD_MS = 7200000; +// §1a dead-man cap: an ask whose sessionId matches no live session will never +// render a card anywhere — a 2h hold would be a 2h invisible hang. +export const UNROUTABLE_HOLD_MS = 60000; + export class HookRelay extends EventEmitter { private server: net.Server | null = null; private running = false; @@ -20,10 +32,30 @@ export class HookRelay extends EventEmitter { // a live permission/AskUserQuestion menu and must not be typed into. private pendingSockets = new Map(); private pipeName: string; + // requestId → the app-owned hold timer for that pending ask. Cleared on + // respond()/closeSocket()/socket-close/stop() — a leaked 2h timer would + // hold a socket reference alive, and firing one after teardown would + // respond() into a dead/reused socket. + private holdTimers = new Map(); + private sessionGate: ((sessionId: string) => boolean) | null = null; + private readonly holdMs: number; + private readonly unroutableHoldMs: number; - constructor(pipeName?: string) { + constructor(pipeName?: string, holdMs: number = APP_HOLD_MS, unroutableHoldMs: number = UNROUTABLE_HOLD_MS) { super(); this.pipeName = pipeName || DEFAULT_PIPE_NAME; + this.holdMs = holdMs; + this.unroutableHoldMs = unroutableHoldMs; + } + + /** main.ts wires this to SessionManager (mirrors setReloadPluginsGate). */ + setSessionGate(gate: (sessionId: string) => boolean): void { + this.sessionGate = gate; + } + + private clearHold(requestId: string): void { + const t = this.holdTimers.get(requestId); + if (t) { clearTimeout(t); this.holdTimers.delete(requestId); } } private parseHookPayload(data: string): HookEvent { @@ -62,13 +94,46 @@ export class HookRelay extends EventEmitter { event.payload._requestId = requestId; this.emit('hook-event', event); + // §1 tier-1: the app ends the wait, with a labeled deny. §1a: an + // unroutable ask (no live session at arrival) gets the short + // dead-man cap instead — restores what the old 300s timeout was + // silently doing for that case. + const routable = this.sessionGate ? this.sessionGate(event.sessionId) : true; + const holdMs = routable ? this.holdMs : this.unroutableHoldMs; + this.holdTimers.set(requestId, setTimeout(() => { + this.holdTimers.delete(requestId); + // Nested { decision: { … } } is load-bearing: relay-blocking.js + // reads appDecision.decision — a flat shape ships undefined. + // The message lands VERBATIM in the denied tool result the model + // reads (verified in the CC 2.1.220 binary), so say what + // happened and invite a re-ask. + this.respond(requestId, { + decision: { + behavior: 'deny', + message: routable + ? `YouCoded auto-denied this request after ${Math.round(this.holdMs / 3600000)} hour(s) with no user response — ask again if still needed.` + : 'YouCoded could not route this request to any open session — auto-denied. Ask again if still needed.', + }, + }); + // respond() deletes the pending entry BEFORE 'close' fires, so + // the close handler's wasOpen guard swallows any emit — + // app-initiated endings must emit explicitly (spec §2). + this.emit('permission-expired', event.sessionId, requestId, + routable ? 'app-timeout' : 'unroutable'); + }, holdMs)); + // When the socket closes (relay timeout, Claude Code kills hook, // or network error), notify listeners so the UI can clear the // awaiting-approval state instead of leaving dead buttons. socket.on('close', () => { + this.clearHold(requestId); const wasOpen = this.pendingSockets.delete(requestId); if (wasOpen) { - this.emit('permission-expired', event.sessionId, requestId); + // Reachable ONLY when the far end went away first (relay + // timeout/death, CC killing the hook): app-initiated paths + // delete the entry before 'close' fires and emit their own + // reason. That asymmetry IS the §2 discrimination. + this.emit('permission-expired', event.sessionId, requestId, 'hook-closed'); } }); } else { @@ -154,6 +219,7 @@ export class HookRelay extends EventEmitter { } respond(requestId: string, decision: object): boolean { + this.clearHold(requestId); const pending = this.pendingSockets.get(requestId); if (!pending || pending.socket.destroyed) { this.pendingSockets.delete(requestId); @@ -166,6 +232,7 @@ export class HookRelay extends EventEmitter { } closeSocket(requestId: string): void { + this.clearHold(requestId); const pending = this.pendingSockets.get(requestId); if (pending && !pending.socket.destroyed) { pending.socket.end(); @@ -187,6 +254,11 @@ export class HookRelay extends EventEmitter { } stop(): void { + // Clear all app-owned hold timers first — otherwise a still-pending + // timer could fire respond() into a socket we're about to tear down. + for (const t of this.holdTimers.values()) clearTimeout(t); + this.holdTimers.clear(); + // Clean up all pending permission sockets for (const [, pending] of this.pendingSockets) { if (!pending.socket.destroyed) { diff --git a/desktop/src/main/main.ts b/desktop/src/main/main.ts index c03fb38dd..ccfa92fd6 100644 --- a/desktop/src/main/main.ts +++ b/desktop/src/main/main.ts @@ -179,6 +179,8 @@ const hookRelay = new HookRelay(pipeName); // live permission/AskUserQuestion menu — typing into that menu presses Enter // on the highlighted option and silently answers the prompt (stray-Enter fix). sessionManager.setReloadPluginsGate((sessionId) => hookRelay.hasPendingPermission(sessionId)); +// §1a: unroutable asks get the 60s dead-man cap instead of the 2h hold. +hookRelay.setSessionGate((sessionId) => sessionManager.hasSession(sessionId)); const remoteConfig = new RemoteConfig(); const skillProvider = new LocalSkillProvider(); skillProvider.ensureMigrated(); @@ -924,12 +926,16 @@ function createWindow(firstRunManager?: FirstRunManager) { } }); - // Notify renderer when a permission request socket closes (timeout/killed) - hookRelay.on('permission-expired', (sessionId: string, requestId: string) => { + // Notify renderer when a permission request ends — either app-initiated + // (hold timer / unroutable cap, with a labeled reason) or because the far + // end went away first (hook-closed). See hook-relay.ts §1/§2. + hookRelay.on('permission-expired', (sessionId: string, requestId: string, reason?: string) => { const evt = { type: 'PermissionExpired', sessionId, - payload: { _requestId: requestId }, + // _reason rides INSIDE the payload — no IPC channel shape change, so + // ipc-channels.test.ts needs nothing and old remote shims just ignore it. + payload: { _requestId: requestId, _reason: reason }, timestamp: Date.now(), }; const ownerId = windowRegistry.getOwner(sessionId); diff --git a/desktop/src/main/session-manager.ts b/desktop/src/main/session-manager.ts index dba322a8a..8ac73f2a7 100644 --- a/desktop/src/main/session-manager.ts +++ b/desktop/src/main/session-manager.ts @@ -315,6 +315,11 @@ export class SessionManager extends EventEmitter { return this.sessions.get(id)?.info; } + /** §1a routability gate — true when this sessionId belongs to a live session. */ + hasSession(sessionId: string): boolean { + return this.sessions.has(sessionId); + } + destroyAll(): void { for (const [id] of this.sessions) { this.destroySession(id); diff --git a/desktop/src/renderer/state/hook-dispatcher.ts b/desktop/src/renderer/state/hook-dispatcher.ts index 83ed37708..40cdde178 100644 --- a/desktop/src/renderer/state/hook-dispatcher.ts +++ b/desktop/src/renderer/state/hook-dispatcher.ts @@ -35,7 +35,9 @@ export function hookEventToAction(event: HookEvent): ChatAction | null { case 'PermissionExpired': { const requestId = payload._requestId as string; if (!requestId) return null; - return { type: 'PERMISSION_EXPIRED', sessionId, requestId }; + const reason = payload._reason as + | 'app-timeout' | 'unroutable' | 'delivery-failed' | 'hook-closed' | undefined; + return { type: 'PERMISSION_EXPIRED', sessionId, requestId, reason }; } default: diff --git a/desktop/tests/hook-relay.test.ts b/desktop/tests/hook-relay.test.ts index e37e03ff5..2b75995f4 100644 --- a/desktop/tests/hook-relay.test.ts +++ b/desktop/tests/hook-relay.test.ts @@ -111,4 +111,77 @@ describe('HookRelay', () => { client.destroy(); }); }); + + describe('tier-1 hold (2026-07-30 spec §1)', () => { + it('auto-denies with nested decision shape + app-timeout reason when the hold fires', async () => { + const short = new HookRelay((relay as any).pipeName + '-hold', 60 /* holdMs */); + await short.start(); + const expired = new Promise<[string, string, string?]>((resolve) => { + short.once('permission-expired', (sid, rid, reason) => resolve([sid, rid, reason])); + }); + const net = await import('net'); + const client = net.createConnection((short as any).pipeName); + await new Promise((res, rej) => { client.on('connect', res); client.on('error', rej); }); + const received = new Promise((resolve) => { + let buf = ''; + client.on('data', (c) => { buf += c; if (buf.includes('\n')) resolve(buf); }); + }); + client.write(JSON.stringify({ hook_event_name: 'PermissionRequest', _desktop_session_id: 'sess-h' }) + '\n'); + + const [sid, , reason] = await expired; + expect(sid).toBe('sess-h'); + expect(reason).toBe('app-timeout'); + // Assert against the RELAY'S OWN parse path: relay-blocking.js reads + // appDecision.decision — a flat shape would make this undefined. + const decision = JSON.parse((await received).trim()); + expect(decision.decision.behavior).toBe('deny'); + expect(decision.decision.message).toContain('auto-denied'); + short.stop(); + client.destroy(); + }); + + it('caps the hold at 60s-tier when the session gate says unroutable', async () => { + const short = new HookRelay((relay as any).pipeName + '-unroutable', 60_000, 40 /* unroutableHoldMs */); + short.setSessionGate(() => false); + await short.start(); + const expired = new Promise((resolve) => { + short.once('permission-expired', (_s, _r, reason) => resolve(reason)); + }); + const net = await import('net'); + const client = net.createConnection((short as any).pipeName); + await new Promise((res, rej) => { client.on('connect', res); client.on('error', rej); }); + client.write(JSON.stringify({ hook_event_name: 'PermissionRequest', _desktop_session_id: 'ghost' }) + '\n'); + expect(await expired).toBe('unroutable'); + short.stop(); + client.destroy(); + }); + + it("far-end death emits 'hook-closed'; respond() cancels the hold and emits nothing", async () => { + const short = new HookRelay((relay as any).pipeName + '-closed', 100); + await short.start(); + const reasons: (string | undefined)[] = []; + short.on('permission-expired', (_s, _r, reason) => reasons.push(reason)); + + const net = await import('net'); + const c1 = net.createConnection((short as any).pipeName); + await new Promise((res, rej) => { c1.on('connect', res); c1.on('error', rej); }); + const evt = new Promise((resolve) => short.once('hook-event', resolve)); + c1.write(JSON.stringify({ hook_event_name: 'PermissionRequest', _desktop_session_id: 'sess-c' }) + '\n'); + const e1 = await evt; + c1.destroy(); // far end dies + await new Promise((r) => setTimeout(r, 30)); + expect(reasons).toEqual(['hook-closed']); + + const c2 = net.createConnection((short as any).pipeName); + await new Promise((res, rej) => { c2.on('connect', res); c2.on('error', rej); }); + const evt2 = new Promise((resolve) => short.once('hook-event', resolve)); + c2.write(JSON.stringify({ hook_event_name: 'PermissionRequest', _desktop_session_id: 'sess-d' }) + '\n'); + const e2 = await evt2; + short.respond(e2.payload._requestId, { decision: { behavior: 'deny' } }); + await new Promise((r) => setTimeout(r, 150)); // past holdMs — timer must be dead + expect(reasons).toEqual(['hook-closed']); // respond() itself emits nothing (caller's job) + short.stop(); + c2.destroy(); + }); + }); }); diff --git a/desktop/tests/permission-timeout-margins.test.ts b/desktop/tests/permission-timeout-margins.test.ts index 29d1eb5f4..742756eab 100644 --- a/desktop/tests/permission-timeout-margins.test.ts +++ b/desktop/tests/permission-timeout-margins.test.ts @@ -53,4 +53,10 @@ describe('permission timeout tier margins (2026-07-30 spec §1)', () => { expect(v).toBeLessThan(2147483647); // overflow fires IMMEDIATELY — the bug, disguised } }); + + const appHold = () => literal('desktop/src/main/hook-relay.ts', /APP_HOLD_MS = ([\d_]+)/); + it('app hold is 2h and strictly under the relay backstop', () => { + expect(appHold()).toBe(7200000); + expect(appHold()).toBeLessThanOrEqual(desktopRelay() - 15 * 60 * 1000); + }); }); From 5cad204b2c63f4c14e2638f3500d6a5f87ce038a Mon Sep 17 00:00:00 2001 From: Destin Date: Fri, 31 Jul 2026 14:11:26 -0700 Subject: [PATCH 15/20] test(workbench): register permission_expired in the fixture-kind guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Task 6 added the permission_expired line kind to the fixture loader but not to this guard's mirror list, so the guard correctly failed. Unregistered kinds are SILENTLY SKIPPED by the loader — which is exactly what this test protects. Co-Authored-By: Claude Fable 5 --- desktop/tests/workbench-fixture-actions.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/desktop/tests/workbench-fixture-actions.test.ts b/desktop/tests/workbench-fixture-actions.test.ts index a884df487..bb8778deb 100644 --- a/desktop/tests/workbench-fixture-actions.test.ts +++ b/desktop/tests/workbench-fixture-actions.test.ts @@ -12,7 +12,7 @@ const FIXTURE_ROOT = join(__dirname, '../src/renderer/dev/workbench/fixtures'); * timeline with no error — which is exactly the failure this guards. */ const KNOWN_KINDS = new Set([ 'text', 'user_message', 'assistant_text', 'tool_use', 'tool_result', - 'permission_request', + 'permission_request', 'permission_expired', ]); function fixtureFiles(dir: string): Array<{ name: string; raw: string }> { From 7ee401071dd6faf1bcf8547773c49edd8aac264f Mon Sep 17 00:00:00 2001 From: Destin Date: Fri, 31 Jul 2026 14:25:11 -0700 Subject: [PATCH 16/20] feat(android): EventBridge 2h permission hold + reasoned PermissionExpired MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Task 8 of the permission-ask-timeout plan — Android half of Task 7's desktop hold timer. EventBridge now runs its own tier-1 hold (PERMISSION_HOLD_MS = 7_200_000L, mirrors desktop's APP_HOLD_MS) per PermissionRequest socket: if 2h pass with no user response, the app itself sends a labeled deny (nested {decision:{behavior:"deny"}} shape — flat would ship decision:undefined through hook-relay-blocking.js) and emits PermissionExpired(reason = "app-timeout"). This must stay strictly under the relay asset's 2h30m backstop and Claude Code's 3h hook-entry timeout, or CC wins and kills the hook with no decision, wedging AskUserQuestion forever (its own timeout defaults to never) — margins are pinned by the extended permission-timeout-margins.test.ts. PermissionExpired now carries an optional `reason` ("app-timeout", "delivery-failed", "hook-closed", or null) threaded through HookEvent, HookSerializer (as `_reason` inside the payload, same convention as desktop's main.ts so the shared hook-dispatcher parses both transports), and ManagedSession's broadcast — keyed on requestId/session id, never the raw event's sessionId (EventBridge's write-failure path emits that as ""). Every path that can end a pending request (respond, closeSocket, the closure monitor, stop) cancels the hold job so a live 2h coroutine never outlives its socket. Fixed a double-emit the brief's sample code would have had: respond() now returns Boolean so the hold-timeout path only emits "app-timeout" when the deny actually went out, since respond() already emits "delivery-failed" itself on a write failure. No routability gate added — EventBridge is per-session (one per PtyBridge), so an ask on a session's own socket is routable by construction; desktop's unroutable cap has no Android analogue. Co-Authored-By: Claude Opus 5 --- .../com/youcoded/app/bridge/HookSerializer.kt | 5 +- .../com/youcoded/app/parser/EventBridge.kt | 70 ++++++++++++++++++- .../com/youcoded/app/parser/HookEvent.kt | 11 +-- .../youcoded/app/runtime/ManagedSession.kt | 12 ++-- .../youcoded/app/bridge/HookSerializerTest.kt | 11 +++ .../tests/permission-timeout-margins.test.ts | 8 +++ 6 files changed, 105 insertions(+), 12 deletions(-) diff --git a/app/src/main/kotlin/com/youcoded/app/bridge/HookSerializer.kt b/app/src/main/kotlin/com/youcoded/app/bridge/HookSerializer.kt index 4bc670e7b..1145627e4 100644 --- a/app/src/main/kotlin/com/youcoded/app/bridge/HookSerializer.kt +++ b/app/src/main/kotlin/com/youcoded/app/bridge/HookSerializer.kt @@ -30,9 +30,12 @@ object HookSerializer { return envelope("PermissionRequest", sessionId, inner) } - fun permissionExpired(sessionId: String, requestId: String): JSONObject { + fun permissionExpired(sessionId: String, requestId: String, reason: String? = null): JSONObject { val inner = JSONObject().apply { put("_requestId", requestId) + // _reason rides inside the payload — same convention as desktop + // main.ts, so the shared hook-dispatcher parses both transports. + if (reason != null) put("_reason", reason) } return envelope("PermissionExpired", sessionId, inner) } diff --git a/app/src/main/kotlin/com/youcoded/app/parser/EventBridge.kt b/app/src/main/kotlin/com/youcoded/app/parser/EventBridge.kt index 89c2c7ada..1549778cb 100644 --- a/app/src/main/kotlin/com/youcoded/app/parser/EventBridge.kt +++ b/app/src/main/kotlin/com/youcoded/app/parser/EventBridge.kt @@ -19,12 +19,25 @@ import java.util.concurrent.ConcurrentHashMap * structured decision back through it (blocking relay protocol). */ class EventBridge(private val socketName: String) { + companion object { + /** §1 tier-1 hold (2h). Must stay UNDER the relay asset's 2h30m and + * Bootstrap's 3h CC hook timeout — margins are load-bearing (the + * losing order kills the hook with no decision and AskUserQuestion + * wedges forever). Pinned by desktop/tests/permission-timeout-margins. */ + const val PERMISSION_HOLD_MS = 7_200_000L + } + private val _events = MutableSharedFlow(extraBufferCapacity = 1000) val events: SharedFlow = _events /** Sockets held open for blocking PermissionRequest responses. */ private val pendingSockets = ConcurrentHashMap() + /** Tier-1 hold timers, keyed by requestId. Cancelled on every path that + * ends a request (respond, closeSocket, closure monitor, stop) so a + * 2h coroutine never outlives the socket it was guarding. */ + private val holdJobs = ConcurrentHashMap() + /** Maps mobile session IDs to Claude Code session IDs. */ private val sessionIdMap = ConcurrentHashMap() @@ -119,6 +132,35 @@ class EventBridge(private val socketName: String) { // hook-relay-blocking.js times out or Claude Code kills the hook. // Desktop equivalent: hook-relay.ts socket.on('close') handler. monitorSocketClosure(requestId, sessionId, client) + + // §1 tier-1: the app ends the wait with a labeled deny. + // Must emit explicitly — respond() removes the pending + // entry BEFORE closing, so the closure monitor stays + // silent for app-initiated endings (its own comment). + monitorScope?.launch(Dispatchers.IO) { + delay(PERMISSION_HOLD_MS) + holdJobs.remove(requestId) + if (pendingSockets.containsKey(requestId)) { + // Nested decision shape is load-bearing: the relay + // reads appDecision.decision. Message lands in the + // tool result the model reads. + val deny = JSONObject().put("decision", JSONObject() + .put("behavior", "deny") + .put("message", "YouCoded auto-denied this request after 2 hours with no user response — ask again if still needed.")) + // Only emit "app-timeout" if the deny actually went out. If + // the write failed, respond() already emitted its own + // "delivery-failed" PermissionExpired — emitting again here + // would violate "at most one expiry per request". + if (respond(requestId, deny)) { + _events.tryEmit(HookEvent.PermissionExpired( + sessionId = sessionId, + hookEventName = "PermissionExpired", + requestId = requestId, + reason = "app-timeout", + )) + } + } + }?.also { holdJobs[requestId] = it } } else { pendingSockets.remove(requestId) client.close() @@ -165,10 +207,14 @@ class EventBridge(private val socketName: String) { // responded to — emit PermissionExpired to clean up the React UI. if (pendingSockets.remove(requestId) != null) { try { client.close() } catch (_: Exception) {} + // Far-end death (relay backstop or CC killing the hook), not our + // own hold firing — cancel the hold job so it doesn't also emit. + holdJobs.remove(requestId)?.cancel() if (!_events.tryEmit(HookEvent.PermissionExpired( sessionId = sessionId, hookEventName = "PermissionExpired", requestId = requestId, + reason = "hook-closed", ))) { android.util.Log.e("EventBridge", "Event buffer full, dropped PermissionExpired") } @@ -176,18 +222,29 @@ class EventBridge(private val socketName: String) { } } - /** Send a decision back through a held PermissionRequest socket. */ - fun respond(requestId: String, decision: JSONObject) { + /** + * Send a decision back through a held PermissionRequest socket. + * Returns true if the write succeeded, false if it failed (in which case + * this method has ALREADY emitted a "delivery-failed" PermissionExpired + * itself — callers must not emit a second one for the same requestId, or + * "at most one expiry per request" breaks for the hold-timeout path). + */ + fun respond(requestId: String, decision: JSONObject): Boolean { + // A decision is about to be delivered (or attempted) — the tier-1 + // hold is no longer needed. Cancel first so it can never race a + // second emit for the same request. + holdJobs.remove(requestId)?.cancel() val socket = pendingSockets.remove(requestId) if (socket == null) { android.util.Log.e("EventBridge", "No pending socket for requestId=$requestId") - return + return false } try { val payload = decision.toString() + "\n" socket.outputStream.write(payload.toByteArray()) socket.outputStream.flush() socket.close() + return true } catch (e: Exception) { // Response couldn't be delivered — permission effectively expired. // Emit PermissionExpired so React UI clears the stale approval card. @@ -197,12 +254,15 @@ class EventBridge(private val socketName: String) { sessionId = "", // ManagedSession uses its own ID for broadcast hookEventName = "PermissionExpired", requestId = requestId, + reason = "delivery-failed", )) + return false } } /** Close a held socket without sending a response (cross-path cleanup). */ fun closeSocket(requestId: String) { + holdJobs.remove(requestId)?.cancel() val socket = pendingSockets.remove(requestId) ?: return try { socket.close() } catch (_: Exception) {} } @@ -219,6 +279,10 @@ class EventBridge(private val socketName: String) { fun hasPendingPermission(): Boolean = pendingSockets.isNotEmpty() fun stop() { + // Cancel any outstanding tier-1 hold timers so they don't fire (and + // try to write to a socket we're about to close) after teardown. + holdJobs.values.forEach { it.cancel() } + holdJobs.clear() // Close all pending sockets for ((_, socket) in pendingSockets) { try { socket.close() } catch (_: Exception) {} diff --git a/app/src/main/kotlin/com/youcoded/app/parser/HookEvent.kt b/app/src/main/kotlin/com/youcoded/app/parser/HookEvent.kt index 072a59d27..659352766 100644 --- a/app/src/main/kotlin/com/youcoded/app/parser/HookEvent.kt +++ b/app/src/main/kotlin/com/youcoded/app/parser/HookEvent.kt @@ -56,14 +56,17 @@ sealed class HookEvent { val requestId: String, ) : HookEvent() - /** Emitted when a held PermissionRequest socket closes before a response - * was sent — e.g., hook-relay-blocking.js timed out (2h30m backstop) or Claude Code - * killed the hook process. React uses this to clear stale approval cards. - * Desktop equivalent: hook-relay.ts socket.on('close') → 'permission-expired'. */ + /** Emitted when a held PermissionRequest ends without a delivered user + * decision. reason discriminates (2026-07-30 spec §2): "app-timeout" + * (our own 2h hold fired — a deny WAS delivered), "delivery-failed" + * (respond() write threw), "hook-closed" (relay died / CC killed the + * hook — the terminal menu may still be live; React retains the card), + * or null (legacy producers; React resolves). */ data class PermissionExpired( override val sessionId: String, override val hookEventName: String, val requestId: String, + val reason: String? = null, ) : HookEvent() companion object { diff --git a/app/src/main/kotlin/com/youcoded/app/runtime/ManagedSession.kt b/app/src/main/kotlin/com/youcoded/app/runtime/ManagedSession.kt index 8c66a933a..e2e597691 100644 --- a/app/src/main/kotlin/com/youcoded/app/runtime/ManagedSession.kt +++ b/app/src/main/kotlin/com/youcoded/app/runtime/ManagedSession.kt @@ -253,13 +253,17 @@ class ManagedSession( )) } is HookEvent.PermissionExpired -> { - // Socket closed before user responded — relay timed out - // or Claude Code killed the hook. Clear the stale approval - // card in React UI. Desktop equivalent: main.ts - // hookRelay.on('permission-expired') handler. + // Ended without a delivered decision (app hold fired, + // respond() write failed, or the relay/hook died). + // reason discriminates which — React uses it to decide + // whether to retain or resolve the stale approval card. + // Routing keys on requestId / this session's own `id`, + // NEVER event.sessionId (EventBridge's write-failure + // path emits that as "" — see EventBridge.kt). server.broadcast(HookSerializer.permissionExpired( sessionId = id, requestId = event.requestId, + reason = event.reason, )) } is HookEvent.Notification -> { diff --git a/app/src/test/kotlin/com/youcoded/app/bridge/HookSerializerTest.kt b/app/src/test/kotlin/com/youcoded/app/bridge/HookSerializerTest.kt index bf21d8c21..a601c5831 100644 --- a/app/src/test/kotlin/com/youcoded/app/bridge/HookSerializerTest.kt +++ b/app/src/test/kotlin/com/youcoded/app/bridge/HookSerializerTest.kt @@ -140,6 +140,17 @@ class HookSerializerTest { assertFalse("Should not have 'requestId'", inner.has("requestId")) } + @Test + fun `permissionExpired carries _reason when present and omits it when null`() { + val with = HookSerializer.permissionExpired("s1", "r1", "hook-closed") + val inner = with.getJSONObject("payload").getJSONObject("payload") + assertEquals("hook-closed", inner.getString("_reason")) + + val without = HookSerializer.permissionExpired("s1", "r1", null) + val innerNone = without.getJSONObject("payload").getJSONObject("payload") + assertFalse(innerNone.has("_reason")) + } + // ── notification ───────────────────────────────────────────────────────── @Test diff --git a/desktop/tests/permission-timeout-margins.test.ts b/desktop/tests/permission-timeout-margins.test.ts index 742756eab..64d9176cf 100644 --- a/desktop/tests/permission-timeout-margins.test.ts +++ b/desktop/tests/permission-timeout-margins.test.ts @@ -59,4 +59,12 @@ describe('permission timeout tier margins (2026-07-30 spec §1)', () => { expect(appHold()).toBe(7200000); expect(appHold()).toBeLessThanOrEqual(desktopRelay() - 15 * 60 * 1000); }); + + const androidHold = () => literal( + 'app/src/main/kotlin/com/youcoded/app/parser/EventBridge.kt', + /PERMISSION_HOLD_MS = ([\d_]+)L/); + it('android hold is 2h and strictly under the relay backstop', () => { + expect(androidHold()).toBe(7200000); + expect(androidHold()).toBeLessThanOrEqual(androidRelay() - 15 * 60 * 1000); + }); }); From b5341ebe9ea7ff081ed9b9ef14b7bbd7f0cddf6a Mon Sep 17 00:00:00 2001 From: Destin Date: Fri, 31 Jul 2026 14:36:54 -0700 Subject: [PATCH 17/20] feat(chat): digit rebind for expired permission cards (digit-gated, menu-labeled) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Task 9 of the permission-ask-timeout plan. Expired cards (hook socket dead, Ink menu possibly still live) can now drive that live menu directly: a new rebindButtons() gate in ink-select-parser.ts refuses the whole rebind unless every option in the fresh parse carries a bare digit (never mixes the digit and arrow+\r write shapes) and never rebinds AskUserQuestion (its TUI is sequential/multi-select with a Skip and free-text box this card doesn't model). ToolCard's Task 5 expired branch is replaced by ExpiredApprovalActions, which polls the terminal buffer every 2s, renders the MENU'S OWN option labels (never the card's original buttons, ruling out a "click No, land on don't-ask-again" misfire), and writes the digit as a deliberate menu-driving PTY passthrough that deliberately bypasses pty-input-gate.ts (same precedent as PlanApprovalButtons — but NOT its known-broken arrow+\r-in-one-write shape). The click never resolves the card itself; resolution stays owned by usePromptDetector's menu-absence rule, so a write that didn't land leaves the card up and the buttons re-arm. Also documents the invariant that makes the branch's "answer it in the terminal" copy safe: `expired` is only ever set on the hook/PTY permission path today, never by the native broker, so a future change that broke that exclusivity needs to be caught here. Co-Authored-By: Claude Opus 5 --- desktop/src/renderer/components/ToolCard.tsx | 149 +++++++++++++++--- .../src/renderer/parser/ink-select-parser.ts | 22 +++ desktop/tests/ink-select-parser.test.ts | 37 ++++- 3 files changed, 186 insertions(+), 22 deletions(-) diff --git a/desktop/src/renderer/components/ToolCard.tsx b/desktop/src/renderer/components/ToolCard.tsx index 94dedcb7f..266437508 100644 --- a/desktop/src/renderer/components/ToolCard.tsx +++ b/desktop/src/renderer/components/ToolCard.tsx @@ -9,6 +9,9 @@ import { isAndroid } from '../platform'; import ToolBody from './tool-views/ToolBody'; import { useExpandAllToggle, getInitialExpanded } from '../hooks/useExpandAllToggle'; import { isTypingTarget } from '../utils/is-typing-target'; +import { getVisibleScreenText } from '../hooks/terminal-registry'; +import { parseInkSelect, rebindButtons, type PromptButton } from '../parser/ink-select-parser'; +import type { ChatAction } from '../state/chat-types'; // --- Helpers for friendly display --- @@ -495,6 +498,115 @@ function PlanApprovalButtons({ requestId, sessionId, onResponded }: { ); } +// --- Expired-card rebind UI (2026-07-30 spec §3) --- +// A card lands here only after Task 4's retention path sets `expired`: the +// hook socket died but CC's Ink menu may STILL be live in the terminal. +// Re-parse the visible buffer periodically and, if the parse yields a menu +// whose every option carries a bare digit, render THAT menu's own labels as +// buttons that type the digit straight into the PTY. + +/** How often to re-parse the terminal buffer while an expired card is on + * screen — cheap because this component only ever mounts on expired cards, + * which are rare and short-lived (the menu-absence rule below resolves them + * within a couple of buffer flushes once the digit lands). */ +const REBIND_POLL_MS = 2000; + +/** How long a clicked rebind button stays disabled before re-arming. The + * click does NOT resolve the card — nothing here confirms the write + * landed, since the hook socket is dead and no response can ever arrive on + * it. Resolution comes exclusively from usePromptDetector's menu-absence + * rule (Task 4): once the Ink menu has been gone from the buffer for two + * consecutive flushes, it dispatches PERMISSION_CARD_RESOLVED. So a + * successful digit write makes the menu disappear, which resolves the card + * on its own; a write that didn't land leaves the menu (and this card) up, + * and the buttons must be clickable again — hence re-arming on a timer + * rather than on any signal we'd have to invent. */ +const REBIND_REARM_MS = 2000; + +/** Expired-card actions. Re-parses the live buffer every REBIND_POLL_MS: if + * CC's menu is still up AND every option has a digit (rebindButtons' + * gate — see its own header for why), render the MENU'S OWN options as + * digit-writing buttons; otherwise fall back to copy + Dismiss only (the + * only case in the Workbench, which has no real terminal buffer to parse). + * + * The digit write is a DELIBERATE menu-driving write — like + * PlanApprovalButtons and TrustGate, it must NOT go through + * pty-input-gate.ts (see that module's header: "driving the menu is their + * whole purpose"). Unlike PlanApprovalButtons' `DOWN.repeat(i) + '\r'` (a + * known-broken write shape — arrows in a write that ends with `\r` are + * discarded, see menuToButtons' header — already filed separately, not + * fixed here), this only ever sends a bare digit: select-and-submit in one + * byte, no `\r`, nothing for CC to collapse. */ +function ExpiredApprovalActions({ sessionId, tool, dispatch }: { + sessionId?: string; + tool: ToolCallState; + dispatch: React.Dispatch; +}) { + const [buttons, setButtons] = useState(null); + const [clicked, setClicked] = useState(false); + const rearmTimerRef = useRef | null>(null); + + useEffect(() => { + if (!sessionId) return; + const parse = () => { + const screen = getVisibleScreenText(sessionId); + setButtons(rebindButtons(screen ? parseInkSelect(screen) : null, tool.toolName)); + }; + parse(); + const poll = setInterval(parse, REBIND_POLL_MS); + return () => clearInterval(poll); + }, [sessionId, tool.toolName]); + + // Cleanup: don't let a stale re-arm timer fire setState after this card + // has already been resolved and unmounted. + useEffect(() => () => { + if (rearmTimerRef.current) clearTimeout(rearmTimerRef.current); + }, []); + + const resolveLocally = () => { + if (!sessionId) return; + const action: ChatAction = { type: 'PERMISSION_CARD_RESOLVED', sessionId, toolUseId: tool.toolUseId }; + dispatch(action); + (window as any).claude?.remote?.broadcastAction?.(action); + }; + + const writeDigit = (button: PromptButton) => { + if (!sessionId || clicked) return; + setClicked(true); + window.claude.session.sendInput(sessionId, button.input); + rearmTimerRef.current = setTimeout(() => setClicked(false), REBIND_REARM_MS); + }; + + return ( +
+

+ The buttons on this card timed out, but Claude may still be waiting + in the terminal.{buttons + ? ' These options come straight from the terminal menu:' + : ' Answer it there — or dismiss this if you already did.'} +

+ {buttons && ( +
+ {buttons.map((b) => ( + + ))} +
+ )} + +
+ ); +} + // --- AskUserQuestion UI --- // Claude Code's AskUserQuestion tool sends 1-4 multiple-choice questions. // Unlike regular tools (allow/deny), we must collect the user's selections @@ -784,29 +896,24 @@ export default React.memo(function ToolCard({ tool, sessionId, inGroup = false } {/* Permission / AskUserQuestion / ExitPlanMode UI */} {tool.status === 'awaiting-approval' && (tool.requestId || tool.expired) && (() => { // Expired: the hook socket is dead (requestId cleared) but CC's Ink - // menu may still be live in the terminal. Task 9 adds digit-rebind - // buttons here; Dismiss is the universal out (only out for - // AskUserQuestion, which never rebinds — spec §3/§1b). Copy says what - // is known (buttons stopped working), not what is assumed (the ask - // failed) — the user may have already answered in the terminal. + // menu may still be live in the terminal. ExpiredApprovalActions + // (Task 9) re-parses the buffer and offers digit-rebind buttons when + // safe; Dismiss is the universal out (only out for AskUserQuestion, + // which never rebinds — spec §3/§1b). Copy says what is known + // (buttons stopped working), not what is assumed (the ask failed) — + // the user may have already answered in the terminal. + // + // WHY "answer it in the terminal" is safe copy: `expired` is only + // ever set by the hook/PTY permission path ('hook-closed' reason, + // src/shared/types.ts). The native permission broker never emits + // that reason, and native sessions have no terminal at all — so an + // expired card always implies a live PTY the user can act in. If a + // future change makes native cards expire too, this copy (and the + // rebind attempt below, which needs a real screen buffer) goes wrong + // silently — check that invariant still holds before touching it. if (tool.expired || !tool.requestId) { - const resolveLocally = () => { - if (!sessionId) return; - const action = { type: 'PERMISSION_CARD_RESOLVED' as const, sessionId, toolUseId: tool.toolUseId }; - dispatch(action); - (window as any).claude?.remote?.broadcastAction(action); - }; return ( -
-

- The buttons on this card timed out, but Claude may still be - waiting in the terminal. Answer it there — or dismiss this if - you already did. -

- -
+ ); } // AskUserQuestion needs its own UI with option selection instead of Yes/No diff --git a/desktop/src/renderer/parser/ink-select-parser.ts b/desktop/src/renderer/parser/ink-select-parser.ts index c7804bf96..c9125a963 100644 --- a/desktop/src/renderer/parser/ink-select-parser.ts +++ b/desktop/src/renderer/parser/ink-select-parser.ts @@ -384,3 +384,25 @@ export function menuToButtons(menu: ParsedMenu): PromptButton[] { return { label, input: DOWN.repeat(steps), submitInput: '\r' }; }); } + +/** §3 rebind gate (2026-07-30 spec): expired-card buttons may drive the + * still-live menu ONLY when every option carries a bare digit. A digit + * selects-and-submits in one byte with no cursor dependency (header above). + * Any arrow-fallback option disqualifies the whole menu — its DOWN math + * depends on selectedIndex freshness, which goes stale the moment the card + * is expired (nothing is left maintaining it), and a mixed row would also + * mix write shapes on one render. AskUserQuestion NEVER rebinds: CC's TUI + * for it is sequential (Q1 then Q2), multi-select, with a Skip and + * free-text box our card doesn't model — replaying that blind has a + * wrong-answer failure mode strictly worse than no feature. Its outs stay + * Dismiss + terminal view. Labels always come from THIS parse (menu.options), + * never from the card's original buttons — matching by index or guess is + * the exact misfire class ("click No, land on don't-ask-again") this gate + * exists to rule out. */ +export function rebindButtons(menu: ParsedMenu | null, toolName: string): PromptButton[] | null { + if (!menu) return null; + if (toolName === 'AskUserQuestion') return null; + const buttons = menuToButtons(menu); + if (buttons.some((b) => b.submitInput !== undefined)) return null; + return buttons; +} diff --git a/desktop/tests/ink-select-parser.test.ts b/desktop/tests/ink-select-parser.test.ts index f4ea83940..9d4229398 100644 --- a/desktop/tests/ink-select-parser.test.ts +++ b/desktop/tests/ink-select-parser.test.ts @@ -1,5 +1,6 @@ import { describe, it, expect } from 'vitest'; -import { parseInkSelect, menuToButtons } from '../src/renderer/parser/ink-select-parser'; +import { parseInkSelect, menuToButtons, rebindButtons } from '../src/renderer/parser/ink-select-parser'; +import type { ParsedMenu } from '../src/renderer/parser/ink-select-parser'; describe('ink-select-parser', () => { describe('parseInkSelect', () => { @@ -423,4 +424,38 @@ Pick a follow-up: expect(menu.options).toEqual(['as is', 'from summary']); }); }); + + describe('rebindButtons (spec §3 gates)', () => { + const digitMenu: ParsedMenu = { + id: 'm1', title: 'Bash command', options: ['Yes', "Yes, and don't ask again", 'No'], + optionNumbers: [1, 2, 3], selectedIndex: 0, + }; + + it('returns digit-writing buttons for a fully-numbered single-select menu', () => { + const btns = rebindButtons(digitMenu, 'Bash')!; + expect(btns.map((b) => b.input)).toEqual(['1', '2', '3']); + expect(btns.every((b) => b.submitInput === undefined)).toBe(true); + // Labels come from the fresh parse, not any guessed/matched original — + // this is what rules out the "click No, land on don't-ask-again" misfire. + expect(btns.map((b) => b.label)).toEqual(digitMenu.options); + }); + + it('rejects a menu with ANY arrow-fallback option — never mix write shapes', () => { + const partial: ParsedMenu = { ...digitMenu, optionNumbers: [1, null, 3] }; + expect(rebindButtons(partial, 'Bash')).toBeNull(); + }); + + it('rejects a menu with NO digits at all (pure arrow-fallback)', () => { + const noDigits: ParsedMenu = { ...digitMenu, optionNumbers: undefined }; + expect(rebindButtons(noDigits, 'Bash')).toBeNull(); + }); + + it('never rebinds AskUserQuestion — sequential multi-question TUI with Skip/free-text', () => { + expect(rebindButtons(digitMenu, 'AskUserQuestion')).toBeNull(); + }); + + it('null menu → null', () => { + expect(rebindButtons(null, 'Bash')).toBeNull(); + }); + }); }); From 5fb40acdb2d7056e86c0c423d89b8e2bb7d1bcac Mon Sep 17 00:00:00 2001 From: Destin Date: Fri, 31 Jul 2026 14:50:09 -0700 Subject: [PATCH 18/20] feat(chat): send-refusal copy names the blocking approval card MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Task 10 of the permission-ask-timeout plan (§4). The gate now refuses sends for up to 2h (Task 9's app-owned hold) instead of the prior 5-min expiry, and an expired card keeps blocking rather than clearing — a generic "Claude is waiting for your response" reads as a mystery lock at that length, and pushes a stuck-feeling user toward InputBar's "Send anyway" force path, which writes straight into the live Ink menu. Adds pendingInteractionKind (pty-input-gate.ts), a pure predicate with the same scan order as hasPendingInteraction so the two can never disagree about whether something is blocking, plus pendingInteractionRefusalCopy as the single copy source both refusal sites read from. An 'approval' card now names itself and its Dismiss out ("Claude asked a question — answer or dismiss the card in the chat before sending") — copy that stays accurate even once the card has expired. Scraped terminal prompts (trust/resume dialogs, no chat card) keep the plainer "answer the prompt first" phrasing. Updated all three sites that show this copy: App.tsx's notifyIfPtyBlocked (command/skill/slash sends), the onSendBlocked toast wired to ChatInputBar (the actual "Send anyway" force-path toast typed messages hit), and InputBar's toast fallback for callers that don't wire onSendBlocked. Co-Authored-By: Claude Opus 5 --- desktop/src/renderer/App.tsx | 18 ++++++-- desktop/src/renderer/components/InputBar.tsx | 7 +++- .../state/__tests__/chat-reducer.test.ts | 13 +++++- desktop/src/renderer/state/pty-input-gate.ts | 41 +++++++++++++++++++ 4 files changed, 73 insertions(+), 6 deletions(-) diff --git a/desktop/src/renderer/App.tsx b/desktop/src/renderer/App.tsx index 8bdfb9ac1..3ce368636 100644 --- a/desktop/src/renderer/App.tsx +++ b/desktop/src/renderer/App.tsx @@ -38,7 +38,7 @@ import { dispatchSlashCommand, type DispatcherResult } from './state/slash-comma import { runNativeSlashAction, routeSlashResult } from './state/native-slash-actions'; import { GameProvider, useGameState, useGameDispatch } from './state/game-context'; import { hookEventToAction } from './state/hook-dispatcher'; -import { hasPendingInteraction, canPtySend } from './state/pty-input-gate'; +import { hasPendingInteraction, pendingInteractionKind, pendingInteractionRefusalCopy, canPtySend } from './state/pty-input-gate'; import { buildOutgoingMessage } from './components/outgoing-message'; import type { SyncWarning } from '../main/sync-state'; import { usePromptDetector } from './hooks/usePromptDetector'; @@ -526,7 +526,11 @@ function AppInner() { const notifyIfPtyBlocked = useCallback((sid: string): boolean => { const session = chatStateMapRef.current.get(sid); if (session && hasPendingInteraction(session)) { - setToast('Claude is waiting for your response — answer the prompt first.'); + // Name the blocker — under the 2h app-owned hold (Task 9) a generic + // line reads as a mystery lock once it's been sitting there a while. + // An 'approval' card carries its own outs (answer / Dismiss) even + // after it's expired, so point at it instead of the generic copy. + setToast(pendingInteractionRefusalCopy(pendingInteractionKind(session))); return true; } return false; @@ -2935,7 +2939,15 @@ function AppInner() { {/* TerminalToolbar (Esc/Tab/Ctrl/arrows) now renders inside ChatInputBar when minimal={isTerminalTouch}, slotted in the QuickChips position so both modes share one container. */} - setResumeRequested(true)} getUsageSnapshot={getUsageSnapshot} onOpenPreferences={() => setPreferencesOpen(true)} onToast={(msg) => setToast(msg)} onSendBlocked={(retry) => setToast({ message: 'Claude is waiting for your response — answer the prompt first.', durationMs: 8000, action: { label: 'Send anyway', onClick: () => { setToast(null); retry(); } } })} getSessionState={(sid) => chatStateMapRef.current.get(sid)} onOpenModelPicker={() => setModelPickerOpen(true)} initialInput={currentSession?.initialInput} provider={currentSession?.provider} /> + setResumeRequested(true)} getUsageSnapshot={getUsageSnapshot} onOpenPreferences={() => setPreferencesOpen(true)} onToast={(msg) => setToast(msg)} onSendBlocked={(retry) => { + // "Send anyway" writes straight into the live Ink menu (the + // exact stray-input class this gate exists to prevent) — + // name the actual blocker so reaching for it is an informed + // choice, not a reflex against a generic-sounding lock. + const blockedSession = chatStateMapRef.current.get(sessionId ?? ''); + const message = blockedSession ? pendingInteractionRefusalCopy(pendingInteractionKind(blockedSession)) : 'Claude is waiting for your response — answer the prompt first.'; + setToast({ message, durationMs: 8000, action: { label: 'Send anyway', onClick: () => { setToast(null); retry(); } } }); + }} getSessionState={(sid) => chatStateMapRef.current.get(sid)} onOpenModelPicker={() => setModelPickerOpen(true)} initialInput={currentSession?.initialInput} provider={currentSession?.provider} /> (function InputBar({ sessionId sendRef.current(true); }); } else { - onToast?.('Claude is waiting for your response — answer the prompt first.'); + // Same copy as the onSendBlocked branch above (and App.tsx's + // notifyIfPtyBlocked) — pendingInteractionRefusalCopy is the one + // source both refusal sites read from so they can't drift apart. + onToast?.(pendingInteractionRefusalCopy(pendingInteractionKind(session))); } return false; } diff --git a/desktop/src/renderer/state/__tests__/chat-reducer.test.ts b/desktop/src/renderer/state/__tests__/chat-reducer.test.ts index 45394c991..ea78d1507 100644 --- a/desktop/src/renderer/state/__tests__/chat-reducer.test.ts +++ b/desktop/src/renderer/state/__tests__/chat-reducer.test.ts @@ -3,7 +3,7 @@ import { chatReducer } from '../chat-reducer'; import { createSessionChatState, serializeChatState } from '../chat-types'; import type { ChatState } from '../chat-types'; import type { ToolCallState } from '../../../shared/types'; -import { hasPendingInteraction, canRetrySubmit } from '../pty-input-gate'; +import { hasPendingInteraction, canRetrySubmit, pendingInteractionKind } from '../pty-input-gate'; function stateWithInFlightTurn(sessionId = 'sess-1', turnId = 'turn-1'): ChatState { const session = createSessionChatState(); @@ -928,4 +928,15 @@ describe('PERMISSION_EXPIRED reasons (2026-07-30 spec §2/§2a/§2c/§2d)', () = expect(tool.status).toBe('failed'); expect(tool.error).toBe('Some real failure'); }); + + // Task 10 (2026-07-30 spec §4): the send-refusal copy needs to distinguish + // an awaiting-approval card (has a Dismiss out, names itself accurately + // even once expired) from a scraped terminal prompt (no card, no Dismiss). + it('pendingInteractionKind distinguishes approval cards from scraped prompts', () => { + const withCard = withPendingAsk(); // Task 3 helper — awaiting-approval tool + expect(pendingInteractionKind(withCard.get('s1')!)).toBe('approval'); + const expired = expire(withCard, { reason: 'hook-closed' }); + expect(pendingInteractionKind(expired.get('s1')!)).toBe('approval'); // expired still blocks + expect(pendingInteractionKind(emptySession().get('s1')!)).toBeNull(); + }); }); diff --git a/desktop/src/renderer/state/pty-input-gate.ts b/desktop/src/renderer/state/pty-input-gate.ts index ec061bd6e..7fff08435 100644 --- a/desktop/src/renderer/state/pty-input-gate.ts +++ b/desktop/src/renderer/state/pty-input-gate.ts @@ -46,6 +46,47 @@ export function hasPendingInteraction(session: SessionChatState): boolean { return false; } +/** + * Which kind of interaction is blocking sends — drives the send-refusal + * copy (§4, 2026-07-30 permission-ask-timeout spec) so a 2h app-owned hold + * never reads as a generic mystery lock: an 'approval' card names itself + * and its Dismiss out even once expired; a scraped 'prompt' has neither, so + * it keeps the plainer "answer the prompt" phrasing. Same scan order and + * same source fields as hasPendingInteraction — kept as a parallel function + * (not derived from its boolean) so the two can never disagree about + * whether something is blocking. + */ +export function pendingInteractionKind(session: SessionChatState): 'approval' | 'prompt' | null { + for (const id of session.activeTurnToolIds) { + if (session.toolCalls.get(id)?.status === 'awaiting-approval') return 'approval'; + } + for (const entry of session.timeline) { + if (entry.kind === 'prompt' + && entry.prompt.promptId !== HISTORY_EXPAND_PROMPT_ID + && !entry.prompt.completed) { + return 'prompt'; + } + } + return null; +} + +/** + * Send-refusal copy for a given pendingInteractionKind — the single source + * both refusal sites (App.tsx's command/skill send guard and InputBar's + * typed-message guard, including its "Send anyway" force-path toast) pull + * from, so they cannot drift apart (§4, 2026-07-30 spec). An 'approval' card + * is named explicitly, with its Dismiss out, because it stays accurate even + * once the app-owned hold has expired the card (still blocking, no longer + * "waiting" in the live sense) — see docs/error-message-standards.md: + * specific+accurate over a generic guess. `null` falls back to the 'prompt' + * copy defensively; callers only invoke this when a kind was found. + */ +export function pendingInteractionRefusalCopy(kind: 'approval' | 'prompt' | null): string { + return kind === 'approval' + ? 'Claude asked a question — answer or dismiss the card in the chat before sending.' + : 'Claude is waiting for your response — answer the prompt first.'; +} + /** * True only when the session is observably idle enough that a recovery `\r` * (useSubmitConfirmation) cannot land on anything but CC's empty input bar: From 19f11d015074699aa4dc57f170efa0d2d46d3d0c Mon Sep 17 00:00:00 2001 From: Destin Date: Fri, 31 Jul 2026 14:58:44 -0700 Subject: [PATCH 19/20] fix(chat): route no-session refusal fallback through shared copy helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two review findings on the send-refusal message shown while a permission card blocks PTY writes: - App.tsx's onSendBlocked hardcoded pendingInteractionRefusalCopy(null)'s exact return string in its no-session fallback branch instead of calling the helper, defeating the single-source guarantee the helper exists for. Route the fallback through the helper too. - The 'approval' copy said "answer or dismiss the card in the chat" but only 2 of the 4 card shapes that collapse into 'approval' (AskUserQuestion, ExpiredApprovalActions) render a Dismiss button — the plain permission triad and ExitPlanMode do not. Reworded to "resolve the card in the chat", which is honest for all four without enumerating them (and matches the "resolve the card" phrasing already used in the design spec). --- desktop/src/renderer/App.tsx | 6 +++++- desktop/src/renderer/state/pty-input-gate.ts | 22 ++++++++++++++------ 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/desktop/src/renderer/App.tsx b/desktop/src/renderer/App.tsx index 3ce368636..23d3c9702 100644 --- a/desktop/src/renderer/App.tsx +++ b/desktop/src/renderer/App.tsx @@ -2945,7 +2945,11 @@ function AppInner() { // name the actual blocker so reaching for it is an informed // choice, not a reflex against a generic-sounding lock. const blockedSession = chatStateMapRef.current.get(sessionId ?? ''); - const message = blockedSession ? pendingInteractionRefusalCopy(pendingInteractionKind(blockedSession)) : 'Claude is waiting for your response — answer the prompt first.'; + // Route through the shared helper even in the no-session + // fallback — hardcoding its `null` output here let this + // site silently drift from the other two on any future + // copy edit (found in review of 5fb40acd). + const message = pendingInteractionRefusalCopy(blockedSession ? pendingInteractionKind(blockedSession) : null); setToast({ message, durationMs: 8000, action: { label: 'Send anyway', onClick: () => { setToast(null); retry(); } } }); }} getSessionState={(sid) => chatStateMapRef.current.get(sid)} onOpenModelPicker={() => setModelPickerOpen(true)} initialInput={currentSession?.initialInput} provider={currentSession?.provider} /> Date: Fri, 31 Jul 2026 15:24:15 -0700 Subject: [PATCH 20/20] fix(permissions): close five review findings on the ask-timeout branch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - hook-relay.ts: only emit permission-expired from the hold-timer path when respond() actually delivered the deny — mirrors Android's if (respond(...)) guard so desktop stops claiming "auto-denied" for a write that never happened. - Both platforms now derive the hold-hours count (and pluralize naturally) from their own tier constant instead of a literal "hour(s)" string (desktop) / hardcoded "2 hours" (Android), so a future tier change can't silently desync the copy. - CompactToolStrip's expired-card button now says "Dismiss — I answered in the terminal" instead of a bare "Dismiss" — matches ToolCard, since dismissing opens the PTY input gates and needs the same user assertion to be safe. - Added resolver coverage to use-prompt-detector.test.tsx (menu present/absent-once/absent-twice/live-card-bail) — this was the only thing exercising usePromptDetector's auto-resolve block, which previously had zero tests despite being the sole path that clears a retained 'hook-closed' card. - Pinned UNROUTABLE_HOLD_MS (60s dead-man cap for unroutable asks) in permission-timeout-margins.test.ts, the one tier constant that wasn't guarded against creeping into invisible-hang territory. Co-Authored-By: Claude Opus 5 --- .../com/youcoded/app/parser/EventBridge.kt | 10 +- desktop/src/main/hook-relay.ts | 23 +++- .../buddy/CompactToolStrip.test.tsx | 4 +- .../components/buddy/CompactToolStrip.tsx | 12 +- .../tests/permission-timeout-margins.test.ts | 12 ++ desktop/tests/use-prompt-detector.test.tsx | 125 +++++++++++++++++- 6 files changed, 172 insertions(+), 14 deletions(-) diff --git a/app/src/main/kotlin/com/youcoded/app/parser/EventBridge.kt b/app/src/main/kotlin/com/youcoded/app/parser/EventBridge.kt index 1549778cb..906dce85c 100644 --- a/app/src/main/kotlin/com/youcoded/app/parser/EventBridge.kt +++ b/app/src/main/kotlin/com/youcoded/app/parser/EventBridge.kt @@ -144,9 +144,17 @@ class EventBridge(private val socketName: String) { // Nested decision shape is load-bearing: the relay // reads appDecision.decision. Message lands in the // tool result the model reads. + // Fix: derive from PERMISSION_HOLD_MS instead of a + // hardcoded "2 hours" — mirrors desktop's + // hook-relay.ts deriving from APP_HOLD_MS, so a + // future tier change can't silently desync the two + // platforms' copy (permission-timeout-margins.test.ts + // still pins the underlying millisecond values). + val holdHours = PERMISSION_HOLD_MS / 3_600_000L + val hoursLabel = if (holdHours == 1L) "hour" else "hours" val deny = JSONObject().put("decision", JSONObject() .put("behavior", "deny") - .put("message", "YouCoded auto-denied this request after 2 hours with no user response — ask again if still needed.")) + .put("message", "YouCoded auto-denied this request after $holdHours $hoursLabel with no user response — ask again if still needed.")) // Only emit "app-timeout" if the deny actually went out. If // the write failed, respond() already emitted its own // "delivery-failed" PermissionExpired — emitting again here diff --git a/desktop/src/main/hook-relay.ts b/desktop/src/main/hook-relay.ts index d9b257d4b..cf1853733 100644 --- a/desktop/src/main/hook-relay.ts +++ b/desktop/src/main/hook-relay.ts @@ -107,19 +107,34 @@ export class HookRelay extends EventEmitter { // The message lands VERBATIM in the denied tool result the model // reads (verified in the CC 2.1.220 binary), so say what // happened and invite a re-ask. - this.respond(requestId, { + // Fix: pluralize naturally (was the literal "hour(s)" landing in + // the model-visible message). Android derives the same way from + // its own PERMISSION_HOLD_MS (EventBridge.kt) — keep both in + // sync if this tier value ever changes; permission-timeout- + // margins.test.ts pins the underlying numbers. + const holdHours = Math.round(this.holdMs / 3600000); + // Fix: only claim "auto-denied" if the deny actually reached the + // socket. respond() returns false when the pending entry is + // already gone or the socket is already destroyed — nothing was + // written in that case, so emitting unconditionally would assert + // a cause that never happened (docs/error-message-standards.md). + // Mirrors Android's `if (respond(requestId, deny)) { emit }` + // guard (EventBridge.kt) — the two platforms deliberately agree. + const delivered = this.respond(requestId, { decision: { behavior: 'deny', message: routable - ? `YouCoded auto-denied this request after ${Math.round(this.holdMs / 3600000)} hour(s) with no user response — ask again if still needed.` + ? `YouCoded auto-denied this request after ${holdHours} hour${holdHours === 1 ? '' : 's'} with no user response — ask again if still needed.` : 'YouCoded could not route this request to any open session — auto-denied. Ask again if still needed.', }, }); // respond() deletes the pending entry BEFORE 'close' fires, so // the close handler's wasOpen guard swallows any emit — // app-initiated endings must emit explicitly (spec §2). - this.emit('permission-expired', event.sessionId, requestId, - routable ? 'app-timeout' : 'unroutable'); + if (delivered) { + this.emit('permission-expired', event.sessionId, requestId, + routable ? 'app-timeout' : 'unroutable'); + } }, holdMs)); // When the socket closes (relay timeout, Claude Code kills hook, diff --git a/desktop/src/renderer/components/buddy/CompactToolStrip.test.tsx b/desktop/src/renderer/components/buddy/CompactToolStrip.test.tsx index cd509892f..d3d7ee7a6 100644 --- a/desktop/src/renderer/components/buddy/CompactToolStrip.test.tsx +++ b/desktop/src/renderer/components/buddy/CompactToolStrip.test.tsx @@ -52,7 +52,7 @@ describe('CompactToolStrip — expired approval card', () => { ); - expect(screen.getByText('Dismiss')).toBeInTheDocument(); + expect(screen.getByText('Dismiss — I answered in the terminal')).toBeInTheDocument(); expect(screen.queryByText('✓ Allow')).toBeNull(); expect(screen.queryByText('✕ Deny')).toBeNull(); expect(screen.queryByText('∞ Always')).toBeNull(); @@ -72,7 +72,7 @@ describe('CompactToolStrip — expired approval card', () => { ); - fireEvent.click(screen.getByText('Dismiss')); + fireEvent.click(screen.getByText('Dismiss — I answered in the terminal')); expect(broadcastAction).toHaveBeenCalledWith({ type: 'PERMISSION_CARD_RESOLVED', sessionId: 's1', diff --git a/desktop/src/renderer/components/buddy/CompactToolStrip.tsx b/desktop/src/renderer/components/buddy/CompactToolStrip.tsx index c14020903..7f57b65ec 100644 --- a/desktop/src/renderer/components/buddy/CompactToolStrip.tsx +++ b/desktop/src/renderer/components/buddy/CompactToolStrip.tsx @@ -274,9 +274,17 @@ function ToolRow({ dispatch(action); (window as any).claude?.remote?.broadcastAction(action); }} - style={denyStyle} + // Fix: this must say the same thing ToolCard's Dismiss says, not + // just "Dismiss". Clicking it opens the PTY input gates (marks + // the card complete), which is only safe because the label + // extracts an assertion that the user already answered the live + // terminal menu — a bare "Dismiss" would let a user clear + // clutter and then type, sending stray keys (+ trailing \r) + // straight into that menu. A `title` tooltip isn't enough + // either — this app runs on touch, where hover never fires. + style={{ ...denyStyle, whiteSpace: 'normal', textAlign: 'center', lineHeight: 1.3 }} > - Dismiss + Dismiss — I answered in the terminal ) : ( diff --git a/desktop/tests/permission-timeout-margins.test.ts b/desktop/tests/permission-timeout-margins.test.ts index 64d9176cf..2d37962dc 100644 --- a/desktop/tests/permission-timeout-margins.test.ts +++ b/desktop/tests/permission-timeout-margins.test.ts @@ -67,4 +67,16 @@ describe('permission timeout tier margins (2026-07-30 spec §1)', () => { expect(androidHold()).toBe(7200000); expect(androidHold()).toBeLessThanOrEqual(androidRelay() - 15 * 60 * 1000); }); + + // Unlike the other five tiers, this one has no "loses the race to CC" cap + // to check against — its whole purpose is to cap an ask whose sessionId + // matches no live session, which will never render a card anywhere. A + // large value here silently turns that into an invisible hang, so pin the + // literal (not an env-resolved value) and require it stay far below the + // routable app hold. + const unroutableHold = () => literal('desktop/src/main/hook-relay.ts', /UNROUTABLE_HOLD_MS = (\d+)/); + it('unroutable dead-man cap is 60s and far below the routable app hold', () => { + expect(unroutableHold()).toBe(60000); + expect(unroutableHold()).toBeLessThanOrEqual(appHold() / 10); + }); }); diff --git a/desktop/tests/use-prompt-detector.test.tsx b/desktop/tests/use-prompt-detector.test.tsx index 2882a0169..f30a43b3f 100644 --- a/desktop/tests/use-prompt-detector.test.tsx +++ b/desktop/tests/use-prompt-detector.test.tsx @@ -16,12 +16,15 @@ const mocks = vi.hoisted(() => ({ callbacks: [] as Array<(sid: string) => void>, screen: { text: '' }, // Minimal ChatStore stand-in (tranche 1: the detector reads the store - // directly instead of subscribing to the whole chat map). These tests drive - // the detector purely through terminal buffer events, so chat state stays - // empty — no awaiting-approval tools — and nothing ever notifies. Identity - // is stable across renders, matching the real store's per-provider lifetime. + // directly instead of subscribing to the whole chat map). Most tests in + // this file drive the detector purely through terminal buffer events with + // an empty `sessions` map (no awaiting-approval tools), so nothing ever + // notifies. The §2 resolver tests below populate `sessions` per-test. + // Identity is stable across renders, matching the real store's + // per-provider lifetime. + sessions: new Map(), store: { - getState: () => new Map(), + getState: () => mocks.sessions, subscribeAll: () => () => {}, }, })); @@ -70,6 +73,7 @@ describe('usePromptDetector prompt lifecycle', () => { mocks.dispatch.mockClear(); mocks.callbacks.length = 0; mocks.screen.text = ''; + mocks.sessions.clear(); }); afterEach(() => { @@ -192,3 +196,114 @@ describe('usePromptDetector prompt lifecycle', () => { expect(dismiss![0].promptId).toBe(shownId); }); }); + +// Coverage for the §2 standing rule (2026-07-30 spec): usePromptDetector is +// the ONLY thing that auto-resolves a 'hook-closed'-retained card. Before +// this block, only the pure helpers in expired-card-resolver.ts had tests — +// deleting the resolver block inside usePromptDetector (the effect body that +// calls expiredToolIds/nextAbsentCount and dispatches PERMISSION_CARD_RESOLVED) +// left the whole suite green while every retained card stayed stuck until the +// user clicked Dismiss by hand. +describe('usePromptDetector — expired card resolver (2026-07-30 spec §2)', () => { + beforeEach(() => { + vi.useFakeTimers(); + mocks.dispatch.mockClear(); + mocks.callbacks.length = 0; + mocks.screen.text = ''; + mocks.sessions.clear(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + // A neutral, unrecognized Ink menu — parses as "a menu is present" without + // matching any SETUP_PROMPT_TITLES entry, so it can't also trigger a + // SHOW_PROMPT dispatch and muddy the PERMISSION_CARD_RESOLVED assertions. + const NEUTRAL_MENU = `Pick a flavor + + ❯ 1. Vanilla + 2. Chocolate`; + const NO_MENU = 'plain output, no menu here'; + + function makeSession(tools: Array>) { + const toolCalls = new Map(tools.map((t) => [t.toolUseId, { input: {}, ...t }])); + return { + toolCalls, + activeTurnToolIds: new Set(tools.map((t) => t.toolUseId)), + }; + } + + function setExpiredCard() { + mocks.sessions.set('s1', makeSession([ + { toolUseId: 'toolu_expired', toolName: 'Bash', status: 'awaiting-approval', expired: true }, + ])); + } + + it('menu present: does not resolve, and resets the absence counter', () => { + setExpiredCard(); + renderHook(() => usePromptDetector()); + + // Build up one absent flush first... + mocks.screen.text = NO_MENU; + fireBuffer('s1'); + // ...then the menu reappears, which must reset the counter to 0. + mocks.screen.text = NEUTRAL_MENU; + fireBuffer('s1'); + expect( + mocks.dispatch.mock.calls.find((c) => c[0].type === 'PERMISSION_CARD_RESOLVED'), + ).toBeUndefined(); + + // If the reset above didn't happen, this single absent flush would be + // the "2nd" and would wrongly resolve. + mocks.screen.text = NO_MENU; + fireBuffer('s1'); + expect( + mocks.dispatch.mock.calls.find((c) => c[0].type === 'PERMISSION_CARD_RESOLVED'), + ).toBeUndefined(); + }); + + it('menu absent for ONE flush: still does not resolve', () => { + setExpiredCard(); + renderHook(() => usePromptDetector()); + + mocks.screen.text = NO_MENU; + fireBuffer('s1'); + + expect( + mocks.dispatch.mock.calls.find((c) => c[0].type === 'PERMISSION_CARD_RESOLVED'), + ).toBeUndefined(); + }); + + it('menu absent for TWO consecutive flushes: dispatches PERMISSION_CARD_RESOLVED for the expired card', () => { + setExpiredCard(); + renderHook(() => usePromptDetector()); + + mocks.screen.text = NO_MENU; + fireBuffer('s1'); + fireBuffer('s1'); + + const resolved = mocks.dispatch.mock.calls.find((c) => c[0].type === 'PERMISSION_CARD_RESOLVED'); + expect(resolved).toBeTruthy(); + expect(resolved![0]).toMatchObject({ + type: 'PERMISSION_CARD_RESOLVED', + sessionId: 's1', + toolUseId: 'toolu_expired', + }); + }); + + it('a LIVE (non-expired) awaiting-approval card still bails prompt detection, unaffected by the resolver', () => { + mocks.sessions.set('s1', makeSession([ + { toolUseId: 'toolu_live', toolName: 'Bash', status: 'awaiting-approval' }, + ])); + renderHook(() => usePromptDetector()); + + // A recognized setup-prompt menu would normally SHOW_PROMPT after the + // debounce — the live awaiting-approval bail must suppress that too. + mocks.screen.text = RESUME_MENU; + fireBuffer('s1'); + act(() => { vi.advanceTimersByTime(400); }); + + expect(mocks.dispatch).not.toHaveBeenCalled(); + }); +});