Skip to content

fix(scripts): gen-sdui-manifest cleanup reaps the session it started instead of leaking a flock-holding orphan#9580

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
os-steve merged 2 commits into from
Aug 18, 2026
+324 −5
Merged

fix(scripts): gen-sdui-manifest cleanup reaps the session it started instead of leaking a flock-holding orphan #9580

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4f10b4c
fix(scripts): gen-sdui-manifest cleanup reaps the session it started
claude Aug 18, 2026
68bdc2e
test(spec): count inherited fds while the session leader is still alive
claude Aug 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
160 changes: 160 additions & 0 deletions packages/spec/scripts/gen-sdui-manifest-cleanup.test.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -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);
});
169 changes: 164 additions & 5 deletions scripts/gen-sdui-manifest.sh
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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"

Expand DownExpand Up@@ -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=$?
Expand Down
Loading