diff --git a/bin/gstack-session-kind b/bin/gstack-session-kind index 8e9bc4e41..b9df19f82 100755 --- a/bin/gstack-session-kind +++ b/bin/gstack-session-kind @@ -17,11 +17,31 @@ # a positive headless signal, since a stray prose message in an unmarked one-shot # `-p` run just ends the turn (harmless), whereas wrongly BLOCKING a real human is not. # +# GSTACK_SESSION_KIND=spawned is the explicit per-command override (step 0 below), +# outranking every ambient marker. Deliberately narrow — only "spawned" is honored: +# Claude Code subagents inherit the parent env byte-for-byte (#2733), so a +# dispatching skill marks its subagent by prefixing the gstack-skill-start +# invocation. "headless" already has GSTACK_HEADLESS; other values are reserved +# and ignored (fall through to detection). +# # Why env vars and not TTY/entrypoint: an interactive Conductor session reports # CLAUDE_CODE_ENTRYPOINT=sdk-ts with no TTY — identical to a headless SDK eval. The # signals that actually discriminate are the host/orchestrator/CI env markers below. set -euo pipefail +# 0. Explicit spawned marker — set per-command by a dispatching skill (e.g. +# /ship Step 18 prefixes the document-release subagent's gstack-skill-start +# with GSTACK_SESSION_KIND=spawned; #2733: Claude Code subagents inherit +# the parent env byte-for-byte, so ambient markers misclassify them as the +# parent's kind). Only "spawned" is honored: headless already has +# GSTACK_HEADLESS, and letting an env var force "interactive" over CI +# markers would be a misclassification footgun. Other values are reserved +# and ignored (fall through) — same contract as empty GSTACK_HEADLESS. +if [ "${GSTACK_SESSION_KIND:-}" = "spawned" ]; then + echo "spawned" + exit 0 +fi + # 1. Orchestrator-spawned session (OpenClaw). Authoritative block lives in the skill; # we only surface the classification. if [ -n "${OPENCLAW_SESSION:-}" ]; then diff --git a/bin/gstack-skill-start b/bin/gstack-skill-start index 671b30550..8fbd43dea 100755 --- a/bin/gstack-skill-start +++ b/bin/gstack-skill-start @@ -88,8 +88,11 @@ case "$_SESSION_KIND" in spawned|headless|interactive) ;; *) _SESSION_KIND="inte echo "SESSION_KIND: $_SESSION_KIND" # Conductor host: AskUserQuestion is unreliable there (native disabled, MCP # variant flaky); skills render decisions as prose. Gated on !headless so an -# eval/CI run INSIDE Conductor still BLOCKs rather than rendering prose to nobody. -if [ "$_SESSION_KIND" != "headless" ] && { [ -n "${CONDUCTOR_WORKSPACE_PATH:-}" ] || [ -n "${CONDUCTOR_PORT:-}" ]; }; then +# eval/CI run INSIDE Conductor still BLOCKs rather than rendering prose to +# nobody, and on !spawned so an orchestrator- or parent-skill-spawned session +# inside a Conductor workspace auto-chooses per its spawned-session block +# instead of rendering prose to nobody (#2733). +if [ "$_SESSION_KIND" != "headless" ] && [ "$_SESSION_KIND" != "spawned" ] && { [ -n "${CONDUCTOR_WORKSPACE_PATH:-}" ] || [ -n "${CONDUCTOR_PORT:-}" ]; }; then echo "CONDUCTOR_SESSION: true" fi _ACTIVATED=$([ -f "$_GH/.activated" ] && echo "yes" || echo "no") @@ -182,7 +185,9 @@ else GSTACK_PLAN_MODE="inactive" fi echo "GSTACK_PLAN_MODE: $GSTACK_PLAN_MODE" -[ -n "${OPENCLAW_SESSION:-}" ] && echo "SPAWNED_SESSION: true" || true +# Keyed on the resolved kind, not raw OPENCLAW_SESSION (#2733): the explicit +# GSTACK_SESSION_KIND=spawned override must light this up too. +[ "$_SESSION_KIND" = "spawned" ] && echo "SPAWNED_SESSION: true" || true # --------------------------------------------------------------------------- # Artifacts sync (the former "Artifacts Sync (skill start)" fence, verbatim @@ -315,6 +320,17 @@ fi _ROOT_DIR=$(dirname "$_BIN") _emit_block() { echo "GSTACK_INSTRUCTION_BEGIN: $1 $_SESSION_ID"; cat; echo "GSTACK_INSTRUCTION_END"; } +# Spawned sessions get NO interactive-onboarding blocks (#2733): no human is +# watching, so an emitted prompt gets auto-answered (config writes nobody +# approved) and ack-at-emit markers get consumed invisibly — the next HUMAN +# session would never see the one-time prompt. Gating EMISSION here (not just +# the prose-skip inside the spawned-session block) also gates the script-side +# marker/state writes: .activated, .first-loop-tip-shown, and scaffold +# telemetry stay untouched, so onboarding fires intact on the next human run. +# The spawned-session block and the privacy-stop-gate below sit OUTSIDE this +# guard: the former is the gate's inverse, the latter is interactive-only. +if [ "$_SESSION_KIND" != "spawned" ]; then + # Upgrade flow (gated: update-check emitted something above). if [ -n "$_UPD" ]; then _emit_block upgrade-flow < { expect(kind({ GSTACK_HEADLESS: '' })).toBe('interactive'); }); }); + +describe('GSTACK_SESSION_KIND explicit override (#2733)', () => { + test('spawned wins over every ambient marker (step 0, explicit beats ambient)', () => { + // Claude Code subagents inherit the parent env byte-for-byte, so the + // per-command marker must outrank whatever the parent session looks like. + expect(kind({ GSTACK_SESSION_KIND: 'spawned' })).toBe('spawned'); + expect(kind({ GSTACK_SESSION_KIND: 'spawned', CONDUCTOR_PORT: '5' })).toBe('spawned'); + expect(kind({ GSTACK_SESSION_KIND: 'spawned', CONDUCTOR_WORKSPACE_PATH: '/x', CI: '1' })).toBe('spawned'); + expect(kind({ GSTACK_SESSION_KIND: 'spawned', GSTACK_HEADLESS: '1' })).toBe('spawned'); + expect(kind({ GSTACK_SESSION_KIND: 'spawned', CLAUDE_CODE_ENTRYPOINT: 'cli' })).toBe('spawned'); + }); + + test('only "spawned" is honored — reserved values fall through to detection', () => { + // Deliberately narrow: "headless" already has GSTACK_HEADLESS, and letting + // an env var force "interactive" over CI markers would be a footgun. + expect(kind({ GSTACK_SESSION_KIND: 'headless' })).toBe('interactive'); + expect(kind({ GSTACK_SESSION_KIND: 'headless', OPENCLAW_SESSION: '1' })).toBe('spawned'); + expect(kind({ GSTACK_SESSION_KIND: 'interactive', CI: '1' })).toBe('headless'); + }); + + test('invalid values are ignored (case-sensitive)', () => { + expect(kind({ GSTACK_SESSION_KIND: 'bogus' })).toBe('interactive'); + expect(kind({ GSTACK_SESSION_KIND: 'bogus', CI: '1' })).toBe('headless'); + expect(kind({ GSTACK_SESSION_KIND: 'SPAWNED' })).toBe('interactive'); + }); + + test('empty GSTACK_SESSION_KIND is treated as unset', () => { + expect(kind({ GSTACK_SESSION_KIND: '' })).toBe('interactive'); + expect(kind({ GSTACK_SESSION_KIND: '', OPENCLAW_SESSION: '1' })).toBe('spawned'); + }); +}); diff --git a/test/gstack-skill-start.test.ts b/test/gstack-skill-start.test.ts index 46dbec9a3..583a4aa11 100644 --- a/test/gstack-skill-start.test.ts +++ b/test/gstack-skill-start.test.ts @@ -197,6 +197,61 @@ describe('gstack-skill-start behavior', () => { } }); + test('spawned override suppresses CONDUCTOR_SESSION, emits SPAWNED_SESSION + block, gates onboarding (#2733)', () => { + const freshGh = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-ss-spawned-')); + fs.writeFileSync(path.join(freshGh, 'config.yaml'), 'update_check: false\n'); + try { + const out = runStart([], { + GSTACK_SESSION_KIND: 'spawned', + CONDUCTOR_WORKSPACE_PATH: '/x', + GSTACK_HOME: freshGh, + }); + expect(out).toMatch(/^SESSION_KIND: spawned$/m); + // spawned outranks Conductor: prose-to-nobody is always wrong. + expect(out).not.toContain('CONDUCTOR_SESSION: true'); + expect(out).toMatch(/^SPAWNED_SESSION: true$/m); + // The ONLY instruction block a spawned session gets is spawned-session — + // none of the 11 interactive-onboarding blocks may emit (no human is + // watching; auto-answered prompts would write config nobody approved). + const ids = (out.match(/^GSTACK_INSTRUCTION_BEGIN: (\S+)/gm) ?? []).map( + (h) => h.replace(/^GSTACK_INSTRUCTION_BEGIN: /, ''), + ); + expect(ids).toEqual(['spawned-session']); + // Script-side ack-at-emit markers stay UNWRITTEN, so the one-time + // prompts fire intact on the next human session. + expect(fs.existsSync(path.join(freshGh, '.activated'))).toBe(false); + expect(fs.existsSync(path.join(freshGh, '.first-loop-tip-shown'))).toBe(false); + expect(fs.existsSync(path.join(freshGh, '.completeness-intro-seen'))).toBe(false); + expect(fs.existsSync(path.join(freshGh, '.telemetry-prompted'))).toBe(false); + } finally { + fs.rmSync(freshGh, { recursive: true, force: true }); + } + }); + + test('legacy OPENCLAW_SESSION still gets full spawned behavior through the kind-keyed gates', () => { + // Regression pin for the raw-marker → $_SESSION_KIND migration (#2733): + // OpenClaw sessions must behave exactly as before the re-keying. + const freshGh = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-ss-openclaw-')); + fs.writeFileSync(path.join(freshGh, 'config.yaml'), 'update_check: false\n'); + try { + const out = runStart([], { + OPENCLAW_SESSION: '1', + CONDUCTOR_WORKSPACE_PATH: '/x', + GSTACK_HOME: freshGh, + }); + expect(out).toMatch(/^SESSION_KIND: spawned$/m); + expect(out).not.toContain('CONDUCTOR_SESSION: true'); + expect(out).toMatch(/^SPAWNED_SESSION: true$/m); + const ids = (out.match(/^GSTACK_INSTRUCTION_BEGIN: (\S+)/gm) ?? []).map( + (h) => h.replace(/^GSTACK_INSTRUCTION_BEGIN: /, ''), + ); + expect(ids).toEqual(['spawned-session']); + expect(fs.existsSync(path.join(freshGh, '.activated'))).toBe(false); + } finally { + fs.rmSync(freshGh, { recursive: true, force: true }); + } + }); + test('display-only tips ack at emit and never re-fire (OV6)', () => { const freshGh = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-ss-refire-')); fs.writeFileSync(path.join(freshGh, 'config.yaml'), 'update_check: false\n'); diff --git a/test/preamble-compose.test.ts b/test/preamble-compose.test.ts index 0ada67f35..c1e6e9454 100644 --- a/test/preamble-compose.test.ts +++ b/test/preamble-compose.test.ts @@ -83,6 +83,9 @@ describe('Conductor signal (skill-start script)', () => { const script = fs.readFileSync(path.join(import.meta.dir, '..', 'bin', 'gstack-skill-start'), 'utf-8'); expect(script).toContain('echo "CONDUCTOR_SESSION: true"'); expect(script).toMatch(/"\$_SESSION_KIND" != "headless"[\s\S]*CONDUCTOR_WORKSPACE_PATH[\s\S]*CONDUCTOR_PORT[\s\S]*CONDUCTOR_SESSION: true/); + // #2733: spawned outranks Conductor — a spawned session inside a Conductor + // workspace auto-chooses instead of rendering prose to nobody. + expect(script).toMatch(/"\$_SESSION_KIND" != "headless"[\s\S]{0,80}"\$_SESSION_KIND" != "spawned"[\s\S]{0,200}CONDUCTOR_SESSION: true/); }); test('claude preamble render invokes the script and interprets CONDUCTOR_SESSION', () => {