* fix(#4624): persist orchestrator-worktree worker lifecycle records * fix(#4624): address review findings on the worker lifecycle protocol * fix(#4624): require the summary path and surface torn records on status --path * fix(#4624): distinguish no-record from torn-record, tolerate older shims in the sweep * docs(#4624): backfill changeset PR number --------- Co-authored-by: sim <sim@local>
This commit is contained in:
5
.changeset/noble-moles-munch.md
Normal file
5
.changeset/noble-moles-munch.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Fixed
|
||||
pr: 4778
|
||||
---
|
||||
**Codex orchestrator-worktree workers now persist a durable lifecycle record** — external executors dispatched by /gsd-execute-phase record their launch and terminal state (worktree worker-record/worker-status/worker-complete), so a resumed session reconciles finished or blocked workers from persisted state instead of manual PID discovery, and never re-dispatches a recorded plan. (#4624)
|
||||
@@ -3017,6 +3017,12 @@ function dispatchOverlayCapabilityCommand({ command, args, cwd, raw, error, load
|
||||
worktreeSafety.cmdWorktreeRecordAgent(cwd, args.slice(2));
|
||||
} else if (subcommand === 'reap-orphans') {
|
||||
worktreeSafety.cmdWorktreeReapOrphans(cwd);
|
||||
} else if (subcommand === 'worker-record') {
|
||||
worktreeSafety.cmdWorktreeWorkerRecord(cwd, args.slice(2));
|
||||
} else if (subcommand === 'worker-status') {
|
||||
worktreeSafety.cmdWorktreeWorkerStatus(cwd, args.slice(2));
|
||||
} else if (subcommand === 'worker-complete') {
|
||||
worktreeSafety.cmdWorktreeWorkerComplete(cwd, args.slice(2));
|
||||
} else if (subcommand === 'base-check') {
|
||||
require('./lib/worktree-base-ref.cjs').cmdWorktreeBaseCheck(cwd, args.slice(2));
|
||||
} else if (subcommand === 'set-baseref') {
|
||||
@@ -3024,7 +3030,7 @@ function dispatchOverlayCapabilityCommand({ command, args, cwd, raw, error, load
|
||||
} else if (subcommand === 'create') {
|
||||
worktreeSafety.cmdWorktreeCreate(cwd, args.slice(2));
|
||||
} else {
|
||||
error('Unknown worktree subcommand. Available: cleanup-wave, record-agent, reap-orphans, base-check, set-baseref, create', ERROR_REASON.SDK_UNKNOWN_COMMAND);
|
||||
error('Unknown worktree subcommand. Available: cleanup-wave, record-agent, reap-orphans, base-check, set-baseref, create, worker-record, worker-status, worker-complete', ERROR_REASON.SDK_UNKNOWN_COMMAND);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -126,6 +126,16 @@ in `execute-phase.md` step 3 (on Claude Code it is literally `isolation="worktre
|
||||
|
||||
The host has no harness-native isolation primitive, so **GSD** creates each worktree and process-spawns the executor into it. Fan-out is OS-level (N processes), not the host's subagent tool. Per the Codex `workspace-write` sandbox constraint, **the orchestrator performs every git operation** — create, merge, cleanup; the spawned executor only edits files and commits inside its own worktree.
|
||||
|
||||
**Resume-first guard (#4624).** The launching turn may end while external workers are still live — that is expected here, not an error. So before dispatching ANY plan in this section, sweep for workers a previous session left behind:
|
||||
|
||||
```bash
|
||||
gsd_run query worktree.worker-status --root "${ORCH_ROOT}/.claude/worktrees" 2>/dev/null || true
|
||||
```
|
||||
|
||||
(An older installed shim without these verbs reports nothing here — its waves stay owned by the #3707 orphan sweep, as before.)
|
||||
|
||||
Every entry with `needsReconciliation: true` (recorded running, process gone) is reconciled from its persisted record — SUMMARY + plan-scoped commits + branch state per `execute-phase/steps/completion-reconciliation.md`, merge if artifact-complete, preserve with recovery information otherwise — then marked with `worker-complete`. An entry that is still `running` with `pidAlive: true` is a live worker — wait on it or leave it running; its terminal state surfaces on the next sweep. One caution: pid reuse on a long-lived host can keep a dead worker reading as alive — when a running entry's `startedAt` is older than the executor timeout budget, inspect its log and worktree before trusting liveness. **Never re-dispatch a recorded plan, and never reconcile on narration alone.**
|
||||
|
||||
Run the loop below once per runnable plan in the wave, **one plan at a time** (`git worktree add` races on `.git/config.lock`).
|
||||
|
||||
**Before running the bash block, substitute the plan's identifiers into it** exactly as you do for the `Agent()` prompt on the harness path: replace `{plan_number}` and `{phase_number}` with this plan's values. They are template placeholders, not shell variables. `$ORCH_ROOT` and `$EXPECTED_BASE` are real shell variables, already assigned earlier in this step; `$WAVE_WORKTREE_MANIFEST` was initialized above.
|
||||
@@ -149,7 +159,7 @@ Assign the composed prompt to a shell variable so it can be passed as one argume
|
||||
# An unreadable source file is a halt condition (#3637 fail-closed),
|
||||
# never a skip — a child without these texts is not a gsd-executor.
|
||||
# 2. Substitute this plan's {plan_number}, {phase_number}, {phase_name},
|
||||
# {phase_dir}, {plan_file}, and {plan_id} placeholders (same values the
|
||||
# {phase_dir}, {plan_file}, {plan_id}, and {plan_padded} placeholders (same values the
|
||||
# harness path substitutes into its Agent() prompt). {plan_id} is this
|
||||
# plan's `id` field from the phase-plan-index JSON — the guard hooks
|
||||
# compare it verbatim against the sentinel the per-plan gate wrote, so a
|
||||
@@ -330,7 +340,31 @@ fi
|
||||
|
||||
`worktree create` records the entry in `$WAVE_WORKTREE_MANIFEST` itself, so **do not** call `worktree.record-agent` for these plans — that verb is the harness-path counterpart, used because the harness creates the worktree behind GSD's back. Double-recording is deduped by path+branch, but the create verb is the single writer here.
|
||||
|
||||
Spawn `EXEC_JSON`'s `command` + `args` as a background process with its working directory set to `EXEC_JSON.cwd`. The `cwd` is returned for **every** host, including those whose descriptor has no cwd flag (`cwdFlag: null`) and therefore bind through the process's own working directory — always set it, never assume the flag did the job. Wait for all spawned executors in the wave before merging.
|
||||
Spawn `EXEC_JSON`'s `command` + `args` as a background process with its working directory set to `EXEC_JSON.cwd`, capturing the PID (`WORKER_PID=$!`) and redirecting stdout+stderr into a per-worker log placed BESIDE the worktree — `${WT_PATH}.worker.log`, not inside it: cleanup removes the worktree, and the log plus the lifecycle record must survive it. The `cwd` is returned for **every** host, including those whose descriptor has no cwd flag (`cwdFlag: null`) and therefore bind through the process's own working directory — always set it, never assume the flag did the job.
|
||||
|
||||
**Record the launch before any wait (#4624).** A bare background `wait` left the wave with no durable lifecycle: when the orchestrator's turn ended first, a finished worker sat undiscovered and a blocked one sat unreported, and a resumed session had nothing to recover from. Immediately after the spawn — before waiting on anything — persist the launch identity and result location:
|
||||
|
||||
```bash
|
||||
WORKER_LOG="${WT_PATH}.worker.log"
|
||||
gsd_run query worktree.worker-record \
|
||||
--path "$WT_PATH" \
|
||||
--pid "$WORKER_PID" \
|
||||
--plan "{plan_number}" \
|
||||
--summary-path "{phase_dir}/{plan_padded}-SUMMARY.md" \
|
||||
--log-file "$WORKER_LOG" || {
|
||||
echo "FATAL: worker launch not recorded for plan {plan_number} — either a running record already exists for this worktree (a resumed session dispatched it: reconcile that worker per completion-reconciliation.md, never spawn a second one) or the record write failed (surface the error output)." >&2
|
||||
exit 1
|
||||
}
|
||||
```
|
||||
|
||||
**Wait until terminal, reconcile BEFORE recording the outcome (#4217/#4624).** Wait for the wave's executors to reach a terminal process state (`wait` on the captured PIDs, or poll `worktree.worker-status --root "${ORCH_ROOT}/.claude/worktrees"` and read `pidAlive`). While a worker's record is `running`, it is a reconciliation candidate the moment its process dies — a crash between classification and recording can never strand it. So the ordering is: run the **existing** artifact reconciliation (`execute-phase/steps/completion-reconciliation.md`) FIRST — SUMMARY + plan-scoped commits + branch state decide the outcome: a worker that exits 0 with no artifacts is not done, and one that exits non-zero with artifacts complete may still merge. Merge artifact-complete workers through the manifest-only gauntlet below; preserve a blocked/failed worktree untouched. Only then record the terminal outcome, carrying the recovery information:
|
||||
|
||||
```bash
|
||||
gsd_run query worktree.worker-complete --path "$WT_PATH" --exit-code "$WORKER_EXIT" \
|
||||
--note "{worker_note}"
|
||||
```
|
||||
|
||||
`{worker_note}` carries the blocker description and the log path for a preserved worktree (`$WT_PATH.worker.log`), or is empty for a clean finish.
|
||||
|
||||
The executor never touches `STATE.md`/`ROADMAP.md`, and that guard needs no new code — `execute-plan` auto-detects worktree mode via the `IS_WORKTREE` (`.git`-is-a-file) primitive, which a GSD-created worktree trips identically to a harness-created one.
|
||||
|
||||
|
||||
@@ -2253,6 +2253,295 @@ function cmdWorktreeReapOrphans(cwd: string, deps: RecordAgentCmdDeps & Worktree
|
||||
write(`${JSON.stringify({ ok: true, reaped: result.filter((r) => r.status === 'reaped').length, entries: result }, null, 2)}\n`);
|
||||
}
|
||||
|
||||
// ─── Worker lifecycle records (#4624) ────────────────────────────────────────
|
||||
// Durable per-worker launch/terminal state for the orchestrator-worktree
|
||||
// backend. The dispatch fragment spawns external executor processes with a
|
||||
// bare background `wait`; when the orchestrator's turn ends before a worker
|
||||
// finishes, nothing records the launch or guarantees reconciliation, and a
|
||||
// resumed session has no state to recover — it re-derives everything from
|
||||
// manual PID/log discovery. These records persist the launch identity, the
|
||||
// result location, and the terminal outcome as a small JSON file beside the
|
||||
// worktree (NOT inside it — cleanup removes the worktree; the record must
|
||||
// survive it), so `worker-status` can answer "who was dispatched, is it
|
||||
// still running, did it finish its artifacts, does it need reconciliation"
|
||||
// deterministically on resume.
|
||||
|
||||
/** Path of the lifecycle record for a worktree: a SIBLING of the worktree dir. */
|
||||
function workerRecordPath(worktreePath: string): string {
|
||||
return `${worktreePath}.worker.json`;
|
||||
}
|
||||
|
||||
interface WorkerRecord {
|
||||
agentId: string;
|
||||
pid: number;
|
||||
plan: string;
|
||||
worktreePath: string;
|
||||
summaryPath: string;
|
||||
logFile: string;
|
||||
startedAt: string;
|
||||
state: 'running' | 'complete';
|
||||
exitCode: number | null;
|
||||
note: string;
|
||||
completedAt: string | null;
|
||||
}
|
||||
|
||||
const RENAME_RETRY_ERRNOS = new Set(['EPERM', 'EBUSY', 'EACCES']);
|
||||
|
||||
// local/require-fs-op-fallback: a concurrent reader or antivirus scanner can
|
||||
// transiently hold the rename target open on Windows (DEFECT.WINDOWS-FS-OPS) —
|
||||
// bounded retry on the transient errnos, per the house pattern.
|
||||
function renameWithRetry(tmp: string, target: string): void {
|
||||
let lastErr: unknown;
|
||||
for (let attempt = 0; attempt < 4; attempt++) {
|
||||
try {
|
||||
fs.renameSync(tmp, target);
|
||||
return;
|
||||
} catch (err) {
|
||||
lastErr = err;
|
||||
const code = (err as NodeJS.ErrnoException).code;
|
||||
if (code && RENAME_RETRY_ERRNOS.has(code) && attempt < 3) {
|
||||
const until = Date.now() + 25 * (attempt + 1);
|
||||
while (Date.now() < until) { /* bounded spin: transient locks clear in <100ms */ }
|
||||
continue;
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
}
|
||||
throw lastErr;
|
||||
}
|
||||
|
||||
function writeWorkerRecord(
|
||||
recordPath: string,
|
||||
record: WorkerRecord,
|
||||
deps: Record<string, unknown> = {}
|
||||
): void {
|
||||
const writeFile = (deps.writeFile as ((p: string, d: string) => void)) || ((p: string, d: string) => fs.writeFileSync(p, d));
|
||||
const tmp = `${recordPath}.tmp-${process.pid}`;
|
||||
writeFile(tmp, `${JSON.stringify(record, null, 2)}\n`);
|
||||
renameWithRetry(tmp, recordPath);
|
||||
}
|
||||
|
||||
function cmdWorktreeWorkerRecord(cwd: string, args: string[] = [], deps: Record<string, unknown> = {}): void {
|
||||
const flag = (name: string): string => {
|
||||
const i = args.indexOf(name);
|
||||
if (i < 0 || i + 1 >= args.length) return '';
|
||||
return args[i + 1];
|
||||
};
|
||||
const write = (deps.write as ((s: string) => void)) || ((s: string) => process.stdout.write(s));
|
||||
const writeErr = (deps.writeErr as ((s: string) => void)) || ((s: string) => process.stderr.write(s));
|
||||
|
||||
const worktreePath = flag('--path');
|
||||
const pidText = flag('--pid');
|
||||
const plan = flag('--plan');
|
||||
const summaryPath = flag('--summary-path');
|
||||
const logFile = flag('--log-file');
|
||||
if (!worktreePath || !pidText || !plan || !summaryPath) {
|
||||
writeErr('Usage: worktree worker-record --path <worktree> --pid <pid> --plan <plan_number> --summary-path <path> [--log-file <path>]\n');
|
||||
process.exitCode = 2;
|
||||
return;
|
||||
}
|
||||
const pid = Number(pidText);
|
||||
if (!Number.isInteger(pid) || pid <= 0) {
|
||||
writeErr(`[gsd] worktree.worker-record: invalid --pid: ${pidText}\n`);
|
||||
process.exitCode = 2;
|
||||
return;
|
||||
}
|
||||
|
||||
const resolvedWorktree = path.resolve(cwd, worktreePath);
|
||||
const recordPath = workerRecordPath(resolvedWorktree);
|
||||
const readFile = (deps.readFile as ((p: string) => string)) || ((p: string) => fs.readFileSync(p, 'utf8'));
|
||||
let existing: WorkerRecord | null = null;
|
||||
try {
|
||||
existing = JSON.parse(readFile(recordPath)) as WorkerRecord;
|
||||
} catch { /* no record yet — first dispatch for this worktree */ }
|
||||
if (existing && existing.state === 'running') {
|
||||
// Duplicate-dispatch guard (#4624): a running record means a resumed
|
||||
// session re-entered the dispatch step. The worker must be reconciled
|
||||
// (worker-status → completion-reconciliation), never re-spawned.
|
||||
const hint = 'A worker is already recorded RUNNING for this worktree. Reconcile it (worktree worker-status, then execute-phase/steps/completion-reconciliation.md) before any new dispatch — never re-dispatch a recorded plan.';
|
||||
writeErr(`[gsd] worktree.worker-record: already_running — ${hint}\n`);
|
||||
write(`${JSON.stringify({ ok: false, reason: 'already_running', hint, record: existing }, null, 2)}\n`);
|
||||
process.exitCode = 1;
|
||||
return;
|
||||
}
|
||||
|
||||
const record: WorkerRecord = {
|
||||
agentId: path.basename(resolvedWorktree),
|
||||
pid,
|
||||
plan,
|
||||
worktreePath: resolvedWorktree,
|
||||
summaryPath: path.resolve(cwd, summaryPath),
|
||||
logFile: logFile ? path.resolve(cwd, logFile) : '',
|
||||
startedAt: new Date().toISOString(),
|
||||
state: 'running',
|
||||
exitCode: null,
|
||||
note: '',
|
||||
completedAt: null,
|
||||
};
|
||||
try {
|
||||
writeWorkerRecord(recordPath, record, deps);
|
||||
} catch (err) {
|
||||
writeErr(`[gsd] worktree.worker-record: write_failed — ${(err as Error).message}\n`);
|
||||
write(`${JSON.stringify({ ok: false, reason: 'write_failed', error: (err as Error).message }, null, 2)}\n`);
|
||||
process.exitCode = 1;
|
||||
return;
|
||||
}
|
||||
write(`${JSON.stringify({ ok: true, record }, null, 2)}\n`);
|
||||
}
|
||||
|
||||
function workerStatusView(
|
||||
record: WorkerRecord,
|
||||
deps: Record<string, unknown> = {}
|
||||
): Record<string, unknown> {
|
||||
// Only ESRCH is dead (defaultIsPidAlive contract): a verdict feeding a
|
||||
// merge/reconcile decision must fail toward ALIVE on unrecognized errnos.
|
||||
const pidAlive = (deps.isPidAlive as ((pid: number) => boolean)) || defaultIsPidAlive;
|
||||
const exists = (deps.existsSync as ((p: string) => boolean)) || fs.existsSync;
|
||||
const summaryExists = exists(record.summaryPath);
|
||||
const alive = record.state === 'running' && pidAlive(record.pid);
|
||||
return {
|
||||
agentId: record.agentId,
|
||||
pid: record.pid,
|
||||
plan: record.plan,
|
||||
worktreePath: record.worktreePath,
|
||||
summaryPath: record.summaryPath,
|
||||
logFile: record.logFile,
|
||||
state: record.state,
|
||||
exitCode: record.exitCode,
|
||||
note: record.note,
|
||||
startedAt: record.startedAt,
|
||||
completedAt: record.completedAt,
|
||||
pidAlive: record.state === 'running' ? alive : null,
|
||||
summaryExists,
|
||||
needsReconciliation: record.state === 'running' && !alive,
|
||||
};
|
||||
}
|
||||
|
||||
function readWorkerRecordsFromRoot(
|
||||
root: string,
|
||||
readFile: (p: string) => string,
|
||||
readdir: (p: string) => string[]
|
||||
): { path: string; record: WorkerRecord }[] {
|
||||
let entries: string[] = [];
|
||||
try {
|
||||
entries = readdir(root).filter((f) => f.endsWith('.worker.json'));
|
||||
} catch { /* root missing — no workers ever recorded */ return []; }
|
||||
const out: { path: string; record: WorkerRecord }[] = [];
|
||||
for (const entry of entries) {
|
||||
const recordPath = path.join(root, entry);
|
||||
try {
|
||||
out.push({ path: recordPath, record: JSON.parse(readFile(recordPath)) as WorkerRecord });
|
||||
} catch { // a torn/partial record must not hide the others
|
||||
out.push({ path: recordPath, record: null as unknown as WorkerRecord });
|
||||
}
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
function cmdWorktreeWorkerStatus(cwd: string, args: string[] = [], deps: Record<string, unknown> = {}): void {
|
||||
const flag = (name: string): string => {
|
||||
const i = args.indexOf(name);
|
||||
if (i < 0 || i + 1 >= args.length) return '';
|
||||
return args[i + 1];
|
||||
};
|
||||
const write = (deps.write as ((s: string) => void)) || ((s: string) => process.stdout.write(s));
|
||||
const writeErr = (deps.writeErr as ((s: string) => void)) || ((s: string) => process.stderr.write(s));
|
||||
|
||||
const worktreePath = flag('--path');
|
||||
const root = flag('--root');
|
||||
if (!worktreePath === !root) { // exactly one of the two
|
||||
writeErr('Usage: worktree worker-status (--path <worktree> | --root <worktrees-dir>)\n');
|
||||
process.exitCode = 2;
|
||||
return;
|
||||
}
|
||||
const readFile = (deps.readFile as ((p: string) => string)) || ((p: string) => fs.readFileSync(p, 'utf8'));
|
||||
const readdir = (deps.readdir as ((p: string) => string[])) || ((p: string) => fs.readdirSync(p));
|
||||
|
||||
const views: Record<string, unknown>[] = [];
|
||||
if (worktreePath) {
|
||||
const recordPath = workerRecordPath(path.resolve(cwd, worktreePath));
|
||||
let record: WorkerRecord | null = null;
|
||||
let unreadable = false;
|
||||
try {
|
||||
record = JSON.parse(readFile(recordPath)) as WorkerRecord;
|
||||
} catch {
|
||||
// A record that exists but cannot be parsed is NOT "never dispatched" —
|
||||
// conflating the two invites a re-dispatch. Surface it as a candidate.
|
||||
unreadable = fs.existsSync(recordPath);
|
||||
}
|
||||
if (unreadable) {
|
||||
// exists but unparseable — a reconciliation candidate, never "never dispatched"
|
||||
views.push({ state: 'unreadable', needsReconciliation: true, recordPath });
|
||||
} else if (record) {
|
||||
views.push(workerStatusView(record, deps));
|
||||
} else {
|
||||
// no record file: genuinely never dispatched (found:false)
|
||||
write(`${JSON.stringify({ ok: true, found: false, workers: [] }, null, 2)}\n`);
|
||||
return;
|
||||
}
|
||||
} else {
|
||||
for (const { record } of readWorkerRecordsFromRoot(path.resolve(cwd, root), readFile, readdir)) {
|
||||
if (!record) { // torn record is itself a reconciliation candidate
|
||||
views.push({ state: 'unreadable', needsReconciliation: true });
|
||||
continue;
|
||||
}
|
||||
views.push(workerStatusView(record, deps));
|
||||
}
|
||||
}
|
||||
write(`${JSON.stringify({ ok: true, found: true, workers: views }, null, 2)}\n`);
|
||||
}
|
||||
|
||||
function cmdWorktreeWorkerComplete(cwd: string, args: string[] = [], deps: Record<string, unknown> = {}): void {
|
||||
const flag = (name: string): string => {
|
||||
const i = args.indexOf(name);
|
||||
if (i < 0 || i + 1 >= args.length) return '';
|
||||
return args[i + 1];
|
||||
};
|
||||
const write = (deps.write as ((s: string) => void)) || ((s: string) => process.stdout.write(s));
|
||||
const writeErr = (deps.writeErr as ((s: string) => void)) || ((s: string) => process.stderr.write(s));
|
||||
|
||||
const worktreePath = flag('--path');
|
||||
const exitText = flag('--exit-code');
|
||||
const note = flag('--note');
|
||||
if (!worktreePath || !exitText) {
|
||||
writeErr('Usage: worktree worker-complete --path <worktree> --exit-code <n> [--note <recovery info>]\n');
|
||||
process.exitCode = 2;
|
||||
return;
|
||||
}
|
||||
const exitCode = Number(exitText);
|
||||
if (!Number.isInteger(exitCode)) {
|
||||
writeErr(`[gsd] worktree.worker-complete: invalid --exit-code: ${exitText}\n`);
|
||||
process.exitCode = 2;
|
||||
return;
|
||||
}
|
||||
|
||||
const recordPath = workerRecordPath(path.resolve(cwd, worktreePath));
|
||||
const readFile = (deps.readFile as ((p: string) => string)) || ((p: string) => fs.readFileSync(p, 'utf8'));
|
||||
let record: WorkerRecord;
|
||||
try {
|
||||
record = JSON.parse(readFile(recordPath)) as WorkerRecord;
|
||||
} catch (err) {
|
||||
writeErr(`[gsd] worktree.worker-complete: no_record — ${(err as Error).message}\n`);
|
||||
write(`${JSON.stringify({ ok: false, reason: 'no_record', error: (err as Error).message }, null, 2)}\n`);
|
||||
process.exitCode = 1;
|
||||
return;
|
||||
}
|
||||
const alreadyComplete = record.state === 'complete';
|
||||
record.state = 'complete';
|
||||
record.exitCode = exitCode;
|
||||
record.note = note || record.note || '';
|
||||
record.completedAt = record.completedAt || new Date().toISOString();
|
||||
try {
|
||||
writeWorkerRecord(recordPath, record, deps);
|
||||
} catch (err) {
|
||||
writeErr(`[gsd] worktree.worker-complete: write_failed — ${(err as Error).message}\n`);
|
||||
write(`${JSON.stringify({ ok: false, reason: 'write_failed', error: (err as Error).message }, null, 2)}\n`);
|
||||
process.exitCode = 1;
|
||||
return;
|
||||
}
|
||||
write(`${JSON.stringify({ ok: true, alreadyComplete, record }, null, 2)}\n`);
|
||||
}
|
||||
|
||||
// Unused exports kept for API compatibility
|
||||
void parseWorktreeListPaths;
|
||||
|
||||
@@ -2338,6 +2627,10 @@ export = {
|
||||
cmdWorktreeCreate,
|
||||
reapOrphanWorktrees,
|
||||
cmdWorktreeReapOrphans,
|
||||
workerRecordPath,
|
||||
cmdWorktreeWorkerRecord,
|
||||
cmdWorktreeWorkerStatus,
|
||||
cmdWorktreeWorkerComplete,
|
||||
resolveWorktreeRoot,
|
||||
pruneOrphanedWorktrees,
|
||||
};
|
||||
|
||||
202
tests/worktree-worker-log.test.cjs
Normal file
202
tests/worktree-worker-log.test.cjs
Normal file
@@ -0,0 +1,202 @@
|
||||
'use strict';
|
||||
|
||||
/**
|
||||
* #4624 — `worktree worker-record` / `worker-status` / `worker-complete`: the
|
||||
* durable per-worker lifecycle record for the orchestrator-worktree backend.
|
||||
*
|
||||
* WHY: the dispatch fragment spawned external executors behind a bare shell
|
||||
* `wait`. When the orchestrator's turn ended before a worker finished, no
|
||||
* launch identity, result location, or terminal outcome was persisted, so a
|
||||
* resumed session re-derived everything from manual PID/log discovery and
|
||||
* could re-dispatch a recorded plan (#4624). These tests pin the state
|
||||
* machine that closes that gap:
|
||||
*
|
||||
* record (running) ──▶ status (pidAlive/summaryExists/needsReconciliation)
|
||||
* ▲ │
|
||||
* │ re-record only after ▼
|
||||
* └── terminal ───── worker-complete (idempotent)
|
||||
*
|
||||
* Determinism: `pidAlive` is injected everywhere (no live-PID probing), the
|
||||
* summary file is real fs, and the CLI end-to-end case uses a self-PID or a
|
||||
* max-int32 pid (ESRCH on every platform) so no assertion depends on the
|
||||
* runner's process table.
|
||||
*/
|
||||
|
||||
const { describe, test, beforeEach, afterEach } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const path = require('node:path');
|
||||
const { createTempDir, cleanup } = require('./helpers.cjs');
|
||||
const { runNode } = require('./helpers/process-seam.cjs');
|
||||
const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs');
|
||||
|
||||
const ROOT = path.join(__dirname, '..');
|
||||
const GSD_TOOLS = path.join(ROOT, 'gsd-core', 'bin', 'gsd-tools.cjs');
|
||||
const {
|
||||
cmdWorktreeWorkerRecord,
|
||||
cmdWorktreeWorkerStatus,
|
||||
cmdWorktreeWorkerComplete,
|
||||
} = require('../gsd-core/bin/lib/worktree-safety.cjs');
|
||||
|
||||
describe('#4624 worktree worker lifecycle records', () => {
|
||||
let dir;
|
||||
let wtPath;
|
||||
let summaryPath;
|
||||
|
||||
beforeEach(() => {
|
||||
dir = createTempDir('rwt-4624');
|
||||
wtPath = path.join(dir, 'agent-p3-1700000000');
|
||||
summaryPath = path.join(dir, '3-SUMMARY.md');
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
cleanup(dir);
|
||||
});
|
||||
|
||||
function run(cmd, args, deps = {}) {
|
||||
let exitCode = 0;
|
||||
let stdout = '';
|
||||
let stderr = '';
|
||||
const scoped = {
|
||||
write: (s) => { stdout += s; },
|
||||
writeErr: (s) => { stderr += s; },
|
||||
...deps,
|
||||
};
|
||||
// The cmd functions set process.exitCode directly; capture it without
|
||||
// leaking into the runner's own exit status.
|
||||
const prior = process.exitCode;
|
||||
process.exitCode = 0;
|
||||
if (cmd === 'record') cmdWorktreeWorkerRecord(dir, args, scoped);
|
||||
else if (cmd === 'status') cmdWorktreeWorkerStatus(dir, args, scoped);
|
||||
else if (cmd === 'complete') cmdWorktreeWorkerComplete(dir, args, scoped);
|
||||
exitCode = process.exitCode || 0;
|
||||
process.exitCode = prior;
|
||||
return { exitCode, stdout, stderr };
|
||||
}
|
||||
|
||||
const dead = () => false;
|
||||
|
||||
test('record persists a running launch record beside the worktree with an absolute summary path', () => {
|
||||
const r = run('record', ['--path', wtPath, '--pid', '4242', '--plan', '3',
|
||||
'--summary-path', summaryPath, '--log-file', `${wtPath}.worker.log`]);
|
||||
assert.equal(r.exitCode, 0);
|
||||
const out = JSON.parse(r.stdout);
|
||||
assert.equal(out.ok, true);
|
||||
assert.equal(out.record.state, 'running');
|
||||
assert.equal(out.record.agentId, 'agent-p3-1700000000');
|
||||
assert.equal(out.record.summaryPath, summaryPath, 'summary path must resolve to absolute (resume reads it from any cwd)');
|
||||
const onDisk = JSON.parse(require('node:fs').readFileSync(`${wtPath}.worker.json`, 'utf8'));
|
||||
assert.equal(onDisk.state, 'running');
|
||||
assert.equal(onDisk.pid, 4242);
|
||||
});
|
||||
|
||||
test('record refuses a second dispatch while a running record exists (duplicate-dispatch guard)', () => {
|
||||
run('record', ['--path', wtPath, '--pid', '4242', '--plan', '3', '--summary-path', summaryPath]);
|
||||
const r = run('record', ['--path', wtPath, '--pid', '4242', '--plan', '3', '--summary-path', summaryPath]);
|
||||
assert.equal(r.exitCode, 1, 're-dispatching a recorded running worker must fail closed');
|
||||
const out = JSON.parse(r.stdout);
|
||||
assert.equal(out.ok, false);
|
||||
assert.equal(out.reason, 'already_running');
|
||||
assert.match(r.stderr, /never re-dispatch a recorded plan/);
|
||||
});
|
||||
|
||||
test('record after a terminal record is allowed (fresh lifecycle at the same path)', () => {
|
||||
run('record', ['--path', wtPath, '--pid', '4242', '--plan', '3', '--summary-path', summaryPath]);
|
||||
run('complete', ['--path', wtPath, '--exit-code', '0']);
|
||||
const r = run('record', ['--path', wtPath, '--pid', '4242', '--plan', '3', '--summary-path', summaryPath]);
|
||||
assert.equal(r.exitCode, 0);
|
||||
const out = JSON.parse(r.stdout);
|
||||
assert.equal(out.ok, true);
|
||||
assert.equal(out.record.state, 'running');
|
||||
});
|
||||
|
||||
test('status composes the recovery view: dead pid + running record = needsReconciliation', () => {
|
||||
run('record', ['--path', wtPath, '--pid', '4242', '--plan', '3', '--summary-path', summaryPath]);
|
||||
const r = run('status', ['--path', wtPath], { isPidAlive: dead });
|
||||
const out = JSON.parse(r.stdout);
|
||||
assert.equal(out.found, true);
|
||||
assert.equal(out.workers[0].state, 'running');
|
||||
assert.equal(out.workers[0].pidAlive, false);
|
||||
assert.equal(out.workers[0].summaryExists, false, 'no SUMMARY written yet — nothing artifact-complete to reconcile');
|
||||
assert.equal(out.workers[0].needsReconciliation, true);
|
||||
});
|
||||
|
||||
test('status surfaces artifact-completion independently of the process result', () => {
|
||||
run('record', ['--path', wtPath, '--pid', '4242', '--plan', '3', '--summary-path', summaryPath]);
|
||||
require('node:fs').writeFileSync(summaryPath, '# SUMMARY\n');
|
||||
const r = run('status', ['--path', wtPath], { isPidAlive: dead });
|
||||
const out = JSON.parse(r.stdout);
|
||||
assert.equal(out.workers[0].summaryExists, true, 'the exit-after-artifact-completion shape the issue describes');
|
||||
assert.equal(out.workers[0].needsReconciliation, true);
|
||||
});
|
||||
|
||||
test('status --root scans every record and flags torn records as reconciliation candidates', () => {
|
||||
run('record', ['--path', wtPath, '--pid', '4242', '--plan', '3', '--summary-path', summaryPath]);
|
||||
require('node:fs').writeFileSync(path.join(dir, 'torn.worker.json'), '{ truncated');
|
||||
const r = run('status', ['--root', dir]);
|
||||
const out = JSON.parse(r.stdout);
|
||||
assert.equal(out.workers.length, 2);
|
||||
const torn = out.workers.find((w) => w.state === 'unreadable');
|
||||
assert.ok(torn, 'a torn record must surface, never silently vanish');
|
||||
assert.equal(torn.needsReconciliation, true);
|
||||
});
|
||||
|
||||
test('status with no record reports found:false (unrecorded worktree)', () => {
|
||||
const r = run('status', ['--path', wtPath]);
|
||||
const out = JSON.parse(r.stdout);
|
||||
assert.equal(out.found, false);
|
||||
assert.deepEqual(out.workers, []);
|
||||
});
|
||||
|
||||
test('complete is idempotent: a resumed session re-marking a terminal worker still succeeds', () => {
|
||||
run('record', ['--path', wtPath, '--pid', '4242', '--plan', '3', '--summary-path', summaryPath]);
|
||||
const first = run('complete', ['--path', wtPath, '--exit-code', '1', '--note', 'blocked: missing toolchain']);
|
||||
assert.equal(first.exitCode, 0);
|
||||
const again = run('complete', ['--path', wtPath, '--exit-code', '1', '--note', 'blocked: missing toolchain']);
|
||||
assert.equal(again.exitCode, 0);
|
||||
const out = JSON.parse(again.stdout);
|
||||
assert.equal(out.alreadyComplete, true);
|
||||
const view = JSON.parse(run('status', ['--path', wtPath], { isPidAlive: dead }).stdout);
|
||||
assert.equal(view.workers[0].state, 'complete');
|
||||
assert.equal(view.workers[0].exitCode, 1);
|
||||
assert.equal(view.workers[0].needsReconciliation, false, 'a terminal record never needs reconciliation');
|
||||
});
|
||||
|
||||
test('complete without a record fails with no_record (never fabricates state)', () => {
|
||||
const r = run('complete', ['--path', wtPath, '--exit-code', '0']);
|
||||
assert.equal(r.exitCode, 1);
|
||||
assert.equal(JSON.parse(r.stdout).reason, 'no_record');
|
||||
});
|
||||
|
||||
test('usage errors: missing/invalid --pid, and status requires exactly one of --path/--root', () => {
|
||||
assert.equal(run('record', ['--path', wtPath, '--plan', '3', '--summary-path', summaryPath]).exitCode, 2);
|
||||
assert.equal(run('record', ['--path', wtPath, '--pid', 'not-a-pid', '--plan', '3', '--summary-path', summaryPath]).exitCode, 2);
|
||||
assert.equal(run('record', ['--path', wtPath, '--pid', '4242', '--plan', '3']).exitCode, 2, '--summary-path is required — an omitted path would make summaryExists vacuously true');
|
||||
assert.equal(run('status', ['--path', wtPath, '--root', dir]).exitCode, 2);
|
||||
assert.equal(run('status', []).exitCode, 2);
|
||||
});
|
||||
|
||||
test('end-to-end through the gsd-tools CLI router: record → status(dead pid) → complete', () => {
|
||||
const record = runNode([GSD_TOOLS, 'query', 'worktree', 'worker-record',
|
||||
'--path', wtPath, '--pid', String(process.pid), '--plan', '3', '--summary-path', summaryPath],
|
||||
{ timeoutMs: PROBE_TIMEOUT_MS });
|
||||
assert.equal(record.exitCode, 0, `record failed: ${record.stderr}`);
|
||||
|
||||
const deadPid = 2147483646; // max int32 — ESRCH on every platform, never a real pid
|
||||
runNode([GSD_TOOLS, 'query', 'worktree', 'worker-record',
|
||||
'--path', path.join(dir, 'agent-p4-1700000001'), '--pid', String(deadPid),
|
||||
'--plan', '4', '--summary-path', summaryPath], { timeoutMs: PROBE_TIMEOUT_MS });
|
||||
|
||||
const status = runNode([GSD_TOOLS, 'query', 'worktree', 'worker-status', '--root', dir],
|
||||
{ timeoutMs: PROBE_TIMEOUT_MS });
|
||||
assert.equal(status.exitCode, 0);
|
||||
const workers = JSON.parse(status.stdout).workers;
|
||||
const self = workers.find((w) => w.pid === process.pid);
|
||||
const gone = workers.find((w) => w.pid === deadPid);
|
||||
assert.equal(self.needsReconciliation, false, 'self pid is alive');
|
||||
assert.equal(gone.needsReconciliation, true, 'dead-pid worker needs reconciliation');
|
||||
|
||||
const complete = runNode([GSD_TOOLS, 'query', 'worktree', 'worker-complete',
|
||||
'--path', wtPath, '--exit-code', '0'], { timeoutMs: PROBE_TIMEOUT_MS });
|
||||
assert.equal(complete.exitCode, 0);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user