Skip to content

Commit b057e53

Browse files
os-steveclaude
andauthored
fix(scripts): gen-sdui-manifest cleanup reaps the session it started instead of leaking a flock-holding orphan (#9580)
* fix(scripts): gen-sdui-manifest cleanup reaps the session it started The EXIT trap armed `kill "$DUMP_DEV_PID"`, ran, and left the dev server 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. Three measured properties of the old form were each independently wrong: - `kill "$!"` does not reap the tree. Descendants reparent to init and no single-pid kill reaches them; signalling the process GROUP does. - The group could not be signalled safely: a background job in a non-interactive shell inherits the SCRIPT's process group, which under the heavy-verify discipline is led by the wrapping `flock` itself. `setsid` makes the group kill bounded. - `flock(1)` holds its lock on an open fd and background children inherit open fds, so the orphan held the caller's lock. Closing inherited descriptors makes a missed kill untidy rather than a container-wide stall. Cleanup now escalates TERM to KILL, verifies, and reports what it could not kill. Traps INT/TERM/HUP too, since bash runs no EXIT trap when signalled. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XqDQYVU5smx29ts9pAErja * test(spec): count inherited fds while the session leader is still alive The lock-fd assertion was a phantom check: it ran after the stub leader had exited, so it only ever inspected the reparented helper, which never inherits the descriptor. Measured — with the fd hygiene deleted the test still passed. Counting while the leader lives turns that ablation red (LOCKFDS=1). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XqDQYVU5smx29ts9pAErja --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent ca2e020 commit b057e53

2 files changed

Lines changed: 324 additions & 5 deletions

File tree

Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,160 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
//
3+
// Pins the cleanup contract of `scripts/gen-sdui-manifest.sh`.
4+
//
5+
// WHY THIS IS WORTH AN EXECUTED TEST AND NOT A SOURCE-LEVEL ASSERTION.
6+
//
7+
// The defect this guards was a cleanup that REPORTED success and did nothing:
8+
// the script armed `trap 'kill "$DUMP_DEV_PID"' EXIT`, the trap ran, the script
9+
// printed its own tidy failure message — and the dev server was measured still
10+
// alive 20 minutes later, holding the container's shared heavy-verify flock, so
11+
// every later agent's build queued out at exit 99 with no signal at all. A
12+
// grep-level test ("does the file mention setsid?") would have passed against
13+
// the broken version too, because the broken version also mentioned `kill`. The
14+
// only assertion that distinguishes them is running the trap path and looking
15+
// at what is still breathing afterwards.
16+
//
17+
// The three properties below are each independently load-bearing; the incident
18+
// needed only one of them to be false.
19+
//
20+
// 1. The server runs in its OWN session. Not cosmetic: a background job in a
21+
// non-interactive shell inherits the SCRIPT's process group, and under the
22+
// agent heavy-verify discipline that group is led by the wrapping `flock`
23+
// itself. So `kill -- -$PGID` on the inherited group would kill the
24+
// caller's lock holder and the script. `setsid` is what makes a group kill
25+
// bounded, and this asserts the boundary actually exists.
26+
//
27+
// 2. Nothing in that session holds a descriptor on the caller's lock file.
28+
// `flock(1)` holds its lock on an open fd and background children inherit
29+
// open fds, which is the single step that converts "a leaked dev server"
30+
// into "this container is closed for business". This half matters even
31+
// when the kill works, because it is what makes a MISSED kill survivable.
32+
//
33+
// 3. After the trap, nothing from that session survives — specifically
34+
// including a process that has been reparented to init. That is the case
35+
// `kill "$!"` provably cannot reach, and the test asserts the orphan
36+
// really existed before cleanup (see ORPHAN_BEFORE below) so that a green
37+
// line here can never mean "nothing was ever spawned".
38+
//
39+
// The stub deliberately does not involve vite or pnpm: this pins the lifecycle
40+
// the script owns, and a real console build is neither available nor relevant
41+
// to whether the trap reaps what it started.
42+
43+
import { describe, it, expect } from 'vitest';
44+
import { execFileSync } from 'node:child_process';
45+
import fs from 'node:fs';
46+
import os from 'node:os';
47+
import path from 'node:path';
48+
import { fileURLToPath } from 'node:url';
49+
50+
const HERE = path.dirname(fileURLToPath(import.meta.url));
51+
const SCRIPT = path.resolve(HERE, '..', '..', '..', 'scripts', 'gen-sdui-manifest.sh');
52+
53+
function have(bin: string): boolean {
54+
try {
55+
execFileSync('sh', ['-c', `command -v ${bin}`], { stdio: 'ignore' });
56+
return true;
57+
} catch {
58+
return false;
59+
}
60+
}
61+
62+
// Linux-only by construction: the failure being pinned is an agent-container
63+
// one, and the harness reads /proc to see which descriptors a child inherited.
64+
const RUNNABLE = process.platform === 'linux' && ['setsid', 'flock', 'pgrep', 'fuser'].every(have);
65+
66+
describe.skipIf(!RUNNABLE)('gen-sdui-manifest.sh cleanup contract', () => {
67+
it('reaps the whole session it started, holds no caller fd, and leaves nothing behind', () => {
68+
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'sdui-cleanup-'));
69+
const lock = path.join(dir, 'caller.lock');
70+
const harness = path.join(dir, 'harness.sh');
71+
fs.writeFileSync(lock, '');
72+
73+
// A stub server that reproduces the shape that defeats `kill "$!"`: it
74+
// spawns a helper and then exits, leaving the helper reparented to init
75+
// while still inside the session the script created.
76+
const stub = [
77+
'const { spawn } = require("node:child_process");',
78+
'spawn(process.execPath, ["-e", "setInterval(() => {}, 1e9)"], { stdio: "ignore" });',
79+
'setTimeout(() => process.exit(0), 1200);',
80+
].join('');
81+
82+
fs.writeFileSync(
83+
harness,
84+
[
85+
'#!/usr/bin/env bash',
86+
'set -euo pipefail',
87+
// Sourcing runs no generation — the script returns right after defining
88+
// its lifecycle helpers, so this exercises the REAL functions.
89+
`source ${JSON.stringify(SCRIPT)}`,
90+
'DUMP_PID_FILE="$(mktemp "${TMPDIR:-/tmp}/sdui-dump-pid.XXXXXX")"',
91+
`trap 'sdui_stop_detached "$DUMP_PID_FILE"' EXIT`,
92+
`sdui_spawn_detached "$DUMP_PID_FILE" ${JSON.stringify(path.join(dir, 'dev.log'))} \\`,
93+
` "$(command -v node)" -e ${JSON.stringify(stub)}`,
94+
'LEADER="$(cat "$DUMP_PID_FILE")"',
95+
'echo "LEADER=$LEADER"',
96+
'echo "LEADER_SID=$(ps -o sid= -p "$LEADER" | tr -d " ")"',
97+
'echo "SCRIPT_PGID=$(ps -o pgid= -p $$ | tr -d " ")"',
98+
// Count inherited descriptors NOW, while the session LEADER is still
99+
// alive. Counting later would inspect only the reparented helper, which
100+
// never inherits the fd anyway (node closes it across its own spawn) —
101+
// an assertion that stays green with the fd hygiene deleted. That green
102+
// was observed before this line moved, so the ordering is the test.
103+
'FDS=0',
104+
'for p in $(pgrep -s "$LEADER"); do',
105+
` n=$(ls -l "/proc/$p/fd" 2>/dev/null | grep -c ${JSON.stringify(path.basename(lock))} || true)`,
106+
' FDS=$((FDS + n))',
107+
'done',
108+
'echo "LOCKFDS=$FDS"',
109+
`echo "LEADER_ALIVE_AT_COUNT=$(kill -0 "$LEADER" 2>/dev/null && echo yes || echo no)"`,
110+
// Now let the stub exit, so its helper is reparented to init before cleanup.
111+
'sleep 2',
112+
'MEMBERS="$(pgrep -s "$LEADER" | tr "\\n" " ")"',
113+
'echo "MEMBERS=$MEMBERS"',
114+
'ORPHAN=""',
115+
'for p in $MEMBERS; do [ "$(ps -o ppid= -p "$p" | tr -d " ")" = "1" ] && ORPHAN="$p"; done',
116+
'echo "ORPHAN_BEFORE=$ORPHAN"',
117+
// Falling off the end fires the EXIT trap, which is the path under test.
118+
].join('\n'),
119+
{ mode: 0o755 },
120+
);
121+
122+
const out = execFileSync(
123+
'flock',
124+
['-w', '120', lock, '-c', `bash ${JSON.stringify(harness)}`],
125+
{ encoding: 'utf8', timeout: 120_000 },
126+
);
127+
const field = (k: string): string => (out.match(new RegExp(`^${k}=(.*)$`, 'm'))?.[1] ?? '').trim();
128+
129+
const leader = field('LEADER');
130+
expect(leader, out).toMatch(/^\d+$/);
131+
132+
// (1) its own session — so the group kill cannot reach the caller's flock.
133+
expect(field('LEADER_SID'), out).toBe(leader);
134+
expect(field('SCRIPT_PGID'), out).not.toBe(leader);
135+
136+
// (3, precondition) the orphan the naive `kill "$!"` cannot reach really
137+
// existed. Without this, the survivor assertion below could pass vacuously.
138+
expect(field('ORPHAN_BEFORE'), out).toMatch(/^\d+$/);
139+
140+
// (2) nothing in the session inherited the caller's lock descriptor —
141+
// counted while the leader was still alive, which is the only moment the
142+
// count can be non-zero and therefore the only moment it means anything.
143+
expect(field('LEADER_ALIVE_AT_COUNT'), out).toBe('yes');
144+
expect(field('LOCKFDS'), out).toBe('0');
145+
146+
// (3) the trap reaped the session, orphan included.
147+
let survivors = '';
148+
try {
149+
survivors = execFileSync('pgrep', ['-s', leader], { encoding: 'utf8' }).trim();
150+
} catch {
151+
survivors = ''; // pgrep exits 1 when nothing matches — the passing case.
152+
}
153+
expect(survivors, `processes survived cleanup:\n${survivors}`).toBe('');
154+
155+
// and the caller's lock is free again.
156+
expect(() => execFileSync('flock', ['-w', '5', lock, '-c', 'true'])).not.toThrow();
157+
158+
fs.rmSync(dir, { recursive: true, force: true });
159+
}, 120_000);
160+
});

‎scripts/gen-sdui-manifest.sh‎

Lines changed: 164 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,148 @@
2323

2424
set -euo pipefail
2525

26+
# ---------------------------------------------------------------------------
27+
# Background dev-server lifecycle.
28+
#
29+
# WHY THIS IS NOT JUST `cmd & ...; trap 'kill $!' EXIT`.
30+
#
31+
# That is what this script used to do, and it was measured failing: the trap
32+
# ran, and the backgrounded process was still alive 20 minutes later, holding
33+
# the container's shared heavy-verify flock. Three separate properties of the
34+
# naive form are wrong, and each one is enough on its own:
35+
#
36+
# 1. `$!` is the `pnpm` wrapper, and killing it does not reap the tree.
37+
# Measured here: SIGTERM to the wrapper leaves descendants reparented to
38+
# init (PPID 1) that no `kill "$!"` can ever reach. Signalling the process
39+
# GROUP is what reaches them.
40+
#
41+
# 2. You cannot signal the group without `setsid` first. A background job in
42+
# a non-interactive shell does NOT get its own process group — it inherits
43+
# the script's. Measured under agent discipline, the backgrounded server's
44+
# PGID was the PID of the wrapping `flock` itself, so the "obvious" fix,
45+
# `kill -- -$PGID`, would have killed the caller's flock and this script.
46+
# `setsid` gives the server its own session so the group kill is bounded.
47+
#
48+
# 3. Background children inherit open descriptors, and `flock(1)` holds its
49+
# lock on an open fd. Measured: the backgrounded wrapper carried the
50+
# caller's lock fd (`fuser -v` listed it as a holder), which is what turns
51+
# "a leaked dev server" into "every later agent in this container queues
52+
# out at exit 99 with no signal". Closing inherited fds in the child means
53+
# a kill that misses is merely untidy instead of a container-wide outage.
54+
#
55+
# A note on what was NOT determined: the exact reason the original SIGTERM
56+
# failed to reap that particular run is not reproducible from here (it needs a
57+
# real vite under a real console build tree). So the cleanup below is
58+
# deliberately cause-agnostic — it escalates TERM -> KILL, it VERIFIES, and it
59+
# says so loudly when it cannot finish the job, rather than assuming any one
60+
# diagnosis. A cleanup that can fail silently is the whole defect.
61+
# ---------------------------------------------------------------------------
62+
63+
SDUI_HAVE_SETSID=0
64+
if command -v setsid > /dev/null 2>&1; then SDUI_HAVE_SETSID=1; fi
65+
66+
# Every pid in the session led by $1 (setsid path), or $1 plus its descendants
67+
# (fallback path). Prints nothing when the tree is gone.
68+
sdui_live_pids() {
69+
local leader="$1" out=""
70+
if [[ "$SDUI_HAVE_SETSID" == 1 ]]; then
71+
out="$(pgrep -s "$leader" 2>/dev/null || true)"
72+
else
73+
kill -0 "$leader" 2>/dev/null && out="$leader"
74+
local frontier="$leader" next=""
75+
while [[ -n "$frontier" ]]; do
76+
next="$(pgrep -P "${frontier// /,}" 2>/dev/null || true)"
77+
[[ -z "$next" ]] && break
78+
out="${out}${out:+ }${next//$'\n'/ }"
79+
frontier="${next//$'\n'/ }"
80+
done
81+
fi
82+
printf '%s' "$out" | tr '\n' ' ' | tr -s ' '
83+
}
84+
85+
# Start "$@" detached: its own session (so the group is safe to signal) and no
86+
# inherited descriptors above stderr (so an orphan cannot hold a caller's lock).
87+
# Writes the leader pid to $1.
88+
sdui_spawn_detached() {
89+
local pidfile="$1" logfile="$2"; shift 2
90+
: > "$pidfile"
91+
92+
# Runs as the new session leader, before exec'ing the real command.
93+
local runner='
94+
pidfile="$1"; shift
95+
printf "%s\n" "$$" > "$pidfile"
96+
if [ -d "/proc/$$/fd" ]; then
97+
for fd in /proc/$$/fd/*; do
98+
n=${fd##*/}
99+
# 0/1/2 are this job'"'"'s own stdio; 255 is reserved by bash itself.
100+
case "$n" in 0|1|2|255) continue ;; esac
101+
eval "exec ${n}>&-" 2>/dev/null || true
102+
done
103+
fi
104+
exec "$@"
105+
'
106+
if [[ "$SDUI_HAVE_SETSID" == 1 ]]; then
107+
setsid bash -c "$runner" sdui-dump "$pidfile" "$@" < /dev/null > "$logfile" 2>&1 &
108+
else
109+
bash -c "$runner" sdui-dump "$pidfile" "$@" < /dev/null > "$logfile" 2>&1 &
110+
fi
111+
112+
local _i
113+
for _i in $(seq 1 100); do
114+
[[ -s "$pidfile" ]] && return 0
115+
sleep 0.05
116+
done
117+
echo "⚠ dev server did not report its pid within 5s — cleanup may be incomplete." >&2
118+
return 0
119+
}
120+
121+
# Stop what sdui_spawn_detached started, and VERIFY. Idempotent.
122+
sdui_stop_detached() {
123+
local pidfile="${1:-}" leader="" sig="" left="" _i
124+
[[ -n "$pidfile" && -s "$pidfile" ]] && leader="$(tr -d '[:space:]' < "$pidfile" 2>/dev/null || true)"
125+
[[ -n "$pidfile" ]] && rm -f "$pidfile" 2>/dev/null || true
126+
case "$leader" in ''|*[!0-9]*) return 0 ;; esac
127+
128+
for sig in TERM KILL; do
129+
left="$(sdui_live_pids "$leader")"
130+
[[ -z "${left// /}" ]] && return 0
131+
if [[ "$SDUI_HAVE_SETSID" == 1 ]]; then
132+
kill -"$sig" -- "-$leader" 2>/dev/null || true
133+
else
134+
# shellcheck disable=SC2086
135+
kill -"$sig" $left 2>/dev/null || true
136+
fi
137+
for _i in $(seq 1 40); do
138+
left="$(sdui_live_pids "$leader")"
139+
[[ -z "${left// /}" ]] && return 0
140+
sleep 0.25
141+
done
142+
done
143+
144+
left="$(sdui_live_pids "$leader")"
145+
if [[ -n "${left// /}" ]]; then
146+
echo "✗ could not stop the SDUI dump dev server; these processes survived SIGKILL:" >&2
147+
# shellcheck disable=SC2086
148+
ps -o pid,ppid,pgid,etime,args -p $left 2>/dev/null >&2 || true
149+
echo " They may hold descriptors inherited from this run (e.g. a flock held by the caller)." >&2
150+
fi
151+
return 0
152+
}
153+
154+
sdui_on_signal() {
155+
local sig="$1"
156+
sdui_stop_detached "${DUMP_PID_FILE:-}"
157+
trap - "$sig" EXIT
158+
kill -"$sig" "$$"
159+
}
160+
161+
# Sourced rather than executed: publish the lifecycle helpers above and do
162+
# nothing else, so the cleanup contract can be exercised without a console
163+
# build. `${BASH_SOURCE[0]}` differs from `$0` exactly when this file is sourced.
164+
if [[ "${BASH_SOURCE[0]}" != "$0" ]]; then
165+
return 0
166+
fi
167+
26168
FRAMEWORK_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
27169
SHA_FILE="${FRAMEWORK_ROOT}/.objectui-sha"
28170

@@ -50,12 +192,29 @@ fi
50192

51193
echo "→ Generating SDUI public-tier manifest (ADR-0080) from objectui@${PINNED_SHA:0:12}..."
52194
pushd "$BUILD_ROOT" > /dev/null
53-
pnpm --filter @object-ui/console exec vite dev --port 5180 > /tmp/sdui-dump-dev.log 2>&1 &
54-
DUMP_DEV_PID=$!
55-
trap 'kill "$DUMP_DEV_PID" 2>/dev/null || true' EXIT
56-
for _ in $(seq 1 90); do curl -sf "http://localhost:5180/" > /dev/null 2>&1 && break; sleep 1; done
57195

58-
if BASE_URL="http://localhost:5180" OUT="${TARGET}/sdui.manifest.json" node scripts/dump-public-manifest.mjs; then
196+
DUMP_PORT=5180
197+
DUMP_DEV_LOG="/tmp/sdui-dump-dev.log"
198+
DUMP_PID_FILE="$(mktemp "${TMPDIR:-/tmp}/sdui-dump-pid.XXXXXX")"
199+
200+
# Armed BEFORE the spawn: everything from here on must be able to fail without
201+
# leaving a server behind. EXIT alone is not enough — bash runs no EXIT trap
202+
# when the script is itself signalled, and being signalled is exactly how an
203+
# agent container reclaims a run.
204+
trap 'sdui_stop_detached "${DUMP_PID_FILE:-}"' EXIT
205+
trap 'sdui_on_signal INT' INT
206+
trap 'sdui_on_signal TERM' TERM
207+
trap 'sdui_on_signal HUP' HUP
208+
209+
sdui_spawn_detached "$DUMP_PID_FILE" "$DUMP_DEV_LOG" \
210+
pnpm --filter @object-ui/console exec vite dev --port "$DUMP_PORT"
211+
212+
# `curl`, not a socket table: `ss`/`netstat` are absent from the agent dispatch
213+
# containers, and `ss -ltn | grep :$DUMP_PORT` prints nothing there whether or
214+
# not anything is listening (docs/qa/platform-checklist/RUNNER.md).
215+
for _ in $(seq 1 90); do curl -sf "http://localhost:${DUMP_PORT}/" > /dev/null 2>&1 && break; sleep 1; done
216+
217+
if BASE_URL="http://localhost:${DUMP_PORT}" OUT="${TARGET}/sdui.manifest.json" node scripts/dump-public-manifest.mjs; then
59218
echo "✓ wrote ${TARGET}/sdui.manifest.json"
60219
else
61220
status=$?

0 commit comments

Comments
 (0)