mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-14 17:05:28 +02:00
Merge remote-tracking branch 'origin/main' into moscow-v4
# Conflicts: # .github/workflows/evals.yml # test/helpers/touchfiles-data.ts
This commit is contained in:
Garry Tan
@@ -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, [], {
|
||||
@@ -528,6 +534,209 @@ 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('Conductor + invalid GSTACK_SESSION_KIND value → prose deny, not spawned (strict-equality fall-through)', () => {
|
||||
// spawnedByEnv() mirrors bin/gstack-session-kind step 0: only the exact
|
||||
// value "spawned" is honored. A reserved/typo'd value inside Conductor
|
||||
// must fall through to the PROSE deny — loosening the comparison to
|
||||
// truthiness would auto-choose past a human who IS watching.
|
||||
const r = runHook(
|
||||
{ session_id: 's3', tool_name: 'AskUserQuestion', tool_use_id: 'tu-s3', tool_input: Q },
|
||||
undefined,
|
||||
{ CONDUCTOR_PORT: '55071', GSTACK_SESSION_KIND: 'bogus' },
|
||||
);
|
||||
expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('deny');
|
||||
const reason = r.parsed?.hookSpecificOutput?.permissionDecisionReason ?? '';
|
||||
expect(reason).not.toContain('[conductor][spawned]');
|
||||
expect(reason).toMatch(/reply with a letter/i);
|
||||
});
|
||||
|
||||
test('spawned marker WITHOUT Conductor → pass-through (deny branch stays nested under isConductor)', () => {
|
||||
// Outside Conductor the tool is reliable; the spawned auto-choose deny is
|
||||
// a Conductor-only rescue. Hoisting spawnedByEnv() above isConductor()
|
||||
// would deny AUQ in every OpenClaw session regardless of host — pin the
|
||||
// nesting.
|
||||
const r = runHook(
|
||||
{
|
||||
session_id: 's4',
|
||||
tool_name: 'AskUserQuestion',
|
||||
tool_use_id: 'tu-s4',
|
||||
tool_input: {
|
||||
questions: [
|
||||
{ question: '<gstack-qid:spawned-nc> Bump VERSION?', options: ['A) Skip (recommended)', 'B) Bump'] },
|
||||
],
|
||||
},
|
||||
},
|
||||
undefined,
|
||||
{ OPENCLAW_SESSION: '1' },
|
||||
);
|
||||
expectPassThrough(r);
|
||||
});
|
||||
|
||||
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'");
|
||||
}
|
||||
});
|
||||
|
||||
test('spawned deny annotates one-way doors per question (#2733 review)', () => {
|
||||
// The auto-choose deny performs no preference lookup, so destructive
|
||||
// questions get a deterministic per-question annotation — a destructive
|
||||
// option marked (recommended) must not be auto-approved on prose alone.
|
||||
const r = runHook(
|
||||
{
|
||||
session_id: 's3',
|
||||
tool_name: 'AskUserQuestion',
|
||||
tool_use_id: 'tu-s3',
|
||||
tool_input: {
|
||||
questions: [
|
||||
{ question: '<gstack-qid:test-q> Force-push and overwrite the remote branch, deleting its history?', options: ['A) Force-push (recommended)', 'B) Abort'] },
|
||||
],
|
||||
},
|
||||
},
|
||||
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(/one-way door detected: Q1/);
|
||||
expect(reason).toMatch(/conservative non-destructive option/);
|
||||
// The driving env var is named (tamper visibility)...
|
||||
expect(reason).toContain('spawned driver: OPENCLAW_SESSION');
|
||||
// ...and the machine-resolved gate leaves a forensic record (the deny
|
||||
// prevents PostToolUse capture; this branch must log its own events).
|
||||
const f = path.join(stateRoot, 'projects', cwdSlug, 'question-log.jsonl');
|
||||
const events = fs.existsSync(f)
|
||||
? fs.readFileSync(f, 'utf-8').trim().split('\n').filter(Boolean).map((l) => JSON.parse(l))
|
||||
: [];
|
||||
expect(events.some((e) => e.source === 'spawned-env-deny')).toBe(true);
|
||||
});
|
||||
|
||||
test('spawned deny catches a destructive OPTION behind a bland question (codex finding)', () => {
|
||||
const r = runHook(
|
||||
{
|
||||
session_id: 's4',
|
||||
tool_name: 'AskUserQuestion',
|
||||
tool_use_id: 'tu-s4',
|
||||
tool_input: {
|
||||
questions: [
|
||||
{ question: '<gstack-qid:test-q> Proceed with the plan?', options: ['A) Force-push over the remote branch (recommended)', 'B) Abort'] },
|
||||
],
|
||||
},
|
||||
},
|
||||
undefined,
|
||||
{ CONDUCTOR_PORT: '55070', GSTACK_SESSION_KIND: 'spawned' },
|
||||
);
|
||||
expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('deny');
|
||||
const reason = r.parsed?.hookSpecificOutput?.permissionDecisionReason ?? '';
|
||||
expect(reason).toMatch(/one-way door detected: Q1/);
|
||||
expect(reason).toContain('spawned driver: GSTACK_SESSION_KIND');
|
||||
});
|
||||
|
||||
test('cross-surface destructive-policy drift guard: every spawned surface carries the canonical phrase', () => {
|
||||
// The conservative-continue destructive policy lives on four surfaces
|
||||
// (shared hook constant, AUQ resolver rule, skill-start spawned block,
|
||||
// ship dispatch prompt). Phrasings vary; the canonical core must not.
|
||||
const surfaces = [
|
||||
path.join(ROOT, 'hosts', 'claude', 'hooks', 'spawned-directive.ts'),
|
||||
path.join(ROOT, 'hosts', 'claude', 'hooks', 'auq-error-fallback-hook.ts'),
|
||||
path.join(ROOT, 'scripts', 'resolvers', 'preamble', 'generate-ask-user-format.ts'),
|
||||
path.join(ROOT, 'bin', 'gstack-skill-start'),
|
||||
path.join(ROOT, 'ship', 'sections', 'pr-body.md.tmpl'),
|
||||
];
|
||||
for (const f of surfaces) {
|
||||
const src = fs.readFileSync(f, 'utf-8');
|
||||
expect(src, `${path.basename(f)} lost the canonical destructive-policy phrase`).toContain('conservative non-destructive');
|
||||
}
|
||||
});
|
||||
|
||||
test('spawnedByEnv() parity with bin/gstack-session-kind over the spawned env matrix', () => {
|
||||
// spawnedByEnv mirrors session-kind steps 0-1 by hand; this pins the
|
||||
// mirror so a new ambient spawned marker added to the script cannot
|
||||
// silently leave Conductor-spawned sessions on the prose-STOP path.
|
||||
const { spawnedByEnv } = require(path.join(ROOT, 'hosts', 'claude', 'hooks', 'spawned-directive.ts'));
|
||||
const BIN = path.join(ROOT, 'bin', 'gstack-session-kind');
|
||||
const cases: Array<Record<string, string>> = [
|
||||
{ OPENCLAW_SESSION: '1' },
|
||||
{ GSTACK_SESSION_KIND: 'spawned' },
|
||||
{ GSTACK_SESSION_KIND: 'spawned', GSTACK_HEADLESS: '1' },
|
||||
{ GSTACK_SESSION_KIND: 'bogus' },
|
||||
{ GSTACK_SESSION_KIND: 'headless' },
|
||||
{ CONDUCTOR_PORT: '5' },
|
||||
{},
|
||||
];
|
||||
for (const env of cases) {
|
||||
const scriptKind = spawnSync(BIN, [], {
|
||||
env: { PATH: process.env.PATH ?? '/usr/bin:/bin', ...env },
|
||||
encoding: 'utf-8',
|
||||
}).stdout.trim();
|
||||
expect(
|
||||
spawnedByEnv(env),
|
||||
`parity break on env ${JSON.stringify(env)}: script says ${scriptKind}`,
|
||||
).toBe(scriptKind === 'spawned');
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
// ----------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user