CalvinBackup/gstack
1
0
Fork 0
mirror of https://github.com/garrytan/gstack.git synced 2026-06-17 07:10:12 +02:00
Code Issues Packages Projects Releases Wiki Activity

feat: PreToolUse hook denies AskUserQuestion in Conductor, redirects to prose

Browse Source 
Conductor disables native AskUserQuestion and routes through a flaky MCP
variant that returns '[Tool result missing due to internal error]'. The
hook now denies any AUQ call in a Conductor session and instructs the model
to render a prose decision brief instead (transport avoidance, not preference
enforcement) — firing for one-way doors too, with a typed-confirmation
requirement for destructive paths.

Precedence: never-ask auto-decide still wins (user already settled those);
Conductor prose is the fallback for everything else; non-Conductor behavior
is byte-for-byte unchanged. Restructured the per-question loop to compute
eligibility without early-returning so the Conductor branch can run as the
fallback while preserving memoryContext on every exit.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Garry Tan
2026-06-11 20:10:40 -07:00
parent ba640b2fb5
commit d74a8744e4
2 changed files with 160 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files
hosts/claude/hooks/question-preference-hook.ts
+54 -33
View File
@@ -40,6 +40,7 @@ import * as fs from 'fs';
import * as path from 'path';
import * as os from 'os';
import { spawnSync } from 'child_process';
import { isConductor } from '../../../lib/is-conductor';
interface HookStdin {
session_id?: string;
@@ -400,57 +401,77 @@ async function main(): Promise<void> {
? '[plan-tune memory] Past answers suggest: ' + contextNuggets.join(' | ')
: undefined;
// Determine whether EVERY question is eligible for never-ask auto-decide.
// We deliberately do NOT early-return defer on the first ineligible question:
// a Conductor session still needs the [conductor] prose deny as a fallback,
// so we compute eligibility, then branch. memoryContext is preserved on every
// non-enforcing exit. (All-or-nothing per-call semantics are unchanged: any
// ineligible question makes the whole call not auto-decidable.)
const autoDecisions: Array<{ id: string; recommended: string }> = [];
let fullyAutoDecidable = true;
for (const q of questions) {
const qText = q.question || '';
const marker = qText.match(MARKER_RE);
if (!marker) {
defer(memoryContext);
return;
}
if (!marker) { fullyAutoDecidable = false; break; }
const questionId = marker[1];
const pref = lookupPreference(slug, questionId);
if (!pref.preference || pref.preference === 'always-ask') {
defer(memoryContext);
return;
}
if (!pref.preference || pref.preference === 'always-ask') { fullyAutoDecidable = false; break; }
const entry = registry[questionId];
const doorType = entry?.door_type || 'two-way';
if (doorType === 'one-way') {
// Safety override — even never-ask doesn't bypass one-way doors.
defer(memoryContext);
return;
}
// Safety override — even never-ask doesn't bypass one-way doors.
if (doorType === 'one-way') { fullyAutoDecidable = false; break; }
const opts = optionLabels(q.options || []);
const { recommended, ambiguous } = extractRecommended(qText, opts);
if (!recommended || ambiguous) {
// Refuse-on-ambiguous per D2 — fail safe, ask normally.
defer(memoryContext);
return;
}
// Refuse-on-ambiguous per D2 — fail safe.
if (!recommended || ambiguous) { fullyAutoDecidable = false; break; }
autoDecisions.push({ id: questionId, recommended });
}
// All questions were eligible for enforcement.
markAutoDecided(stdin.session_id, stdin.tool_use_id);
if (fullyAutoDecidable && autoDecisions.length > 0) {
// All questions were eligible for enforcement.
markAutoDecided(stdin.session_id, stdin.tool_use_id);
// Log each auto-decided question now, since deny prevents PostToolUse from
// firing. /plan-tune Recent auto-decisions reads source=auto-decided events.
for (let i = 0; i < autoDecisions.length; i++) {
const d = autoDecisions[i];
const q = questions[i];
const qText = (q.question || '').replace(MARKER_RE, '').trim();
const opts = optionLabels(q.options || []);
logAutoDecided(d.id, qText, d.recommended, opts.length, stdin.session_id, stdin.tool_use_id, stdin.cwd);
// Log each auto-decided question now, since deny prevents PostToolUse from
// firing. /plan-tune Recent auto-decisions reads source=auto-decided events.
for (let i = 0; i < autoDecisions.length; i++) {
const d = autoDecisions[i];
const q = questions[i];
const qText = (q.question || '').replace(MARKER_RE, '').trim();
const opts = optionLabels(q.options || []);
logAutoDecided(d.id, qText, d.recommended, opts.length, stdin.session_id, stdin.tool_use_id, stdin.cwd);
}
const reasonLines = autoDecisions.map(
(d) =>
`[plan-tune auto-decide] ${d.id} → ${d.recommended} (your never-ask preference). Proceed with that option without re-prompting. Change with /plan-tune.`,
);
deny(reasonLines.join('\n'));
return;
}
const reasonLines = autoDecisions.map(
(d) =>
`[plan-tune auto-decide] ${d.id} → ${d.recommended} (your never-ask preference). Proceed with that option without re-prompting. Change with /plan-tune.`,
);
deny(reasonLines.join('\n'));
// 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). ' +
'Do NOT call AskUserQuestion (native or any mcp__*__AskUserQuestion). Render this decision as a ' +
'PROSE decision brief now: a D<N> 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 ' +
'to reply with a letter, then STOP. For a one-way/destructive confirmation, require an explicit ' +
'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);
return;
}
defer(memoryContext);
}
main().catch((e) => {
test/question-preference-hook.test.ts
+106 -2
View File
@@ -60,7 +60,7 @@ function writeGlobalPref(questionId: string, preference: string): void {
fs.writeFileSync(f, JSON.stringify(prefs, null, 2));
}
function runHook(stdin: object, cwd?: string): {
function runHook(stdin: object, cwd?: string, extraEnv?: Record<string, string>): {
stdout: string;
stderr: string;
status: number;
@@ -75,10 +75,12 @@ function runHook(stdin: object, cwd?: string): {
// 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.
// [conductor] prose deny. The Conductor cases below opt back in explicitly
// via extraEnv.
delete env.CONDUCTOR_WORKSPACE_PATH;
delete env.CONDUCTOR_PORT;
env.GSTACK_QUESTION_LOG_NO_DERIVE = '1';
if (extraEnv) Object.assign(env, extraEnv);
const res = spawnSync(HOOK, [], {
env,
input: JSON.stringify({ ...stdin, cwd: cwd || fixtureCwd }),
@@ -343,6 +345,108 @@ describe('MCP variant', () => {
});
});
// ----------------------------------------------------------------------
// Conductor: deny + prose redirect (transport avoidance, not preference)
// ----------------------------------------------------------------------
describe('Conductor prose redirect', () => {
const CONDUCTOR = { CONDUCTOR_PORT: '55070' };
test('two-way, no preference → deny with [conductor] prose directive', () => {
const r = runHook({
session_id: 'c1',
tool_name: 'AskUserQuestion',
tool_use_id: 'tu-c1',
tool_input: {
questions: [
{ question: '<gstack-qid:test-q> Need approval?', options: ['A) Yes (recommended)', 'B) No'] },
],
},
}, undefined, CONDUCTOR);
expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('deny');
expect(r.parsed?.hookSpecificOutput?.permissionDecisionReason).toContain('[conductor]');
expect(r.parsed?.hookSpecificOutput?.permissionDecisionReason).toMatch(/do not call askuserquestion/i);
expect(r.parsed?.hookSpecificOutput?.permissionDecisionReason).toMatch(/reply with a letter/i);
});
test('UNMARKED question (modal path) → deny with prose directive', () => {
const r = runHook({
session_id: 'c2',
tool_name: 'AskUserQuestion',
tool_use_id: 'tu-c2',
tool_input: {
questions: [
{ question: 'No marker — an ad-hoc question', options: ['A) Yes (recommended)', 'B) No'] },
],
},
}, undefined, CONDUCTOR);
expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('deny');
expect(r.parsed?.hookSpecificOutput?.permissionDecisionReason).toContain('[conductor]');
});
test('one-way door → deny with prose directive (NOT defer — destructive must reach human via prose)', () => {
const r = runHook({
session_id: 'c3',
tool_name: 'AskUserQuestion',
tool_use_id: 'tu-c3',
tool_input: {
questions: [
{
question: '<gstack-qid:ship-test-failure-triage> Tests failed.',
options: ['A) Fix now (recommended)', 'B) Investigate', 'C) Ack and ship'],
},
],
},
}, undefined, CONDUCTOR);
expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('deny');
expect(r.parsed?.hookSpecificOutput?.permissionDecisionReason).toContain('[conductor]');
expect(r.parsed?.hookSpecificOutput?.permissionDecisionReason).toMatch(/typed confirmation/i);
});
test('CONDUCTOR_WORKSPACE_PATH alone also triggers the redirect', () => {
const r = runHook({
session_id: 'c4',
tool_name: 'mcp__conductor__AskUserQuestion',
tool_use_id: 'tu-c4',
tool_input: {
questions: [{ question: '<gstack-qid:test-q> Pick?', options: ['A) X (recommended)', 'B) Y'] }],
},
}, undefined, { CONDUCTOR_WORKSPACE_PATH: '/Users/x/conductor/ws' });
expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('deny');
expect(r.parsed?.hookSpecificOutput?.permissionDecisionReason).toContain('[conductor]');
});
test('PRECEDENCE: full never-ask auto-decide still wins over Conductor prose', () => {
writeProjectPref('ship-pre-landing-review-fix', 'never-ask');
const r = runHook({
session_id: 'c5',
tool_name: 'AskUserQuestion',
tool_use_id: 'tu-c5',
tool_input: {
questions: [
{
question: '<gstack-qid:ship-pre-landing-review-fix> Pre-landing review flagged issue.',
options: ['A) Fix now (recommended)', 'B) Skip'],
},
],
},
}, undefined, CONDUCTOR);
expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('deny');
// auto-decide reason, NOT the conductor prose reason
expect(r.parsed?.hookSpecificOutput?.permissionDecisionReason).toContain('plan-tune auto-decide');
expect(r.parsed?.hookSpecificOutput?.permissionDecisionReason).not.toContain('[conductor]');
});
test('non-AUQ tool in Conductor → still defer (no redirect on unrelated tools)', () => {
const r = runHook(
{ session_id: 'c6', tool_name: 'Bash', tool_use_id: 'tu-c6', tool_input: {} },
undefined,
CONDUCTOR,
);
expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('defer');
});
});
// ----------------------------------------------------------------------
// Auto-decided event logging (since PostToolUse never fires on deny)
// ----------------------------------------------------------------------