diff --git a/packages/spec/scripts/gen-sdui-manifest-cleanup.test.ts b/packages/spec/scripts/gen-sdui-manifest-cleanup.test.ts new file mode 100644 index 0000000000..1b56e35e99 --- /dev/null +++ b/packages/spec/scripts/gen-sdui-manifest-cleanup.test.ts @@ -0,0 +1,160 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// Pins the cleanup contract of `scripts/gen-sdui-manifest.sh`. +// +// WHY THIS IS WORTH AN EXECUTED TEST AND NOT A SOURCE-LEVEL ASSERTION. +// +// The defect this guards was a cleanup that REPORTED success and did nothing: +// the script armed `trap 'kill "$DUMP_DEV_PID"' EXIT`, the trap ran, the script +// printed its own tidy failure message — and the dev server was measured still +// alive 20 minutes later, holding the container's shared heavy-verify flock, so +// every later agent's build queued out at exit 99 with no signal at all. A +// grep-level test ("does the file mention setsid?") would have passed against +// the broken version too, because the broken version also mentioned `kill`. The +// only assertion that distinguishes them is running the trap path and looking +// at what is still breathing afterwards. +// +// The three properties below are each independently load-bearing; the incident +// needed only one of them to be false. +// +// 1. The server runs in its OWN session. Not cosmetic: a background job in a +// non-interactive shell inherits the SCRIPT's process group, and under the +// agent heavy-verify discipline that group is led by the wrapping `flock` +// itself. So `kill -- -$PGID` on the inherited group would kill the +// caller's lock holder and the script. `setsid` is what makes a group kill +// bounded, and this asserts the boundary actually exists. +// +// 2. Nothing in that session holds a descriptor on the caller's lock file. +// `flock(1)` holds its lock on an open fd and background children inherit +// open fds, which is the single step that converts "a leaked dev server" +// into "this container is closed for business". This half matters even +// when the kill works, because it is what makes a MISSED kill survivable. +// +// 3. After the trap, nothing from that session survives — specifically +// including a process that has been reparented to init. That is the case +// `kill "$!"` provably cannot reach, and the test asserts the orphan +// really existed before cleanup (see ORPHAN_BEFORE below) so that a green +// line here can never mean "nothing was ever spawned". +// +// The stub deliberately does not involve vite or pnpm: this pins the lifecycle +// the script owns, and a real console build is neither available nor relevant +// to whether the trap reaps what it started. + +import { describe, it, expect } from 'vitest'; +import { execFileSync } from 'node:child_process'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const HERE = path.dirname(fileURLToPath(import.meta.url)); +const SCRIPT = path.resolve(HERE, '..', '..', '..', 'scripts', 'gen-sdui-manifest.sh'); + +function have(bin: string): boolean { + try { + execFileSync('sh', ['-c', `command -v ${bin}`], { stdio: 'ignore' }); + return true; + } catch { + return false; + } +} + +// Linux-only by construction: the failure being pinned is an agent-container +// one, and the harness reads /proc to see which descriptors a child inherited. +const RUNNABLE = process.platform === 'linux' && ['setsid', 'flock', 'pgrep', 'fuser'].every(have); + +describe.skipIf(!RUNNABLE)('gen-sdui-manifest.sh cleanup contract', () => { + it('reaps the whole session it started, holds no caller fd, and leaves nothing behind', () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'sdui-cleanup-')); + const lock = path.join(dir, 'caller.lock'); + const harness = path.join(dir, 'harness.sh'); + fs.writeFileSync(lock, ''); + + // A stub server that reproduces the shape that defeats `kill "$!"`: it + // spawns a helper and then exits, leaving the helper reparented to init + // while still inside the session the script created. + const stub = [ + 'const { spawn } = require("node:child_process");', + 'spawn(process.execPath, ["-e", "setInterval(() => {}, 1e9)"], { stdio: "ignore" });', + 'setTimeout(() => process.exit(0), 1200);', + ].join(''); + + fs.writeFileSync( + harness, + [ + '#!/usr/bin/env bash', + 'set -euo pipefail', + // Sourcing runs no generation — the script returns right after defining + // its lifecycle helpers, so this exercises the REAL functions. + `source ${JSON.stringify(SCRIPT)}`, + 'DUMP_PID_FILE="$(mktemp "${TMPDIR:-/tmp}/sdui-dump-pid.XXXXXX")"', + `trap 'sdui_stop_detached "$DUMP_PID_FILE"' EXIT`, + `sdui_spawn_detached "$DUMP_PID_FILE" ${JSON.stringify(path.join(dir, 'dev.log'))} \\`, + ` "$(command -v node)" -e ${JSON.stringify(stub)}`, + 'LEADER="$(cat "$DUMP_PID_FILE")"', + 'echo "LEADER=$LEADER"', + 'echo "LEADER_SID=$(ps -o sid= -p "$LEADER" | tr -d " ")"', + 'echo "SCRIPT_PGID=$(ps -o pgid= -p $$ | tr -d " ")"', + // Count inherited descriptors NOW, while the session LEADER is still + // alive. Counting later would inspect only the reparented helper, which + // never inherits the fd anyway (node closes it across its own spawn) — + // an assertion that stays green with the fd hygiene deleted. That green + // was observed before this line moved, so the ordering is the test. + 'FDS=0', + 'for p in $(pgrep -s "$LEADER"); do', + ` n=$(ls -l "/proc/$p/fd" 2>/dev/null | grep -c ${JSON.stringify(path.basename(lock))} || true)`, + ' FDS=$((FDS + n))', + 'done', + 'echo "LOCKFDS=$FDS"', + `echo "LEADER_ALIVE_AT_COUNT=$(kill -0 "$LEADER" 2>/dev/null && echo yes || echo no)"`, + // Now let the stub exit, so its helper is reparented to init before cleanup. + 'sleep 2', + 'MEMBERS="$(pgrep -s "$LEADER" | tr "\\n" " ")"', + 'echo "MEMBERS=$MEMBERS"', + 'ORPHAN=""', + 'for p in $MEMBERS; do [ "$(ps -o ppid= -p "$p" | tr -d " ")" = "1" ] && ORPHAN="$p"; done', + 'echo "ORPHAN_BEFORE=$ORPHAN"', + // Falling off the end fires the EXIT trap, which is the path under test. + ].join('\n'), + { mode: 0o755 }, + ); + + const out = execFileSync( + 'flock', + ['-w', '120', lock, '-c', `bash ${JSON.stringify(harness)}`], + { encoding: 'utf8', timeout: 120_000 }, + ); + const field = (k: string): string => (out.match(new RegExp(`^${k}=(.*)$`, 'm'))?.[1] ?? '').trim(); + + const leader = field('LEADER'); + expect(leader, out).toMatch(/^\d+$/); + + // (1) its own session — so the group kill cannot reach the caller's flock. + expect(field('LEADER_SID'), out).toBe(leader); + expect(field('SCRIPT_PGID'), out).not.toBe(leader); + + // (3, precondition) the orphan the naive `kill "$!"` cannot reach really + // existed. Without this, the survivor assertion below could pass vacuously. + expect(field('ORPHAN_BEFORE'), out).toMatch(/^\d+$/); + + // (2) nothing in the session inherited the caller's lock descriptor — + // counted while the leader was still alive, which is the only moment the + // count can be non-zero and therefore the only moment it means anything. + expect(field('LEADER_ALIVE_AT_COUNT'), out).toBe('yes'); + expect(field('LOCKFDS'), out).toBe('0'); + + // (3) the trap reaped the session, orphan included. + let survivors = ''; + try { + survivors = execFileSync('pgrep', ['-s', leader], { encoding: 'utf8' }).trim(); + } catch { + survivors = ''; // pgrep exits 1 when nothing matches — the passing case. + } + expect(survivors, `processes survived cleanup:\n${survivors}`).toBe(''); + + // and the caller's lock is free again. + expect(() => execFileSync('flock', ['-w', '5', lock, '-c', 'true'])).not.toThrow(); + + fs.rmSync(dir, { recursive: true, force: true }); + }, 120_000); +}); diff --git a/scripts/gen-sdui-manifest.sh b/scripts/gen-sdui-manifest.sh index 5bfe3e5d4f..b14caa1984 100755 --- a/scripts/gen-sdui-manifest.sh +++ b/scripts/gen-sdui-manifest.sh @@ -23,6 +23,148 @@ set -euo pipefail +# --------------------------------------------------------------------------- +# Background dev-server lifecycle. +# +# WHY THIS IS NOT JUST `cmd & ...; trap 'kill $!' EXIT`. +# +# That is what this script used to do, and it was measured failing: the trap +# ran, and the backgrounded process was still alive 20 minutes later, holding +# the container's shared heavy-verify flock. Three separate properties of the +# naive form are wrong, and each one is enough on its own: +# +# 1. `$!` is the `pnpm` wrapper, and killing it does not reap the tree. +# Measured here: SIGTERM to the wrapper leaves descendants reparented to +# init (PPID 1) that no `kill "$!"` can ever reach. Signalling the process +# GROUP is what reaches them. +# +# 2. You cannot signal the group without `setsid` first. A background job in +# a non-interactive shell does NOT get its own process group — it inherits +# the script's. Measured under agent discipline, the backgrounded server's +# PGID was the PID of the wrapping `flock` itself, so the "obvious" fix, +# `kill -- -$PGID`, would have killed the caller's flock and this script. +# `setsid` gives the server its own session so the group kill is bounded. +# +# 3. Background children inherit open descriptors, and `flock(1)` holds its +# lock on an open fd. Measured: the backgrounded wrapper carried the +# caller's lock fd (`fuser -v` listed it as a holder), which is what turns +# "a leaked dev server" into "every later agent in this container queues +# out at exit 99 with no signal". Closing inherited fds in the child means +# a kill that misses is merely untidy instead of a container-wide outage. +# +# A note on what was NOT determined: the exact reason the original SIGTERM +# failed to reap that particular run is not reproducible from here (it needs a +# real vite under a real console build tree). So the cleanup below is +# deliberately cause-agnostic — it escalates TERM -> KILL, it VERIFIES, and it +# says so loudly when it cannot finish the job, rather than assuming any one +# diagnosis. A cleanup that can fail silently is the whole defect. +# --------------------------------------------------------------------------- + +SDUI_HAVE_SETSID=0 +if command -v setsid > /dev/null 2>&1; then SDUI_HAVE_SETSID=1; fi + +# Every pid in the session led by $1 (setsid path), or $1 plus its descendants +# (fallback path). Prints nothing when the tree is gone. +sdui_live_pids() { + local leader="$1" out="" + if [[ "$SDUI_HAVE_SETSID" == 1 ]]; then + out="$(pgrep -s "$leader" 2>/dev/null || true)" + else + kill -0 "$leader" 2>/dev/null && out="$leader" + local frontier="$leader" next="" + while [[ -n "$frontier" ]]; do + next="$(pgrep -P "${frontier// /,}" 2>/dev/null || true)" + [[ -z "$next" ]] && break + out="${out}${out:+ }${next//$'\n'/ }" + frontier="${next//$'\n'/ }" + done + fi + printf '%s' "$out" | tr '\n' ' ' | tr -s ' ' +} + +# Start "$@" detached: its own session (so the group is safe to signal) and no +# inherited descriptors above stderr (so an orphan cannot hold a caller's lock). +# Writes the leader pid to $1. +sdui_spawn_detached() { + local pidfile="$1" logfile="$2"; shift 2 + : > "$pidfile" + + # Runs as the new session leader, before exec'ing the real command. + local runner=' +pidfile="$1"; shift +printf "%s\n" "$$" > "$pidfile" +if [ -d "/proc/$$/fd" ]; then + for fd in /proc/$$/fd/*; do + n=${fd##*/} + # 0/1/2 are this job'"'"'s own stdio; 255 is reserved by bash itself. + case "$n" in 0|1|2|255) continue ;; esac + eval "exec ${n}>&-" 2>/dev/null || true + done +fi +exec "$@" +' + if [[ "$SDUI_HAVE_SETSID" == 1 ]]; then + setsid bash -c "$runner" sdui-dump "$pidfile" "$@" < /dev/null > "$logfile" 2>&1 & + else + bash -c "$runner" sdui-dump "$pidfile" "$@" < /dev/null > "$logfile" 2>&1 & + fi + + local _i + for _i in $(seq 1 100); do + [[ -s "$pidfile" ]] && return 0 + sleep 0.05 + done + echo "⚠ dev server did not report its pid within 5s — cleanup may be incomplete." >&2 + return 0 +} + +# Stop what sdui_spawn_detached started, and VERIFY. Idempotent. +sdui_stop_detached() { + local pidfile="${1:-}" leader="" sig="" left="" _i + [[ -n "$pidfile" && -s "$pidfile" ]] && leader="$(tr -d '[:space:]' < "$pidfile" 2>/dev/null || true)" + [[ -n "$pidfile" ]] && rm -f "$pidfile" 2>/dev/null || true + case "$leader" in ''|*[!0-9]*) return 0 ;; esac + + for sig in TERM KILL; do + left="$(sdui_live_pids "$leader")" + [[ -z "${left// /}" ]] && return 0 + if [[ "$SDUI_HAVE_SETSID" == 1 ]]; then + kill -"$sig" -- "-$leader" 2>/dev/null || true + else + # shellcheck disable=SC2086 + kill -"$sig" $left 2>/dev/null || true + fi + for _i in $(seq 1 40); do + left="$(sdui_live_pids "$leader")" + [[ -z "${left// /}" ]] && return 0 + sleep 0.25 + done + done + + left="$(sdui_live_pids "$leader")" + if [[ -n "${left// /}" ]]; then + echo "✗ could not stop the SDUI dump dev server; these processes survived SIGKILL:" >&2 + # shellcheck disable=SC2086 + ps -o pid,ppid,pgid,etime,args -p $left 2>/dev/null >&2 || true + echo " They may hold descriptors inherited from this run (e.g. a flock held by the caller)." >&2 + fi + return 0 +} + +sdui_on_signal() { + local sig="$1" + sdui_stop_detached "${DUMP_PID_FILE:-}" + trap - "$sig" EXIT + kill -"$sig" "$$" +} + +# Sourced rather than executed: publish the lifecycle helpers above and do +# nothing else, so the cleanup contract can be exercised without a console +# build. `${BASH_SOURCE[0]}` differs from `$0` exactly when this file is sourced. +if [[ "${BASH_SOURCE[0]}" != "$0" ]]; then + return 0 +fi + FRAMEWORK_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" SHA_FILE="${FRAMEWORK_ROOT}/.objectui-sha" @@ -50,12 +192,29 @@ fi echo "→ Generating SDUI public-tier manifest (ADR-0080) from objectui@${PINNED_SHA:0:12}..." pushd "$BUILD_ROOT" > /dev/null -pnpm --filter @object-ui/console exec vite dev --port 5180 > /tmp/sdui-dump-dev.log 2>&1 & -DUMP_DEV_PID=$! -trap 'kill "$DUMP_DEV_PID" 2>/dev/null || true' EXIT -for _ in $(seq 1 90); do curl -sf "http://localhost:5180/" > /dev/null 2>&1 && break; sleep 1; done -if BASE_URL="http://localhost:5180" OUT="${TARGET}/sdui.manifest.json" node scripts/dump-public-manifest.mjs; then +DUMP_PORT=5180 +DUMP_DEV_LOG="/tmp/sdui-dump-dev.log" +DUMP_PID_FILE="$(mktemp "${TMPDIR:-/tmp}/sdui-dump-pid.XXXXXX")" + +# Armed BEFORE the spawn: everything from here on must be able to fail without +# leaving a server behind. EXIT alone is not enough — bash runs no EXIT trap +# when the script is itself signalled, and being signalled is exactly how an +# agent container reclaims a run. +trap 'sdui_stop_detached "${DUMP_PID_FILE:-}"' EXIT +trap 'sdui_on_signal INT' INT +trap 'sdui_on_signal TERM' TERM +trap 'sdui_on_signal HUP' HUP + +sdui_spawn_detached "$DUMP_PID_FILE" "$DUMP_DEV_LOG" \ + pnpm --filter @object-ui/console exec vite dev --port "$DUMP_PORT" + +# `curl`, not a socket table: `ss`/`netstat` are absent from the agent dispatch +# containers, and `ss -ltn | grep :$DUMP_PORT` prints nothing there whether or +# not anything is listening (docs/qa/platform-checklist/RUNNER.md). +for _ in $(seq 1 90); do curl -sf "http://localhost:${DUMP_PORT}/" > /dev/null 2>&1 && break; sleep 1; done + +if BASE_URL="http://localhost:${DUMP_PORT}" OUT="${TARGET}/sdui.manifest.json" node scripts/dump-public-manifest.mjs; then echo "✓ wrote ${TARGET}/sdui.manifest.json" else status=$?