From a5e5a0b8768d05d92d6b9b0658f1b949a3500e3b Mon Sep 17 00:00:00 2001 From: Daniel Ochoa Date: Wed, 21 Oct 2026 16:28:58 -0500 Subject: [PATCH 1/4] fix(vibe): never signal pid 1 or an unowned process group - A helpers.test.mjs run recorded pid 1 and stopped it; endProcesses counted EPERM as alive and sent kill(-1), which signals every process the user owns. It ended every app on the operator's machine twice. - helpers.mjs refuses to record a pid of 1 or lower. endProcesses never probes or signals one, nor its own or its parent's pid. It signals a whole group only for a pid recorded with --group whose pgid still equals the pid, and never signals a pid that EPERM says is not ours. It takes an injected kill, so its tests send no real signal. - Every other group signal (run-bounded, precedent-check, the vibe-workers ledger and Claude worker, two tests) checks the pid above 1 first; dist rebuilt. - test_vibe_signal_safety_contract.py fails when a plugin test signals 0, 1, a negative literal or its own pid, writes one into a pids ledger, or calls endProcesses without a fake kill, and when any source signals a group without checking the pid above 1. - The cli-worker fake Codex ends with process.abort() instead of signalling its own pid. Testing: node --test, pytest, ruff, pyright and the vibe-workers vitest and build on a second machine; each safety check shown failing with its fix removed. Risks: a helper started without --group now has only its own pid signalled, so children it spawned in another group need their own recorded pids. --- .../scripts/cli-worker-session.test.mjs | 6 +- ...ct-resume-cross-package.black-box.test.mjs | 1 + .../fleet-orchestrator/scripts/helpers.mjs | 208 ++++++++++++++++-- .../scripts/helpers.test.mjs | 190 +++++++++++++++- .../vibe/scripts/dist/claude-worker.mjs | 5 +- .../skills/vibe/scripts/dist/writer-state.mjs | 1 + .../skills/vibe/scripts/precedent-check.mjs | 73 +++++- .../vibe/scripts/precedent-check.test.mjs | 36 ++- .../vibe/skills/vibe/scripts/run-bounded.mjs | 2 + .../test_vibe_signal_safety_contract.py | 172 +++++++++++++++ tools/vibe-workers/src/claude-worker.ts | 4 +- tools/vibe-workers/src/ledger.test.ts | 2 +- tools/vibe-workers/src/ledger.ts | 1 + 13 files changed, 657 insertions(+), 44 deletions(-) create mode 100644 plugins/vibe/tools/python/test_vibe_signal_safety_contract.py diff --git a/plugins/vibe/skills/cl-sweep/scripts/cli-worker-session.test.mjs b/plugins/vibe/skills/cl-sweep/scripts/cli-worker-session.test.mjs index d5aa57d9..2eee59ef 100644 --- a/plugins/vibe/skills/cl-sweep/scripts/cli-worker-session.test.mjs +++ b/plugins/vibe/skills/cl-sweep/scripts/cli-worker-session.test.mjs @@ -448,7 +448,9 @@ if (args.includes('--help')) { process.stdout.write(args.includes('resume') ? 'Usage: codex exec resume [SESSION_ID] [PROMPT]\\n-m, --model \\n' : '-C, --cd \\n-m, --model \\n'); process.exit(0); } -process.kill(process.pid, 'SIGKILL'); +// Ends by a signal without signalling any pid; stderr is closed so abort's report does not replace the signal. +require('node:fs').closeSync(2); +process.abort(); `); chmodSync(fakeCodex, 0o755); const session = join(state, 'session.json'); @@ -458,7 +460,7 @@ process.kill(process.pid, 'SIGKILL'); const failed = spawnSync('node', [script, 'run', '--session-file', session, '--prompt-file', prompt, '--events-file', join(state, 'events.jsonl'), '--codex', fakeCodex], { encoding: 'utf8' }); assert.notEqual(failed.status, 0); assert.doesNotMatch(failed.stderr, /Codex exited null/); - assert.match(JSON.parse(readFileSync(session, 'utf8')).lastFailure, /SIGKILL/); + assert.match(JSON.parse(readFileSync(session, 'utf8')).lastFailure, /SIGABRT/); const healthy = setup().fakeCodex; const retryPrompt = join(state, 'retry.txt'); diff --git a/plugins/vibe/skills/cl-sweep/scripts/project-resume-cross-package.black-box.test.mjs b/plugins/vibe/skills/cl-sweep/scripts/project-resume-cross-package.black-box.test.mjs index 2e16a774..23c72969 100644 --- a/plugins/vibe/skills/cl-sweep/scripts/project-resume-cross-package.black-box.test.mjs +++ b/plugins/vibe/skills/cl-sweep/scripts/project-resume-cross-package.black-box.test.mjs @@ -50,6 +50,7 @@ function spawnPublicCli(args, env) { async function killPublicCli(processHandle) { assert.equal(processHandle.child.exitCode, null, 'public CLI must still be alive before forced termination'); + assert.ok(processHandle.child.pid > 1, 'signalling group 1 would reach every process the user owns'); process.kill(-processHandle.child.pid, 'SIGKILL'); const exit = await processHandle.exited; assert.equal(exit.code, null); diff --git a/plugins/vibe/skills/fleet-orchestrator/scripts/helpers.mjs b/plugins/vibe/skills/fleet-orchestrator/scripts/helpers.mjs index f13bfbef..aa387f02 100644 --- a/plugins/vibe/skills/fleet-orchestrator/scripts/helpers.mjs +++ b/plugins/vibe/skills/fleet-orchestrator/scripts/helpers.mjs @@ -5,29 +5,135 @@ // brief at the next trigger. `status` lists every helper whose result was // accepted more than --idle-minutes ago and that is not recorded as stopped. // -// Usage: helpers.mjs start [--brief ] +// Usage: helpers.mjs start [--brief ] [--stack ] +// helpers.mjs pid [--group] // helpers.mjs accepted -// helpers.mjs stopped +// helpers.mjs stopped [--grace-seconds ] +// helpers.mjs hand --to +// helpers.mjs unhand // helpers.mjs status [--idle-minutes ] [--now ] // -// `status` prints `STOP accepted m ago` lines, then `helpers: active, to stop`, -// and exits 1 when any helper should be stopped. +// `pid` records processes a helper started (dev servers, app stacks); the +// helper records its own. Pass `--group` only for a process the helper spawned +// detached, so it leads its own process group. `stopped` ends every recorded +// process that is still alive and ours (SIGTERM, then SIGKILL after the grace) +// and records the helper stopped only when none is left; otherwise it prints +// `ALIVE ` and exits 1. A whole group is signalled only for a +// `--group` pid whose process group id still equals the pid; any other pid is +// signalled alone. A pid of 1 or lower, this process or its parent is never +// recorded or signalled, and neither is a pid the system says is not ours +// (EPERM): a recorded pid can be reused by an unrelated process. +// `hand` marks a running app stack (a worktree path, port or URL) as handed to +// a human QA guide; `start --stack` on a handed stack exits 1, so a rerun that +// toggles flags or moves data launches its own stack. +// +// `status` prints `STOP accepted m ago` and `ORPHAN still running` +// lines, then `helpers: active, to stop, orphaned`, +// and exits 1 when any helper should be stopped or left processes running. +import { spawnSync } from "node:child_process"; import { existsSync, readFileSync, renameSync, writeFileSync } from "node:fs"; import path from "node:path"; import { pathToFileURL } from "node:url"; import { parseArgs } from "node:util"; const LEDGER = "helpers.json"; +const STACKS = "#stacks"; const DEFAULT_IDLE_MINUTES = 5; +const DEFAULT_GRACE_SECONDS = 10; +const POLL_MS = 200; /** Helpers to stop: accepted more than `idleMinutes` before `now` and never recorded stopped. */ export function stopCandidates(ledger, now, idleMinutes = DEFAULT_IDLE_MINUTES) { - return Object.entries(ledger) + return helperEntries(ledger) .filter(([, helper]) => helper.acceptedAt && !helper.stoppedAt && now - Date.parse(helper.acceptedAt) > idleMinutes * 60_000) .map(([name, helper]) => ({ name, minutes: Math.floor((now - Date.parse(helper.acceptedAt)) / 60_000), brief: helper.brief ?? null })); } +/** Stopped helpers whose recorded processes still run. Stopped QA agents once left dev servers running for 13 to 16 hours on a loaded machine. */ +export function orphans(ledger, alive = isAlive) { + return helperEntries(ledger) + .filter(([, helper]) => helper.stoppedAt) + .map(([name, helper]) => ({ name, pids: (helper.pids ?? []).filter(alive) })) + .filter((helper) => helper.pids.length > 0); +} + +/** Whether endProcesses may signal a pid: kill(-1) reaches every process the user owns and kill(0) the caller's own group. */ +export function safePid(pid) { + return Number.isSafeInteger(pid) && pid > 1 && pid !== process.pid && pid !== process.ppid; +} + +/** Parses recorded pids, refusing anything that is not an integer above 1. */ +export function parsePids(texts) { + const pids = texts.map(Number); + if (pids.length === 0 || !pids.every((pid) => Number.isSafeInteger(pid) && pid > 1)) { + throw new Error("pid needs one or more process ids above 1."); + } + return pids; +} + +/** `alive`, `gone`, or `foreign` when the process exists but is not ours (EPERM). */ +export function probe(pid, kill = defaultKill) { + try { + kill(pid, 0); + return "alive"; + } catch (error) { + return error?.code === "EPERM" ? "foreign" : "gone"; + } +} + +/** Whether a process exists, ours or not. */ +export function isAlive(pid) { + return probe(pid) !== "gone"; +} + +/** The process group id of `pid`, or null when it cannot be read. */ +export function readGroupId(pid) { + const result = spawnSync("ps", ["-o", "pgid=", "-p", String(pid)], { encoding: "utf8" }); + const group = Number(result.stdout?.trim()); + return result.status === 0 && Number.isSafeInteger(group) ? group : null; +} + +/** + * Ends `targets` ({ pid, group }), SIGKILLs what outlives the grace, and + * returns the pids still alive. Unsafe and foreign pids are never signalled. + * `kill` and `groupOf` are injectable so tests can check every decision + * without sending a real signal. + */ +export async function endProcesses(targets, graceSeconds = DEFAULT_GRACE_SECONDS, { kill = defaultKill, groupOf = readGroupId } = {}) { + const owned = targets.filter((target) => safePid(target.pid)); + const live = () => owned.filter((target) => probe(target.pid, kill) === "alive"); + const signal = ({ pid, group }, name) => { + // Checked again here so no edit to the filter above can ever reach kill(-1). + if (!(pid > 1)) return; + const wholeGroup = group === true && groupOf(pid) === pid; + try { + kill(wholeGroup ? -pid : pid, name); + } catch { + // Already gone. + } + }; + for (const target of live()) signal(target, "SIGTERM"); + const deadline = Date.now() + graceSeconds * 1000; + while (live().length > 0 && Date.now() < deadline) { + await new Promise((resolve) => setTimeout(resolve, POLL_MS)); + } + for (const target of live()) signal(target, "SIGKILL"); + const killDeadline = Date.now() + 2000; + while (live().length > 0 && Date.now() < killDeadline) { + await new Promise((resolve) => setTimeout(resolve, POLL_MS)); + } + return live().map((target) => target.pid); +} + +function defaultKill(pid, name) { + process.kill(pid, name); +} + +function helperEntries(ledger) { + return Object.entries(ledger).filter(([name]) => name !== STACKS); +} + function read(orchDir) { const file = path.join(orchDir, LEDGER); return existsSync(file) ? JSON.parse(readFileSync(file, "utf8")) : {}; @@ -39,46 +145,100 @@ function write(orchDir, ledger) { renameSync(`${file}.tmp`, file); } -function main(argv) { +function requireName(action, name) { + if (!name) { + throw new Error(`${action} needs a name.`); + } + if (name === STACKS) { + throw new Error(`${name} is reserved.`); + } + return name; +} + +async function main(argv) { const [orchDir, action, ...rest] = argv; const { values, positionals } = parseArgs({ args: rest, allowPositionals: true, - options: { brief: { type: "string" }, "idle-minutes": { type: "string" }, now: { type: "string" } }, + options: { + brief: { type: "string" }, + stack: { type: "string" }, + to: { type: "string" }, + "idle-minutes": { type: "string" }, + "grace-seconds": { type: "string" }, + group: { type: "boolean" }, + now: { type: "string" }, + }, }); - if (!orchDir || !["start", "accepted", "stopped", "status"].includes(action)) { - throw new Error("Usage: helpers.mjs start|accepted|stopped | status [--idle-minutes ]"); + if (!orchDir || !["start", "pid", "accepted", "stopped", "hand", "unhand", "status"].includes(action)) { + throw new Error("Usage: helpers.mjs start|pid|accepted|stopped|hand|unhand | status [--idle-minutes ]"); } const ledger = read(orchDir); const now = values.now ? Date.parse(values.now) : Date.now(); + const at = new Date(now).toISOString(); if (action === "status") { const stop = stopCandidates(ledger, now, values["idle-minutes"] === undefined ? DEFAULT_IDLE_MINUTES : Number(values["idle-minutes"])); for (const helper of stop) { process.stdout.write(`STOP ${helper.name} accepted ${helper.minutes}m ago${helper.brief ? `; relaunch from ${helper.brief}` : ""}\n`); } - const active = Object.values(ledger).filter((helper) => !helper.stoppedAt).length; - process.stdout.write(`helpers: ${active} active, ${stop.length} to stop\n`); - return stop.length === 0 ? 0 : 1; + const left = orphans(ledger); + for (const helper of left) { + process.stdout.write(`ORPHAN ${helper.name} ${helper.pids.join(" ")} still running\n`); + } + const active = helperEntries(ledger).filter(([, helper]) => !helper.stoppedAt).length; + process.stdout.write(`helpers: ${active} active, ${stop.length} to stop, ${left.length} orphaned\n`); + return stop.length + left.length === 0 ? 0 : 1; } - const [name] = positionals; - if (!name) { - throw new Error(`${action} needs a helper name.`); + const name = requireName(action, positionals[0]); + const stacks = ledger[STACKS] ?? {}; + if (action === "hand" || action === "unhand") { + if (action === "hand") { + if (!values.to) { + throw new Error("hand needs --to ."); + } + stacks[name] = { guide: values.to, handedAt: at }; + } else { + delete stacks[name]; + } + ledger[STACKS] = stacks; + write(orchDir, ledger); + return 0; } - const at = new Date(now).toISOString(); if (action === "start") { - ledger[name] = { startedAt: at, ...(values.brief ? { brief: values.brief } : {}) }; + // A QA rerun must never change the stack a human is testing on. + if (values.stack && stacks[values.stack]) { + process.stderr.write(`stack ${values.stack} is handed to ${stacks[values.stack].guide}; launch this helper on its own stack\n`); + return 1; + } + ledger[name] = { startedAt: at, ...(values.brief ? { brief: values.brief } : {}), ...(values.stack ? { stack: values.stack } : {}) }; + } else if (action === "pid") { + const pids = parsePids(positionals.slice(1)); + const entry = ledger[name] ?? {}; + const groups = values.group ? [...new Set([...(entry.groups ?? []), ...pids])] : (entry.groups ?? []); + ledger[name] = { ...entry, pids: [...new Set([...(entry.pids ?? []), ...pids])], ...(groups.length > 0 ? { groups } : {}) }; + } else if (action === "accepted") { + ledger[name] = { ...(ledger[name] ?? {}), acceptedAt: at }; } else { - ledger[name] = { ...(ledger[name] ?? {}), [action === "accepted" ? "acceptedAt" : "stoppedAt"]: at }; + const grace = values["grace-seconds"] === undefined ? DEFAULT_GRACE_SECONDS : Number(values["grace-seconds"]); + const entry = ledger[name] ?? {}; + const groups = new Set(entry.groups ?? []); + const survivors = await endProcesses((entry.pids ?? []).map((pid) => ({ pid, group: groups.has(pid) })), grace); + if (survivors.length > 0) { + process.stdout.write(`ALIVE ${name} ${survivors.join(" ")}\n`); + return 1; + } + ledger[name] = { ...(ledger[name] ?? {}), stoppedAt: at }; } write(orchDir, ledger); return 0; } if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { - try { - process.exit(main(process.argv.slice(2))); - } catch (error) { - process.stderr.write(`${error instanceof Error ? error.message : String(error)}\n`); - process.exit(2); - } + main(process.argv.slice(2)).then( + (code) => process.exit(code), + (error) => { + process.stderr.write(`${error instanceof Error ? error.message : String(error)}\n`); + process.exit(2); + } + ); } diff --git a/plugins/vibe/skills/fleet-orchestrator/scripts/helpers.test.mjs b/plugins/vibe/skills/fleet-orchestrator/scripts/helpers.test.mjs index b52cc67d..73b7a4aa 100644 --- a/plugins/vibe/skills/fleet-orchestrator/scripts/helpers.test.mjs +++ b/plugins/vibe/skills/fleet-orchestrator/scripts/helpers.test.mjs @@ -1,15 +1,91 @@ import assert from 'node:assert/strict'; import { spawnSync } from 'node:child_process'; -import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; import os from 'node:os'; import path from 'node:path'; import { test } from 'node:test'; import { fileURLToPath } from 'node:url'; import { historyOrigin } from './history-origin.mjs'; -import { stopCandidates } from './helpers.mjs'; +import { endProcesses, isAlive, orphans, parsePids, stopCandidates } from './helpers.mjs'; const HERE = path.dirname(fileURLToPath(import.meta.url)); const run = (args) => spawnSync(process.execPath, [path.join(HERE, 'helpers.mjs'), ...args], { encoding: 'utf8' }); +const tempDir = (t) => { + const dir = mkdtempSync(path.join(os.tmpdir(), 'fleet-helpers-')); + t.after(() => rmSync(dir, { recursive: true, force: true })); + return dir; +}; +/** A pid this test started; anything else (empty output parses as 0) fails the test before any signal. */ +const ownedPid = (text) => { + const pid = Number(String(text).trim()); + assert.ok(Number.isSafeInteger(pid) && pid > 1 && pid !== process.pid && pid !== process.ppid, `not a pid this test started: ${text}`); + return pid; +}; +/** Signals one owned pid, never its group. */ +const killOwned = (pid) => { + if (!(pid > 1) || pid === process.pid) return; + try { + process.kill(pid, 'SIGKILL'); + } catch { + // Already gone. + } +}; +/** + * A background process the test owns, started through a shell that exits at + * once, so its parent is not this test (a dead direct child would stay a + * zombie while spawnSync blocks the loop). Killed in t.after if left running. + */ +const child = (t, command, args) => { + const quoted = [command, ...args].map((word) => `'${word.replace(/'/g, "'\\''")}'`).join(' '); + const pid = ownedPid(spawnSync('sh', ['-c', `${quoted} >/dev/null 2>&1 & echo $!`], { encoding: 'utf8' }).stdout); + t.after(() => killOwned(pid)); + return pid; +}; +/** + * A detached shell that leads its own process group and waits on a `sleep` + * in that group, started through a node process that exits at once. + */ +const groupLeader = async (t, dir) => { + const memberFile = path.join(dir, 'member.pid'); + const script = `const c = require('node:child_process').spawn('sh', ['-c', 'sleep 60 & echo $! > "$0"; wait', ${JSON.stringify(memberFile)}], { detached: true, stdio: 'ignore' }); c.unref(); process.stdout.write(String(c.pid));`; + const leader = ownedPid(spawnSync(process.execPath, ['-e', script], { encoding: 'utf8' }).stdout); + t.after(() => killOwned(leader)); + for (let i = 0; i < 100 && !existsSync(memberFile); i += 1) await new Promise((resolve) => setTimeout(resolve, 50)); + const member = ownedPid(readFileSync(memberFile, 'utf8')); + t.after(() => killOwned(member)); + return { leader, member }; +}; +/** + * A fake process table for endProcesses, so its decisions are checked without + * one real signal. Pids sit above every real pid_max, in case one leaks. + */ +const fakeTable = (processes) => { + const table = new Map(processes.map((entry) => [entry.pid, { ...entry }])); + const calls = []; + const fail = (code) => Object.assign(new Error(code), { code }); + const kill = (target, signal) => { + calls.push([target, signal]); + if (target < 0) { + const members = [...table.values()].filter((entry) => entry.pgid === -target && entry.state === 'alive'); + if (members.length === 0) throw fail('ESRCH'); + for (const entry of members) entry.state = 'gone'; + return; + } + const entry = table.get(target); + if (!entry || entry.state === 'gone') throw fail('ESRCH'); + if (entry.state === 'foreign') throw fail('EPERM'); + if (signal !== 0) entry.state = 'gone'; + }; + const groupOf = (pid) => table.get(pid)?.pgid ?? null; + const sent = () => calls.filter(([, signal]) => signal !== 0); + const state = (pid) => table.get(pid)?.state; + return { kill, groupOf, calls, sent, state }; +}; +const FAKE = 2_147_480_000; +const waitUntil = async (predicate) => { + for (let i = 0; i < 100 && !predicate(); i += 1) await new Promise((resolve) => setTimeout(resolve, 50)); + return predicate(); +}; test('a helper whose result was accepted and that still runs is a stop candidate', (t) => { const dir = mkdtempSync(path.join(os.tmpdir(), 'fleet-helpers-')); @@ -42,3 +118,113 @@ test('history attribution refuses a shallow clone', (t) => { writeFileSync(path.join(dir, '.git', 'shallow'), `${git('rev-parse', 'HEAD')}\n`); assert.match(historyOrigin(dir, 'route.ts').reason, /shallow/); }); + +test('stopping a helper ends the processes it recorded, and a stopped helper with live processes is an orphan', async (t) => { + const dir = tempDir(t); + const server = child(t, 'sleep', ['60']); + assert.equal(run([dir, 'start', 'qa-a']).status, 0); + assert.equal(run([dir, 'pid', 'qa-a', String(server), String(server)]).status, 0); + assert.equal(run([dir, 'stopped', 'qa-a', '--grace-seconds', '5']).status, 0); + assert.ok(await waitUntil(() => !isAlive(server)), 'the recorded dev server is gone'); + assert.equal(run([dir, 'status']).status, 0); + + const left = child(t, 'sleep', ['60']); + assert.deepEqual(orphans({ 'qa-b': { stoppedAt: 'x', pids: [left] }, 'qa-c': { pids: [left] } }), [{ name: 'qa-b', pids: [left] }]); + assert.deepEqual(orphans({ '#stacks': { s: { guide: 'g' } } }), []); +}); + +test('a process that ignores SIGTERM is killed after the grace; a survivor keeps the helper unstopped', async (t) => { + const dir = tempDir(t); + const stubborn = child(t, process.execPath, ['-e', "process.on('SIGTERM',()=>{});setInterval(()=>{},1000)"]); + await new Promise((resolve) => setTimeout(resolve, 300)); + assert.equal(run([dir, 'start', 'qa-d']).status, 0); + assert.equal(run([dir, 'pid', 'qa-d', String(stubborn)]).status, 0); + const stopped = run([dir, 'stopped', 'qa-d', '--grace-seconds', '1']); + assert.equal(stopped.status, 0, stopped.stdout + stopped.stderr); + assert.ok(await waitUntil(() => !isAlive(stubborn))); + + const ledger = path.join(dir, 'helpers.json'); + const left = child(t, 'sleep', ['60']); + writeFileSync(ledger, JSON.stringify({ 'qa-e': { startedAt: 'x', pids: [left] }, 'qa-f': { stoppedAt: 'x', pids: [left] } })); + const status = run([dir, 'status']); + assert.equal(status.status, 1); + assert.match(status.stdout, new RegExp(`^ORPHAN qa-f ${left} still running$`, 'm')); + assert.match(status.stdout, /helpers: 1 active, 0 to stop, 1 orphaned/); + assert.equal(run([dir, 'pid', 'qa-e', 'abc']).status, 2); +}); + +test('a pid of 1 or lower is never recorded: signalling group 1 reaches every process the user owns', () => { + for (const unsafe of [['1'], ['0'], ['-1'], ['-42'], ['1.5'], ['abc'], []]) { + assert.throws(() => parsePids(unsafe), /above 1/, unsafe.join(' ')); + } + assert.deepEqual(parsePids(['2', String(FAKE)]), [2, FAKE]); +}); + +test('endProcesses never signals a pid of 1 or lower, this process or its parent, even when one is recorded', async () => { + const fake = fakeTable([]); + const unsafe = [0, 1, -1, -42, Number.NaN, 1.5, process.pid, process.ppid].map((pid) => ({ pid, group: true })); + assert.deepEqual(await endProcesses(unsafe, 0, { kill: fake.kill, groupOf: fake.groupOf }), []); + assert.deepEqual(fake.calls, [], 'not even a liveness probe'); +}); + +test('endProcesses signals a pid alone unless it was recorded as a group the helper spawned and still leads', async () => { + const fake = fakeTable([ + { pid: FAKE + 1, pgid: FAKE + 1, state: 'alive' }, + { pid: FAKE + 2, pgid: FAKE + 1, state: 'alive' }, + { pid: FAKE + 3, pgid: FAKE + 3, state: 'alive' }, + { pid: FAKE + 4, pgid: FAKE + 3, state: 'alive' }, + { pid: FAKE + 5, pgid: FAKE + 9, state: 'alive' }, + { pid: FAKE + 6, pgid: FAKE + 9, state: 'alive' }, + ]); + const survivors = await endProcesses( + [ + { pid: FAKE + 1, group: false }, + { pid: FAKE + 3, group: true }, + { pid: FAKE + 5, group: true }, + ], + 0, + { kill: fake.kill, groupOf: fake.groupOf } + ); + assert.deepEqual(survivors, []); + assert.deepEqual(fake.sent(), [ + [FAKE + 1, 'SIGTERM'], + [-(FAKE + 3), 'SIGTERM'], + [FAKE + 5, 'SIGTERM'], + ]); + assert.equal(fake.state(FAKE + 2), 'alive', 'a group leader recorded without --group is signalled alone'); + assert.equal(fake.state(FAKE + 4), 'gone', 'a recorded group that the pid still leads is ended whole'); + assert.equal(fake.state(FAKE + 6), 'alive', 'a recorded group pid that no longer leads its group is signalled alone'); +}); + +test('endProcesses never signals a pid the system says is not ours (EPERM), and does not count it as a survivor', async () => { + const fake = fakeTable([{ pid: FAKE + 7, pgid: FAKE + 7, state: 'foreign' }]); + assert.deepEqual(await endProcesses([{ pid: FAKE + 7, group: true }], 0, { kill: fake.kill, groupOf: fake.groupOf }), []); + assert.deepEqual(fake.sent(), []); + assert.equal(isAlive(FAKE + 7), false, 'a fake pid above pid_max does not exist'); +}); + +test('stopping a helper recorded with --group ends its whole process group', { timeout: 30_000 }, async (t) => { + const dir = tempDir(t); + const { leader, member } = await groupLeader(t, dir); + assert.equal(run([dir, 'start', 'qa-h']).status, 0); + assert.equal(run([dir, 'pid', 'qa-h', String(leader), '--group']).status, 0); + assert.deepEqual(JSON.parse(readFileSync(path.join(dir, 'helpers.json'), 'utf8'))['qa-h'].groups, [leader]); + const stopped = run([dir, 'stopped', 'qa-h', '--grace-seconds', '5']); + assert.equal(stopped.status, 0, stopped.stdout + stopped.stderr); + assert.ok(await waitUntil(() => !isAlive(leader)), 'the leader is gone'); + assert.ok(await waitUntil(() => !isAlive(member)), 'the process it started in its group is gone'); +}); + +test('a stack handed to a human cannot be used by another helper until it is released', (t) => { + const dir = tempDir(t); + assert.equal(run([dir, 'hand', 'preview-3000', '--to', 'qa-guide-12']).status, 0); + const refused = run([dir, 'start', 'qa-rerun', '--stack', 'preview-3000']); + assert.equal(refused.status, 1); + assert.match(refused.stderr, /handed to qa-guide-12; launch this helper on its own stack/); + assert.equal(run([dir, 'start', 'qa-rerun', '--stack', 'qa-rerun-own']).status, 0); + assert.equal(run([dir, 'hand', 'x']).status, 2, 'hand needs a guide'); + assert.equal(run([dir, 'unhand', 'preview-3000']).status, 0); + assert.equal(run([dir, 'start', 'qa-rerun-2', '--stack', 'preview-3000']).status, 0); + const status = run([dir, 'status']); + assert.match(status.stdout, /helpers: 2 active, 0 to stop, 0 orphaned/); +}); diff --git a/plugins/vibe/skills/vibe/scripts/dist/claude-worker.mjs b/plugins/vibe/skills/vibe/scripts/dist/claude-worker.mjs index 79462ec5..ba377690 100644 --- a/plugins/vibe/skills/vibe/scripts/dist/claude-worker.mjs +++ b/plugins/vibe/skills/vibe/scripts/dist/claude-worker.mjs @@ -11982,6 +11982,7 @@ function summary(ledger) { } function proveGroupStopped(processGroupId) { if (process.platform === "win32") throw new Error("Owned process-group proof is unsupported on this platform"); + if (!(processGroupId > 1)) throw new Error("Unable to prove the owned process group stopped; retain the lease"); try { process.kill(-processGroupId, 0); } catch (error) { @@ -12977,7 +12978,7 @@ function safeFailure(error) { return { code: "WORKER_TURN_FAILED", summary: "The worker turn failed; inspect its private evidence and retain the same writer identity." }; } async function finishOwnedGroup(child) { - if (!child.pid) return; + if (!child.pid || child.pid <= 1) return; if (process.platform === "win32") { throw new OwnedCleanupError("Owned descendant cleanup is unsupported on this platform; writer lease retained for stopped-turn proof"); } @@ -12995,7 +12996,7 @@ async function finishOwnedGroup(child) { throw new OwnedCleanupError("Owned process group is still present; writer lease retained for stopped-turn proof"); } function signalOwnedChild(child, signal) { - if (!child.pid) return; + if (!child.pid || child.pid <= 1) return; try { if (process.platform === "win32") child.kill(signal); else process.kill(-child.pid, signal); diff --git a/plugins/vibe/skills/vibe/scripts/dist/writer-state.mjs b/plugins/vibe/skills/vibe/scripts/dist/writer-state.mjs index 90a3de4d..16223c24 100644 --- a/plugins/vibe/skills/vibe/scripts/dist/writer-state.mjs +++ b/plugins/vibe/skills/vibe/scripts/dist/writer-state.mjs @@ -12009,6 +12009,7 @@ function summary(ledger) { } function proveGroupStopped(processGroupId) { if (process.platform === "win32") throw new Error("Owned process-group proof is unsupported on this platform"); + if (!(processGroupId > 1)) throw new Error("Unable to prove the owned process group stopped; retain the lease"); try { process.kill(-processGroupId, 0); } catch (error) { diff --git a/plugins/vibe/skills/vibe/scripts/precedent-check.mjs b/plugins/vibe/skills/vibe/scripts/precedent-check.mjs index aa57fcdc..4a917e62 100644 --- a/plugins/vibe/skills/vibe/scripts/precedent-check.mjs +++ b/plugins/vibe/skills/vibe/scripts/precedent-check.mjs @@ -20,6 +20,13 @@ // keeps, never the system temp folder) and prints // {"ok":true,"report":...,"answer":...,"source":...}, or // {"ok":false,"error":...,"report":...} when the run failed or timed out. +// precedent-check.mjs check --report [--no-browser ] +// Refuses (exit 1) a report not settled by a live look: an `undocumented` +// answer or a public-material source. Precedent means the named product's +// live app in the operator's signed-in browser; a help-center search once +// came back undocumented while the live app had the answer. `--no-browser` +// with the reason no browser could look accepts a public or undocumented +// report and echoes the reason. // // `--apps` names the apps to try after Asana, in order (default: Linear, then // the service's own screens). Every command prints one JSON object. @@ -39,6 +46,8 @@ const DEFAULT_TIMEOUT_SECONDS = 900; const MAX_TEXT = 2000; const ANSWER_LINE = /^Answer:\s*(.+)$/m; const SOURCE_LINE = /^Source:\s*(.+)$/m; +const PUBLIC_SOURCE = /https?:\/\/|help center|help-center|documentation|docs/i; +const UNDOCUMENTED = /^undocumented\b/i; /** The research brief for one product question. */ export function precedentPrompt({ question, situation, apps = DEFAULT_APPS, today = new Date() }) { @@ -83,14 +92,49 @@ export function claudeArgs() { return ["-p", "--chrome", "--allowedTools", CLAUDE_TOOLS]; } -/** The answer and source lines of a report, or null for each line it lacks. */ +/** + * The answer and source lines of a report, or null for each line it lacks, and + * the kind of source: `observed` (a look at the live app), `public` (help + * center or documentation only) or `none`. + */ export function summarize(report) { + const answer = ANSWER_LINE.exec(report)?.[1].trim() ?? null; + const source = SOURCE_LINE.exec(report)?.[1].trim() ?? null; + return { answer, source, sourceKind: sourceKind(source) }; +} + +/** Whether a report is settled by a live look; `noBrowser` names why none could look. */ +export function checkReport(report, noBrowser) { + const { answer, source, sourceKind: kind } = summarize(report); + if (!answer || !source) { + return { ok: false, error: "the report has no Answer or Source line" }; + } + const settledLive = kind === "observed" && !UNDOCUMENTED.test(answer); + if (settledLive) { + return { ok: true, answer, source, sourceKind: kind }; + } + if (typeof noBrowser === "string" && noBrowser.trim() !== "") { + return { ok: true, answer, source, sourceKind: kind, noBrowser: noBrowser.trim() }; + } return { - answer: ANSWER_LINE.exec(report)?.[1].trim() ?? null, - source: SOURCE_LINE.exec(report)?.[1].trim() ?? null, + ok: false, + answer, + source, + sourceKind: kind, + error: `the answer is ${UNDOCUMENTED.test(answer) ? "undocumented" : `from ${kind === "public" ? "public material" : "no source"}`}, not a live look; run precedent-check.mjs run for a live look in the signed-in browser first, or pass --no-browser when no browser can look`, }; } +function sourceKind(source) { + if (!source) { + return "none"; + } + if (/\bobserved\b/i.test(source)) { + return "observed"; + } + return PUBLIC_SOURCE.test(source) && !UNDOCUMENTED.test(source) ? "public" : "none"; +} + /** Runs the headless look and resolves with its result; never rejects. */ export function runClaude({ prompt, out, claude = "claude", timeoutSeconds = DEFAULT_TIMEOUT_SECONDS }) { mkdirSync(out, { recursive: true }); @@ -110,7 +154,8 @@ export function runClaude({ prompt, out, claude = "claude", timeoutSeconds = DEF const timer = setTimeout(() => { timedOut = true; try { - process.kill(-child.pid, "SIGKILL"); + // kill(-1) would signal every process the user owns. + if (child.pid > 1) process.kill(-child.pid, "SIGKILL"); } catch { // Already gone. } @@ -124,7 +169,7 @@ export function runClaude({ prompt, out, claude = "claude", timeoutSeconds = DEF clearTimeout(timer); report.end(() => { const text = readFileSync(reportFile, "utf8"); - const { answer, source } = summarize(text); + const { answer, source, sourceKind: kind } = summarize(text); if (timedOut) { resolve({ ok: false, error: `timed out after ${timeoutSeconds}s`, prompt: promptFile, report: reportFile }); } else if (code !== 0 || !answer || !source) { @@ -135,7 +180,7 @@ export function runClaude({ prompt, out, claude = "claude", timeoutSeconds = DEF report: reportFile, }); } else { - resolve({ ok: true, answer, source, prompt: promptFile, report: reportFile }); + resolve({ ok: true, answer, source, sourceKind: kind, prompt: promptFile, report: reportFile }); } }); }); @@ -163,11 +208,23 @@ async function main() { out: { type: "string" }, claude: { type: "string" }, "timeout-seconds": { type: "string" }, + report: { type: "string" }, + "no-browser": { type: "string" }, }, }); + if (command === "check") { + if (!values.report) { + throw new Error("check needs --report "); + } + const result = checkReport(readFileSync(values.report, "utf8"), values["no-browser"]); + if (!result.ok) { + process.exitCode = 1; + } + return result; + } const prompt = precedentPrompt({ question: values.question, situation: values.situation, apps: values.apps }); if (command === "prompt") { - return { ok: true, prompt }; + return { ok: true, liveLook: false, prompt }; } if (command === "run") { const timeout = Number(values["timeout-seconds"]); @@ -178,7 +235,7 @@ async function main() { timeoutSeconds: Number.isFinite(timeout) && timeout > 0 ? timeout : DEFAULT_TIMEOUT_SECONDS, }); } - throw new Error("command must be prompt or run"); + throw new Error("command must be prompt, run or check"); } if (process.argv[1] && import.meta.url === pathToFileURL(path.resolve(process.argv[1])).href) { diff --git a/plugins/vibe/skills/vibe/scripts/precedent-check.test.mjs b/plugins/vibe/skills/vibe/scripts/precedent-check.test.mjs index 3f5d59da..dbe4c44f 100644 --- a/plugins/vibe/skills/vibe/scripts/precedent-check.test.mjs +++ b/plugins/vibe/skills/vibe/scripts/precedent-check.test.mjs @@ -5,7 +5,7 @@ import os from "node:os"; import path from "node:path"; import test from "node:test"; import { fileURLToPath } from "node:url"; -import { CLAUDE_TOOLS, TASK_PREFIX, TEMP_PROJECT, precedentPrompt, precedentRoot, summarize } from "./precedent-check.mjs"; +import { CLAUDE_TOOLS, TASK_PREFIX, TEMP_PROJECT, checkReport, precedentPrompt, precedentRoot, summarize } from "./precedent-check.mjs"; import { makeHome } from "./test-fixtures.mjs"; const SCRIPT = path.join(path.dirname(fileURLToPath(import.meta.url)), "precedent-check.mjs"); @@ -83,6 +83,7 @@ test("prompt mode prints the brief for the Codex orchestrator and starts nothing const { status, json } = run(["prompt", "--question", QUESTION, "--situation", "Start connecting GitHub, then cancel"]); assert.equal(status, 0); assert.equal(json.ok, true); + assert.equal(json.liveLook, false, "the fallback brief is not a live look"); assert.match(json.prompt, /Situation to reproduce: Start connecting GitHub, then cancel/); }); @@ -96,6 +97,7 @@ test("run mode starts headless claude with Chrome and only the research tools, t ok: true, answer: "Returns to the integrations page with nothing connected and no error", source: "Asana, observed 2026-10-16", + sourceKind: "observed", prompt: path.join(out, "prompt.md"), report: path.join(out, "report.md"), }); @@ -126,9 +128,11 @@ test("the summary reads only the report's Answer and Source lines", () => { assert.deepEqual(summarize(REPORT), { answer: "Returns to the integrations page with nothing connected and no error", source: "Asana, observed 2026-10-16", + sourceKind: "observed", }); - assert.deepEqual(summarize("Answer: undocumented\nSource: undocumented\n"), { answer: "undocumented", source: "undocumented" }); - assert.deepEqual(summarize("nothing"), { answer: null, source: null }); + assert.deepEqual(summarize("Answer: undocumented\nSource: undocumented\n"), { answer: "undocumented", source: "undocumented", sourceKind: "none" }); + assert.deepEqual(summarize("Answer: x\nSource: https://help.asana.com/hc/en-us/articles/1\n"), { answer: "x", source: "https://help.asana.com/hc/en-us/articles/1", sourceKind: "public" }); + assert.deepEqual(summarize("nothing"), { answer: null, source: null, sourceKind: "none" }); }); test("without --out the report folder is under the vibe home a reboot keeps, never the temp folder", (t) => { @@ -142,3 +146,29 @@ test("without --out the report folder is under the vibe home a reboot keeps, nev assert.equal(precedentRoot("/Users/someone"), path.join("/Users/someone", ".codex", "vibe", "precedent")); assert.ok(!precedentRoot().startsWith(os.tmpdir())); }); + +test("check refuses an undocumented or help-center-only answer until a live look ran, unless no browser can look", (t) => { + const dir = workspace(t); + const write = (name, text) => { + const file = path.join(dir, name); + writeFileSync(file, text); + return file; + }; + const live = write("live.md", REPORT); + const publicOnly = write("public.md", "Answer: Keeps the list as it was\nSource: https://help.asana.com/hc/en-us/articles/1\n"); + const undocumentedOnline = write("undocumented.md", "Answer: undocumented\nSource: undocumented\n"); + const observedUndocumented = write("observed-undocumented.md", "Answer: undocumented\nSource: Asana, observed 2026-10-21\n"); + assert.deepEqual(run(["check", "--report", live]), { status: 0, json: { ok: true, answer: summarize(REPORT).answer, source: "Asana, observed 2026-10-16", sourceKind: "observed" } }); + for (const file of [publicOnly, undocumentedOnline, observedUndocumented]) { + const refused = run(["check", "--report", file]); + assert.equal(refused.status, 1, file); + assert.equal(refused.json.ok, false); + assert.match(refused.json.error, /run precedent-check\.mjs run for a live look in the signed-in browser first/); + } + const accepted = run(["check", "--report", publicOnly, "--no-browser", "no signed-in claude CLI on this machine"]); + assert.equal(accepted.status, 0); + assert.equal(accepted.json.sourceKind, "public"); + assert.equal(accepted.json.noBrowser, "no signed-in claude CLI on this machine"); + assert.equal(checkReport("I looked around.").ok, false); + assert.equal(run(["check"]).status, 1); +}); diff --git a/plugins/vibe/skills/vibe/scripts/run-bounded.mjs b/plugins/vibe/skills/vibe/scripts/run-bounded.mjs index 102eac04..68d8e88f 100644 --- a/plugins/vibe/skills/vibe/scripts/run-bounded.mjs +++ b/plugins/vibe/skills/vibe/scripts/run-bounded.mjs @@ -28,6 +28,8 @@ export function runBounded(command, args, { seconds, stdio = "inherit" }) { let timedOut = false; let killTimer; const signalGroup = (signal) => { + // kill(-1) would signal every process the user owns. + if (!(child.pid > 1)) return; try { process.kill(-child.pid, signal); } catch (error) { diff --git a/plugins/vibe/tools/python/test_vibe_signal_safety_contract.py b/plugins/vibe/tools/python/test_vibe_signal_safety_contract.py new file mode 100644 index 00000000..5023538d --- /dev/null +++ b/plugins/vibe/tools/python/test_vibe_signal_safety_contract.py @@ -0,0 +1,172 @@ +"""Signal safety checks (operator incident, 2026-10-21). + +A plugin test recorded pid 1 for a helper and then stopped it. The stop path +group-killed the recorded pid, so it ran kill(-1), which signals every process +the user owns, and every app on the operator's laptop died, twice. + +1. No plugin test passes 0, 1, a negative literal, its own pid or its parent's + pid to a call that sends a signal, writes such a pid into a `pids` ledger, + or calls `endProcesses` without injecting a fake `kill`. +2. No plugin source signals a process group (`process.kill(-x)`, `kill(-x)`, + `kill -- -x`) unless `x` is checked above 1 on the lines just before it. +""" + +from __future__ import annotations + +import re +from pathlib import Path + + +PLUGIN_ROOT = Path(__file__).resolve().parents[2] +WORKERS_SOURCE = PLUGIN_ROOT.parents[1] / "tools" / "vibe-workers" / "src" +SOURCE_SUFFIXES = (".mjs", ".js", ".cjs", ".ts", ".sh", ".zsh", ".bash") +GUARD_WINDOW = 8 +UNARY_BEFORE = set("?:(,=!&|[{;") +IDENTIFIER = r"[A-Za-z_$][\w$.]*" +JS_KILL = re.compile(r"(? list[Path]: + return [path for root in (PLUGIN_ROOT, WORKERS_SOURCE) for path in sorted(root.rglob("*")) + if path.is_file() and path.suffix in SOURCE_SUFFIXES + and "node_modules" not in path.parts and "dist" not in path.parts] + + +def _tests() -> list[Path]: + return [path for path in _sources() if ".test." in path.name] + + +def _call_args(text: str, open_paren: int) -> list[str]: + """Top-level arguments of the call whose `(` is at `open_paren`.""" + depth, start, args = 0, open_paren + 1, [] + for index in range(open_paren, len(text)): + char = text[index] + if char in "([{": + depth += 1 + elif char in ")]}": + depth -= 1 + if depth == 0: + args.append(text[start:index]) + return [arg.strip() for arg in args] + elif char == "," and depth == 1: + args.append(text[start:index]) + start = index + 1 + return [arg.strip() for arg in args] + + +def _in_comment(text: str, at: int) -> bool: + """Whether `at` follows a `//`, `/*`, `#` or doc-comment `*` on its line, so prose naming kill(-1) is not a call.""" + return COMMENT.search(text[text.rfind("\n", 0, at) + 1:at]) is not None + + +def _negated_targets(arg: str) -> list[str]: + """Each operand a unary minus is applied to in a kill target, such as `pid` in `group ? -pid : pid`.""" + targets = [] + for match in NEGATED.finditer(arg): + before = arg[:match.start()].rstrip() + if not before or before[-1] in UNARY_BEFORE: + targets.append(match.group(1)) + return targets + + +def _guarded(text: str, at: int, target: str, shell: bool) -> bool: + if target.isdigit(): + return False + line_start = text.rfind("\n", 0, at) + 1 + window_start = line_start + for _ in range(GUARD_WINDOW): + window_start = text.rfind("\n", 0, max(window_start - 1, 0)) + 1 + if window_start == 0: + break + window = text[window_start:at] + name = re.escape(target.rstrip("!")) + if shell: + guard = r"\$\{?" + name + r"\}?\"?\s+-(?:gt\s+1|ge\s+2|le\s+1|lt\s+2)\b" + else: + guard = r"(?\s*1\b|>=\s*2\b|<=\s*1\b|<\s*2\b)" + return re.search(guard, window) is not None + + +def group_kill_offenders(text: str) -> list[str]: + """Group signals whose target is not checked above 1 just before the call.""" + offenders = [] + for match in JS_KILL.finditer(text): + if _in_comment(text, match.start()): + continue + args = _call_args(text, match.end() - 1) + for target in _negated_targets(args[0] if args else ""): + if not _guarded(text, match.start(), target, shell=False): + offenders.append(f"kill(-{target})") + for match in SHELL_GROUP.finditer(text): + if _in_comment(text, match.start()): + continue + target = match.group(1) or match.group(2) + if not _guarded(text, match.start(), target, shell=True): + offenders.append(f"kill -{target}") + return offenders + + +def unsafe_test_signals(text: str) -> list[str]: + """Signals a test sends, or sets up, to pid 0 or 1, a negative literal, or its own or parent pid.""" + offenders = [] + for match in JS_KILL.finditer(text): + if not text[:match.start()].endswith("process.") or _in_comment(text, match.start()): + continue + args = _call_args(text, match.end() - 1) + if len(args) > 1 and args[1] == "0": + continue + if args and UNSAFE_PID.search(args[0]): + offenders.append(f"process.kill({args[0]})") + offenders += [f"shell {match.group(0)}" for match in SHELL_UNSAFE.finditer(text)] + offenders += [f"ledger {match.group(0)}" for match in UNSAFE_LEDGER.finditer(text)] + for match in re.finditer(r"(? None: + offenders = [f"{path.relative_to(PLUGIN_ROOT.parents[1])}: {found}" + for path in _tests() for found in unsafe_test_signals(path.read_text())] + assert offenders == [] + + +def test_no_plugin_source_signals_a_group_it_did_not_check_above_1() -> None: + offenders = [f"{path.relative_to(PLUGIN_ROOT.parents[1])}: {found}" + for path in _sources() for found in group_kill_offenders(path.read_text())] + assert offenders == [] + + +def test_the_scans_catch_the_shapes_that_killed_every_process() -> None: + for unsafe in ("process.kill(1, 'SIGTERM');", "process.kill(0, 'SIGKILL');", "process.kill(-1, 'SIGTERM');", + "process.kill(process.pid, 'SIGKILL');", "process.kill(process.ppid, 'SIGTERM');", + "kill -9 $$", "kill -TERM -- -1", "kill 0", "kill -KILL \"$PPID\"", + "JSON.stringify({ a: { pids: [process.pid] } })", "{ pids: [1] }", "{ pids: [4242, 0] }", + "await endProcesses([{ pid }], 0);"): + assert unsafe_test_signals(unsafe), unsafe + for safe in ("process.kill(pid, 0);", "process.kill(process.pid, 0);", "process.kill(child.pid, 'SIGKILL');", + "{ pids: [left, 12] }", "await endProcesses(unsafe, 0, { kill: fake.kill });", "kill -0 \"$pid\"", + "// process.kill(-1) would signal every process the user owns."): + assert unsafe_test_signals(safe) == [], safe + + for unguarded in ("process.kill(-pid, 'SIGTERM');", "process.kill(-child.pid, signal);", + "kill(group ? -pid : pid, name);", "process.kill(-1, 'SIGKILL');", + "kill -TERM -- -\"$pgid\"", "kill -- -$pid", "process.kill(- pid, 'SIGTERM');", + "if (pid > 1) log();\n" + "\n" * GUARD_WINDOW + "process.kill(-pid, 'SIGTERM');"): + assert group_kill_offenders(unguarded), unguarded + for guarded in ("if (!(pid > 1)) return;\nprocess.kill(-pid, 'SIGTERM');", + "if (child.pid > 1) process.kill(-child.pid, 'SIGKILL');", + "if (!child.pid || child.pid <= 1) return;\nprocess.kill(-child.pid, signal);", + "if (!(pid > 1)) return;\nconst whole = true;\nkill(whole ? -pid : pid, name);", + "[ \"$pgid\" -gt 1 ] || exit 1\nkill -TERM -- -\"$pgid\"", + "process.kill(pid, 'SIGTERM');", "kill(total - pid, 0);", "kill -9 \"$pid\"", "kill -0 \"$(cat \"$dir/pid\")\"", + " // kill(-1) would signal every process the user owns.", "# never kill -- -$pid", + "/** kill(-1) reaches every process the user owns. */"): + assert group_kill_offenders(guarded) == [], guarded diff --git a/tools/vibe-workers/src/claude-worker.ts b/tools/vibe-workers/src/claude-worker.ts index 100d7f4d..3da4371c 100644 --- a/tools/vibe-workers/src/claude-worker.ts +++ b/tools/vibe-workers/src/claude-worker.ts @@ -356,7 +356,7 @@ function safeFailure(error: unknown): { code: string; summary: string; personAct /** Completion is not a free writer slot until the owned process group is proven stopped. */ async function finishOwnedGroup(child: ChildProcessWithoutNullStreams) { - if (!child.pid) return; + if (!child.pid || child.pid <= 1) return; if (process.platform === "win32") { throw new OwnedCleanupError("Owned descendant cleanup is unsupported on this platform; writer lease retained for stopped-turn proof"); } @@ -374,7 +374,7 @@ async function finishOwnedGroup(child: ChildProcessWithoutNullStreams) { } function signalOwnedChild(child: ChildProcessWithoutNullStreams, signal: NodeJS.Signals) { - if (!child.pid) return; + if (!child.pid || child.pid <= 1) return; try { if (process.platform === "win32") child.kill(signal); else process.kill(-child.pid, signal); diff --git a/tools/vibe-workers/src/ledger.test.ts b/tools/vibe-workers/src/ledger.test.ts index f5aa9ebc..86fb1177 100644 --- a/tools/vibe-workers/src/ledger.test.ts +++ b/tools/vibe-workers/src/ledger.test.ts @@ -158,7 +158,7 @@ describe("persistent writer ledger", () => { expect(() => changeWriter("finish", { ...input, lease: claimed.turn?.lease, status: "FAILED" })).toThrow(/still running/); expect(readWriterSummary(value.worktree)?.processGroupId).toBe(child.pid); expect(readWriterSummary(value.worktree)?.running).toBe(true); - } finally { process.kill(-child.pid!, "SIGTERM"); await closed; } + } finally { const group = child.pid ?? 0; if (group > 1) process.kill(-group, "SIGTERM"); await closed; } changeWriter("finish", { ...input, lease: claimed.turn?.lease, status: "FAILED" }); expect(readWriterSummary(value.worktree)?.running).toBe(false); expect(readWriterSummary(value.worktree)?.workerId).toBe(registered.workerId); diff --git a/tools/vibe-workers/src/ledger.ts b/tools/vibe-workers/src/ledger.ts index 71f2aa54..391ac6ef 100644 --- a/tools/vibe-workers/src/ledger.ts +++ b/tools/vibe-workers/src/ledger.ts @@ -350,6 +350,7 @@ function summary(ledger: Ledger) { function proveGroupStopped(processGroupId: number) { if (process.platform === "win32") throw new Error("Owned process-group proof is unsupported on this platform"); + if (!(processGroupId > 1)) throw new Error("Unable to prove the owned process group stopped; retain the lease"); try { process.kill(-processGroupId, 0); } catch (error) { if (error instanceof Error && "code" in error && error.code === "ESRCH") return; From a70cbb38d5eef15e48888778f91846886f2835a0 Mon Sep 17 00:00:00 2001 From: Daniel Ochoa Date: Wed, 21 Oct 2026 16:29:08 -0500 Subject: [PATCH 2/4] fix(vibe): encode the open 2026-10-21 fleet lessons - fleet-orchestrator: board.mjs keeps the fleet board, flags rows behind the pushed head, and tracks rulings and ready QA checkpoints until every writer and QA guide is told directly. qa-plan-check.mjs defaults to zero human checkpoints. helpers.mjs records and ends a helper's processes, lists orphans, and keeps a handed stack from QA reruns. - codex-fleet-monitor: watch.sh writes watcher.json and watcher.mjs reports a lapsed watcher; the supervisor template restarts with bootout then bootstrap and throttles 30 seconds. - vibe: push-cadence.mjs and pushDue on every commit, shepherd.mjs for the final stage, main-sync-push refuses out-of-order migrations, e2e-lock.mjs gives each run its own compose project, and precedent-check.mjs check refuses an answer no live look settled. - handoff: repo-declared path gates (.vibe/path-gates.json) become required runs on the published commit. - Skill prose points at each check; vibe 1.0.74 and its changelog. Testing: node --test, pytest, ruff, pyright and the vibe-workers vitest and build on a second machine. Risks: None identified; every new check is additive and a repo without .vibe/path-gates.json requires no gate. --- CHANGELOG.md | 16 ++ plugins/vibe/.claude-plugin/plugin.json | 2 +- plugins/vibe/.codex-plugin/plugin.json | 2 +- plugins/vibe/agents/vibe-verify-worker.md | 4 +- .../vibe/skills/codex-fleet-monitor/SKILL.md | 4 +- .../scripts/templates.test.mjs | 23 +++ .../codex-fleet-monitor/scripts/watch.sh | 11 +- .../codex-fleet-monitor/scripts/watcher.mjs | 72 +++++++ .../scripts/watcher.test.mjs | 82 ++++++++ .../templates/codex-fleet-supervisor.plist | 5 +- .../vibe/skills/fleet-orchestrator/SKILL.md | 24 ++- .../references/templates.md | 2 + .../fleet-orchestrator/scripts/board.mjs | 191 ++++++++++++++++++ .../fleet-orchestrator/scripts/board.test.mjs | 65 ++++++ .../scripts/qa-plan-check.mjs | 90 +++++++++ .../scripts/qa-plan-check.test.mjs | 50 +++++ plugins/vibe/skills/handoff/SKILL.md | 13 +- .../skills/handoff/scripts/diff-rules.mjs | 3 +- .../handoff/scripts/diff-rules.test.mjs | 8 + .../skills/handoff/scripts/path-gates.mjs | 48 +++++ .../handoff/scripts/path-gates.test.mjs | 34 ++++ plugins/vibe/skills/vibe/SKILL.md | 5 +- .../skills/vibe/references/environment.md | 4 +- .../skills/vibe/references/quality-loop.md | 4 + .../skills/vibe/scripts/commit-worktree.mjs | 14 +- .../vibe/scripts/commit-worktree.test.mjs | 2 + plugins/vibe/skills/vibe/scripts/e2e-lock.mjs | 82 +++++++- .../skills/vibe/scripts/e2e-lock.test.mjs | 53 ++++- .../skills/vibe/scripts/migration-order.mjs | 88 ++++++++ .../vibe/scripts/migration-order.test.mjs | 96 +++++++++ .../vibe/skills/vibe/scripts/push-cadence.mjs | 123 +++++++++++ .../skills/vibe/scripts/push-cadence.test.mjs | 93 +++++++++ plugins/vibe/skills/vibe/scripts/shepherd.mjs | 83 ++++++++ .../skills/vibe/scripts/shepherd.test.mjs | 47 +++++ .../skills/vibe/scripts/vibe-sessions.mjs | 10 + .../python/test_vibe_1074_lessons_contract.py | 62 ++++++ 36 files changed, 1484 insertions(+), 31 deletions(-) create mode 100644 plugins/vibe/skills/codex-fleet-monitor/scripts/templates.test.mjs create mode 100644 plugins/vibe/skills/codex-fleet-monitor/scripts/watcher.mjs create mode 100644 plugins/vibe/skills/codex-fleet-monitor/scripts/watcher.test.mjs create mode 100644 plugins/vibe/skills/fleet-orchestrator/scripts/board.mjs create mode 100644 plugins/vibe/skills/fleet-orchestrator/scripts/board.test.mjs create mode 100644 plugins/vibe/skills/fleet-orchestrator/scripts/qa-plan-check.mjs create mode 100644 plugins/vibe/skills/fleet-orchestrator/scripts/qa-plan-check.test.mjs create mode 100644 plugins/vibe/skills/handoff/scripts/path-gates.mjs create mode 100644 plugins/vibe/skills/handoff/scripts/path-gates.test.mjs create mode 100644 plugins/vibe/skills/vibe/scripts/migration-order.mjs create mode 100644 plugins/vibe/skills/vibe/scripts/migration-order.test.mjs create mode 100644 plugins/vibe/skills/vibe/scripts/push-cadence.mjs create mode 100644 plugins/vibe/skills/vibe/scripts/push-cadence.test.mjs create mode 100644 plugins/vibe/skills/vibe/scripts/shepherd.mjs create mode 100644 plugins/vibe/skills/vibe/scripts/shepherd.test.mjs create mode 100644 plugins/vibe/tools/python/test_vibe_1074_lessons_contract.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 4b7b90da..420b6450 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,22 @@ All notable changes to the claude-plugins project will be documented in this fil The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). Entries are listed newest-first; each plugin section is treated as released when merged to `main`. +### vibe v1.0.74 + +Signal safety after a plugin test ended every process on an operator's machine, and the open lessons from the 2026-10-21 ledger entries, each with a mechanical check where one fits. + +#### Fixed +- `helpers.mjs stopped` no longer signals a process group it does not own. A test recorded pid 1 and stopped it; `endProcesses` sent `kill(-1)`, which signals every process the user owns, twice. Pids of 1 or lower are refused at record time and never signalled, nor is the stopper's own pid or its parent. A whole group is signalled only for a pid recorded with `--group` whose process group id still equals the pid; any other pid is signalled alone. A pid the system reports as not ours (EPERM, a reused pid) is never signalled and does not block `stopped`. `endProcesses` takes an injected `kill`, so its tests check every decision without a real signal. +- Every group signal in the plugin (`run-bounded.mjs`, `precedent-check.mjs`, the vibe-workers ledger and Claude worker, and their tests) checks the pid above 1 first. The dist bundles are rebuilt. + +#### Added +- `test_vibe_signal_safety_contract.py`: no plugin test passes 0, 1, a negative literal, its own pid or its parent's pid to a call that sends a signal, writes one into a `pids` ledger, or calls `endProcesses` without an injected `kill`; no plugin source signals a group (`process.kill(-x)`, `kill(-x)`, `kill -- -x`) unless `x` is checked above 1 just before. +- fleet-orchestrator: `board.mjs` keeps the fleet board every writer reads, flags rows behind the pushed head, and tracks rulings (saved once as a reference file) and ready QA checkpoints until each writer and QA guide is told directly; writers message each other (Codex through the app-server orchestrator's send, Claude through SendMessage). +- fleet-orchestrator: `qa-plan-check.mjs` makes zero human checkpoints the default and refuses a human checkpoint without a reason an agent cannot cover, a visual judgment included (agents compare screenshots with the reference). `helpers.mjs` records the processes a helper starts (`pid`), ends them on `stopped`, lists `ORPHAN` helpers, and keeps a stack handed to a QA guide (`hand`, `unhand`) from QA reruns. +- codex-fleet-monitor: `watch.sh` writes `watcher.json` and `watcher.mjs` reports a lapsed watcher; the supervisor template restarts with bootout then bootstrap, never kickstart, and respawns after 30 seconds. +- vibe: `push-cadence.mjs` and `pushDue` on every commit (a tested checkpoint is pushed at least every 90 minutes); `shepherd.mjs` polls a pull request every two minutes in the final stage and stops on a conflict; the final main merge pushes before its e2e run; `main-sync-push` refuses a migration that sorts before main's newest (`migration-order.mjs`); `e2e-lock.mjs` gives each run its own compose project and honors `--foreign-compose`; `precedent-check.mjs check` refuses a public-material or undocumented answer unless no browser could look. +- handoff: a repository can declare gates required when paths match (`.vibe/path-gates.json`); each matching gate is a required run on the published commit. + ### vibe v1.0.73 The open lessons from the 2026-10-20 and 2026-10-21 ledger entries, with a mechanical check wherever the lesson had a checkable shape. diff --git a/plugins/vibe/.claude-plugin/plugin.json b/plugins/vibe/.claude-plugin/plugin.json index 17e2653a..b3cd03a0 100644 --- a/plugins/vibe/.claude-plugin/plugin.json +++ b/plugins/vibe/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "vibe", "description": "Vibe sessions, handoff, seed refresh, and ClosedLoop ticket workflows", - "version": "1.0.73", + "version": "1.0.74", "dependencies": ["closedloop-core"], "author": {"name": "ClosedLoop", "email": "support@closedloop.ai"} } diff --git a/plugins/vibe/.codex-plugin/plugin.json b/plugins/vibe/.codex-plugin/plugin.json index e7578f90..6e13cb31 100644 --- a/plugins/vibe/.codex-plugin/plugin.json +++ b/plugins/vibe/.codex-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "vibe", - "version": "1.0.73", + "version": "1.0.74", "description": "Vibe sessions, handoff, seed refresh, and ClosedLoop ticket workflows", "author": {"name": "ClosedLoop", "email": "support@closedloop.ai"}, "homepage": "https://github.com/closedloop-ai/claude-plugins", diff --git a/plugins/vibe/agents/vibe-verify-worker.md b/plugins/vibe/agents/vibe-verify-worker.md index 54fdab94..6162786c 100644 --- a/plugins/vibe/agents/vibe-verify-worker.md +++ b/plugins/vibe/agents/vibe-verify-worker.md @@ -68,7 +68,9 @@ Validation is targeted, never a broad or full local suite. Run: included), plus the inventory's `diffRules.requiredTests` and `requiredChecks`, each through its owning package's runner on those test files alone (for example - `pnpm --filter exec vitest run `). + `pnpm --filter exec vitest run `). A required test + whose path is a command comes from the repo's `.vibe/path-gates.json`: run + that command and record it in `testRuns` with the command as its `path`. Never run `pnpm test`, `pnpm verify`, `pnpm test:affected`, `pnpm test:lanes --exec`, a whole lane or a package's whole suite locally; CI runs the broad suites on the diff --git a/plugins/vibe/skills/codex-fleet-monitor/SKILL.md b/plugins/vibe/skills/codex-fleet-monitor/SKILL.md index fa3c5e90..74cdd3a1 100644 --- a/plugins/vibe/skills/codex-fleet-monitor/SKILL.md +++ b/plugins/vibe/skills/codex-fleet-monitor/SKILL.md @@ -63,7 +63,7 @@ The label is what appears in the output lines. scripts/watch.sh [intervalSeconds] ``` -Start it with `run_in_background`. You are notified when it exits. Read its output, handle every line, then start it again; it only watches while running. Editing `sessions.txt` or `prs.txt` takes effect on the next re-arm. `scripts/snapshot.sh ` prints the current state once. +Start it with `run_in_background`. You are notified when it exits. Read its output, handle every line, then start it again; it only watches while running. `watch.sh` records its pid, each tick and its exit in `/watcher.json`, and `node scripts/watcher.mjs ` prints `watcher ok` or `watcher MISSING()` (exit 1): check it before ending any turn that handled an event, and re-arm on `MISSING`. Editing `sessions.txt` or `prs.txt` takes effect on the next re-arm. `scripts/snapshot.sh ` prints the current state once. ## Reading the output @@ -121,7 +121,7 @@ Every action is an `EVENT recovery ...` line in the events log, which the `recov - Each pass is bounded by `PASS_TIMEOUT_SECONDS` (default 300, enough for a daemon restart and the resumes after it). A pass that runs longer gets a `pass-timeout` event, its App Server connections are closed so it unwinds and releases its lock, and the loop goes on. - After every pass it writes `/.fleet-recovery/heartbeat.json`: time, pid, interval, pass count, last pass duration, last error and RSS. The watcher's `supervisor` line reads it, so a hung, dead or leaking supervisor is an event. Once an hour it logs a `supervise: hourly` line (passes, errors, longest pass, RSS). A clean stop removes the heartbeat. - It logs to stdout, which the templates append to `/supervisor.log`; at `LOG_MAX_BYTES` (default 1 MB) it copies that file to `supervisor.log.1` and empties it. `SUPERVISOR_LOG` names the file when it is elsewhere. The events log rotates the same way to `events.log.1`. -- Keep it running with `templates/codex-fleet-supervisor.plist` (launchd, macOS) or `templates/codex-fleet-supervisor.service` (systemd user unit, Linux). Each template says where to save it and how to load it; nothing is installed for you. +- Keep it running with `templates/codex-fleet-supervisor.plist` (launchd, macOS) or `templates/codex-fleet-supervisor.service` (systemd user unit, Linux). Each template says where to save it and how to load it; nothing is installed for you. On macOS restart it with `launchctl bootout` then `launchctl bootstrap`, never `launchctl kickstart -k`, which hung the calling shell and left the job waiting its throttle before it respawned. ## Notes diff --git a/plugins/vibe/skills/codex-fleet-monitor/scripts/templates.test.mjs b/plugins/vibe/skills/codex-fleet-monitor/scripts/templates.test.mjs new file mode 100644 index 00000000..65325148 --- /dev/null +++ b/plugins/vibe/skills/codex-fleet-monitor/scripts/templates.test.mjs @@ -0,0 +1,23 @@ +import assert from 'node:assert/strict'; +import { readFileSync } from 'node:fs'; +import { test } from 'node:test'; +import { fileURLToPath } from 'node:url'; + +const PLIST = readFileSync(fileURLToPath(new URL('../templates/codex-fleet-supervisor.plist', import.meta.url)), 'utf8'); + +test('launchd respawns the supervisor within a minute', () => { + const throttle = /ThrottleInterval<\/key>\s*(\d+)<\/integer>/.exec(PLIST); + assert.ok(throttle, 'the template sets ThrottleInterval'); + assert.ok(Number(throttle[1]) <= 60, `ThrottleInterval ${throttle[1]} leaves the host without a supervisor for too long`); +}); + +test('the template restarts with bootout then bootstrap and never recommends kickstart', () => { + assert.match(PLIST, /launchctl bootout gui\/\$\(id -u\)\/codex-fleet-supervisor/); + assert.match(PLIST, /launchctl bootstrap gui\/\$\(id -u\)/); + const lines = PLIST.split('\n'); + const kickstart = lines.map((line, index) => [line, index]).filter(([line]) => /kickstart/.test(line)); + assert.ok(kickstart.length > 0, 'the template warns against kickstart'); + for (const [line, index] of kickstart) { + assert.match(line, /never/i, `line ${index + 1} names kickstart without saying never: ${line.trim()}`); + } +}); diff --git a/plugins/vibe/skills/codex-fleet-monitor/scripts/watch.sh b/plugins/vibe/skills/codex-fleet-monitor/scripts/watch.sh index 1dc5b25d..70391a93 100755 --- a/plugins/vibe/skills/codex-fleet-monitor/scripts/watch.sh +++ b/plugins/vibe/skills/codex-fleet-monitor/scripts/watch.sh @@ -1,14 +1,23 @@ #!/bin/zsh # Usage: watch.sh [intervalSeconds]. Exits with the diff on the first change. K=${0:A:h}; W=${1:?workDir}; I=${2:-120} +# watcher.json records this watcher's pid and last tick, and its exit, so watcher.mjs can tell a lapsed watcher. +started=$(date -u +%Y-%m-%dT%H:%M:%SZ) +beat() { + local extra=${1:-} + print -r -- "{\"pid\":$$,\"startedAt\":\"$started\",\"lastTickAt\":\"$(date -u +%Y-%m-%dT%H:%M:%SZ)\",\"intervalSeconds\":$I$extra}" > $W/watcher.json.tmp && mv $W/watcher.json.tmp $W/watcher.json +} +beat # Compare against the last snapshot the previous run saw, so a change between runs is still reported. if [ -s $W/last-snapshot.txt ]; then base=$(cat $W/last-snapshot.txt); else base=$(zsh $K/snapshot.sh $W); echo "$base" > $W/last-snapshot.txt; fi for _ in $(seq 1 200); do sleep $I # Each tick runs a recovery pass first (recover.mjs), so a transient failure is resumed without you. now=$(RECOVER=1 zsh $K/snapshot.sh $W) + beat if [ -n "$now" ] && [ "$now" != "$base" ]; then - echo "$now" > $W/last-snapshot.txt; echo EVENT; diff <(echo "$base") <(echo "$now") | grep '^[<>]'; exit 0 + echo "$now" > $W/last-snapshot.txt; beat ",\"exitedAt\":\"$(date -u +%Y-%m-%dT%H:%M:%SZ)\""; echo EVENT; diff <(echo "$base") <(echo "$now") | grep '^[<>]'; exit 0 fi done +beat ",\"exitedAt\":\"$(date -u +%Y-%m-%dT%H:%M:%SZ)\"" echo TIMEOUT; cat $W/last-snapshot.txt diff --git a/plugins/vibe/skills/codex-fleet-monitor/scripts/watcher.mjs b/plugins/vibe/skills/codex-fleet-monitor/scripts/watcher.mjs new file mode 100644 index 00000000..f14e1ea8 --- /dev/null +++ b/plugins/vibe/skills/codex-fleet-monitor/scripts/watcher.mjs @@ -0,0 +1,72 @@ +#!/usr/bin/env node +// Is the watcher for a work dir running? A watcher re-arm was lost in an +// interrupted tool batch and nobody noticed for about two hours, so pushes sat +// unhandled. watch.sh writes /watcher.json at start, after every +// tick and on exit; this reads it. +// +// Usage: watcher.mjs [--now ] +// +// Prints `watcher ok pid=

tick=s ago`, or `watcher MISSING()` and +// exits 1. A watcher is live when watcher.json has no exitedAt, its pid is +// alive and its last tick is within twice its interval plus 120 seconds. + +import { readFileSync } from "node:fs"; +import path from "node:path"; +import { pathToFileURL } from "node:url"; +import { parseArgs } from "node:util"; + +const TICK_SLACK_SECONDS = 120; + +/** The watcher's state for `workDir`: `{ ok, line }`. */ +export function watcherState(workDir, now = Date.now()) { + let state; + try { + state = JSON.parse(readFileSync(path.join(workDir, "watcher.json"), "utf8")); + } catch { + return missing("no watcher.json"); + } + if (state.exitedAt) { + return missing(`exited ${Math.max(0, Math.floor((now - Date.parse(state.exitedAt)) / 60_000))}m ago`); + } + if (!isAlive(state.pid)) { + return missing(`pid ${state.pid} dead`); + } + const age = Math.max(0, Math.floor((now - Date.parse(state.lastTickAt ?? state.startedAt)) / 1000)); + const interval = Number(state.intervalSeconds) || 0; + if (!Number.isFinite(age) || age > 2 * interval + TICK_SLACK_SECONDS) { + return missing(`no tick for ${Number.isFinite(age) ? age : "?"}s`); + } + return { ok: true, line: `watcher ok pid=${state.pid} tick=${age}s ago` }; +} + +function missing(why) { + return { ok: false, line: `watcher MISSING(${why})` }; +} + +function isAlive(pid) { + if (!Number.isInteger(pid) || pid <= 0) { + return false; + } + try { + process.kill(pid, 0); + return true; + } catch (error) { + return error.code === "EPERM"; + } +} + +function main() { + const { values, positionals } = parseArgs({ allowPositionals: true, options: { now: { type: "string" } } }); + const [workDir] = positionals; + if (!workDir) { + process.stderr.write("Usage: watcher.mjs [--now ]\n"); + return 2; + } + const { ok, line } = watcherState(workDir, values.now ? Date.parse(values.now) : Date.now()); + process.stdout.write(`${line}\n`); + return ok ? 0 : 1; +} + +if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { + process.exit(main()); +} diff --git a/plugins/vibe/skills/codex-fleet-monitor/scripts/watcher.test.mjs b/plugins/vibe/skills/codex-fleet-monitor/scripts/watcher.test.mjs new file mode 100644 index 00000000..48815fde --- /dev/null +++ b/plugins/vibe/skills/codex-fleet-monitor/scripts/watcher.test.mjs @@ -0,0 +1,82 @@ +import assert from 'node:assert/strict'; +import { spawn, spawnSync } from 'node:child_process'; +import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { test } from 'node:test'; +import { fileURLToPath } from 'node:url'; +import { watcherState } from './watcher.mjs'; + +const WATCH = fileURLToPath(new URL('./watch.sh', import.meta.url)); +const WATCHER = fileURLToPath(new URL('./watcher.mjs', import.meta.url)); +const NOW = Date.parse('2026-10-21T12:00:00Z'); + +function workDir(t) { + const dir = mkdtempSync(join(tmpdir(), 'fleet-watcher-')); + t.after(() => rmSync(dir, { recursive: true, force: true })); + return dir; +} + +const write = (dir, state) => writeFileSync(join(dir, 'watcher.json'), JSON.stringify(state)); +const at = (seconds) => new Date(NOW - seconds * 1000).toISOString(); + +function deadPid() { + const child = spawnSync(process.execPath, ['-e', 'process.stdout.write(String(process.pid))'], { encoding: 'utf8' }); + return Number(child.stdout); +} + +test('a live watcher that ticked recently is ok', (t) => { + const dir = workDir(t); + write(dir, { pid: process.pid, startedAt: at(600), lastTickAt: at(100), intervalSeconds: 120 }); + assert.deepEqual(watcherState(dir, NOW), { ok: true, line: `watcher ok pid=${process.pid} tick=100s ago` }); +}); + +test('each lapse is MISSING with its reason', (t) => { + const dir = workDir(t); + assert.deepEqual(watcherState(dir, NOW), { ok: false, line: 'watcher MISSING(no watcher.json)' }); + write(dir, { pid: process.pid, startedAt: at(9000), lastTickAt: at(7300), intervalSeconds: 120, exitedAt: at(7200) }); + assert.equal(watcherState(dir, NOW).line, 'watcher MISSING(exited 120m ago)'); + const dead = deadPid(); + write(dir, { pid: dead, startedAt: at(600), lastTickAt: at(10), intervalSeconds: 120 }); + assert.equal(watcherState(dir, NOW).line, `watcher MISSING(pid ${dead} dead)`); + write(dir, { pid: process.pid, startedAt: at(900), lastTickAt: at(361), intervalSeconds: 120 }); + assert.equal(watcherState(dir, NOW).line, 'watcher MISSING(no tick for 361s)'); + write(dir, { pid: process.pid, startedAt: at(900), lastTickAt: at(360), intervalSeconds: 120 }); + assert.equal(watcherState(dir, NOW).ok, true, 'twice the interval plus 120 seconds is still live'); +}); + +test('the command prints the line and exits 1 for a missing watcher', (t) => { + const dir = workDir(t); + const missing = spawnSync(process.execPath, [WATCHER, dir], { encoding: 'utf8' }); + assert.equal(missing.status, 1); + assert.equal(missing.stdout, 'watcher MISSING(no watcher.json)\n'); + write(dir, { pid: process.pid, startedAt: at(60), lastTickAt: at(30), intervalSeconds: 120 }); + const live = spawnSync(process.execPath, [WATCHER, dir, '--now', new Date(NOW).toISOString()], { encoding: 'utf8' }); + assert.equal(live.status, 0); +}); + +test('watch.sh records its pid at start, and a killed watcher reads as dead', { timeout: 30_000 }, async (t) => { + const dir = workDir(t); + writeFileSync(join(dir, 'last-snapshot.txt'), 'session local a RUN\n'); + // Its own process group, so killing it also ends the `sleep` it is waiting in. + const child = spawn('zsh', [WATCH, dir, '600'], { stdio: 'ignore', detached: true }); + const killGroup = () => { + if (!(child.pid > 1)) return; + try { + process.kill(-child.pid, 'SIGKILL'); + } catch { + // Already gone. + } + }; + t.after(killGroup); + const file = join(dir, 'watcher.json'); + for (let i = 0; i < 100 && !existsSync(file); i += 1) await new Promise((resolve) => setTimeout(resolve, 100)); + const state = JSON.parse(readFileSync(file, 'utf8')); + assert.equal(state.pid, child.pid); + assert.equal(state.intervalSeconds, 600); + assert.equal(watcherState(dir).ok, true); + const exited = new Promise((resolve) => child.on('exit', resolve)); + killGroup(); + await exited; + assert.equal(watcherState(dir).line, `watcher MISSING(pid ${child.pid} dead)`); +}); diff --git a/plugins/vibe/skills/codex-fleet-monitor/templates/codex-fleet-supervisor.plist b/plugins/vibe/skills/codex-fleet-monitor/templates/codex-fleet-supervisor.plist index bef3193c..4b782f0e 100644 --- a/plugins/vibe/skills/codex-fleet-monitor/templates/codex-fleet-supervisor.plist +++ b/plugins/vibe/skills/codex-fleet-monitor/templates/codex-fleet-supervisor.plist @@ -5,6 +5,9 @@ skill directory) and WORK_DIR (the supervisor's work dir), save it as ~/Library/LaunchAgents/codex-fleet-supervisor.plist, then run launchctl bootstrap gui/$(id -u) ~/Library/LaunchAgents/codex-fleet-supervisor.plist + To restart it, run launchctl bootout gui/$(id -u)/codex-fleet-supervisor, then the same + launchctl bootstrap line. Never use launchctl kickstart -k, which hung the calling shell and left + the job waiting ThrottleInterval before it respawned. The login shell puts node and codex on PATH. launchd appends to supervisor.log, and the supervisor rotates it at LOG_MAX_BYTES (default 1 MB) into supervisor.log.1. --> @@ -23,7 +26,7 @@ KeepAlive ThrottleInterval - 300 + 30 StandardOutPath WORK_DIR/supervisor.log StandardErrorPath diff --git a/plugins/vibe/skills/fleet-orchestrator/SKILL.md b/plugins/vibe/skills/fleet-orchestrator/SKILL.md index 7984d9cb..c5068b07 100644 --- a/plugins/vibe/skills/fleet-orchestrator/SKILL.md +++ b/plugins/vibe/skills/fleet-orchestrator/SKILL.md @@ -75,7 +75,7 @@ The closedloop-graph MCP server is required; decide from it, not from guesses. T Run [codex-fleet-monitor](../codex-fleet-monitor/SKILL.md) (`vibe:codex-fleet-monitor`) on the same `` in the background for the whole program and act on its lines as its table says, including usage bands, paid-credit spending and request files. It reads the session state files `fleet.sh` writes, listed as `local|remote ` in `sessions.txt`. - The watcher resumes a session after a transient provider failure on its own; read its `recovery` lines when you wake. Run its supervisor on every machine that hosts Codex writers, so recovery also happens while you are not running. Set `AUTO_RESTART_DAEMON=1` in `config.env` only when the operator agrees that a stuck daemon may be restarted unattended. -- Re-arm the watcher in the same step you read each event. It only watches while running, and it costs nothing while waiting. +- Re-arm the watcher in the same step you read each event. It only watches while running, and it costs nothing while waiting. Before ending any turn that handled an event, `node ../codex-fleet-monitor/scripts/watcher.mjs ` must print `watcher ok`; on `MISSING`, re-arm it. A lost re-arm once left pushes unhandled for two hours. - Before you or a reviewer inspect a head, check it equals the pushed PR head (`gh pr view --json headRefOid`). Sessions sometimes report local, unpushed commits. ## Codex sessions @@ -104,7 +104,17 @@ scripts/fleet.sh local|remote session last [--count ] - Launch each Claude writer with the [worker preamble](./references/templates.md#worker-preamble) saved to `prompts/`. It starts with invoking vibe and carries the worktree, PR, reading, test, git, thread-reply and report rules so no writer improvises them. - A message to a busy worker arrives only when it goes idle; if it idles without acting on one, resend it shortly. - A pane teammate sees your messages only when its current turn ends. For an urgent redirect, ask the operator to press Esc in the named pane, which ends the turn and delivers them; otherwise expect delivery at the next turn boundary. Keep long briefs in a file and send its path. -- Stop a helper (reviewer, verifier, QA prep, poller) as soon as you accept its result; its brief and findings are on disk, so the next trigger launches a fresh one. Idle helper panes hide the active ones from the operator. Record each with `scripts/helpers.mjs start|accepted|stopped `, and run `scripts/helpers.mjs status` with every monitor event: each `STOP` line is a helper to stop now. +- Stop a helper (reviewer, verifier, QA prep, poller) as soon as you accept its result; its brief and findings are on disk, so the next trigger launches a fresh one. Idle helper panes hide the active ones from the operator. Record each with `scripts/helpers.mjs start|accepted|stopped `, and run `scripts/helpers.mjs status` with every monitor event: each `STOP` line is a helper to stop now, each `ORPHAN` line a stopped helper whose processes still run. A helper records every process it starts (dev servers, app stacks) with `helpers.mjs pid `, adding `--group` only for one it spawned detached; `stopped` ends them and refuses while any lives. Stopped QA agents once left dev servers running for 13 to 16 hours. + +## Fleet board and direct messages + +Writers that could not see each other's work collided, a QA guide never heard its checkpoint was ready, and a UX ruling reached one writer while another PR rendered the same behavior. `scripts/board.mjs` keeps `/board.md`, the page every writer reads: + +- When you launch a writer, add its row (`board.mjs writer --pr --repo --head --status --deps `) and give it the board path and the messaging recipe ([worker preamble](./references/templates.md#worker-preamble)). Update the row on every push report. +- Writers message each other directly: Codex writers through the `app-server-orchestrator` skill's send, Claude workers and QA guides through SendMessage. +- A settled UX ruling is saved once as a reference file with its screenshots in a shared evidence location. Find every renderer of the behavior with `code_grep` across each open PR's diff and main, then `board.mjs deliver --kind ruling --reference --to `. Every QA plan for that behavior cites the reference, and one shared component beats each PR building its own. +- When QA prep marks a human checkpoint ready, or rebinds results, `deliver --kind checkpoint --head --to ` and message the guide yourself with the checkpoint, its launch steps and the head. +- After each message, `board.mjs told `. Run `board.mjs status` with every monitor event: each `STALE` row is a writer whose pushed head the board does not show, each `UNTOLD` line someone still to tell. ## Moving work between runtimes and accounts @@ -120,16 +130,16 @@ scripts/fleet.sh local|remote session last [--count ] - Run heavy checks (full test suites, typecheck, repo-wide lint and other gates, component story runs) on the least loaded machine, in the worker's own worktree there ([checklist step 4](./references/setup.md#checklist) gets the branch there). Keep the operator's daily machine for edits and git. - Balance heavy work by measured load, not a fixed machine: before each heavy run, compare load per core (`uptime` against the core count) on each machine and send the run to the lower one. A fixed choice once drove one machine to more than twice its core count while the other sat idle. - Cap heavy runs per machine at its `HEAVY_SLOTS` or `REMOTE_HEAVY_SLOTS`: `scripts/fleet.sh local|remote slot heavy `. -- A shared e2e stack with fixed ports runs only under vibe's machine-wide e2e lock, the one every vibe session's e2e command already takes: `node ../vibe/scripts/e2e-lock.mjs run ` on the machine that runs the stack. A second lock for the same stack would let two runs collide. Only runs that start that stack take it; tests against a stand-in server, component story runs, unit tests and typecheck run outside it in parallel. +- A shared e2e stack with fixed ports runs only under vibe's machine-wide e2e lock, the one every vibe session's e2e command already takes: `node ../vibe/scripts/e2e-lock.mjs run ` on the machine that runs the stack. A second lock for the same stack would let two runs collide. The lock gives each run its own compose project, so a teardown removes only its own stack; name a stack another tool started on the same ports with `--foreign-compose `. Only runs that start that stack take it; tests against a stand-in server, component story runs, unit tests and typecheck run outside it in parallel. - `scripts/slot-lock.sh` underneath frees the slot when the command exits or is interrupted and reclaims one whose holder pid died; `scripts/slot-lock.sh --status ` on that machine lists holders. -- Kill only processes you started, matched by pid or worktree path, never an unscoped `pkill`. Other runs on that machine belong to someone else. +- Kill only processes you started, matched by pid or worktree path, never an unscoped `pkill`. Other runs on that machine belong to someone else. Never signal a pid of 1 or lower or a process group you did not spawn: `kill(-1)` from a plugin test once ended every app on the operator's machine. Tests that send signals run on another machine, never the operator's daily one. - Plugin marketplaces and env files on a remote machine live in persistent paths too. ## Quality gates and QA - QA happens before merge; never call a PR ready to enqueue before its QA is done. - Each PR moves through, in order: independent review → writer fixes → verification of the fixes → design or visual review where UI changed → full check at the final head → QA plan (agent-verified plus human-only checkpoints) → operator QA → one main merge → operator enqueues. A head change sends it back to the earliest step the change affects. -- QA runs on the feature-complete head, even when it conflicts with main; on a fast-moving main, a head kept current before QA goes stale during QA. vibe's publication leaves main unmerged until then; after QA, tell the writer to set final-merge mode (`vibe-sessions.mjs touch --main-merge final`), so its next publication merges main once, resolving version-file conflicts, reruns only the tests `code_tests_for` selects for the files the merge touched, and the operator enqueues at once; the merge queue retests against main. Before telling the operator to enqueue, `node ../handoff/scripts/qa-merge-order.mjs --worktree --qa-head ` must pass. Never ask a writer to merge main mid-work unless a conflict blocks the build itself. +- QA runs on the feature-complete head, even when it conflicts with main; on a fast-moving main, a head kept current before QA goes stale during QA. vibe's publication leaves main unmerged until then; after QA, tell the writer to set final-merge mode (`vibe-sessions.mjs touch --main-merge final`), so its next publication merges main once, resolving version-file conflicts, reruns only the tests `code_tests_for` selects for the files the merge touched, and the operator enqueues at once; the merge queue retests against main. Before telling the operator to enqueue, `node ../handoff/scripts/qa-merge-order.mjs --worktree --qa-head ` must pass. That merge waits on no e2e run: the writer pushes first, then runs only the e2e specs covering conflict-resolved files, and shepherds its PR (`../vibe/scripts/shepherd.mjs`, every two minutes, re-merging main at once on a conflict). Never ask a writer to merge main mid-work unless a conflict blocks the build itself. - Criteria are fixed before the build, because nobody is in the loop to ask for them while it runs. - A writer starts only once its plan holds a numbered acceptance-criteria list. - Each criterion is sourced to a ticket AC, a PRD line or an operator ruling. @@ -137,7 +147,9 @@ scripts/fleet.sh local|remote session last [--count ] An unrequested write path, a missed operator ruling and a mislabeled defect each traced back to having no such list. - The QA plan is a two-way trace between that list and the diff: each checkpoint cites one criterion ID, a diff change mapping to no criterion is a finding, never a checkpoint, and a criterion with no change and no coverage is a gap. Regression checks on neighboring surfaces the diff touches are labeled separately; a regression failure is a bug, never a new requirement. -- Agent-first QA: an agent verifies everything it can (e2e on the current head, DOM and accessibility reads, API second views, headless screenshots it reads itself) and records it. The operator gets only checkpoints that need a human: perceptual judgment, real-device feel, assistive tech, product judgment, flows an agent cannot drive; measure the rest (contrast ratios, box geometry, focus targets). A defect an agent already observed goes to the writer with its measurement, not to the human checklist. +- Agent-first QA, zero human checkpoints by default: an agent verifies everything it can (e2e on the current head, DOM and accessibility reads, API second views, headless screenshots it compares with the saved reference itself) and records it. The operator gets only checkpoints an agent cannot cover: real-device feel, assistive tech, flows an agent cannot drive, a product call no reference settles; measure the rest (contrast ratios, box geometry, focus targets). `node scripts/qa-plan-check.mjs --plan ` must pass before the plan reaches the operator; it refuses a human checkpoint without such a reason, a visual one included. A defect an agent already observed goes to the writer with its measurement, not to the human checklist. +- A QA rerun that toggles flags or moves data never uses the stack handed to the operator: `helpers.mjs hand --to ` when the guide gets it, `unhand` when QA ends, and start the rerun helper with `--stack ` (refused on a handed one). +- Writers push a tested checkpoint at least every 90 minutes of active work (vibe's commit reports `pushDue`); review findings after that go out as follow-up commits reviewed per diff, and never add new scope to an unpushed checkpoint. A critical-path PR once sat unpushed for 5 hours while each review pass added scope. - Human QA runs in a dedicated Claude QA-guide session per PR that the operator talks to directly. When the PR's QA plan is ready: 1. Save the [QA guide prompt](./references/templates.md#qa-guide-prompt) to `prompts/`. 2. Launch it headless from the repo root (`claude -p --output-format json` with the tools it needs allowed). diff --git a/plugins/vibe/skills/fleet-orchestrator/references/templates.md b/plugins/vibe/skills/fleet-orchestrator/references/templates.md index 97cf535b..65c60ece 100644 --- a/plugins/vibe/skills/fleet-orchestrator/references/templates.md +++ b/plugins/vibe/skills/fleet-orchestrator/references/templates.md @@ -34,6 +34,8 @@ Heavy checks go through . Never ru Git: commit messages per the repo's conventions; regular push only, never force or amend after push. Review threads: reply on the thread naming the fixing SHA and resolve it only after the fix is verified in code. Artifacts for people (tickets, PR bodies, PR comments, review replies): never name a path on the operator's machine (home, temp, scratchpad or worktree paths) or say something is on someone's laptop. Embed screenshots as uploaded images and quote text evidence inline, briefly. A plan is attached, never pointed at: link the implementation plan as a plan document, and upload a QA plan or other working file as a ticket attachment named by its attachment name. +Fleet board: /board.md shows every writer's PR, head, status and dependencies. Message another writer directly when your work touches theirs: Codex writers through the app-server-orchestrator skill's send, Claude workers and QA guides through SendMessage. +Push a tested checkpoint at least every 90 minutes of active work; send review findings after that as follow-up commits, and add no new scope to an unpushed checkpoint. Report when done: pushed head SHA, files changed, tests run and results, open items, anything that needs the operator. Task: diff --git a/plugins/vibe/skills/fleet-orchestrator/scripts/board.mjs b/plugins/vibe/skills/fleet-orchestrator/scripts/board.mjs new file mode 100644 index 00000000..f496f034 --- /dev/null +++ b/plugins/vibe/skills/fleet-orchestrator/scripts/board.mjs @@ -0,0 +1,191 @@ +#!/usr/bin/env node +// The fleet board: what every writer is doing, and what each writer or QA +// guide must be told. Writers that could not see each other's work collided, +// a QA guide never heard that its checkpoint was ready, and a UX ruling +// reached one writer as text while another PR rendered the same behavior. +// +// Usage: board.mjs writer [--pr ] [--repo ] [--head ] [--status ] [--deps ] +// board.mjs deliver --to [--kind ruling|checkpoint|note] [--reference ] [--head ] +// board.mjs told +// board.mjs status [--pushed =]... +// +// `writer` adds or updates a writer's row (owner, PR, head, status, +// dependencies). `deliver` records something each recipient must be told +// directly (a ruling saved once as a reference file, a QA checkpoint that is +// ready); `told` marks one recipient told. Every write re-renders +// /board.md, the page every writer reads, with a log line. +// +// `status` prints `STALE board head , pushed ` for a row whose +// head differs from the PR's pushed head (`--pushed`, or `gh pr view` for rows +// with a PR and repo; `BOARD_GH` overrides the gh binary), and +// `UNTOLD ` for each recipient not yet told, then +// `board: writers, stale, untold`; exits 1 when any. + +import { execFileSync } from "node:child_process"; +import { existsSync, readFileSync, renameSync, writeFileSync } from "node:fs"; +import path from "node:path"; +import { pathToFileURL } from "node:url"; +import { parseArgs } from "node:util"; + +const STATE = "board.json"; +const PAGE = "board.md"; +const KINDS = new Set(["ruling", "checkpoint", "note"]); +const LOG_LINES = 50; + +/** Rows whose recorded head differs from the pushed head; a row with no known pushed head is not judged. */ +export function staleRows(board, pushed) { + return Object.entries(board.writers) + .filter(([name, row]) => pushed[name] && row.head !== pushed[name]) + .map(([name, row]) => ({ name, head: row.head ?? "none", pushed: pushed[name] })); +} + +/** Every recipient of every delivery not yet told. */ +export function untold(board) { + return Object.entries(board.deliveries).flatMap(([item, delivery]) => + delivery.to.filter((recipient) => !delivery.told[recipient]).map((recipient) => ({ item, recipient, delivery }))); +} + +/** The page writers read. */ +export function render(board) { + const rows = Object.entries(board.writers).map(([name, row]) => + `| ${name} | ${row.pr ? `${row.repo ?? ""}#${row.pr}` : ""} | ${row.head ?? ""} | ${row.status ?? ""} | ${(row.deps ?? []).join(", ")} | ${row.updatedAt} |`); + const open = untold(board).map(({ item, recipient, delivery }) => + `- ${item} (${delivery.kind}) for ${recipient}${delivery.reference ? `: ${delivery.reference}` : ""}${delivery.head ? ` at ${delivery.head}` : ""}`); + return [ + "# Fleet board", + "", + "| Writer | PR | Head | Status | Depends on | Updated |", + "|---|---|---|---|---|---|", + ...rows, + "", + "## Not yet told", + "", + ...(open.length > 0 ? open : ["- nothing"]), + "", + "## Log", + "", + ...board.log.slice(-LOG_LINES).map((line) => `- ${line}`), + "", + ].join("\n"); +} + +function read(orchDir) { + const file = path.join(orchDir, STATE); + const board = existsSync(file) ? JSON.parse(readFileSync(file, "utf8")) : {}; + return { writers: board.writers ?? {}, deliveries: board.deliveries ?? {}, log: board.log ?? [] }; +} + +function write(orchDir, board, line, at) { + board.log.push(`${at} ${line}`); + board.log = board.log.slice(-LOG_LINES); + for (const [name, text] of [[STATE, `${JSON.stringify(board, null, 2)}\n`], [PAGE, render(board)]]) { + const file = path.join(orchDir, name); + writeFileSync(`${file}.tmp`, text); + renameSync(`${file}.tmp`, file); + } +} + +function list(text) { + return (text ?? "").split(",").map((part) => part.trim()).filter(Boolean); +} + +function ghHead(row) { + try { + return execFileSync(process.env.BOARD_GH || "gh", ["pr", "view", String(row.pr), "-R", row.repo, "--json", "headRefOid", "-q", ".headRefOid"], + { encoding: "utf8", stdio: ["ignore", "pipe", "ignore"], timeout: 30_000 }).trim() || null; + } catch { + return null; + } +} + +function pushedHeads(board, pairs) { + const pushed = Object.fromEntries(pairs.map((pair) => { + const at = pair.indexOf("="); + if (at <= 0) { + throw new Error(`--pushed needs =, got ${pair}`); + } + return [pair.slice(0, at), pair.slice(at + 1)]; + })); + for (const [name, row] of Object.entries(board.writers)) { + if (!pushed[name] && row.pr && row.repo) { + const head = ghHead(row); + if (head) pushed[name] = head; + } + } + return pushed; +} + +function main(argv) { + const [orchDir, action, ...rest] = argv; + const { values, positionals } = parseArgs({ + args: rest, + allowPositionals: true, + options: { + pr: { type: "string" }, repo: { type: "string" }, head: { type: "string" }, status: { type: "string" }, deps: { type: "string" }, + to: { type: "string" }, kind: { type: "string" }, reference: { type: "string" }, + pushed: { type: "string", multiple: true }, now: { type: "string" }, + }, + }); + if (!orchDir || !["writer", "deliver", "told", "status"].includes(action)) { + throw new Error("Usage: board.mjs writer|deliver|told|status ..."); + } + const board = read(orchDir); + const at = new Date(values.now ? Date.parse(values.now) : Date.now()).toISOString(); + if (action === "status") { + const stale = staleRows(board, pushedHeads(board, values.pushed ?? [])); + for (const row of stale) process.stdout.write(`STALE ${row.name} board head ${row.head}, pushed ${row.pushed}\n`); + const open = untold(board); + for (const entry of open) process.stdout.write(`UNTOLD ${entry.item} ${entry.recipient}\n`); + process.stdout.write(`board: ${Object.keys(board.writers).length} writers, ${stale.length} stale, ${open.length} untold\n`); + return stale.length + open.length === 0 ? 0 : 1; + } + const [name, recipient] = positionals; + if (!name) { + throw new Error(`${action} needs a name.`); + } + if (action === "writer") { + const row = board.writers[name] ?? {}; + const update = Object.fromEntries(Object.entries({ pr: values.pr, repo: values.repo, head: values.head, status: values.status }) + .filter(([, value]) => value !== undefined)); + board.writers[name] = { ...row, ...update, ...(values.deps === undefined ? {} : { deps: list(values.deps) }), updatedAt: at }; + write(orchDir, board, `${name}: ${Object.entries(update).map(([key, value]) => `${key} ${value}`).join(", ") || "touched"}`, at); + return 0; + } + if (action === "deliver") { + const to = list(values.to); + const kind = values.kind ?? "note"; + if (to.length === 0 || !KINDS.has(kind)) { + throw new Error("deliver needs --to and a --kind of ruling, checkpoint or note."); + } + if (kind === "ruling" && !values.reference) { + throw new Error("a ruling is saved once as a reference file (its screenshots beside it); pass --reference ."); + } + const previous = board.deliveries[name]; + board.deliveries[name] = { + kind, + to: [...new Set([...(previous?.to ?? []), ...to])], + told: previous?.told ?? {}, + ...(values.reference ? { reference: values.reference } : {}), + ...(values.head ? { head: values.head } : {}), + at, + }; + write(orchDir, board, `${name} (${kind}) for ${to.join(", ")}`, at); + return 0; + } + const delivery = board.deliveries[name]; + if (!delivery || !recipient || !delivery.to.includes(recipient)) { + throw new Error(`told needs a delivery and one of its recipients (${delivery ? delivery.to.join(", ") : "no such delivery"}).`); + } + delivery.told[recipient] = at; + write(orchDir, board, `${recipient} told ${name}`, at); + return 0; +} + +if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { + try { + process.exit(main(process.argv.slice(2))); + } catch (error) { + process.stderr.write(`${error instanceof Error ? error.message : String(error)}\n`); + process.exit(2); + } +} diff --git a/plugins/vibe/skills/fleet-orchestrator/scripts/board.test.mjs b/plugins/vibe/skills/fleet-orchestrator/scripts/board.test.mjs new file mode 100644 index 00000000..e8e05866 --- /dev/null +++ b/plugins/vibe/skills/fleet-orchestrator/scripts/board.test.mjs @@ -0,0 +1,65 @@ +import assert from 'node:assert/strict'; +import { spawnSync } from 'node:child_process'; +import { chmodSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { test } from 'node:test'; +import { fileURLToPath } from 'node:url'; + +const HERE = path.dirname(fileURLToPath(import.meta.url)); +const tempDir = (t) => { + const dir = mkdtempSync(path.join(os.tmpdir(), 'fleet-board-')); + t.after(() => rmSync(dir, { recursive: true, force: true })); + return dir; +}; +const run = (args, env = {}) => spawnSync(process.execPath, [path.join(HERE, 'board.mjs'), ...args], { encoding: 'utf8', env: { ...process.env, ...env } }); + +test('a writer row whose head is behind the pushed head is stale until the writer row is updated', (t) => { + const dir = tempDir(t); + assert.equal(run([dir, 'writer', 'w1', '--pr', '101', '--head', 'aaa', '--status', 'building', '--deps', 'w2']).status, 0); + assert.equal(run([dir, 'writer', 'w2', '--head', 'ccc']).status, 0); + const stale = run([dir, 'status', '--pushed', 'w1=bbb', '--pushed', 'w2=ccc']); + assert.equal(stale.status, 1); + assert.match(stale.stdout, /^STALE w1 board head aaa, pushed bbb$/m); + assert.doesNotMatch(stale.stdout, /STALE w2/); + assert.match(stale.stdout, /board: 2 writers, 1 stale, 0 untold/); + + assert.equal(run([dir, 'writer', 'w1', '--head', 'bbb', '--status', 'QA ready']).status, 0); + assert.equal(run([dir, 'status', '--pushed', 'w1=bbb']).status, 0); + const page = readFileSync(path.join(dir, 'board.md'), 'utf8'); + assert.match(page, /^\| w1 \| #101 \| bbb \| QA ready \| w2 \| /m, 'the update keeps the PR and dependencies'); + assert.match(page, /w1: head bbb, status QA ready/); +}); + +test('the pushed head is read from gh for a row with a PR and repo', (t) => { + const dir = tempDir(t); + const gh = path.join(dir, 'gh'); + writeFileSync(gh, '#!/bin/sh\necho "pushed-$3"\n'); + chmodSync(gh, 0o755); + assert.equal(run([dir, 'writer', 'w1', '--pr', '7', '--repo', 'acme/app', '--head', 'old']).status, 0); + const stale = run([dir, 'status'], { BOARD_GH: gh }); + assert.match(stale.stdout, /^STALE w1 board head old, pushed pushed-7$/m); +}); + +test('a ruling and a ready checkpoint stay untold until each recipient is told directly', (t) => { + const dir = tempDir(t); + const refused = run([dir, 'deliver', 'drag-chip', '--kind', 'ruling', '--to', 'w1,w2']); + assert.equal(refused.status, 2); + assert.match(refused.stderr, /saved once as a reference file/); + assert.equal(run([dir, 'deliver', 'drag-chip', '--kind', 'ruling', '--reference', 'rulings/drag-chip.md', '--to', 'w1,w2,guide-102']).status, 0); + assert.equal(run([dir, 'deliver', 'p-06', '--kind', 'checkpoint', '--head', 'abc', '--to', 'guide-102']).status, 0); + assert.equal(run([dir, 'told', 'drag-chip', 'w1']).status, 0); + assert.equal(run([dir, 'told', 'drag-chip', 'w9']).status, 2, 'only a named recipient can be told'); + const status = run([dir, 'status']); + assert.equal(status.status, 1); + assert.deepEqual(status.stdout.split('\n').filter((line) => line.startsWith('UNTOLD')), [ + 'UNTOLD drag-chip w2', + 'UNTOLD drag-chip guide-102', + 'UNTOLD p-06 guide-102', + ]); + assert.match(readFileSync(path.join(dir, 'board.md'), 'utf8'), /- p-06 \(checkpoint\) for guide-102 at abc/); + for (const [item, who] of [['drag-chip', 'w2'], ['drag-chip', 'guide-102'], ['p-06', 'guide-102']]) { + assert.equal(run([dir, 'told', item, who]).status, 0); + } + assert.match(run([dir, 'status']).stdout, /board: 0 writers, 0 stale, 0 untold/); +}); diff --git a/plugins/vibe/skills/fleet-orchestrator/scripts/qa-plan-check.mjs b/plugins/vibe/skills/fleet-orchestrator/scripts/qa-plan-check.mjs new file mode 100644 index 00000000..c38d91f0 --- /dev/null +++ b/plugins/vibe/skills/fleet-orchestrator/scripts/qa-plan-check.mjs @@ -0,0 +1,90 @@ +#!/usr/bin/env node +// Human QA only when nothing else can do it. The default is zero human +// checkpoints: an agent verifies what it can, and judges a screenshot against +// the saved reference itself. A checkpoint goes to the operator only for a +// reason an agent cannot cover: a real device or its feel, assistive +// technology, a flow an agent cannot drive (real sign-in, OS dialogs, +// hardware, a third-party screen), or a product call no reference settles. +// +// Usage: qa-plan-check.mjs --plan +// +// Reads each `### : ` checkpoint and its `- Routing:` line (the +// guided-manual-qa record). A routing that starts with AGENT_VERIFIED or +// E2E_COVERED is the agent's; any other routing is a human checkpoint and +// must give a reason from the list above. Prints `HUMAN <id>: <reason>` for +// each accepted one and `REFUSED <id>: <why>` for each refused one, then +// `human checkpoints: <n>, refused: <n>`; exits 1 when any is refused. + +import { readFileSync } from "node:fs"; +import { pathToFileURL } from "node:url"; +import { parseArgs } from "node:util"; + +const CHECKPOINT = /^###\s+([A-Za-z0-9_-]+):/; +const ROUTING = /^-\s*Routing[^:]*:\s*(.*)$/; +const AGENT = /^`?(?:AGENT_VERIFIED|E2E_COVERED)\b/; +const HUMAN_ONLY = /\b(?:real device|physical device|on-device|device feel|(?:touch|scroll|gesture) feel|assistive|screen reader|voiceover|talkback|os dialog|system dialog|oauth|real sign-in|hardware|camera|microphone|third-party|cannot drive|can't drive|product (?:judgment|call|decision))\b/i; +const VISUAL_ONLY = /\b(?:visual|layout|looks?|overlap|spacing|alignment|colou?r|screenshot|perceptual|pixel|render(?:s|ing)?)\b/i; + +/** Every checkpoint of a QA record with its routing line, or null when it has none. */ +export function checkpoints(text) { + const found = []; + for (const line of text.split("\n")) { + const heading = CHECKPOINT.exec(line); + if (heading) { + found.push({ id: heading[1], routing: null }); + continue; + } + const routing = ROUTING.exec(line); + if (routing && found.length > 0 && found.at(-1).routing === null) { + found.at(-1).routing = routing[1].trim(); + } + } + return found; +} + +/** The verdict for one checkpoint: agent, human (with its reason) or refused (with why). */ +export function verdict({ routing }) { + if (routing === null || routing === "") { + return { kind: "refused", why: "no Routing line; route it to the agent with its evidence, or give the reason it needs a human" }; + } + if (AGENT.test(routing)) { + return { kind: "agent" }; + } + if (HUMAN_ONLY.test(routing)) { + return { kind: "human", reason: routing }; + } + if (VISUAL_ONLY.test(routing)) { + return { kind: "refused", why: "a visual judgment is the agent's: compare its screenshot with the saved reference and record AGENT_VERIFIED" }; + } + return { kind: "refused", why: "no reason an agent cannot cover (real device or feel, assistive technology, a flow it cannot drive, a product call)" }; +} + +function main(argv) { + const { values } = parseArgs({ args: argv, options: { plan: { type: "string" } } }); + if (!values.plan) { + throw new Error("Usage: qa-plan-check.mjs --plan <QA record>"); + } + let human = 0; + let refused = 0; + for (const checkpoint of checkpoints(readFileSync(values.plan, "utf8"))) { + const result = verdict(checkpoint); + if (result.kind === "human") { + human += 1; + process.stdout.write(`HUMAN ${checkpoint.id}: ${result.reason}\n`); + } else if (result.kind === "refused") { + refused += 1; + process.stdout.write(`REFUSED ${checkpoint.id}: ${result.why}\n`); + } + } + process.stdout.write(`human checkpoints: ${human}, refused: ${refused}\n`); + return refused === 0 ? 0 : 1; +} + +if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { + try { + process.exit(main(process.argv.slice(2))); + } catch (error) { + process.stderr.write(`${error instanceof Error ? error.message : String(error)}\n`); + process.exit(2); + } +} diff --git a/plugins/vibe/skills/fleet-orchestrator/scripts/qa-plan-check.test.mjs b/plugins/vibe/skills/fleet-orchestrator/scripts/qa-plan-check.test.mjs new file mode 100644 index 00000000..1889975c --- /dev/null +++ b/plugins/vibe/skills/fleet-orchestrator/scripts/qa-plan-check.test.mjs @@ -0,0 +1,50 @@ +import assert from 'node:assert/strict'; +import { spawnSync } from 'node:child_process'; +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { test } from 'node:test'; +import { fileURLToPath } from 'node:url'; +import { checkpoints, verdict } from './qa-plan-check.mjs'; + +const HERE = path.dirname(fileURLToPath(import.meta.url)); +const record = (sections) => sections.map(([id, routing]) => `### ${id}: title\n\n- Risk / requirement: AC-1\n${routing === null ? '' : `- Routing (\`AGENT_VERIFIED\` with its evidence, or the reason this checkpoint needs a human): ${routing}\n`}- Expected: x\n`).join('\n'); + +test('agent-verified and e2e-covered checkpoints are the agent\'s; a human one needs a reason no agent can cover', () => { + const found = checkpoints(record([ + ['QA-001', 'AGENT_VERIFIED: DOM read on head abc'], + ['QA-002', '`E2E_COVERED` by the move spec'], + ['QA-003', 'needs a real device: trackpad scroll feel on the tree grid'], + ['QA-004', 'screen reader announcement order (VoiceOver)'], + ['QA-005', 'visual: whether the drag chip looks right'], + ['QA-006', 'the operator should take a look'], + ['QA-007', null], + ['QA-008', 'the layout looks and feels right'], + ])); + assert.deepEqual(found.map((checkpoint) => [checkpoint.id, verdict(checkpoint).kind]), [ + ['QA-001', 'agent'], + ['QA-002', 'agent'], + ['QA-003', 'human'], + ['QA-004', 'human'], + ['QA-005', 'refused'], + ['QA-006', 'refused'], + ['QA-007', 'refused'], + ['QA-008', 'refused'], + ]); + assert.match(verdict(found[4]).why, /compare its screenshot with the saved reference/); +}); + +test('the command exits 1 on a refused checkpoint and 0 for a plan with zero human checkpoints', (t) => { + const dir = mkdtempSync(path.join(os.tmpdir(), 'qa-plan-')); + t.after(() => rmSync(dir, { recursive: true, force: true })); + const plan = path.join(dir, 'qa.md'); + writeFileSync(plan, record([['QA-001', 'AGENT_VERIFIED: capture compared with the reference'], ['QA-002', 'whether the spacing looks right']])); + const refused = spawnSync(process.execPath, [path.join(HERE, 'qa-plan-check.mjs'), '--plan', plan], { encoding: 'utf8' }); + assert.equal(refused.status, 1); + assert.match(refused.stdout, /^REFUSED QA-002: a visual judgment is the agent's/m); + assert.match(refused.stdout, /human checkpoints: 0, refused: 1/); + writeFileSync(plan, record([['QA-001', 'AGENT_VERIFIED: capture compared with the reference']])); + const clean = spawnSync(process.execPath, [path.join(HERE, 'qa-plan-check.mjs'), '--plan', plan], { encoding: 'utf8' }); + assert.equal(clean.status, 0); + assert.equal(clean.stdout, 'human checkpoints: 0, refused: 0\n'); +}); diff --git a/plugins/vibe/skills/handoff/SKILL.md b/plugins/vibe/skills/handoff/SKILL.md index e79f6268..3c363376 100644 --- a/plugins/vibe/skills/handoff/SKILL.md +++ b/plugins/vibe/skills/handoff/SKILL.md @@ -491,7 +491,10 @@ step 5's checks and step 7's validation: screen, and no other spec. - Run them through the repo's containerized path (the `local-ci` skill's `dagger call e-2-e`) in the verify worker's checks mode, one stack per machine through `../vibe/scripts/e2e-lock.mjs` (the verify worker's - "Existing checks"), never natively on the person's Mac. + "Existing checks"), never natively on the person's Mac. The lock gives + each run its own compose project, so a teardown removes only its own + stack; `--foreign-compose <project>` names a stack started outside the + lock, which keeps the machine busy while it runs. - A missing Docker or Dagger is recorded as that exact limitation, never a pass. @@ -907,4 +910,10 @@ the next pass through step 7's main-sync publication gate is the one main merge. Rerun only the tests selected for the files the merge touched, and tell the person it is ready to enqueue right away, once `node scripts/qa-merge-order.mjs --worktree "<wt>" --qa-head <QA'd sha>` -passes; the merge queue retests it against main. +passes; the merge queue retests it against main. That merge waits on no e2e +run: push it first, then run only the e2e specs covering the files whose +conflicts were resolved. From then on the pull request is shepherded: +`node ../vibe/scripts/shepherd.mjs --pr <n> --repo <owner/repo>` polls it +every two minutes and stops on a conflict (merge main at once in final-merge +mode, push, run it again), failed checks (the SAME writer fixes them), a +merge or a close. diff --git a/plugins/vibe/skills/handoff/scripts/diff-rules.mjs b/plugins/vibe/skills/handoff/scripts/diff-rules.mjs index b8d44b91..2747d6cd 100644 --- a/plugins/vibe/skills/handoff/scripts/diff-rules.mjs +++ b/plugins/vibe/skills/handoff/scripts/diff-rules.mjs @@ -10,6 +10,7 @@ import { existsSync, readFileSync } from "node:fs"; import path from "node:path"; import { git } from "../../vibe/scripts/session-record.mjs"; import { E2E_ROOT_PATHSPECS, isTestFile } from "../../vibe/scripts/test-files.mjs"; +import { pathGateTests } from "./path-gates.mjs"; import { routeSpecs } from "./route-specs.mjs"; import { escapeRegExp, IMPORT_SPECIFIER, resolveImport, SOURCE_FILE } from "./source-imports.mjs"; import { isUnversionedStory, storyExportNames, storyMetaTitle } from "./storybook-taxonomy.mjs"; @@ -151,7 +152,7 @@ export function evaluateDiff(files, changeLog, repo) { checkCopyProvenance(live, files, repo, logged, add); checkCapabilityRemovals(live, files, repo, logged, add); checkRestoredRemovalsDeepening(live, files, repo, changeLog, add); - requiredTests.push(...routeSpecs(live, repo), ...gridParityTests(files, repo), ...sidebarOrderTests(files, repo)); + requiredTests.push(...routeSpecs(live, repo), ...gridParityTests(files, repo), ...sidebarOrderTests(files, repo), ...pathGateTests(live, (file) => repo.head(file))); return { ok: blocking.length === 0, blocking, requiredTests, requiredChecks }; } diff --git a/plugins/vibe/skills/handoff/scripts/diff-rules.test.mjs b/plugins/vibe/skills/handoff/scripts/diff-rules.test.mjs index 0137eaaa..57b5d39c 100644 --- a/plugins/vibe/skills/handoff/scripts/diff-rules.test.mjs +++ b/plugins/vibe/skills/handoff/scripts/diff-rules.test.mjs @@ -409,3 +409,11 @@ test("a spec importing only the helper export that does not reach the changed en }); assert.deepEqual(result.requiredTests.map((entry) => entry.path), ["e2e/move.spec.ts"]); }); + +test("a repo-declared path gate becomes a required run when the diff touches its paths", (t) => { + const gates = JSON.stringify({ gates: [{ name: "catalog-imports", paths: ["apps/prototypes/**"], command: "node apps/prototypes/scripts/check-catalog-imports.mjs" }] }); + const touched = run(t, { base: { ".vibe/path-gates.json": gates }, head: { "apps/prototypes/src/page.tsx": "export const x = 1;\n" } }); + assert.ok(touched.requiredTests.some((entry) => entry.path === "node apps/prototypes/scripts/check-catalog-imports.mjs")); + const elsewhere = run(t, { base: { ".vibe/path-gates.json": gates }, head: { "apps/app/page.tsx": "export const x = 1;\n" } }); + assert.ok(!elsewhere.requiredTests.some((entry) => entry.path.includes("check-catalog-imports"))); +}); diff --git a/plugins/vibe/skills/handoff/scripts/path-gates.mjs b/plugins/vibe/skills/handoff/scripts/path-gates.mjs new file mode 100644 index 00000000..bb161c0c --- /dev/null +++ b/plugins/vibe/skills/handoff/scripts/path-gates.mjs @@ -0,0 +1,48 @@ +// Gates a repository requires when a diff touches given paths. Some gates +// read files by scanning them rather than by import, so neither graph test +// selection nor PR CI picks them; a prototype import the repo's catalog gate +// rejects merged that way and broke main. +// +// The repository declares them in `.vibe/path-gates.json`: +// {"gates": [{"name": "catalog-imports", "paths": ["apps/prototypes/**"], +// "command": "node apps/prototypes/scripts/check-catalog-imports.mjs"}]} +// A path is a prefix, or a glob with `*` (one segment) and `**` (any depth). +// Each gate a changed file matches becomes a required test whose path is the +// gate's command, so the evidence gate needs a passing run of that command on +// the published commit (`testRuns` entry with `path` set to the command). + +export const PATH_GATES_FILE = ".vibe/path-gates.json"; + +/** The gates' required-test entries for `files`; a missing or unreadable declaration requires none and says why. */ +export function pathGateTests(files, read) { + const text = read(PATH_GATES_FILE); + if (text === null || text === undefined) { + return []; + } + let gates; + try { + gates = JSON.parse(text).gates; + } catch (error) { + return [{ path: PATH_GATES_FILE, reason: `could not be parsed (${error instanceof Error ? error.message : String(error)}); fix it so its gates run` }]; + } + if (!Array.isArray(gates)) { + return [{ path: PATH_GATES_FILE, reason: "has no gates list; fix it so its gates run" }]; + } + return gates.flatMap((gate) => { + if (typeof gate?.command !== "string" || !Array.isArray(gate.paths)) { + return [{ path: PATH_GATES_FILE, reason: `gate ${JSON.stringify(gate?.name ?? null)} needs a command and a paths list` }]; + } + const matchers = gate.paths.map(pathMatcher); + const hit = files.map((file) => file.path).find((file) => matchers.some((matches) => matches(file))); + return hit ? [{ path: gate.command, reason: `${gate.name ?? "a path gate"} in ${PATH_GATES_FILE}: ${hit} changed; record a passing run of this command on the published commit` }] : []; + }); +} + +function pathMatcher(pattern) { + if (!pattern.includes("*")) { + return (file) => file === pattern || file.startsWith(pattern.endsWith("/") ? pattern : `${pattern}/`); + } + const source = pattern.split("**").map((part) => part.split("*").map((piece) => piece.replace(/[.+?^${}()|[\]\\]/g, "\\$&")).join("[^/]*")).join(".*"); + const regex = new RegExp(`^${source}$`); + return (file) => regex.test(file); +} diff --git a/plugins/vibe/skills/handoff/scripts/path-gates.test.mjs b/plugins/vibe/skills/handoff/scripts/path-gates.test.mjs new file mode 100644 index 00000000..76d09cae --- /dev/null +++ b/plugins/vibe/skills/handoff/scripts/path-gates.test.mjs @@ -0,0 +1,34 @@ +import assert from 'node:assert/strict'; +import { test } from 'node:test'; +import { PATH_GATES_FILE, pathGateTests } from './path-gates.mjs'; + +const declared = (gates) => (file) => (file === PATH_GATES_FILE ? JSON.stringify({ gates }) : null); +const CATALOG = { name: 'catalog-imports', paths: ['apps/prototypes/**'], command: 'node apps/prototypes/scripts/check-catalog-imports.mjs' }; + +test('a change under a declared path requires a passing run of the gate command', () => { + const required = pathGateTests([{ path: 'apps/prototypes/src/branches/page.tsx' }], declared([CATALOG])); + assert.deepEqual(required.map((entry) => entry.path), [CATALOG.command]); + assert.match(required[0].reason, /catalog-imports in \.vibe\/path-gates\.json: apps\/prototypes\/src\/branches\/page\.tsx changed/); +}); + +test('a change outside every declared path, or a repo with no declaration, requires nothing', () => { + assert.deepEqual(pathGateTests([{ path: 'apps/app/page.tsx' }], declared([CATALOG])), []); + assert.deepEqual(pathGateTests([{ path: 'apps/prototypes/x.tsx' }], () => null), []); +}); + +test('prefixes and single-segment globs match only what they name', () => { + const gates = [ + { name: 'prefix', paths: ['packages/db'], command: 'db-gate' }, + { name: 'one', paths: ['e2e/*.spec.ts'], command: 'spec-gate' }, + ]; + assert.deepEqual(pathGateTests([{ path: 'packages/db/schema.sql' }], declared(gates)).map((entry) => entry.path), ['db-gate']); + assert.deepEqual(pathGateTests([{ path: 'packages/dbx/a.ts' }], declared(gates)), []); + assert.deepEqual(pathGateTests([{ path: 'e2e/a.spec.ts' }], declared(gates)).map((entry) => entry.path), ['spec-gate']); + assert.deepEqual(pathGateTests([{ path: 'e2e/nested/a.spec.ts' }], declared(gates)), []); +}); + +test('a broken declaration is itself a required fix, never silently ignored', () => { + assert.match(pathGateTests([{ path: 'a' }], () => '{nope')[0].reason, /could not be parsed/); + assert.match(pathGateTests([{ path: 'a' }], () => '{}')[0].reason, /no gates list/); + assert.match(pathGateTests([{ path: 'a' }], declared([{ name: 'x', paths: ['a'] }]))[0].reason, /needs a command/); +}); diff --git a/plugins/vibe/skills/vibe/SKILL.md b/plugins/vibe/skills/vibe/SKILL.md index 1ef968ff..a0fd658c 100644 --- a/plugins/vibe/skills/vibe/SKILL.md +++ b/plugins/vibe/skills/vibe/SKILL.md @@ -885,7 +885,10 @@ that waited, and one deploy after the last of them carries them all. A The subject stays under 72 characters and never mentions AI tools. The script leaves out the session's `localFixes` and files that never belong in a commit, and the repository's commit hook runs. `committed: false` - means nothing changed; continue. `"ok":false` means the hook refused: + means nothing changed; continue. A commit reporting `pushDue: true` + (`scripts/push-cadence.mjs`) has work that waited 90 minutes unpushed: + publish this tested checkpoint now, send later review findings as + follow-up commits, and add no new scope to an unpushed checkpoint. `"ok":false` means the hook refused: resume the SAME persistent writer in fix mode with the error, then commit again. An existing test or fixture the change broke is repaired there (`references/guardrails.md`, "Repair, don't author"). When the writer diff --git a/plugins/vibe/skills/vibe/references/environment.md b/plugins/vibe/skills/vibe/references/environment.md index 7d05bcae..42269e1f 100644 --- a/plugins/vibe/skills/vibe/references/environment.md +++ b/plugins/vibe/skills/vibe/references/environment.md @@ -369,7 +369,9 @@ not silently included or discarded. A clean result needs no extra local commit. worker/lease; replay or another helper turn is not the same continuation. 6. Create/redeploy uses `vibe-sessions.mjs main-sync-push`, which pushes the explicit validated SHA through normal hooks, then `dispatch-inputs` with - that context. Prototype share uses `main-sync-share` admission plus the + that context. It refuses while a migration the branch adds sorts before + main's newest one in the same folder (`scripts/migration-order.mjs`): the + SAME writer renames it to a later timestamp, contents unchanged. Prototype share uses `main-sync-share` admission plus the same exact-SHA push gate, then only canonical readiness/immutable URL inspection. No helper or source writer commits. Failures stop publication; the root resumes the same writer, not a replacement author. diff --git a/plugins/vibe/skills/vibe/references/quality-loop.md b/plugins/vibe/skills/vibe/references/quality-loop.md index bd77b8f4..fa26ffbd 100644 --- a/plugins/vibe/skills/vibe/references/quality-loop.md +++ b/plugins/vibe/skills/vibe/references/quality-loop.md @@ -90,6 +90,10 @@ recorded, what was observed directly stays separate from help center or forum material, on-screen wording is quoted exactly as evidence, and a situation nothing settles is reported as `undocumented`, never guessed. A failed run, or an `undocumented` answer from every app, leaves the question to the person. +Before an answer is recorded, `node scripts/precedent-check.mjs check +--report <report>` must pass: it refuses an answer from public material or +`undocumented` unless `--no-browser <reason>` says why no browser could look +(precedent mode's public-material path passes that reason). The answer goes to the SAME writer, which records it in the session decision table as the decision for the affected rows, with its source: the ticket or diff --git a/plugins/vibe/skills/vibe/scripts/commit-worktree.mjs b/plugins/vibe/skills/vibe/scripts/commit-worktree.mjs index 203c72c3..6936839e 100644 --- a/plugins/vibe/skills/vibe/scripts/commit-worktree.mjs +++ b/plugins/vibe/skills/vibe/scripts/commit-worktree.mjs @@ -14,6 +14,7 @@ // Hooks always run; a refused commit commits nothing and reports the end of // the hook output. Never amends, squashes, or skips hooks. // +// A commit also reports `pushDue` and `unpushedMinutes` (`push-cadence.mjs`). // Prints one JSON object on stdout. Errors print {"ok":false,"error":...} and // exit non-zero. @@ -22,6 +23,7 @@ import { writeFileSync } from "node:fs"; import path from "node:path"; import { parseArgs } from "node:util"; import { committedLocalPlans, isLocalPlanPath } from "./local-plans.mjs"; +import { unpushedAge } from "./push-cadence.mjs"; import { git, readSessionRecord } from "./session-record.mjs"; import { mainSyncFailure, prepareMainSync, readMainSyncInput } from "./dist/main-sync.mjs"; @@ -94,7 +96,17 @@ function commitWorktree() { `The commit was refused (${reason}); nothing was committed. Last lines:\n${output.slice(-HOOK_OUTPUT_LINES).join("\n")}` ); } - return { committed: true, commit: git(worktree, ["rev-parse", "HEAD"]), subject, merge: merging, files, excluded }; + return { committed: true, commit: git(worktree, ["rev-parse", "HEAD"]), subject, merge: merging, files, excluded, ...pushCadence(worktree, [...excluded, ...localFixes]) }; +} + +// A tested checkpoint is pushed at least every 90 minutes (`push-cadence.mjs`); the commit never fails on it. +function pushCadence(worktree, ignore) { + try { + const { minutes, due } = unpushedAge(worktree, { ignore }); + return { pushDue: due, unpushedMinutes: minutes }; + } catch { + return {}; + } } function requireSubject(subject) { diff --git a/plugins/vibe/skills/vibe/scripts/commit-worktree.test.mjs b/plugins/vibe/skills/vibe/scripts/commit-worktree.test.mjs index e2a2582d..355c4fb8 100644 --- a/plugins/vibe/skills/vibe/scripts/commit-worktree.test.mjs +++ b/plugins/vibe/skills/vibe/scripts/commit-worktree.test.mjs @@ -58,6 +58,8 @@ test("commits the session's changes but never its local fixes or files that neve assert.equal(git(checkout, ["log", "-1", "--format=%s"], home), "ISS-7: Add the new panel"); assert.equal(git(checkout, ["log", "-1", "--format=%b"], home), "Screens: projects"); assert.equal(result.json.commit, git(checkout, ["rev-parse", "HEAD"], home)); + assert.equal(result.json.pushDue, false, "a commit made just now is not due, and excluded files left behind do not age it"); + assert.equal(typeof result.json.unpushedMinutes, "number"); const leftOver = git(checkout, ["status", "--porcelain", "--untracked-files=all"], home).split("\n").map((line) => line.slice(3)); assert.deepEqual(leftOver.sort(), [".control/run.json", ".env.local", "apps/app/.env", "scripts/fix.sh", "tools/patched/a.ts"]); }); diff --git a/plugins/vibe/skills/vibe/scripts/e2e-lock.mjs b/plugins/vibe/skills/vibe/scripts/e2e-lock.mjs index a30ae9e8..2f121858 100644 --- a/plugins/vibe/skills/vibe/scripts/e2e-lock.mjs +++ b/plugins/vibe/skills/vibe/scripts/e2e-lock.mjs @@ -7,7 +7,7 @@ // stack to be free, runs the command, and releases the lock when the command // ends. A lock left by a process that no longer exists is taken over. // -// Usage: e2e-lock.mjs run [--wait-seconds <n>] [--lock-dir <dir>] <command> [args...] +// Usage: e2e-lock.mjs run [--wait-seconds <n>] [--lock-dir <dir>] [--foreign-compose <project>]... <command> [args...] // e2e-lock.mjs status [--lock-dir <dir>] // // The lock's own options come first; the first other word starts the command, @@ -18,12 +18,21 @@ // and exits 76, so a session batches its selected specs into one run instead // of queueing one entry per spec. // +// A run overlapped a stack another tool started on the same fixed compose +// project and ports, and its teardown removed that live stack. So `run` gives +// the command its own compose project (`COMPOSE_PROJECT_NAME=vibe-e2e-<id>`, +// unless the caller already set one), and a teardown inside it removes only +// the project it created. Each `--foreign-compose <project>` (or the +// comma-separated `VIBE_E2E_FOREIGN_COMPOSE`) names a stack started outside +// this lock: while `docker compose ls` shows it running, the stack counts as +// busy. Without Docker, or when the listing fails, nothing is checked. +// // `run` exits with the command's own code. When the stack stays busy past // `--wait-seconds` (default 3600) it runs nothing, prints // {"status":"busy","holder":{...}} and exits 75. `status` prints // {"status":"free"} or {"status":"held","holder":{...}}. -import { execFileSync, spawn } from "node:child_process"; +import { execFileSync, spawn, spawnSync } from "node:child_process"; import { randomUUID } from "node:crypto"; import { mkdirSync, readdirSync, readFileSync, rmSync, statSync, writeFileSync } from "node:fs"; import os from "node:os"; @@ -36,6 +45,7 @@ const DEFAULT_WAIT_SECONDS = 3600; const POLL_MS = 5_000; const OWNER_FILE = "owner.json"; const OWNERLESS_GRACE_MS = 60_000; +const COMPOSE_PREFIX = "vibe-e2e-"; /** The machine-wide lock folder, shared by every worktree and both harnesses. */ export function defaultLockDir(home = os.homedir()) { @@ -155,27 +165,59 @@ function isAlive(pid) { } } -const OWN_OPTIONS = new Set(["--wait-seconds", "--lock-dir"]); +const OWN_OPTIONS = new Set(["--wait-seconds", "--lock-dir", "--foreign-compose"]); +const REPEATED_OPTIONS = new Set(["--foreign-compose"]); + +/** The first of `projects` that `docker compose ls` lists as running, or null; no Docker means none. */ +export function runningForeignCompose(projects, docker = process.env.VIBE_E2E_DOCKER || "docker") { + if (projects.length === 0) { + return null; + } + const listed = spawnSync(docker, ["compose", "ls", "--format", "json"], { encoding: "utf8", timeout: 30_000 }); + if (listed.error || listed.status !== 0) { + return null; + } + let stacks; + try { + stacks = JSON.parse(listed.stdout || "[]"); + } catch { + return null; + } + const running = new Set((Array.isArray(stacks) ? stacks : []) + .filter((stack) => /running/i.test(String(stack?.Status ?? ""))) + .map((stack) => stack.Name)); + return projects.find((project) => running.has(project)) ?? null; +} + +/** The compose project a run's command gets: the caller's own, or one named for this run. */ +export function composeProjectFor(env, token) { + return env.COMPOSE_PROJECT_NAME || `${COMPOSE_PREFIX}${token.replace(/-/g, "").slice(0, 8)}`; +} // The action, then the lock's own `--option value` pairs, then the command and its arguments untouched. function parseCommandLine(argv) { const [action, ...rest] = argv; - const values = {}; + const values = { "foreign-compose": [] }; let index = 0; while (OWN_OPTIONS.has(rest[index])) { if (rest[index + 1] === undefined) { throw new Error(`${rest[index]} needs a value.`); } - values[rest[index].slice(2)] = rest[index + 1]; + const key = rest[index].slice(2); + if (REPEATED_OPTIONS.has(rest[index])) { + values[key].push(rest[index + 1]); + } else { + values[key] = rest[index + 1]; + } index += 2; } const [command, ...args] = rest.slice(index); return { action, values, command, args }; } -function runCommand(command, args) { +function runCommand(command, args, env) { return new Promise((resolve, reject) => { - const child = spawn(command, args, { stdio: "inherit" }); + const child = spawn(command, args, { stdio: "inherit", env }); const forward = (signal) => child.kill(signal); process.on("SIGINT", forward); process.on("SIGTERM", forward); @@ -198,7 +240,7 @@ async function main(argv) { return 0; } if (action !== "run" || !command) { - throw new Error("Usage: e2e-lock.mjs run [--wait-seconds <n>] [--lock-dir <dir>] <command> [args...]"); + throw new Error("Usage: e2e-lock.mjs run [--wait-seconds <n>] [--lock-dir <dir>] [--foreign-compose <project>]... <command> [args...]"); } const waitSeconds = values["wait-seconds"] === undefined ? DEFAULT_WAIT_SECONDS : Number(values["wait-seconds"]); if (!Number.isFinite(waitSeconds) || waitSeconds < 0) { @@ -217,12 +259,30 @@ async function main(argv) { process.stdout.write(`${JSON.stringify({ status: "duplicate", entry: duplicate, reason: "this worktree already holds or waits for the stack; put every selected spec in one run per head" })}\n`); return DUPLICATE_EXIT; } + const foreign = [...values["foreign-compose"], ...(process.env.VIBE_E2E_FOREIGN_COMPOSE ?? "").split(",")] + .map((name) => name.trim()).filter(Boolean); const waiter = path.join(waitersDir(lockDir), `${owner.token}.json`); mkdirSync(waitersDir(lockDir), { recursive: true }); writeFileSync(waiter, JSON.stringify(owner), { mode: 0o600 }); + const deadline = Date.now() + waitSeconds * 1000; let attempt; try { - attempt = await acquire(lockDir, owner, { waitSeconds }); + for (;;) { + attempt = await acquire(lockDir, owner, { waitSeconds: Math.max(0, (deadline - Date.now()) / 1000) }); + if (!attempt.acquired) { + break; + } + const busyProject = runningForeignCompose(foreign); + if (!busyProject) { + break; + } + release(lockDir, owner); + attempt = { acquired: false, holder: { composeProject: busyProject } }; + if (Date.now() >= deadline) { + break; + } + await new Promise((resolve) => setTimeout(resolve, Math.min(POLL_MS, Math.max(1, deadline - Date.now())))); + } } finally { rmSync(waiter, { force: true }); } @@ -230,8 +290,10 @@ async function main(argv) { process.stdout.write(`${JSON.stringify({ status: "busy", holder: attempt.holder })}\n`); return BUSY_EXIT; } + const composeProject = composeProjectFor(process.env, owner.token); + process.stderr.write(`e2e-lock: compose project ${composeProject}\n`); try { - return await runCommand(command, args); + return await runCommand(command, args, { ...process.env, COMPOSE_PROJECT_NAME: composeProject }); } finally { release(lockDir, owner); } diff --git a/plugins/vibe/skills/vibe/scripts/e2e-lock.test.mjs b/plugins/vibe/skills/vibe/scripts/e2e-lock.test.mjs index 07fbee66..b6b74497 100644 --- a/plugins/vibe/skills/vibe/scripts/e2e-lock.test.mjs +++ b/plugins/vibe/skills/vibe/scripts/e2e-lock.test.mjs @@ -1,11 +1,11 @@ import assert from "node:assert/strict"; import { spawn, spawnSync } from "node:child_process"; -import { existsSync, mkdirSync, mkdtempSync, readFileSync, realpathSync, rmSync, utimesSync, writeFileSync } from "node:fs"; +import { chmodSync, existsSync, mkdirSync, mkdtempSync, readFileSync, realpathSync, rmSync, utimesSync, writeFileSync } from "node:fs"; import os from "node:os"; import path from "node:path"; import { after, test } from "node:test"; import { fileURLToPath } from "node:url"; -import { BUSY_EXIT, DUPLICATE_EXIT, defaultLockDir, readHolder, release, sameWorktreeEntry, tryAcquire } from "./e2e-lock.mjs"; +import { BUSY_EXIT, DUPLICATE_EXIT, composeProjectFor, defaultLockDir, readHolder, release, runningForeignCompose, sameWorktreeEntry, tryAcquire } from "./e2e-lock.mjs"; const SCRIPT = path.join(path.dirname(fileURLToPath(import.meta.url)), "e2e-lock.mjs"); const root = mkdtempSync(path.join(os.tmpdir(), "vibe-e2e-lock-")); @@ -117,6 +117,55 @@ test("a missing command is refused with usage", () => { assert.match(result.stderr, /Usage/); }); +test("each run gets its own compose project unless the caller set one", () => { + const lockDir = path.join(root, "compose-own"); + const print = [process.execPath, "-e", "process.stdout.write(process.env.COMPOSE_PROJECT_NAME)"]; + const env = { ...process.env }; + Reflect.deleteProperty(env, "COMPOSE_PROJECT_NAME"); + Reflect.deleteProperty(env, "VIBE_E2E_FOREIGN_COMPOSE"); + const first = spawnSync(process.execPath, [SCRIPT, "run", "--lock-dir", lockDir, ...print], { encoding: "utf8", env }); + const second = spawnSync(process.execPath, [SCRIPT, "run", "--lock-dir", lockDir, ...print], { encoding: "utf8", env }); + assert.equal(first.status, 0, first.stderr); + assert.match(first.stdout, /^vibe-e2e-[0-9a-f]{8}$/); + assert.notEqual(first.stdout, second.stdout, "two runs never share a project, so one teardown cannot remove the other's stack"); + assert.match(first.stderr, new RegExp(`compose project ${first.stdout}`)); + const kept = spawnSync(process.execPath, [SCRIPT, "run", "--lock-dir", lockDir, ...print], { encoding: "utf8", env: { ...env, COMPOSE_PROJECT_NAME: "callers-own" } }); + assert.equal(kept.stdout, "callers-own"); + assert.equal(composeProjectFor({}, "0123abcd-ef45-6789"), "vibe-e2e-0123abcd"); +}); + +test("a stack started outside the lock on a named compose project keeps the stack busy; no Docker checks nothing", () => { + const lockDir = path.join(root, "compose-foreign"); + const listing = path.join(root, "compose-ls.json"); + const docker = path.join(root, "fake-docker"); + writeFileSync(docker, `#!/bin/sh\n[ "$1 $2 $3 $4" = "compose ls --format json" ] || exit 9\ncat "${listing}"\n`); + chmodSync(docker, 0o755); + writeFileSync(listing, JSON.stringify([{ Name: "fixed-e2e", Status: "running(5)" }, { Name: "other", Status: "exited(1)" }])); + assert.equal(runningForeignCompose(["other", "fixed-e2e"], docker), "fixed-e2e"); + assert.equal(runningForeignCompose(["other"], docker), null, "an exited project is not a running stack"); + assert.equal(runningForeignCompose(["fixed-e2e"], path.join(root, "no-docker")), null); + const marker = path.join(root, "foreign-ran"); + const env = { ...process.env, VIBE_E2E_DOCKER: docker }; + Reflect.deleteProperty(env, "VIBE_E2E_FOREIGN_COMPOSE"); + const command = [process.execPath, "-e", `require("node:fs").writeFileSync(${JSON.stringify(marker)}, "ran")`]; + const busy = spawnSync(process.execPath, [SCRIPT, "run", "--wait-seconds", "0", "--lock-dir", lockDir, "--foreign-compose", "fixed-e2e", ...command], { encoding: "utf8", env }); + assert.equal(busy.status, BUSY_EXIT, busy.stderr); + assert.deepEqual(JSON.parse(busy.stdout).holder, { composeProject: "fixed-e2e" }); + assert.equal(existsSync(marker), false); + assert.equal(existsSync(lockDir), false, "the lock is released while the foreign stack runs"); + const fromEnv = spawnSync(process.execPath, [SCRIPT, "run", "--wait-seconds", "0", "--lock-dir", lockDir, ...command], { encoding: "utf8", env: { ...env, VIBE_E2E_FOREIGN_COMPOSE: "x, fixed-e2e" } }); + assert.equal(fromEnv.status, BUSY_EXIT); + writeFileSync(listing, "[]"); + const free = spawnSync(process.execPath, [SCRIPT, "run", "--wait-seconds", "0", "--lock-dir", lockDir, "--foreign-compose", "fixed-e2e", ...command], { encoding: "utf8", env }); + assert.equal(free.status, 0, free.stderr); + assert.equal(existsSync(marker), true); + rmSync(marker); + const noDocker = spawnSync(process.execPath, [SCRIPT, "run", "--wait-seconds", "0", "--lock-dir", lockDir, "--foreign-compose", "fixed-e2e", ...command], + { encoding: "utf8", env: { ...env, VIBE_E2E_DOCKER: path.join(root, "no-docker") } }); + assert.equal(noDocker.status, 0, noDocker.stderr); + assert.equal(existsSync(marker), true); +}); + async function waitFor(predicate) { for (let attempt = 0; attempt < 200; attempt += 1) { if (predicate()) { diff --git a/plugins/vibe/skills/vibe/scripts/migration-order.mjs b/plugins/vibe/skills/vibe/scripts/migration-order.mjs new file mode 100644 index 00000000..c3be9d5f --- /dev/null +++ b/plugins/vibe/skills/vibe/scripts/migration-order.mjs @@ -0,0 +1,88 @@ +#!/usr/bin/env node +// Every migration a branch adds must sort after the newest migration main +// already has in the same folder, checked before every push. A branch +// migration minted before a newer one landed on main ran out of order after +// the merge and failed the repo's smoke and rebuild checks. The fix is a +// rename to a later timestamp with the migration's contents unchanged. +// +// A migration is a folder or file directly under any `migrations` folder whose +// name starts with a timestamp of at least 8 digits +// (`.../migrations/20261021000000_name/migration.sql`, `.../migrations/20261021_name.sql`). +// +// Usage: migration-order.mjs --worktree <path> [--main <ref, default origin/main>] +// Prints one JSON object {ok, problems}; exits 1 when a migration sorts too early. +// A missing main ref is not checked (nothing to compare against) and says so. + +import { execFileSync } from "node:child_process"; +import path from "node:path"; +import { pathToFileURL } from "node:url"; +import { parseArgs } from "node:util"; + +const MIGRATION = /^(.*?(?:^|\/)migrations)\/(\d{8,})[^/]*(?:\/|$)/; +const MAX_BUFFER = 64 * 1024 * 1024; + +/** Compares two digit strings as numbers of any length. */ +function compareStamps(a, b) { + return a.length === b.length ? a.localeCompare(b) : a.length - b.length; +} + +/** The migrations folder and timestamp of a path, or null when it is not a migration. */ +export function migrationOf(file) { + const match = MIGRATION.exec(file); + return match ? { dir: match[1], entry: file.slice(match[1].length + 1).split("/")[0], stamp: match[2] } : null; +} + +/** Problems with the migrations `worktree`'s branch adds, against `main`. */ +export function migrationOrderProblems(worktree, main = "origin/main") { + const git = (args) => execFileSync("git", ["-C", worktree, ...args], { encoding: "utf8", maxBuffer: MAX_BUFFER, stdio: ["ignore", "pipe", "ignore"] }).trim(); + try { + git(["rev-parse", "--verify", "--quiet", `${main}^{commit}`]); + } catch { + return { checked: false, problems: [], reason: `${main} is not available` }; + } + const base = git(["merge-base", "HEAD", main]); + const added = git(["diff", "--name-only", "--diff-filter=A", base, "HEAD"]).split("\n").filter(Boolean); + const branch = new Map(); + for (const migration of added.map(migrationOf).filter(Boolean)) { + branch.set(`${migration.dir}/${migration.entry}`, migration); + } + const newest = new Map(); + const problems = []; + for (const migration of branch.values()) { + if (!newest.has(migration.dir)) { + const entries = lsTreeSafe(worktree, main, migration.dir).split("\n").filter(Boolean); + newest.set(migration.dir, { entries: new Set(entries), stamp: entries.map((entry) => /^\d{8,}/.exec(entry)?.[0]).filter(Boolean).sort(compareStamps).at(-1) ?? null }); + } + const onMain = newest.get(migration.dir); + if (onMain.entries.has(migration.entry) || onMain.stamp === null) { + continue; + } + if (compareStamps(migration.stamp, onMain.stamp) <= 0) { + problems.push(`${migration.dir}/${migration.entry} sorts before or with main's newest ${onMain.stamp}; rename it to a timestamp after ${onMain.stamp}, contents unchanged`); + } + } + return { checked: true, problems }; +} + +function lsTreeSafe(worktree, main, dir) { + try { + return execFileSync("git", ["-C", worktree, "ls-tree", "--name-only", `${main}:${dir}`], { encoding: "utf8", stdio: ["ignore", "pipe", "ignore"] }); + } catch { + return ""; + } +} + +function main() { + const { values } = parseArgs({ options: { worktree: { type: "string" }, main: { type: "string" } } }); + if (!values.worktree) { + process.stdout.write(`${JSON.stringify({ ok: false, error: "--worktree is required." })}\n`); + process.exit(1); + } + const result = migrationOrderProblems(path.resolve(values.worktree), values.main); + process.stdout.write(`${JSON.stringify({ ok: result.problems.length === 0, ...result }, null, 2)}\n`); + process.exit(result.problems.length === 0 ? 0 : 1); +} + +if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { + main(); +} diff --git a/plugins/vibe/skills/vibe/scripts/migration-order.test.mjs b/plugins/vibe/skills/vibe/scripts/migration-order.test.mjs new file mode 100644 index 00000000..207fdf58 --- /dev/null +++ b/plugins/vibe/skills/vibe/scripts/migration-order.test.mjs @@ -0,0 +1,96 @@ +import assert from "node:assert/strict"; +import { spawnSync } from "node:child_process"; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import test from "node:test"; +import { fileURLToPath } from "node:url"; +import { migrationOf, migrationOrderProblems } from "./migration-order.mjs"; + +const SCRIPT = path.join(path.dirname(fileURLToPath(import.meta.url)), "migration-order.mjs"); +const DIR = "db/prisma/migrations"; + +function repo(t) { + const dir = mkdtempSync(path.join(os.tmpdir(), "vibe-migration-order-")); + t.after(() => rmSync(dir, { recursive: true, force: true })); + const git = (...args) => { + const result = spawnSync("git", ["-C", dir, "-c", "user.name=t", "-c", "user.email=t@t", ...args], { encoding: "utf8" }); + assert.equal(result.status, 0, result.stderr); + return result.stdout.trim(); + }; + const add = (name) => { + mkdirSync(path.join(dir, DIR, name), { recursive: true }); + writeFileSync(path.join(dir, DIR, name, "migration.sql"), `-- ${name}\n`); + }; + git("init", "--quiet", "-b", "main"); + add("20261001000000_first"); + git("add", "-A"); + git("commit", "--quiet", "-m", "first"); + return { dir, git, add }; +} + +test("a migration path is any timestamped entry directly under a migrations folder", () => { + assert.deepEqual(migrationOf("apps/x/prisma/migrations/20261021000000_a/migration.sql"), { dir: "apps/x/prisma/migrations", entry: "20261021000000_a", stamp: "20261021000000" }); + assert.deepEqual(migrationOf("migrations/20261021_a.sql"), { dir: "migrations", entry: "20261021_a.sql", stamp: "20261021" }); + assert.equal(migrationOf("db/migrations/README.md"), null); + assert.equal(migrationOf("src/migrations-helper/20261021000000.ts"), null); +}); + +test("a branch migration that sorts before main's newest blocks, and a later rename passes", (t) => { + const { dir, git, add } = repo(t); + git("checkout", "--quiet", "-b", "feature"); + add("20261010000000_branch"); + git("add", "-A"); + git("commit", "--quiet", "-m", "branch migration"); + git("checkout", "--quiet", "main"); + add("20261015000000_main"); + git("add", "-A"); + git("commit", "--quiet", "-m", "newer main migration"); + git("checkout", "--quiet", "feature"); + + const late = migrationOrderProblems(dir, "main"); + assert.equal(late.checked, true); + assert.equal(late.problems.length, 1); + assert.match(late.problems[0], /20261010000000_branch sorts before or with main's newest 20261015000000/); + const cli = spawnSync(process.execPath, [SCRIPT, "--worktree", dir, "--main", "main"], { encoding: "utf8" }); + assert.equal(cli.status, 1); + assert.equal(JSON.parse(cli.stdout).ok, false); + + git("mv", `${DIR}/20261010000000_branch`, `${DIR}/20261016000000_branch`); + git("commit", "--quiet", "-m", "re-mint"); + assert.deepEqual(migrationOrderProblems(dir, "main").problems, []); +}); + +test("main's own migrations, a folder main lacks and a missing main ref are not problems", (t) => { + const { dir, git, add } = repo(t); + git("checkout", "--quiet", "-b", "feature"); + mkdirSync(path.join(dir, "other", "migrations", "20200101000000_new"), { recursive: true }); + writeFileSync(path.join(dir, "other", "migrations", "20200101000000_new", "migration.sql"), "-- new\n"); + add("20261020000000_branch"); + git("add", "-A"); + git("commit", "--quiet", "-m", "branch"); + assert.deepEqual(migrationOrderProblems(dir, "main").problems, []); + const missing = migrationOrderProblems(dir, "origin/main"); + assert.equal(missing.checked, false); + assert.match(missing.reason, /not available/); +}); + +test("vibe-sessions main-sync-push refuses before pushing while a branch migration sorts too early", (t) => { + const { dir, git, add } = repo(t); + git("checkout", "--quiet", "-b", "feature"); + add("20261010000000_branch"); + git("add", "-A"); + git("commit", "--quiet", "-m", "branch migration"); + git("checkout", "--quiet", "main"); + add("20261015000000_main"); + git("add", "-A"); + git("commit", "--quiet", "-m", "newer main migration"); + git("update-ref", "refs/remotes/origin/main", "main"); + git("checkout", "--quiet", "feature"); + const sessions = path.join(path.dirname(SCRIPT), "vibe-sessions.mjs"); + const result = spawnSync(process.execPath, [sessions, "main-sync-push", "--worktree", dir], { encoding: "utf8", input: "" }); + assert.equal(result.status, 1); + const json = JSON.parse(result.stdout); + assert.equal(json.status, "NEEDS_CHANGE"); + assert.match(json.error, /Nothing was pushed: .*20261010000000_branch sorts before or with main's newest 20261015000000/); +}); diff --git a/plugins/vibe/skills/vibe/scripts/push-cadence.mjs b/plugins/vibe/skills/vibe/scripts/push-cadence.mjs new file mode 100644 index 00000000..4963f19f --- /dev/null +++ b/plugins/vibe/skills/vibe/scripts/push-cadence.mjs @@ -0,0 +1,123 @@ +#!/usr/bin/env node +// How long a worktree's work has waited unpushed. A writer kept a validated +// checkpoint unpushed for hours while each review pass added scope, so QA +// could not start; a tested checkpoint is pushed at least every 90 minutes of +// active work, and review findings after that go out as follow-up commits. +// +// Usage: push-cadence.mjs --worktree <path> [--limit-minutes <n, default 90>] [--now <iso>] +// +// The age is measured from the oldest work not on the remote: the committer +// time of the oldest commit missing from the upstream (or from origin/<branch>, +// or, with no remote branch at all, from every remote ref), and the oldest +// modification time of a changed or untracked file. Prints one JSON object +// {"minutes","due","since","reason"}; exits 1 when the push is due and 2 on an +// error. Changes nothing. + +import { execFileSync } from "node:child_process"; +import { statSync } from "node:fs"; +import path from "node:path"; +import { pathToFileURL } from "node:url"; +import { parseArgs } from "node:util"; + +export const DEFAULT_LIMIT_MINUTES = 90; + +/** Minutes since the oldest unpushed work in `worktree`, and whether a push is due. */ +export function unpushedAge(worktree, { now = Date.now(), limitMinutes = DEFAULT_LIMIT_MINUTES, ignore = [] } = {}) { + const commit = oldestUnpushedCommit(worktree); + const file = oldestChangedFile(worktree, ignore); + const candidates = [commit, file].filter(Boolean).sort((a, b) => a.at - b.at); + if (candidates.length === 0) { + return { minutes: 0, due: false, since: null, reason: "nothing unpushed" }; + } + const oldest = candidates[0]; + const minutes = Math.max(0, Math.floor((now - oldest.at) / 60_000)); + return { minutes, due: minutes >= limitMinutes, since: new Date(oldest.at).toISOString(), reason: oldest.reason }; +} + +function git(worktree, args) { + return execFileSync("git", ["-C", worktree, ...args], { encoding: "utf8", stdio: ["ignore", "pipe", "ignore"], maxBuffer: 64 * 1024 * 1024 }).trim(); +} + +function tryGit(worktree, args) { + try { + return git(worktree, args); + } catch { + return null; + } +} + +/** The unpushed commit range: upstream, then origin/<branch>, then every remote ref. */ +function unpushedRange(worktree) { + if (tryGit(worktree, ["rev-parse", "--abbrev-ref", "--symbolic-full-name", "@{upstream}"])) { + return ["@{upstream}..HEAD"]; + } + const branch = tryGit(worktree, ["symbolic-ref", "--short", "-q", "HEAD"]); + if (branch && tryGit(worktree, ["rev-parse", "--verify", "-q", `refs/remotes/origin/${branch}`])) { + return [`refs/remotes/origin/${branch}..HEAD`]; + } + return ["HEAD", "--not", "--remotes"]; +} + +function oldestUnpushedCommit(worktree) { + const times = (tryGit(worktree, ["log", "--format=%ct", ...unpushedRange(worktree)]) ?? "") + .split("\n").filter(Boolean).map(Number).filter(Number.isFinite); + if (times.length === 0) { + return null; + } + return { at: Math.min(...times) * 1000, reason: `${times.length} unpushed commit(s)` }; +} + +function oldestChangedFile(worktree, ignore) { + const entries = git(worktree, ["status", "--porcelain", "-z", "--untracked-files=all"]).split("\0").filter(Boolean); + let oldest = null; + let count = 0; + for (let index = 0; index < entries.length; index += 1) { + const entry = entries[index]; + const status = entry.slice(0, 2); + const file = entry.slice(3); + if (status.startsWith("R") || status.startsWith("C")) { + index += 1; + } + if (status.includes("D") || ignore.some((skip) => file === skip || file.startsWith(`${skip}/`))) { + continue; + } + let mtime; + try { + mtime = statSync(path.join(worktree, file)).mtimeMs; + } catch { + continue; + } + count += 1; + if (!oldest || mtime < oldest) { + oldest = mtime; + } + } + return oldest === null ? null : { at: oldest, reason: `${count} uncommitted file(s)` }; +} + +function main() { + const { values } = parseArgs({ options: { worktree: { type: "string" }, "limit-minutes": { type: "string" }, now: { type: "string" } } }); + if (!values.worktree) { + throw new Error("--worktree is required."); + } + const limit = values["limit-minutes"] === undefined ? DEFAULT_LIMIT_MINUTES : Number(values["limit-minutes"]); + if (!Number.isFinite(limit) || limit <= 0) { + throw new Error("--limit-minutes must be a positive number."); + } + const now = values.now ? Date.parse(values.now) : Date.now(); + if (!Number.isFinite(now)) { + throw new Error("--now must be an ISO time."); + } + const result = unpushedAge(path.resolve(values.worktree), { now, limitMinutes: limit }); + process.stdout.write(`${JSON.stringify(result)}\n`); + return result.due ? 1 : 0; +} + +if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { + try { + process.exit(main()); + } catch (error) { + process.stdout.write(`${JSON.stringify({ ok: false, error: error instanceof Error ? error.message : String(error) })}\n`); + process.exit(2); + } +} diff --git a/plugins/vibe/skills/vibe/scripts/push-cadence.test.mjs b/plugins/vibe/skills/vibe/scripts/push-cadence.test.mjs new file mode 100644 index 00000000..e69a0544 --- /dev/null +++ b/plugins/vibe/skills/vibe/scripts/push-cadence.test.mjs @@ -0,0 +1,93 @@ +import assert from "node:assert/strict"; +import { execFileSync, spawnSync } from "node:child_process"; +import { mkdtempSync, realpathSync, rmSync, utimesSync, writeFileSync } from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import test from "node:test"; +import { fileURLToPath } from "node:url"; +import { unpushedAge } from "./push-cadence.mjs"; + +const SCRIPT = path.join(path.dirname(fileURLToPath(import.meta.url)), "push-cadence.mjs"); +const T0 = Date.parse("2026-10-21T10:00:00Z"); + +function repos(t) { + const root = realpathSync(mkdtempSync(path.join(os.tmpdir(), "push-cadence-"))); + t.after(() => rmSync(root, { recursive: true, force: true })); + const origin = path.join(root, "origin.git"); + const work = path.join(root, "work"); + const git = (cwd, args, when = T0) => { + const date = new Date(when).toISOString(); + return execFileSync("git", args, { + cwd, + encoding: "utf8", + env: { ...process.env, GIT_CONFIG_NOSYSTEM: "1", GIT_AUTHOR_NAME: "t", GIT_AUTHOR_EMAIL: "t@t", GIT_COMMITTER_NAME: "t", GIT_COMMITTER_EMAIL: "t@t", GIT_AUTHOR_DATE: date, GIT_COMMITTER_DATE: date }, + }).trim(); + }; + git(root, ["init", "--quiet", "--bare", origin]); + git(root, ["init", "--quiet", "-b", "feature", work]); + writeFileSync(path.join(work, "a.txt"), "1\n"); + git(work, ["add", "-A"]); + git(work, ["commit", "--quiet", "-m", "base"]); + git(work, ["remote", "add", "origin", origin]); + git(work, ["push", "--quiet", "-u", "origin", "feature"]); + const commit = (file, when) => { + writeFileSync(path.join(work, file), `${when}\n`); + git(work, ["add", "-A"]); + git(work, ["commit", "--quiet", "-m", file], when); + }; + return { work, git, commit }; +} + +const minutes = (n) => T0 + n * 60_000; + +test("nothing unpushed is never due", (t) => { + const { work } = repos(t); + assert.deepEqual(unpushedAge(work, { now: minutes(500) }), { minutes: 0, due: false, since: null, reason: "nothing unpushed" }); +}); + +test("the oldest unpushed commit sets the age, and 90 minutes makes the push due", (t) => { + const { work, commit } = repos(t); + commit("b.txt", minutes(10)); + commit("c.txt", minutes(60)); + const early = unpushedAge(work, { now: minutes(80) }); + assert.equal(early.minutes, 70); + assert.equal(early.due, false); + assert.match(early.reason, /2 unpushed commit/); + const late = unpushedAge(work, { now: minutes(100) }); + assert.equal(late.minutes, 90); + assert.equal(late.due, true); + assert.equal(late.since, new Date(minutes(10)).toISOString()); + assert.equal(unpushedAge(work, { now: minutes(100), limitMinutes: 120 }).due, false); +}); + +test("an uncommitted file counts by its modification time, and ignored paths do not", (t) => { + const { work } = repos(t); + writeFileSync(path.join(work, "draft.txt"), "x\n"); + utimesSync(path.join(work, "draft.txt"), new Date(minutes(5)), new Date(minutes(5))); + const result = unpushedAge(work, { now: minutes(100) }); + assert.equal(result.minutes, 95); + assert.equal(result.due, true); + assert.match(result.reason, /1 uncommitted file/); + assert.equal(unpushedAge(work, { now: minutes(100), ignore: ["draft.txt"] }).due, false); +}); + +test("with no upstream, commits missing from origin/<branch> or every remote ref count", (t) => { + const { work, git, commit } = repos(t); + git(work, ["branch", "--quiet", "--unset-upstream"]); + commit("b.txt", minutes(20)); + assert.equal(unpushedAge(work, { now: minutes(30) }).minutes, 10); + git(work, ["checkout", "--quiet", "-b", "never-pushed"]); + commit("c.txt", minutes(25)); + assert.equal(unpushedAge(work, { now: minutes(30) }).minutes, 10, "b.txt is on no remote ref either"); +}); + +test("the command prints the age and exits 1 when the push is due", (t) => { + const { work, commit } = repos(t); + commit("b.txt", minutes(0)); + const run = (now) => spawnSync(process.execPath, [SCRIPT, "--worktree", work, "--now", new Date(now).toISOString()], { encoding: "utf8" }); + const due = run(minutes(91)); + assert.equal(due.status, 1); + assert.equal(JSON.parse(due.stdout).due, true); + assert.equal(run(minutes(30)).status, 0); + assert.equal(spawnSync(process.execPath, [SCRIPT], { encoding: "utf8" }).status, 2); +}); diff --git a/plugins/vibe/skills/vibe/scripts/shepherd.mjs b/plugins/vibe/skills/vibe/scripts/shepherd.mjs new file mode 100644 index 00000000..9bf8b875 --- /dev/null +++ b/plugins/vibe/skills/vibe/scripts/shepherd.mjs @@ -0,0 +1,83 @@ +#!/usr/bin/env node +// The writer shepherds its own pull request in the final stage (after QA, +// once final-merge mode is set): it polls the pull request every two minutes +// and stops at the first thing it must act on, so a conflict with main is +// merged at once instead of waiting for someone to notice it. A pull request +// that went conflicting once sat for hours with nobody watching it. +// +// Usage: shepherd.mjs --pr <number> --repo <owner/repo> [--interval-seconds 120] [--max-polls <n>] +// +// Prints one JSON object when it stops: +// {"event":"merged"} exit 0 +// {"event":"closed"} exit 2 +// {"event":"waiting","polls":n} exit 3 (--max-polls reached) +// {"event":"conflict","head":sha} exit 10 (merge main now in final-merge mode, push, run shepherd again) +// {"event":"checks-failed","failed":[...]} exit 11 (fix on the branch, push, run shepherd again) +// {"event":"unreadable","error":...} exit 12 (three reads in a row failed) +// `gh` is read from PATH; a single failed read is retried on the next poll. + +import { execFileSync } from "node:child_process"; +import { pathToFileURL } from "node:url"; +import { parseArgs } from "node:util"; + +const FIELDS = "state,mergeable,headRefOid,statusCheckRollup"; +const FAILED = new Set(["FAILURE", "CANCELLED", "TIMED_OUT", "ACTION_REQUIRED", "ERROR", "STARTUP_FAILURE"]); +const MAX_READ_FAILURES = 3; +export const EXIT = { merged: 0, closed: 2, waiting: 3, conflict: 10, "checks-failed": 11, unreadable: 12 }; + +/** What one pull request read asks the writer to do, or null to keep waiting. */ +export function shepherdEvent(pr) { + if (pr.state === "MERGED") return { event: "merged" }; + if (pr.state === "CLOSED") return { event: "closed" }; + if (pr.mergeable === "CONFLICTING") return { event: "conflict", head: pr.headRefOid }; + const failed = (pr.statusCheckRollup ?? []) + .filter((check) => FAILED.has(check.conclusion ?? check.state)) + .map((check) => check.name ?? check.context); + return failed.length > 0 ? { event: "checks-failed", head: pr.headRefOid, failed } : null; +} + +/** Polls until an event; resolves with it. */ +export async function shepherd({ pr, repo, intervalSeconds = 120, maxPolls = Infinity, read = readPr }) { + let failures = 0; + for (let polls = 1; ; polls++) { + try { + const event = shepherdEvent(read(pr, repo)); + failures = 0; + if (event) return event; + } catch (error) { + failures++; + if (failures >= MAX_READ_FAILURES) { + return { event: "unreadable", error: String(error?.message ?? error).split("\n")[0] }; + } + } + if (polls >= maxPolls) return { event: "waiting", polls }; + await new Promise((resolve) => setTimeout(resolve, intervalSeconds * 1000)); + } +} + +function readPr(pr, repo) { + return JSON.parse(execFileSync("gh", ["pr", "view", String(pr), "--repo", repo, "--json", FIELDS], { encoding: "utf8", stdio: ["ignore", "pipe", "pipe"] })); +} + +async function main() { + const { values } = parseArgs({ + options: { pr: { type: "string" }, repo: { type: "string" }, "interval-seconds": { type: "string" }, "max-polls": { type: "string" } }, + }); + if (!/^\d+$/.test(values.pr ?? "") || !/^[\w.-]+\/[\w.-]+$/.test(values.repo ?? "")) { + process.stdout.write(`${JSON.stringify({ event: "usage", error: "--pr <number> and --repo <owner/repo> are required" })}\n`); + process.exit(64); + } + const interval = Number(values["interval-seconds"] ?? 120); + const result = await shepherd({ + pr: Number(values.pr), + repo: values.repo, + intervalSeconds: Number.isFinite(interval) && interval >= 0 ? interval : 120, + maxPolls: values["max-polls"] ? Math.max(1, Number(values["max-polls"])) : Infinity, + }); + process.stdout.write(`${JSON.stringify(result)}\n`); + process.exit(EXIT[result.event]); +} + +if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { + await main(); +} diff --git a/plugins/vibe/skills/vibe/scripts/shepherd.test.mjs b/plugins/vibe/skills/vibe/scripts/shepherd.test.mjs new file mode 100644 index 00000000..a8257c08 --- /dev/null +++ b/plugins/vibe/skills/vibe/scripts/shepherd.test.mjs @@ -0,0 +1,47 @@ +import assert from "node:assert/strict"; +import { spawnSync } from "node:child_process"; +import { chmodSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import test from "node:test"; +import { fileURLToPath } from "node:url"; +import { EXIT, shepherd, shepherdEvent } from "./shepherd.mjs"; + +const SCRIPT = path.join(path.dirname(fileURLToPath(import.meta.url)), "shepherd.mjs"); +const OPEN = { state: "OPEN", mergeable: "MERGEABLE", headRefOid: "abc", statusCheckRollup: [{ name: "lint", conclusion: "SUCCESS" }, { name: "e2e", conclusion: null, status: "IN_PROGRESS" }] }; + +test("each pull request state maps to the action the writer takes", () => { + assert.equal(shepherdEvent(OPEN), null); + assert.deepEqual(shepherdEvent({ ...OPEN, state: "MERGED" }), { event: "merged" }); + assert.deepEqual(shepherdEvent({ ...OPEN, state: "CLOSED" }), { event: "closed" }); + assert.deepEqual(shepherdEvent({ ...OPEN, mergeable: "CONFLICTING" }), { event: "conflict", head: "abc" }); + assert.deepEqual( + shepherdEvent({ ...OPEN, statusCheckRollup: [{ name: "lint", conclusion: "FAILURE" }, { context: "deploy", state: "ERROR" }] }), + { event: "checks-failed", head: "abc", failed: ["lint", "deploy"] } + ); + assert.equal(shepherdEvent({ ...OPEN, mergeable: "UNKNOWN" }), null, "unknown mergeability keeps waiting"); +}); + +test("it keeps polling until a conflict appears, and a single failed read is retried", async () => { + const reads = [() => OPEN, () => { throw new Error("gh: 502"); }, () => OPEN, () => ({ ...OPEN, mergeable: "CONFLICTING" })]; + let calls = 0; + const result = await shepherd({ pr: 1, repo: "o/r", intervalSeconds: 0, read: () => reads[calls++]() }); + assert.deepEqual(result, { event: "conflict", head: "abc" }); + assert.equal(calls, 4); + const failing = await shepherd({ pr: 1, repo: "o/r", intervalSeconds: 0, read: () => { throw new Error("gh: auth\nmore"); } }); + assert.deepEqual(failing, { event: "unreadable", error: "gh: auth" }); + assert.deepEqual(await shepherd({ pr: 1, repo: "o/r", intervalSeconds: 0, maxPolls: 2, read: () => OPEN }), { event: "waiting", polls: 2 }); +}); + +test("the command reads the pull request through gh and exits with the event's code", (t) => { + const dir = mkdtempSync(path.join(os.tmpdir(), "vibe-shepherd-")); + t.after(() => rmSync(dir, { recursive: true, force: true })); + writeFileSync(path.join(dir, "pr.json"), JSON.stringify({ ...OPEN, mergeable: "CONFLICTING" })); + writeFileSync(path.join(dir, "gh"), `#!/bin/sh\nprintf '%s\\n' "$@" > "${dir}/args.txt"\ncat "${dir}/pr.json"\n`); + chmodSync(path.join(dir, "gh"), 0o755); + const env = { ...process.env, PATH: `${dir}:${process.env.PATH}` }; + const result = spawnSync(process.execPath, [SCRIPT, "--pr", "42", "--repo", "o/r", "--interval-seconds", "0"], { encoding: "utf8", env }); + assert.equal(result.status, EXIT.conflict); + assert.deepEqual(JSON.parse(result.stdout), { event: "conflict", head: "abc" }); + assert.equal(spawnSync(process.execPath, [SCRIPT, "--pr", "x"], { encoding: "utf8" }).status, 64); +}); diff --git a/plugins/vibe/skills/vibe/scripts/vibe-sessions.mjs b/plugins/vibe/skills/vibe/scripts/vibe-sessions.mjs index 1c168c9b..961098ce 100755 --- a/plugins/vibe/skills/vibe/scripts/vibe-sessions.mjs +++ b/plugins/vibe/skills/vibe/scripts/vibe-sessions.mjs @@ -93,6 +93,7 @@ import { readWriterSummary } from "./dist/writer-state.mjs"; import { admitMainSyncRequest, admitMainSyncShare, MainSyncError, mainSyncFailure, pushMainSync, readMainSyncInput, readMainSyncProvenance, prepareMainSyncInputs, validateMainSync } from "./dist/main-sync.mjs"; import { listedFlagKeys } from "./flag-keys.mjs"; +import { migrationOrderProblems } from "./migration-order.mjs"; import { isOwnedPrototypeSession, PROTOTYPE_BRANCH_PREFIX, savePrototypePublication } from "./prototype-session.mjs"; import { adoptStartupQuestions } from "./session-questions.mjs"; import { git, readSessionRecord, sessionLiveTicket, writeSessionRecord as writeRecord } from "./session-record.mjs"; @@ -240,6 +241,7 @@ async function run(command) { case "main-sync-inputs": return { mainSync: prepareMainSyncInputs(await readMainSyncInput(requireOption("worktree"))) }; case "main-sync-push": + refuseLateMigrations(requireOption("worktree")); return { mainSync: pushMainSync(await readMainSyncInput(requireOption("worktree"))) }; case "main-sync-share": return { mainSync: admitMainSyncShare(await readMainSyncInput(requireOption("worktree"))) }; @@ -992,3 +994,11 @@ function recordStoryReplacement() { writeRecord(worktree, updated); return { worktree, ...updated }; } + +/** Refuses a push while a branch migration sorts before main's newest one (`migration-order.mjs`). */ +function refuseLateMigrations(worktree) { + const { problems } = migrationOrderProblems(worktree); + if (problems.length > 0) { + throw new MainSyncError(`Nothing was pushed: ${problems.join("; ")}`, "NEEDS_CHANGE"); + } +} diff --git a/plugins/vibe/tools/python/test_vibe_1074_lessons_contract.py b/plugins/vibe/tools/python/test_vibe_1074_lessons_contract.py new file mode 100644 index 00000000..e57764b9 --- /dev/null +++ b/plugins/vibe/tools/python/test_vibe_1074_lessons_contract.py @@ -0,0 +1,62 @@ +"""Contract coverage for the vibe 1.0.74 lessons: each new check is wired where its reader acts.""" + +from pathlib import Path + +from vibe_contract_text import agent, flat + + +PLUGIN = Path(__file__).resolve().parents[2] +VIBE = PLUGIN / "skills" / "vibe" +HANDOFF = PLUGIN / "skills" / "handoff" +FLEET = PLUGIN / "skills" / "fleet-orchestrator" +MONITOR = PLUGIN / "skills" / "codex-fleet-monitor" + + +def _text(path: Path) -> str: + return flat(path.read_text()) + + +def test_fleet_board_ruling_and_checkpoint_delivery_are_run_from_the_skill() -> None: + skill = _text(FLEET / "SKILL.md") + templates = _text(FLEET / "references" / "templates.md") + assert (FLEET / "scripts" / "board.mjs").exists() + for wired in ("board.mjs <orchDir> writer", "--kind ruling --reference", "--kind checkpoint", "board.mjs <orchDir> status"): + assert wired in skill, wired + assert "app-server-orchestrator skill's send" in templates + assert "through SendMessage" in templates + + +def test_human_qa_defaults_to_zero_and_reruns_never_use_the_handed_stack() -> None: + skill = _text(FLEET / "SKILL.md") + assert (FLEET / "scripts" / "qa-plan-check.mjs").exists() + assert "zero human checkpoints by default" in skill + assert "qa-plan-check.mjs --plan" in skill + assert "hand <stack> --to <guide>" in skill + + +def test_helpers_record_and_end_their_processes_and_the_watcher_is_checked() -> None: + skill = _text(FLEET / "SKILL.md") + monitor = _text(MONITOR / "SKILL.md") + assert "helpers.mjs <orchDir> pid <name> <pid...>" in skill + assert "`--group` only for one it spawned detached" in skill + assert "watcher.mjs <orchDir>" in skill + assert "scripts/watcher.mjs <workDir>" in monitor + assert "never `launchctl kickstart -k`" in monitor + + +def test_push_cadence_final_merge_and_shepherding_are_wired() -> None: + assert "pushDue" in (VIBE / "scripts" / "commit-worktree.mjs").read_text() + assert "`pushDue: true`" in _text(VIBE / "SKILL.md") + handoff = _text(HANDOFF / "SKILL.md") + assert "That merge waits on no e2e run" in handoff + assert "shepherd.mjs --pr <n> --repo <owner/repo>" in handoff + assert "every 90 minutes" in _text(FLEET / "SKILL.md") + + +def test_migration_order_compose_projects_precedent_and_path_gates_are_wired() -> None: + assert "migrationOrderProblems" in (VIBE / "scripts" / "vibe-sessions.mjs").read_text() + assert "scripts/migration-order.mjs" in _text(VIBE / "references" / "environment.md") + assert "its own compose project" in _text(HANDOFF / "SKILL.md") + assert "precedent-check.mjs check --report <report>" in _text(VIBE / "references" / "quality-loop.md") + assert "pathGateTests" in (HANDOFF / "scripts" / "diff-rules.mjs").read_text() + assert ".vibe/path-gates.json" in flat(agent("vibe-verify-worker")) From 4669022c7525ce352abc7ecd8fbf5851638854d4 Mon Sep 17 00:00:00 2001 From: Daniel Ochoa <daniel.ochoa@closedloop.ai> Date: Wed, 21 Oct 2026 16:33:13 -0500 Subject: [PATCH 3/4] test(vibe): use plain checkpoint ids in the QA plan check test - The fleet skill's generic test reads QA-001 as a ticket id; the check accepts any checkpoint id, so the fixtures use C1 to C8. Testing: qa-plan-check, board and generic tests on a second machine. Risks: None identified. --- .../scripts/qa-plan-check.test.mjs | 38 +++++++++---------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/plugins/vibe/skills/fleet-orchestrator/scripts/qa-plan-check.test.mjs b/plugins/vibe/skills/fleet-orchestrator/scripts/qa-plan-check.test.mjs index 1889975c..4bfbd893 100644 --- a/plugins/vibe/skills/fleet-orchestrator/scripts/qa-plan-check.test.mjs +++ b/plugins/vibe/skills/fleet-orchestrator/scripts/qa-plan-check.test.mjs @@ -12,24 +12,24 @@ const record = (sections) => sections.map(([id, routing]) => `### ${id}: title\n test('agent-verified and e2e-covered checkpoints are the agent\'s; a human one needs a reason no agent can cover', () => { const found = checkpoints(record([ - ['QA-001', 'AGENT_VERIFIED: DOM read on head abc'], - ['QA-002', '`E2E_COVERED` by the move spec'], - ['QA-003', 'needs a real device: trackpad scroll feel on the tree grid'], - ['QA-004', 'screen reader announcement order (VoiceOver)'], - ['QA-005', 'visual: whether the drag chip looks right'], - ['QA-006', 'the operator should take a look'], - ['QA-007', null], - ['QA-008', 'the layout looks and feels right'], + ['C1', 'AGENT_VERIFIED: DOM read on head abc'], + ['C2', '`E2E_COVERED` by the move spec'], + ['C3', 'needs a real device: trackpad scroll feel on the tree grid'], + ['C4', 'screen reader announcement order (VoiceOver)'], + ['C5', 'visual: whether the drag chip looks right'], + ['C6', 'the operator should take a look'], + ['C7', null], + ['C8', 'the layout looks and feels right'], ])); assert.deepEqual(found.map((checkpoint) => [checkpoint.id, verdict(checkpoint).kind]), [ - ['QA-001', 'agent'], - ['QA-002', 'agent'], - ['QA-003', 'human'], - ['QA-004', 'human'], - ['QA-005', 'refused'], - ['QA-006', 'refused'], - ['QA-007', 'refused'], - ['QA-008', 'refused'], + ['C1', 'agent'], + ['C2', 'agent'], + ['C3', 'human'], + ['C4', 'human'], + ['C5', 'refused'], + ['C6', 'refused'], + ['C7', 'refused'], + ['C8', 'refused'], ]); assert.match(verdict(found[4]).why, /compare its screenshot with the saved reference/); }); @@ -38,12 +38,12 @@ test('the command exits 1 on a refused checkpoint and 0 for a plan with zero hum const dir = mkdtempSync(path.join(os.tmpdir(), 'qa-plan-')); t.after(() => rmSync(dir, { recursive: true, force: true })); const plan = path.join(dir, 'qa.md'); - writeFileSync(plan, record([['QA-001', 'AGENT_VERIFIED: capture compared with the reference'], ['QA-002', 'whether the spacing looks right']])); + writeFileSync(plan, record([['C1', 'AGENT_VERIFIED: capture compared with the reference'], ['C2', 'whether the spacing looks right']])); const refused = spawnSync(process.execPath, [path.join(HERE, 'qa-plan-check.mjs'), '--plan', plan], { encoding: 'utf8' }); assert.equal(refused.status, 1); - assert.match(refused.stdout, /^REFUSED QA-002: a visual judgment is the agent's/m); + assert.match(refused.stdout, /^REFUSED C2: a visual judgment is the agent's/m); assert.match(refused.stdout, /human checkpoints: 0, refused: 1/); - writeFileSync(plan, record([['QA-001', 'AGENT_VERIFIED: capture compared with the reference']])); + writeFileSync(plan, record([['C1', 'AGENT_VERIFIED: capture compared with the reference']])); const clean = spawnSync(process.execPath, [path.join(HERE, 'qa-plan-check.mjs'), '--plan', plan], { encoding: 'utf8' }); assert.equal(clean.status, 0); assert.equal(clean.stdout, 'human checkpoints: 0, refused: 0\n'); From 9cfa37bf08add260a4276db6481f96e529f2e110 Mon Sep 17 00:00:00 2001 From: Daniel Ochoa <daniel.ochoa@closedloop.ai> Date: Wed, 21 Oct 2026 16:59:26 -0500 Subject: [PATCH 4/4] fix(vibe): never signal a reused pid from the helper ledger - A recorded pid can be handed to one of the user's own apps after the helper exits; probe() saw it as alive, and a launchd app leads its own group, so stopped would have killed it. - pid records each process's start time and command and refuses one it cannot read; endProcesses re-reads them before every signal and treats a mismatch, an unreadable value or an older record without identity as not ours. The ps reader is injected like groupOf. - run-bounded's group test waits, bounded, for the killed grandchild to be reaped before asserting it is gone. Testing: helpers and run-bounded tests on a second machine; making the identity check always pass fails all four identity tests. Risks: a process that execs into another command after it is recorded is no longer signalled by stopped; record the final process instead. --- CHANGELOG.md | 2 +- .../fleet-orchestrator/scripts/helpers.mjs | 46 +++++++++--- .../scripts/helpers.test.mjs | 73 ++++++++++++++++--- .../skills/vibe/scripts/run-bounded.test.mjs | 14 +++- 4 files changed, 112 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 420b6450..cd84ebe5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). Signal safety after a plugin test ended every process on an operator's machine, and the open lessons from the 2026-10-21 ledger entries, each with a mechanical check where one fits. #### Fixed -- `helpers.mjs stopped` no longer signals a process group it does not own. A test recorded pid 1 and stopped it; `endProcesses` sent `kill(-1)`, which signals every process the user owns, twice. Pids of 1 or lower are refused at record time and never signalled, nor is the stopper's own pid or its parent. A whole group is signalled only for a pid recorded with `--group` whose process group id still equals the pid; any other pid is signalled alone. A pid the system reports as not ours (EPERM, a reused pid) is never signalled and does not block `stopped`. `endProcesses` takes an injected `kill`, so its tests check every decision without a real signal. +- `helpers.mjs stopped` no longer signals a process group it does not own. A test recorded pid 1 and stopped it; `endProcesses` sent `kill(-1)`, which signals every process the user owns, twice. Pids of 1 or lower are refused at record time and never signalled, nor is the stopper's own pid or its parent. A whole group is signalled only for a pid recorded with `--group` whose process group id still equals the pid; any other pid is signalled alone. A pid the system reports as not ours (EPERM) is never signalled and does not block `stopped`. `pid` records each process's start time and command, and every signal re-reads them first: a reused pid (even one of the user's own apps, which launchd makes a group leader), a changed or unreadable identity, or an older record without one is never signalled. `endProcesses` takes an injected `kill`, so its tests check every decision without a real signal. - Every group signal in the plugin (`run-bounded.mjs`, `precedent-check.mjs`, the vibe-workers ledger and Claude worker, and their tests) checks the pid above 1 first. The dist bundles are rebuilt. #### Added diff --git a/plugins/vibe/skills/fleet-orchestrator/scripts/helpers.mjs b/plugins/vibe/skills/fleet-orchestrator/scripts/helpers.mjs index aa387f02..06257eec 100644 --- a/plugins/vibe/skills/fleet-orchestrator/scripts/helpers.mjs +++ b/plugins/vibe/skills/fleet-orchestrator/scripts/helpers.mjs @@ -22,7 +22,11 @@ // `--group` pid whose process group id still equals the pid; any other pid is // signalled alone. A pid of 1 or lower, this process or its parent is never // recorded or signalled, and neither is a pid the system says is not ours -// (EPERM): a recorded pid can be reused by an unrelated process. +// (EPERM). A recorded pid can be reused, even by one of the user's own apps, +// so `pid` stores each process's start time and command (`ps -o +// lstart=,command=`), refusing one it cannot read, and every signal re-reads +// them first: a pid whose identity differs, cannot be read, or was never +// recorded is treated as not ours and never signalled, alone or as a group. // `hand` marks a running app stack (a worktree path, port or URL) as handed to // a human QA guide; `start --stack` on a handed stack exits 1, so a rerun that // toggles flags or moves data launches its own stack. @@ -87,6 +91,13 @@ export function isAlive(pid) { return probe(pid) !== "gone"; } +/** A process's start time and command (`ps -o lstart=,command=`), or null when it cannot be read. */ +export function readIdentity(pid) { + const result = spawnSync("ps", ["-o", "lstart=,command=", "-p", String(pid)], { encoding: "utf8", env: { ...process.env, LC_ALL: "C" } }); + const identity = result.stdout?.trim(); + return result.status === 0 && identity ? identity : null; +} + /** The process group id of `pid`, or null when it cannot be read. */ export function readGroupId(pid) { const result = spawnSync("ps", ["-o", "pgid=", "-p", String(pid)], { encoding: "utf8" }); @@ -95,17 +106,22 @@ export function readGroupId(pid) { } /** - * Ends `targets` ({ pid, group }), SIGKILLs what outlives the grace, and - * returns the pids still alive. Unsafe and foreign pids are never signalled. - * `kill` and `groupOf` are injectable so tests can check every decision - * without sending a real signal. + * Ends `targets` ({ pid, group, identity }), SIGKILLs what outlives the + * grace, and returns the pids still alive and ours. Unsafe and foreign pids + * are never signalled; a pid is ours only while `identityOf` still returns + * the identity recorded for it. `kill`, `groupOf` and `identityOf` are + * injectable so tests can check every decision without sending a real signal. */ -export async function endProcesses(targets, graceSeconds = DEFAULT_GRACE_SECONDS, { kill = defaultKill, groupOf = readGroupId } = {}) { +export async function endProcesses(targets, graceSeconds = DEFAULT_GRACE_SECONDS, { kill = defaultKill, groupOf = readGroupId, identityOf = readIdentity } = {}) { const owned = targets.filter((target) => safePid(target.pid)); - const live = () => owned.filter((target) => probe(target.pid, kill) === "alive"); - const signal = ({ pid, group }, name) => { + const same = (target) => typeof target.identity === "string" && target.identity !== "" && identityOf(target.pid) === target.identity; + const live = () => owned.filter((target) => probe(target.pid, kill) === "alive" && same(target)); + const signal = (target, name) => { + const { pid, group } = target; // Checked again here so no edit to the filter above can ever reach kill(-1). if (!(pid > 1)) return; + // Re-read right before the signal: the pid may have been reused since the last check. + if (!same(target)) return; const wholeGroup = group === true && groupOf(pid) === pid; try { kill(wholeGroup ? -pid : pid, name); @@ -213,16 +229,26 @@ async function main(argv) { ledger[name] = { startedAt: at, ...(values.brief ? { brief: values.brief } : {}), ...(values.stack ? { stack: values.stack } : {}) }; } else if (action === "pid") { const pids = parsePids(positionals.slice(1)); + const identities = Object.fromEntries(pids.map((pid) => [pid, readIdentity(pid)])); + const unreadable = pids.filter((pid) => !identities[pid]); + if (unreadable.length > 0) { + throw new Error(`cannot read the start time and command of ${unreadable.join(" ")}; record only a running process this helper started.`); + } const entry = ledger[name] ?? {}; const groups = values.group ? [...new Set([...(entry.groups ?? []), ...pids])] : (entry.groups ?? []); - ledger[name] = { ...entry, pids: [...new Set([...(entry.pids ?? []), ...pids])], ...(groups.length > 0 ? { groups } : {}) }; + ledger[name] = { + ...entry, + pids: [...new Set([...(entry.pids ?? []), ...pids])], + identities: { ...(entry.identities ?? {}), ...identities }, + ...(groups.length > 0 ? { groups } : {}), + }; } else if (action === "accepted") { ledger[name] = { ...(ledger[name] ?? {}), acceptedAt: at }; } else { const grace = values["grace-seconds"] === undefined ? DEFAULT_GRACE_SECONDS : Number(values["grace-seconds"]); const entry = ledger[name] ?? {}; const groups = new Set(entry.groups ?? []); - const survivors = await endProcesses((entry.pids ?? []).map((pid) => ({ pid, group: groups.has(pid) })), grace); + const survivors = await endProcesses((entry.pids ?? []).map((pid) => ({ pid, group: groups.has(pid), identity: entry.identities?.[pid] })), grace); if (survivors.length > 0) { process.stdout.write(`ALIVE ${name} ${survivors.join(" ")}\n`); return 1; diff --git a/plugins/vibe/skills/fleet-orchestrator/scripts/helpers.test.mjs b/plugins/vibe/skills/fleet-orchestrator/scripts/helpers.test.mjs index 73b7a4aa..1dc234ea 100644 --- a/plugins/vibe/skills/fleet-orchestrator/scripts/helpers.test.mjs +++ b/plugins/vibe/skills/fleet-orchestrator/scripts/helpers.test.mjs @@ -60,7 +60,7 @@ const groupLeader = async (t, dir) => { * one real signal. Pids sit above every real pid_max, in case one leaks. */ const fakeTable = (processes) => { - const table = new Map(processes.map((entry) => [entry.pid, { ...entry }])); + const table = new Map(processes.map((entry) => [entry.pid, { identity: `id-${entry.pid}`, ...entry }])); const calls = []; const fail = (code) => Object.assign(new Error(code), { code }); const kill = (target, signal) => { @@ -74,13 +74,24 @@ const fakeTable = (processes) => { const entry = table.get(target); if (!entry || entry.state === 'gone') throw fail('ESRCH'); if (entry.state === 'foreign') throw fail('EPERM'); + if (signal === 'SIGTERM' && entry.reusedOnTerm) { + // The process exits on SIGTERM and the system hands its pid to another of the user's apps. + entry.identity = entry.reusedOnTerm; + return; + } if (signal !== 0) entry.state = 'gone'; }; const groupOf = (pid) => table.get(pid)?.pgid ?? null; + const identityOf = (pid) => { + const entry = table.get(pid); + return entry && entry.state !== 'gone' ? entry.identity : null; + }; const sent = () => calls.filter(([, signal]) => signal !== 0); const state = (pid) => table.get(pid)?.state; - return { kill, groupOf, calls, sent, state }; + return { kill, groupOf, identityOf, calls, sent, state }; }; +/** A target as `stopped` builds it from the ledger, with the identity recorded at `pid` time. */ +const recorded = (pid, group, identity = `id-${pid}`) => ({ pid, group, identity }); const FAKE = 2_147_480_000; const waitUntil = async (predicate) => { for (let i = 0; i < 100 && !predicate(); i += 1) await new Promise((resolve) => setTimeout(resolve, 50)); @@ -163,7 +174,7 @@ test('a pid of 1 or lower is never recorded: signalling group 1 reaches every pr test('endProcesses never signals a pid of 1 or lower, this process or its parent, even when one is recorded', async () => { const fake = fakeTable([]); const unsafe = [0, 1, -1, -42, Number.NaN, 1.5, process.pid, process.ppid].map((pid) => ({ pid, group: true })); - assert.deepEqual(await endProcesses(unsafe, 0, { kill: fake.kill, groupOf: fake.groupOf }), []); + assert.deepEqual(await endProcesses(unsafe, 0, { kill: fake.kill, groupOf: fake.groupOf, identityOf: fake.identityOf }), []); assert.deepEqual(fake.calls, [], 'not even a liveness probe'); }); @@ -177,13 +188,9 @@ test('endProcesses signals a pid alone unless it was recorded as a group the hel { pid: FAKE + 6, pgid: FAKE + 9, state: 'alive' }, ]); const survivors = await endProcesses( - [ - { pid: FAKE + 1, group: false }, - { pid: FAKE + 3, group: true }, - { pid: FAKE + 5, group: true }, - ], + [recorded(FAKE + 1, false), recorded(FAKE + 3, true), recorded(FAKE + 5, true)], 0, - { kill: fake.kill, groupOf: fake.groupOf } + { kill: fake.kill, groupOf: fake.groupOf, identityOf: fake.identityOf } ); assert.deepEqual(survivors, []); assert.deepEqual(fake.sent(), [ @@ -198,11 +205,57 @@ test('endProcesses signals a pid alone unless it was recorded as a group the hel test('endProcesses never signals a pid the system says is not ours (EPERM), and does not count it as a survivor', async () => { const fake = fakeTable([{ pid: FAKE + 7, pgid: FAKE + 7, state: 'foreign' }]); - assert.deepEqual(await endProcesses([{ pid: FAKE + 7, group: true }], 0, { kill: fake.kill, groupOf: fake.groupOf }), []); + assert.deepEqual(await endProcesses([recorded(FAKE + 7, true)], 0, { kill: fake.kill, groupOf: fake.groupOf, identityOf: fake.identityOf }), []); assert.deepEqual(fake.sent(), []); assert.equal(isAlive(FAKE + 7), false, 'a fake pid above pid_max does not exist'); }); +test('a reused pid whose start time or command differs from the record is never signalled', async () => { + const fake = fakeTable([{ pid: FAKE + 10, pgid: FAKE + 30, state: 'alive', identity: 'Wed Oct 21 16:00:01 2026 /Applications/Slack.app/Contents/MacOS/Slack' }]); + const targets = [recorded(FAKE + 10, false, 'Wed Oct 21 15:00:00 2026 node dev-server.js')]; + assert.deepEqual(await endProcesses(targets, 0, { kill: fake.kill, groupOf: fake.groupOf, identityOf: fake.identityOf }), []); + assert.deepEqual(fake.sent(), []); + assert.equal(fake.state(FAKE + 10), 'alive'); +}); + +test('a reused pid that now leads its own group, as a launchd app does, is never group-killed', async () => { + const fake = fakeTable([ + { pid: FAKE + 11, pgid: FAKE + 11, state: 'alive', identity: 'Wed Oct 21 16:05:00 2026 /Applications/Google Chrome.app/Contents/MacOS/Google Chrome' }, + { pid: FAKE + 12, pgid: FAKE + 11, state: 'alive', identity: 'Wed Oct 21 16:05:01 2026 Google Chrome Helper' }, + ]); + const targets = [recorded(FAKE + 11, true, 'Wed Oct 21 15:00:00 2026 sh -c next dev')]; + assert.deepEqual(await endProcesses(targets, 0, { kill: fake.kill, groupOf: fake.groupOf, identityOf: fake.identityOf }), []); + assert.deepEqual(fake.sent(), []); + assert.equal(fake.state(FAKE + 12), 'alive', 'the app and its group are untouched'); +}); + +test('a record with no identity, as an older ledger has, is never signalled', async () => { + const fake = fakeTable([{ pid: FAKE + 13, pgid: FAKE + 13, state: 'alive' }]); + for (const identity of [undefined, '', null]) { + assert.deepEqual(await endProcesses([{ pid: FAKE + 13, group: true, identity }], 0, { kill: fake.kill, groupOf: fake.groupOf, identityOf: fake.identityOf }), []); + } + assert.deepEqual(fake.sent(), []); + assert.equal(fake.state(FAKE + 13), 'alive'); +}); + +test('identity is read again before every signal: a pid reused during the grace gets no SIGKILL', async () => { + const fake = fakeTable([{ pid: FAKE + 14, pgid: FAKE + 40, state: 'alive', reusedOnTerm: 'Wed Oct 21 16:10:00 2026 /Applications/iTerm.app/Contents/MacOS/iTerm2' }]); + assert.deepEqual(await endProcesses([recorded(FAKE + 14, false)], 0, { kill: fake.kill, groupOf: fake.groupOf, identityOf: fake.identityOf }), []); + assert.deepEqual(fake.sent(), [[FAKE + 14, 'SIGTERM']]); + assert.equal(fake.state(FAKE + 14), 'alive', 'the app now holding the pid is not killed'); +}); + +test('pid records each process with its start time and command, and refuses one it cannot read', (t) => { + const dir = tempDir(t); + const server = child(t, 'sleep', ['60']); + assert.equal(run([dir, 'pid', 'qa-i', String(server)]).status, 0); + const identity = JSON.parse(readFileSync(path.join(dir, 'helpers.json'), 'utf8'))['qa-i'].identities[server]; + assert.match(identity, /sleep 60/); + const refused = run([dir, 'pid', 'qa-j', String(FAKE + 15)]); + assert.equal(refused.status, 2); + assert.match(refused.stderr, /cannot read the start time and command/); +}); + test('stopping a helper recorded with --group ends its whole process group', { timeout: 30_000 }, async (t) => { const dir = tempDir(t); const { leader, member } = await groupLeader(t, dir); diff --git a/plugins/vibe/skills/vibe/scripts/run-bounded.test.mjs b/plugins/vibe/skills/vibe/scripts/run-bounded.test.mjs index e6db118e..42831450 100644 --- a/plugins/vibe/skills/vibe/scripts/run-bounded.test.mjs +++ b/plugins/vibe/skills/vibe/scripts/run-bounded.test.mjs @@ -25,7 +25,7 @@ test("a command past its limit is stopped with everything it started, exiting 12 assert.equal(code, TIMED_OUT_EXIT); }); -test("the stopped command's whole process group is gone after a time out", () => { +test("the stopped command's whole process group is gone after a time out", async () => { const result = spawnSync(process.execPath, [SCRIPT, "1", process.execPath, "-e", ` const { spawn } = require("node:child_process"); const grandchild = spawn(process.execPath, ["-e", "setInterval(() => {}, 1000)"], { stdio: "ignore" }); @@ -35,7 +35,17 @@ test("the stopped command's whole process group is gone after a time out", () => assert.equal(result.status, TIMED_OUT_EXIT, result.stderr); assert.match(result.stderr, /stopped after 1s/); const grandchild = Number(result.stdout.trim()); - assert.ok(grandchild > 0); + assert.ok(grandchild > 1); + // A killed grandchild whose parent already exited still answers a liveness probe until it is reaped, so wait, bounded, for it to go. + const gone = () => { + try { + process.kill(grandchild, 0); + return false; + } catch (error) { + return error?.code === "ESRCH"; + } + }; + for (let waited = 0; waited < 5000 && !gone(); waited += 50) await new Promise((resolve) => setTimeout(resolve, 50)); assert.throws(() => process.kill(grandchild, 0), { code: "ESRCH" }); });