fix(skills): keep the seam gate's preview attached so --project works again - #3463
Conversation
… again #3310 made `hyperframes preview` pick its launch mode from the TTY: with no `--foreground`, a non-interactive shell now gets a managed background preview. seam-gate.mjs spawns the launcher with piped stdio, so it always takes that path. The launcher exits 0 before the server is ready, which the readiness loop reads as `preview server exited early`, and the detached server escapes the process group the cleanup handler kills. Pass `--foreground`, which the CLI added for exactly this case ("Force an attached preview in a non-interactive shell"). That restores both invariants the function already relied on: a non-null exit code means the server really died, and the server stays in the killable process group. Fixes the false failure and a leaked preview server per --project run. — Rames Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
miguel-heygen
left a comment
There was a problem hiding this comment.
Exact-head review at 947f1a38543c58a2db092ca0ca649c55761c8d08.
The fix is at the correct boundary. motion-doctrine/scripts/seam-gate.mjs:74-78 opts this known non-interactive caller into the CLI’s existing attached mode instead of undoing #3310 globally. That preserves the readiness invariant (child.exitCode means the serving process group actually died) and keeps the foreground preview under the detached launcher group that cleanup already signals. Both skill mirrors are byte-identical.
I traced the dependency rather than taking the body: previewLaunchMode maps non-interactive/no-flag to background and explicit foreground to attached dev/local modes; the managed background child is detached/unref’d, so the old gate cleanup could not reach it. The one flag closes both the false early-exit and leaked-server mechanisms.
No blocking findings. Current-head CI is fully green; the relevant Test: skills, skill mirror/lint, CodeQL, format, and regression contexts all pass.
Verdict: APPROVE
Reasoning: The change uses the supported caller-level opt-out, restores the gate’s existing process/liveness assumptions, and leaves global preview behavior untouched.
— Magi
What
Pass
--foregroundin the seam gate's default preview command, soseam-gate.mjs verify --projectstarts an attached preview again.Why
seam-gate verify --projecthas been failing withpreview server exited earlysince #3310. The film is fine; the gate is misreading a healthy launch.#3310 made
hyperframes previewchoose its launch mode from the TTY:seam-gate.mjsspawns the launcher withstdio: ["ignore", "pipe", "pipe"], sointeractiveis alwaysfalseand it silently takes the background path. Two things then break:0as soon as it has handed off to the managed background server, which is before the server is serving. The readiness loop'schild.exitCode !== nullcheck treats that as a dead server and throws.spawnDetachedPreviewusesdetached: true+unref(), so the server is its own process-group leader. The gate's cleanup kills the launcher's group (process.kill(-child.pid, ...)), which no longer reaches it — every--projectrun leaks a preview server.The misdirection is the expensive part: the message says the film's preview died when the launch actually succeeded.
How
One flag on the default
--server-cmd.--foregroundis the CLI's own supported opt-out, added by #3310 for precisely this caller — its help string is "Force an attached preview in a non-interactive shell."That restores both invariants the surrounding code already assumes: a non-null exit code means the server really died, and the server stays inside the process group cleanup kills. So it fixes the false failure and the leak with the same change.
Deliberately not doing what looked like the obvious alternative — making
previewforeground-by-default again. Backgrounding in every launch mode is what #3310 set out to do, and the opt-out for non-interactive callers already exists. The gate was the thing that hadn't been updated.Both mirrored copies (
.claude/skills/and.agents/skills/) are updated;scripts/check-skill-mirror.mjspasses byte-for-byte.Test plan
The causal chain is pinned by evidence rather than inference:
The gate's spawn really is non-interactive. Spawned a child with the gate's exact
stdio: ["ignore","pipe","pipe"]shape:stdin.isTTY=undefined stdout.isTTY=undefined => interactive=false.Non-interactive without
--foregroundresolves to background. Already pinned by this repo's own committed table inpackages/cli/src/commands/preview.test.ts:146—{background:false, foreground:false, interactive:false, ...}→"background".--foregroundescapes it even when non-interactive. Same table,preview.test.ts:166and:176—{foreground:true, interactive:false, ...}→"dev"/"local", both attached modes.node --checkon both copies,oxfmt --checkclean,check-skill-mirror.mjspasses.Unit tests added/updated — no new test; the behaviour this depends on is already pinned by the existing
previewLaunchModetable, and the changed line is a default command string with no test harness around it.Manual testing performed
Documentation updated (if applicable) — the
--projectcontract is unchanged, soreferences/seam-gate.mdstill describes it correctly.— Rames