diff --git a/hosts/claude/hooks/auq-error-fallback-hook.ts b/hosts/claude/hooks/auq-error-fallback-hook.ts index 45d86200f..b68e443dc 100755 --- a/hosts/claude/hooks/auq-error-fallback-hook.ts +++ b/hosts/claude/hooks/auq-error-fallback-hook.ts @@ -32,6 +32,7 @@ import * as fs from 'fs'; import * as path from 'path'; import * as os from 'os'; import { runBin } from './spawn-bin'; +import { SPAWNED_ESCAPE_SENTENCE } from './spawned-directive'; interface HookStdin { tool_name?: string; @@ -158,12 +159,17 @@ export function directiveFor(kind: 'spawned' | 'headless' | 'interactive'): stri ); case 'interactive': default: + // #2733: the shell-out above runs in the HARNESS env, so a subagent + // marked spawned via a per-command env prefix still classifies as + // interactive here — the escape sentence is the only lever for that + // topology (see spawned-directive.ts). return ( lead + 'SESSION_KIND=interactive — render the decision as a PROSE message now: a clear ELI10 of the issue, ' + 'then a Recommendation line, then ONE paragraph per choice carrying its `(recommended)` marker, its ' + '`Completeness: X/10`, and 2-4 sentences of reasoning. Tell the user to reply with a letter, then STOP. ' + - '(Retry the call once first only if no answer could have surfaced.)' + '(Retry the call once first only if no answer could have surfaced.) ' + + SPAWNED_ESCAPE_SENTENCE ); } } diff --git a/hosts/claude/hooks/question-preference-hook.ts b/hosts/claude/hooks/question-preference-hook.ts index 950d9c139..90ba9b37f 100644 --- a/hosts/claude/hooks/question-preference-hook.ts +++ b/hosts/claude/hooks/question-preference-hook.ts @@ -46,6 +46,7 @@ import * as os from 'os'; import { runBin, repoRoot } from './spawn-bin'; import { isConductor } from '../../../lib/is-conductor'; import { classifyQuestion } from '../../../scripts/one-way-doors'; +import { SPAWNED_ESCAPE_SENTENCE, CONDUCTOR_SPAWNED_DENY_REASON, spawnedByEnv } from './spawned-directive'; interface HookStdin { session_id?: string; @@ -484,6 +485,17 @@ async function main(): Promise { // preference, or door type — including one-way doors, which must reach the // human via prose rather than the unreliable tool. if (isConductor()) { + // #2733: env-level spawned sessions (OpenClaw inside a Conductor + // workspace, or a harness launched with GSTACK_SESSION_KIND=spawned in + // its env) get an auto-choose deny — a prose brief has no reader there. + // LIMITATION: a per-command GSTACK_SESSION_KIND prefix inside a + // subagent's bash never reaches this hook (hooks inherit the harness + // env); that case is covered by the escape sentence below plus the + // dispatching skill's prompt. + if (spawnedByEnv()) { + deny(CONDUCTOR_SPAWNED_DENY_REASON + (memoryContext ? `\n${memoryContext}` : '')); + return; + } 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 ' + @@ -491,7 +503,8 @@ async function main(): Promise { '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).' + + '(PostToolUse will not fire on a prose path). ' + + SPAWNED_ESCAPE_SENTENCE + (memoryContext ? `\n${memoryContext}` : ''); deny(conductorReason); return; diff --git a/hosts/claude/hooks/spawned-directive.ts b/hosts/claude/hooks/spawned-directive.ts new file mode 100644 index 000000000..bbaf1cdbc --- /dev/null +++ b/hosts/claude/hooks/spawned-directive.ts @@ -0,0 +1,47 @@ +/** + * Shared spawned-session directive text for the AUQ hooks (#2733). + * + * Hook processes inherit the HARNESS env, so a per-command + * `GSTACK_SESSION_KIND=spawned` prefix inside a subagent's bash call can + * never reach a hook — the only levers a hook has for the subagent case are + * (a) env-level markers that ARE session-wide (OPENCLAW_SESSION, or a harness + * launched with GSTACK_SESSION_KIND in its env) and (b) directive TEXT the + * model reads. Both AUQ hooks (question-preference PreToolUse deny, + * auq-error-fallback PostToolUse directive) carry the same escape sentence; + * it lives here as one constant so the two paths can never drift into + * contradictory instructions. + * + * Destructive semantics are unified across every spawned surface (dispatch + * prompt, spawned-session block, AUQ prose rule, both hooks): + * conservative-continue, never prose-STOP — a prose brief with no reader is + * always wrong in a spawned session, and the conservative choice guarantees + * nothing irreversible happens. + */ + +/** Appended to prose-directing hook texts so a marked subagent that slips + * and calls AUQ still resolves to auto-choose instead of prose-STOP. */ +export const SPAWNED_ESCAPE_SENTENCE = + 'If this session was spawned by an orchestrator or a parent agent and no human reads its ' + + 'output mid-run (e.g. your dispatch prompt says you are a spawned subagent), do not render ' + + 'the prose brief either — auto-choose the recommended option and continue; at a destructive ' + + 'or irreversible gate, do not execute the destructive action: take the conservative ' + + 'non-destructive choice (skip/defer), record it, and continue.'; + +/** Deterministic deny reason for env-detected spawned sessions inside Conductor. */ +export const CONDUCTOR_SPAWNED_DENY_REASON = + '[conductor][spawned] AskUserQuestion is unreliable in Conductor and this session is ' + + 'orchestrator-spawned — no human reads its output. Do NOT retry the tool and do NOT render ' + + 'a prose decision brief: auto-choose the recommended option for each question above, note ' + + 'the choice, and continue the workflow. Exception: never auto-approve a destructive or ' + + 'irreversible option — take the conservative non-destructive choice (skip/defer), note it, ' + + 'and continue.'; + +/** + * Env-level spawned detection (direct env read — PreToolUse hot path, no + * shell-out). Mirrors bin/gstack-session-kind steps 0-1. True only for + * session-wide markers; a per-command prefix in subagent bash is invisible + * here by construction. + */ +export function spawnedByEnv(env: NodeJS.ProcessEnv = process.env): boolean { + return !!env.OPENCLAW_SESSION || env.GSTACK_SESSION_KIND === 'spawned'; +} diff --git a/test/auq-error-fallback-hook.test.ts b/test/auq-error-fallback-hook.test.ts index 21505c04b..2265c6312 100644 --- a/test/auq-error-fallback-hook.test.ts +++ b/test/auq-error-fallback-hook.test.ts @@ -74,6 +74,15 @@ describe('directiveFor — per-session-kind instruction', () => { test('spawned directive auto-chooses', () => { expect(directiveFor('spawned')).toMatch(/auto-choose/i); }); + + test('interactive directive carries the spawned escape sentence (#2733)', () => { + // The sessionKind() shell-out runs in the HARNESS env, so a subagent + // marked spawned via a per-command prefix classifies interactive here — + // the directive text is the only lever for that topology. + const d = directiveFor('interactive'); + expect(d).toMatch(/spawned subagent[\s\S]*auto-choose the recommended option/i); + expect(d).toMatch(/destructive or irreversible gate[\s\S]*conservative/i); + }); }); /** Spawn the hook with synthetic stdin + controlled env; parse its JSON stdout. */ @@ -113,6 +122,15 @@ describe('hook integration — invoked as PostToolUse', () => { expect(out.additionalContext).toMatch(/auto-choose/i); }); + test('error result + GSTACK_SESSION_KIND=spawned env → override beats Conductor-interactive (#2733)', () => { + const out = runHook( + { tool_name: 'AskUserQuestion', tool_response: { is_error: true } }, + { GSTACK_SESSION_KIND: 'spawned', CONDUCTOR_PORT: '55010' }, + ); + expect(out.additionalContext).toMatch(/SESSION_KIND=spawned/); + expect(out.additionalContext).toMatch(/auto-choose/i); + }); + test('SUCCESSFUL answer → no injection (inert on real answers)', () => { const out = runHook( { tool_name: 'AskUserQuestion', tool_response: { answers: [{ option_label: 'A' }] } }, diff --git a/test/question-preference-hook.test.ts b/test/question-preference-hook.test.ts index d96843f4e..c66612855 100644 --- a/test/question-preference-hook.test.ts +++ b/test/question-preference-hook.test.ts @@ -82,6 +82,12 @@ function runHook(stdin: object, cwd?: string, extraEnv?: Record) // via extraEnv. delete env.CONDUCTOR_WORKSPACE_PATH; delete env.CONDUCTOR_PORT; + // Same reasoning for the spawned markers (#2733): running the suite inside + // an OpenClaw/spawned-marked session would flip the [conductor] prose deny + // into the [conductor][spawned] auto-choose deny. Spawned cases opt back in + // explicitly via extraEnv. + delete env.OPENCLAW_SESSION; + delete env.GSTACK_SESSION_KIND; env.GSTACK_QUESTION_LOG_NO_DERIVE = '1'; if (extraEnv) Object.assign(env, extraEnv); const res = spawnSync(HOOK, [], { @@ -527,6 +533,72 @@ describe('Conductor prose redirect', () => { ); expectPassThrough(r); }); + + test('prose deny carries the spawned-subagent escape sentence (#2733)', () => { + // A per-command env prefix in a subagent's bash can never reach this hook + // (hooks inherit the harness env), so the deny TEXT must carry the escape + // hatch — otherwise a marked subagent that slips and calls AUQ is + // instructed to prose-STOP, recreating the bug through the hook layer. + const r = runHook({ + session_id: 'c7', + tool_name: 'AskUserQuestion', + tool_use_id: 'tu-c7', + tool_input: { + questions: [ + { question: ' Need approval?', options: ['A) Yes (recommended)', 'B) No'] }, + ], + }, + }, undefined, CONDUCTOR); + const reason = r.parsed?.hookSpecificOutput?.permissionDecisionReason ?? ''; + expect(reason).toMatch(/spawned subagent[\s\S]*auto-choose the recommended option/i); + // Destructive exclusion rides the same sentence (unified semantics). + expect(reason).toMatch(/destructive or irreversible gate[\s\S]*conservative/i); + }); +}); + +// ---------------------------------------------------------------------- +// Conductor + env-detected spawned: auto-choose deny, not prose (#2733) +// ---------------------------------------------------------------------- + +describe('Conductor spawned deny (#2733)', () => { + const Q = { + questions: [ + { question: ' Bump VERSION?', options: ['A) Skip (recommended)', 'B) Bump'] }, + ], + }; + + test('Conductor + OPENCLAW_SESSION → [conductor][spawned] auto-choose deny, not prose', () => { + const r = runHook( + { session_id: 's1', tool_name: 'AskUserQuestion', tool_use_id: 'tu-s1', tool_input: Q }, + undefined, + { CONDUCTOR_PORT: '55070', OPENCLAW_SESSION: '1' }, + ); + expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('deny'); + const reason = r.parsed?.hookSpecificOutput?.permissionDecisionReason ?? ''; + expect(reason).toContain('[conductor][spawned]'); + expect(reason).toMatch(/auto-choose the recommended option/i); + expect(reason).not.toMatch(/reply with a letter/i); + }); + + test('Conductor + GSTACK_SESSION_KIND=spawned env → same auto-choose deny', () => { + const r = runHook( + { session_id: 's2', tool_name: 'AskUserQuestion', tool_use_id: 'tu-s2', tool_input: Q }, + undefined, + { CONDUCTOR_WORKSPACE_PATH: '/Users/x/conductor/ws', GSTACK_SESSION_KIND: 'spawned' }, + ); + expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('deny'); + const reason = r.parsed?.hookSpecificOutput?.permissionDecisionReason ?? ''; + expect(reason).toContain('[conductor][spawned]'); + expect(reason).toMatch(/never auto-approve a destructive or irreversible option/i); + }); + + test('both hooks source their spawned directive from the shared constant (drift guard)', () => { + const hooksDir = path.join(ROOT, 'hosts', 'claude', 'hooks'); + for (const f of ['question-preference-hook.ts', 'auq-error-fallback-hook.ts']) { + const src = fs.readFileSync(path.join(hooksDir, f), 'utf-8'); + expect(src, `${f} must import the shared spawned directive`).toContain("from './spawned-directive'"); + } + }); }); // ----------------------------------------------------------------------