mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-09 06:28:59 +02:00
fix(hooks): spawned-session escape in Conductor AUQ deny; override coverage in AUQ-error fallback (#2733)
Hooks inherit the harness env, so a per-command GSTACK_SESSION_KIND prefix inside a subagent's bash can never reach them. Levers added: a deterministic [conductor][spawned] auto-choose deny for env-level spawned sessions (OPENCLAW_SESSION or session-wide GSTACK_SESSION_KIND), and a spawned escape sentence appended to both hooks' prose directives so a marked subagent that slips and calls AUQ resolves to auto-choose instead of prose-STOP. The sentence lives in one shared constant (hosts/claude/hooks/spawned-directive.ts) so the two paths can never drift; destructive semantics are unified to conservative-continue. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
5486cd6620
commit
9c7b0a2c95
@@ -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
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<void> {
|
||||
// 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<void> {
|
||||
'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;
|
||||
|
||||
@@ -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';
|
||||
}
|
||||
@@ -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' }] } },
|
||||
|
||||
@@ -82,6 +82,12 @@ function runHook(stdin: object, cwd?: string, extraEnv?: Record<string, string>)
|
||||
// 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: '<gstack-qid:test-q> 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: '<gstack-qid:test-q> 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'");
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
// ----------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user