From 78d7d30ec993169df630ac33984798b240fd699a Mon Sep 17 00:00:00 2001 From: kalpajit279 Date: Wed, 1 Jul 2026 23:42:21 +0530 Subject: [PATCH] fix: pre-empt broken AskUserQuestion on the Claude Desktop host MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The PreToolUse transport-avoidance in question-preference-hook only guarded against Conductor. The Claude Desktop app (CLAUDE_CODE_ENTRYPOINT=claude-desktop) advertises + enables AskUserQuestion (CLAUDE_CODE_ENABLE_ASK_USER_QUESTION_TOOL=true) but its handler returns "[Tool result missing due to internal error]". PostToolUse hooks do NOT fire on that transport-error path, so auq-error-fallback-hook cannot rescue it after the fact — the model just sees the raw error. Extend the existing PreToolUse deny to also fire when CLAUDE_CODE_ENTRYPOINT=claude-desktop, redirecting to the same prose decision brief. Generalize the reason label ([conductor] / [claude-desktop]). Tests: mirror the ambient-env strip (delete CLAUDE_CODE_ENTRYPOINT) in the two hook test runners so a suite run inside Claude Desktop stays deterministic, and add a "Claude Desktop prose redirect" describe block paralleling the Conductor cases. Verified on claude.exe 2.1.197 (Windows): the identical AUQ call that returned the missing-result error now returns the [claude-desktop] prose-redirect deny. Co-Authored-By: Claude Opus 4.8 --- .../claude/hooks/question-preference-hook.ts | 30 ++++--- test/memory-cache-injection.test.ts | 6 +- test/question-preference-hook.test.ts | 87 +++++++++++++++++-- 3 files changed, 106 insertions(+), 17 deletions(-) diff --git a/hosts/claude/hooks/question-preference-hook.ts b/hosts/claude/hooks/question-preference-hook.ts index 12cbd5ea2..d245bc2e4 100644 --- a/hosts/claude/hooks/question-preference-hook.ts +++ b/hosts/claude/hooks/question-preference-hook.ts @@ -451,15 +451,25 @@ async function main(): Promise { return; } - // Not fully auto-decidable. In Conductor, AskUserQuestion is unreliable - // (native is disabled, the mcp__conductor__AskUserQuestion variant is flaky), - // so deny the tool and redirect to a prose decision brief. This is TRANSPORT - // AVOIDANCE, not preference enforcement: it fires regardless of marker, - // preference, or door type — including one-way doors, which must reach the - // human via prose rather than the unreliable tool. - if (isConductor()) { - const conductorReason = - '[conductor] AskUserQuestion is unreliable in Conductor (native disabled, MCP variant flaky). ' + + // Not fully auto-decidable. On some hosts AskUserQuestion is unreliable and + // must be avoided entirely: + // - Conductor: native AUQ is disabled and the mcp__conductor__AskUserQuestion + // variant is flaky. + // - Claude Desktop app (CLAUDE_CODE_ENTRYPOINT=claude-desktop): the tool is + // advertised + enabled (CLAUDE_CODE_ENABLE_ASK_USER_QUESTION_TOOL=true) but + // its handler returns "[Tool result missing due to internal error]" — a host + // bug verified 2026-07-01 on claude.exe 2.1.197. Critically, PostToolUse + // hooks do NOT fire on that transport-error path, so auq-error-fallback-hook + // can't rescue it after the fact; it must be pre-empted here in PreToolUse. + // On those hosts, deny the tool and redirect to a prose decision brief. This is + // TRANSPORT AVOIDANCE, not preference enforcement: it fires regardless of + // marker, preference, or door type — including one-way doors, which must reach + // the human via prose rather than the unreliable tool. + const isClaudeDesktop = (process.env.CLAUDE_CODE_ENTRYPOINT || '') === 'claude-desktop'; + if (isConductor() || isClaudeDesktop) { + const host = isConductor() ? 'conductor' : 'claude-desktop'; + const unreliableReason = + `[${host}] AskUserQuestion is unreliable in this host (missing-result / transport error). ` + 'Do NOT call AskUserQuestion (native or any mcp__*__AskUserQuestion). Render this decision as a ' + 'PROSE decision brief now: a D label, an ELI10 of the issue, a Recommendation line, then one ' + 'paragraph per choice carrying its `(recommended)` marker and `Completeness: X/10`; tell the user ' + @@ -467,7 +477,7 @@ async function main(): Promise { 'typed confirmation and do NOT proceed on a vague reply. Capture the decision with gstack-question-log ' + '(PostToolUse will not fire on a prose path).' + (memoryContext ? `\n${memoryContext}` : ''); - deny(conductorReason); + deny(unreliableReason); return; } diff --git a/test/memory-cache-injection.test.ts b/test/memory-cache-injection.test.ts index 3ab6a2144..3e7f56a57 100644 --- a/test/memory-cache-injection.test.ts +++ b/test/memory-cache-injection.test.ts @@ -44,10 +44,12 @@ function runHook(stdin: object): { stdout: string; stderr: string; status: numbe env.GSTACK_QUESTION_LOG_NO_DERIVE = '1'; delete env.GSTACK_HOME; // These cases assert the defer-path memoryContext injection. Strip ambient - // Conductor markers so running inside Conductor (CONDUCTOR_WORKSPACE_PATH/PORT - // set) doesn't flip the hook into the [conductor] prose deny instead of defer. + // host markers so running inside Conductor (CONDUCTOR_WORKSPACE_PATH/PORT set) + // or the Claude Desktop app (CLAUDE_CODE_ENTRYPOINT=claude-desktop) doesn't flip + // the hook into the prose deny instead of defer. delete env.CONDUCTOR_WORKSPACE_PATH; delete env.CONDUCTOR_PORT; + delete env.CLAUDE_CODE_ENTRYPOINT; const res = spawnSync(HOOK, [], { env, input: JSON.stringify({ ...stdin, cwd: fixtureCwd }), diff --git a/test/question-preference-hook.test.ts b/test/question-preference-hook.test.ts index 39de02f4e..921c5f2f9 100644 --- a/test/question-preference-hook.test.ts +++ b/test/question-preference-hook.test.ts @@ -72,13 +72,15 @@ function runHook(stdin: object, cwd?: string, extraEnv?: Record) } env.GSTACK_STATE_ROOT = stateRoot; delete env.GSTACK_HOME; - // Strip ambient Conductor markers so these cases characterize NON-Conductor - // behavior deterministically — otherwise running the suite inside Conductor - // (CONDUCTOR_WORKSPACE_PATH/PORT set) would flip every defer into the - // [conductor] prose deny. The Conductor cases below opt back in explicitly - // via extraEnv. + // Strip ambient host markers so these cases characterize the plain + // (non-transport-avoidance) behavior deterministically — otherwise running the + // suite inside Conductor (CONDUCTOR_WORKSPACE_PATH/PORT set) or the Claude + // Desktop app (CLAUDE_CODE_ENTRYPOINT=claude-desktop) would flip every defer + // into the prose deny. The transport-avoidance cases below opt back in + // explicitly via extraEnv. delete env.CONDUCTOR_WORKSPACE_PATH; delete env.CONDUCTOR_PORT; + delete env.CLAUDE_CODE_ENTRYPOINT; env.GSTACK_QUESTION_LOG_NO_DERIVE = '1'; if (extraEnv) Object.assign(env, extraEnv); const res = spawnSync(HOOK, [], { @@ -447,6 +449,81 @@ describe('Conductor prose redirect', () => { }); }); +// ---------------------------------------------------------------------- +// Claude Desktop app: deny + prose redirect (same transport avoidance as +// Conductor — CLAUDE_CODE_ENTRYPOINT=claude-desktop; AUQ handler returns a +// missing-result internal error, and PostToolUse can't rescue it) +// ---------------------------------------------------------------------- + +describe('Claude Desktop prose redirect', () => { + const DESKTOP = { CLAUDE_CODE_ENTRYPOINT: 'claude-desktop' }; + + test('two-way, no preference → deny with [claude-desktop] prose directive', () => { + const r = runHook({ + session_id: 'd1', + tool_name: 'AskUserQuestion', + tool_use_id: 'tu-d1', + tool_input: { + questions: [ + { question: ' Need approval?', options: ['A) Yes (recommended)', 'B) No'] }, + ], + }, + }, undefined, DESKTOP); + expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('deny'); + expect(r.parsed?.hookSpecificOutput?.permissionDecisionReason).toContain('[claude-desktop]'); + expect(r.parsed?.hookSpecificOutput?.permissionDecisionReason).toMatch(/do not call askuserquestion/i); + expect(r.parsed?.hookSpecificOutput?.permissionDecisionReason).toMatch(/reply with a letter/i); + }); + + test('one-way door → deny (destructive must reach human via prose, not the broken tool)', () => { + const r = runHook({ + session_id: 'd2', + tool_name: 'AskUserQuestion', + tool_use_id: 'tu-d2', + tool_input: { + questions: [ + { + question: ' Tests failed.', + options: ['A) Fix now (recommended)', 'B) Investigate', 'C) Ack and ship'], + }, + ], + }, + }, undefined, DESKTOP); + expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('deny'); + expect(r.parsed?.hookSpecificOutput?.permissionDecisionReason).toContain('[claude-desktop]'); + expect(r.parsed?.hookSpecificOutput?.permissionDecisionReason).toMatch(/typed confirmation/i); + }); + + test('full never-ask auto-decide still wins over the desktop prose redirect', () => { + writeProjectPref('ship-pre-landing-review-fix', 'never-ask'); + const r = runHook({ + session_id: 'd3', + tool_name: 'AskUserQuestion', + tool_use_id: 'tu-d3', + tool_input: { + questions: [ + { + question: ' Pre-landing review flagged issue.', + options: ['A) Fix now (recommended)', 'B) Skip'], + }, + ], + }, + }, undefined, DESKTOP); + expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('deny'); + expect(r.parsed?.hookSpecificOutput?.permissionDecisionReason).toContain('plan-tune auto-decide'); + expect(r.parsed?.hookSpecificOutput?.permissionDecisionReason).not.toContain('[claude-desktop]'); + }); + + test('non-AUQ tool on desktop → still defer (no redirect on unrelated tools)', () => { + const r = runHook( + { session_id: 'd4', tool_name: 'Bash', tool_use_id: 'tu-d4', tool_input: {} }, + undefined, + DESKTOP, + ); + expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('defer'); + }); +}); + // ---------------------------------------------------------------------- // Auto-decided event logging (since PostToolUse never fires on deny) // ----------------------------------------------------------------------