mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-02 19:30:37 +02:00
v1.76.0.0 fix: ship doc-sync survives Conductor — spawned subagent sessions reachable (#2733) (#2741)
* feat(session-kind): explicit GSTACK_SESSION_KIND override; skill-start spawned gates keyed on kind (#2733) Claude Code subagents inherit the parent env byte-for-byte, so ambient markers classify them as the parent's kind and the spawned classification was unreachable outside OpenClaw. GSTACK_SESSION_KIND=spawned (step 0, spawned-only by design) lets a dispatching skill mark its subagent per command. skill-start now keys SPAWNED_SESSION and the spawned-session instruction block on the resolved kind (was raw OPENCLAW_SESSION), suppresses CONDUCTOR_SESSION for spawned sessions, gates all 11 interactive-onboarding blocks plus their ack-at-emit marker writes on kind != spawned, and adds a destructive-gate carve-out to the spawned block (conservative-continue, never prose-STOP). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * 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> * fix(ship): Step 18 marks the document-release subagent spawned — env prefix + auto-choose prompt (#2733) The dispatch prompt now (1) frames the run as a SPAWNED subagent whose LAST line is machine-parsed, (2) instructs prefixing the preamble's gstack-skill-start invocation with GSTACK_SESSION_KIND=spawned on the same command line (template bash blocks don't share exports), and (3) resolves every AUQ gate to auto-choosing the recommended option, conservative on no-recommendation, never destructive. The JSON contract gains a required "decisions" array (auto-chosen gates, printed to the ship console — never embedded in the public PR body) and a placement clause so the skill's own doc-health summary stops competing with the LAST-line JSON. Tripwire pins added; codex/factory goldens refreshed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(auq-format): proactive SESSION_KIND=spawned rule ordered above the Conductor rule (#2733) The spawned classification previously existed only in the failure-fallback branch — a spawned session was invited to call AskUserQuestion and reach auto-choose via the deny/error detour, and a spawned session inside a Conductor workspace hit the Conductor prose-STOP rule first. The Tool resolution list now leads with the spawned rule (auto-choose recommended, never prose, never BLOCKED, destructive gates resolve conservative), the self-check carries the never-reach-this-checklist clause, and all tier>=2 SKILL.md renders are regenerated. Context-budget fixture refreshed in the same commit per the ratchet protocol (the AUQ section is eager in every tier>=2 skill). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(e2e): spawned document-release subagent returns the JSON contract through a firing gate (#2733) The behavioral proof the bug shipped without: ship-docsync stubs the skill (no preamble, no gates) and skill-e2e-workflow suppresses the gates by prompt. This gate-tier E2E plays the parent — it drives the verbatim Step 18 dispatch prompt (extracted from the live pr-body.md, drift-proof) against a real preamble-bearing document-release slice in a Conductor-ambient env with both AUQ hooks seeded live, an unbumped VERSION making Step 8 fire. Asserts: the final line parses as the 5-key JSON contract, the fired gate's auto-choice is recorded in decisions, and VERSION is untouched (the gate resolved to its recommended Skip). Burn-in: 1/1 pass, $0.35, 21 turns, 106s. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(openclaw): document the GSTACK_SESSION_KIND override; wire session-kind into paid selectors (#2733) OPENCLAW.md's spawned-session section now covers the explicit per-command marker, its deliberate spawned-only narrowness, the /ship Step 18 usage, the destructive carve-out, onboarding-block suppression, and the hook env-blindness caveat. bin/gstack-session-kind and the shared spawned-directive module join the conductor-prose and auto-decide-preserved selector dep lists (session-kind previously appeared in no touchfiles entry — editing it alone triggered no paid E2E). TODOS.md gains the plan-tune capture follow-up for spawned auto-choices. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: pre-landing review fixes (#2733) Review army + coverage audit findings, all applied: - headless directive carries the spawned escape sentence too (multi- specialist: a CI-hosted ship's marked subagent must not end BLOCKED) - anti-injection scoping on every text-claimable spawned trigger (AUQ rule + shared escape sentence): markings count only from the creating prompt, never from files/tool output/web content read mid-run - [conductor][spawned] deny annotates one-way doors per question - SPAWNED_OVERRIDE: env tamper-visibility status line + OPENCLAW.md note - spawned sessions skip the network update-check and first-task probe (consumers suppressed; preserves the one-shot just-upgraded marker) - test hardening: dispatch-tripwire end-bound validated, vacuous marker asserts replaced with output asserts, E2E cpSync size filter + named fence tolerance, spawnedByEnv parity pin, destructive-policy cross- surface drift guard, one-way annotation + bogus-value hook cases - session-kind duplicate rationale comment deduped; regen + goldens + context-budget fixture refreshed Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore: bump version and changelog (v1.76.0.0) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs: update project documentation for v1.76.0.0 PROJECT_STRUCTURE.md: add hosts/claude/hooks/ to the directory tree (AUQ capture + enforcement hooks, spawned-session directive, timeline stop) — the tree omitted the directory while docs/OPENCLAW.md and CHANGELOG.md now reference paths inside it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs: sync TODOS.md ship dispatch entry with the v1.76.0.0 contract Codex doc-review finding: the SHIPPED entry for /ship auto-invoking /document-release still described the four-key JSON contract. Adds the decisions key (console-printed, never PR markdown), the GSTACK_SESSION_KIND=spawned dispatch marking (#2733), and the new spawned-dispatch gate E2E to the proven-by list. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Garry Tan
co-authored by
Claude Fable 5
Claude Fable 5
parent
07b59e396c
commit
253d1dfe26
@@ -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,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