Skip to content

Commit 0fcf46b

Browse files
authored
fix(worktree): a full disk must not crash the daemon — guard preserve-marker write + classify ENOSPC as infra (INT-2521) (#270)
Observed live: the disk hit 100% and the daemon crash-looped. Root cause was an unguarded writeFileSync of the .openswarm-preserved marker in preserveWorktree(): ENOSPC threw straight through executePipeline and took the whole runner process down (Node printed its version and exited). preserveWorktree() now: - never throws on a marker-write failure — the crash this guard exists to prevent; - reports NOT preserved (false) instead of a false 'true'. The marker is the only thing that protects a tree from the next createWorktree() recreate and the heartbeat orphan-sweep, so claiming preservation without it is a lie that would lose the work on retry anyway (reviewer-caught); - best-effort drops any truncated partial marker, then best-effort removes the worktree to RECLAIM space — exactly what helps when the disk is full. Both steps are guarded so neither re-introduces the crash. isInfraError() now classifies ENOSPC / 'no space left on device' / ENOMEM as infrastructure (host resource exhaustion) so a disk-full failure gets a backoff retry instead of counting toward STUCK — the contract preserveWorktree's comment now relies on. 'no space left on device' uses the full phrase so prose like 'no space left for the label' is not mis-flagged. Added to the classification matrix. Verified: tsc + oxlint clean, full suite 148 files / 1756 passed; openswarm review APPROVE. Found while unblocking the daemon (disk 100% full) during a /goal review-and-merge session.
1 parent a40d821 commit 0fcf46b

5 files changed

Lines changed: 60 additions & 6 deletions

File tree

src/adapters/errorClassification.test.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,16 @@ describe('isInfraError (INT-2010)', () => {
6262
expect(isInfraError(new Error('the reviewer stage should reject empty diffs'))).toBe(false);
6363
});
6464

65+
it('recognises host resource exhaustion (disk full / OOM) as infra, not a task failure (INT-2521)', () => {
66+
// A full disk crashed the daemon in production; ENOSPC must backoff-retry, not STUCK.
67+
expect(isInfraError(new Error('ENOSPC: no space left on device, open \'/repo/worktree/.openswarm-preserved\''))).toBe(true);
68+
expect(isInfraError(new Error('write failed: no space left on device'))).toBe(true);
69+
expect(isInfraError(Object.assign(new Error('spawn failed'), { cause: { code: 'ENOSPC' } }))).toBe(true);
70+
expect(isInfraError(new Error('ENOMEM: not enough memory'))).toBe(true);
71+
// …but ordinary prose mentioning space is NOT infra
72+
expect(isInfraError(new Error('the layout leaves no space left for the label'))).toBe(false);
73+
});
74+
6575
it('recognises local server capacity failures (5xx / loading / overloaded) (INT-2520)', () => {
6676
expect(isInfraError(new Error('Local API error (503): model is loading'))).toBe(true);
6777
expect(isInfraError(new Error('Local API error (502): bad gateway'))).toBe(true);

src/adapters/errorClassification.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,9 @@ const INFRA_ERROR_PATTERNS = [
3333
'enotfound',
3434
'socket hang up',
3535
'network error',
36+
'enospc', // disk full — an environment condition, not a task verdict; backoff-retry once space frees (INT-2521)
37+
'no space left on device', // the human-readable form of ENOSPC (full phrase, so prose like "no space left for the label" is not mis-flagged)
38+
'enomem', // out of memory — same class: host resource exhaustion, not a bad edit
3639
'git-tracker:', // git snapshot/diff failed mid-run — infra, not a task verdict (colon-anchored to avoid prose) (INT-2521)
3740
'reviewer-stage:', // reviewer ran but its output couldn't be parsed into a verdict — infra, not a quality reject (INT-2521)
3841
'fetch failed', // undici: the real code hides in error.cause.code (checked below)

src/adapters/errorClassificationContract.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ const CASES: Array<{ name: string; err: string | Error; bucket: Bucket }> = [
5454
{ name: 'git-tracker snapshot/diff failure', err: 'git-tracker: diff since snapshot failed: fatal: bad object', bucket: 'infra_error' },
5555
{ name: 'reviewer parse crash', err: 'reviewer-stage: produced no parseable verdict: TypeError x', bucket: 'infra_error' },
5656
{ name: 'scheduler hard watchdog', err: 'Task timed out after 3600000ms (scheduler hard watchdog)', bucket: 'infra_error' },
57+
{ name: 'disk full (ENOSPC)', err: "ENOSPC: no space left on device, open '/repo/worktree/.openswarm-preserved'", bucket: 'infra_error' },
5758

5859
// ---- genuine task failures (MUST count toward STUCK) ----
5960
{ name: 'code TypeError', err: 'TypeError: cannot read property foo of undefined', bucket: 'task_failure' },

src/support/worktreeManager.test.ts

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { execFileSync } from 'node:child_process';
2-
import { existsSync, lstatSync, mkdirSync, readFileSync, realpathSync, rmSync, writeFileSync } from 'node:fs';
2+
import { chmodSync, existsSync, lstatSync, mkdirSync, readFileSync, realpathSync, rmSync, writeFileSync } from 'node:fs';
33
import { tmpdir } from 'node:os';
44
import { join, resolve } from 'node:path';
55
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
@@ -209,6 +209,24 @@ describe('preserveWorktree → createWorktree resume roundtrip (INT-2503)', () =
209209
expect(existsSync(info.worktreePath)).toBe(false);
210210
});
211211

212+
it('does NOT crash when the preserve-marker write fails (ENOSPC/EACCES) — reports NOT preserved (INT-2521)', async () => {
213+
const info = await createWorktree(repo, 'INT-9', 'swarm/INT-9-test');
214+
writeFileSync(join(info.worktreePath, 'app.py'), 'base\npartial-impl\n'); // dirty work
215+
// A read-only worktree dir makes the marker writeFileSync throw — exactly what a
216+
// full disk (ENOSPC) did in production, where an unguarded write crashed the whole
217+
// daemon via executePipeline. preserveWorktree must swallow it and honestly report
218+
// NOT preserved (false): the marker is the only thing that would protect the tree
219+
// from the next createWorktree()/sweep, so it must never claim a preservation it
220+
// can't back. It also never leaves a marker behind.
221+
chmodSync(info.worktreePath, 0o555);
222+
try {
223+
await expect(preserveWorktree(info, 'disk full')).resolves.toBe(false);
224+
expect(existsSync(join(info.worktreePath, '.openswarm-preserved'))).toBe(false); // marker never written
225+
} finally {
226+
if (existsSync(info.worktreePath)) chmodSync(info.worktreePath, 0o755); // restore for cleanup
227+
}
228+
});
229+
212230
it('PRESERVES (does not delete) when git status FAILS — cannot confirm clean (INT-2521)', async () => {
213231
const info = await createWorktree(repo, 'INT-9', 'swarm/INT-9-test');
214232
writeFileSync(join(info.worktreePath, 'app.py'), 'base\nreal-partial-work\n');

src/support/worktreeManager.ts

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -240,11 +240,33 @@ export async function preserveWorktree(info: WorktreeInfo, reason: string): Prom
240240
return false;
241241
}
242242
const fileCount = dirty === null ? 0 : dirty.split('\n').filter(Boolean).length;
243-
writeFileSync(
244-
join(worktreePath, PRESERVE_MARKER),
245-
JSON.stringify({ issueId: info.issueId, branchName: info.branchName, reason, at: new Date().toISOString() }, null, 2),
246-
'utf8',
247-
);
243+
// The resume marker is the ONLY thing that protects this tree from the next
244+
// createWorktree() recreate (which deletes the branch + worktree, ~L286/L299) and
245+
// the heartbeat orphan-sweep (pruneWorktrees drops marker-less trees). So if we
246+
// can't write it, we CANNOT honestly claim the work is preserved. Two rules here:
247+
// 1. Never throw. An unguarded writeFileSync threw ENOSPC straight through
248+
// executePipeline and crashed the whole daemon under a full disk (observed
249+
// live: disk 100% → runner crash-loop). (INT-2521)
250+
// 2. Don't lie. Best-effort remove the tree to reclaim space — which is exactly
251+
// what helps when the disk is full — and report NOT preserved. ENOSPC is an
252+
// infra error, so the task retries fresh from origin/main once space frees.
253+
const markerPath = join(worktreePath, PRESERVE_MARKER);
254+
try {
255+
writeFileSync(
256+
markerPath,
257+
JSON.stringify({ issueId: info.issueId, branchName: info.branchName, reason, at: new Date().toISOString() }, null, 2),
258+
'utf8',
259+
);
260+
} catch (err) {
261+
console.warn(`[Worktree] Preserve-marker write failed — cannot preserve, reclaiming space: ${worktreePath}`, err instanceof Error ? err.message : err);
262+
// A failed write can leave a truncated marker behind. Drop it first so a later
263+
// createWorktree()/prune doesn't treat the corpse as a real preserved tree, THEN
264+
// best-effort remove the whole worktree to reclaim space. Both are best-effort —
265+
// neither may throw (that is the very crash this guard exists to prevent).
266+
try { rmSync(markerPath, { force: true }); } catch { /* read-only/ENOSPC — leave it; prune treats an unreadable marker as sweepable */ }
267+
await removeWorktree(info).catch((e) => console.warn(`[Worktree] Cleanup after failed preserve also failed: ${worktreePath}`, e instanceof Error ? e.message : e));
268+
return false;
269+
}
248270
const label = dirty === null ? 'git status unavailable — preserved to be safe' : `${fileCount} dirty files`;
249271
console.log(`[Worktree] Preserved for retry (${label}, ${reason}): ${worktreePath}`);
250272
return true;

0 commit comments

Comments
 (0)