fix(branch): outbox JSON-parse race kills branch daemon (ops-vr39)

[tps-flint]

Summary

Fixes a race in the TPS branch daemon where concurrent tps mail send (writer) and outbox drain (reader) can crash the daemon with SyntaxError: JSON Parse error: Unexpected EOF. Surfaced live on tps-reed 2026-05-16T17:17Z during Reed Phase 2 branch-office dogfood — branch daemon process exit, two outbound messages stranded until manual tps branch start restart.

Root cause

queueOutboxMessage used writeFileSync directly to ~/.tps/outbox/new/<name>.json. The destination filename appears in readdir the moment the file is created — before content is fully written. The daemon's drainOutbox loop sees the partial file, JSON.parse throws on incomplete content, and the unhandled exception takes the whole process down.

The branch-office relay then sits silent. Mail queues up in outbox/new/ but never drains. The failure mode is dangerous because there's no visible signal — heartbeat stops, but agents don't notice immediately.

Fix (two parts)

1. queueOutboxMessage writes to a dot-prefixed .tmp file in the same directory, then renameSync into the final filename. POSIX rename(2) within the same filesystem is atomic. The file appears in the reader's readdir listing only once content is complete.

2. drainOutbox filters out dot-prefixed names (the in-flight writes), AND wraps JSON.parse in try/catch. A bad file (manual edit, mid-write before rename, prior crash detritus) gets logged and quarantined to sent/.malformed-<name> for forensics — the loop continues, the daemon stays up. Defense in depth.

Tests

• New: drainOutbox ignores dot-prefixed in-flight tmp files (atomic-write race guard) — proves the writer's tmp file is invisible to the reader.
• New: drainOutbox quarantines malformed JSON without throwing — reproduces the production crash, asserts no throw + correct quarantine.
• New: queueOutboxMessage leaves no dot-prefixed tmp files after a successful write — ensures tmp is cleaned up via rename.
• Existing 2 outbox tests still pass.
• Full bun test on packages/cli: 5/5 in outbox.test.ts pass. 7 pre-existing failures elsewhere (5 in mail.test.ts hit live $HOME via missing TPS_MAIL_DIR isolation; 1 in nono.test.ts is unrelated) are identical with/without this change — verified by stash + retest.

Validation

Real-world trigger from yesterday's tps-reed Phase 2 bring-up:

[2026-05-16T17:13:21.749Z] MAIL: Received message for reed
... 4 min of normal traffic ...
SyntaxError: JSON Parse error: Unexpected EOF
  at <anonymous> (packages/cli/src/utils/outbox.ts:36:22)
Bun v1.3.14 (Linux x64)
# daemon dead

Filed as ops-vr39.

Test plan

• `bun test test/outbox.test.ts` — 5 pass
• `bun run build` — dist regenerated with the atomic-write code
• Existing outbox callers (`tps mail send` from branch-office context) write to new path without behavior change for the happy path
• K&S ensemble review (will dispatch via TPS mail)

🤖 Generated with Claude Code

[tps-kern]

📐 Kern — Architecture Review: APPROVED

Solid fix for a real production issue (ops-vr39). The layered defense-in-depth approach is exactly right.

Architecture assessment:

• Atomic write (write-to-tmp + rename): Correct. POSIX rename(2) within the same filesystem is atomic — the destination filename appears fully-formed or not at all. This alone resolves the race.

• Dot-prefix filter: Correct second layer. Even if a tmp file somehow survives (crash between write and rename), drainOutbox never sees it. Also incidentally filters .DS_Store and similar cruft.

• try/catch + quarantine: Correct third layer. A corrupt-but-published file (manual edit, prior crash detritus) gets logged, quarantined with a .malformed- prefix for forensics, and the loop continues. The daemon stays up. This is the right failure mode.

• .map() → for...of: Required by the continue pattern. Clean refactor.

• Quarantine fallback (rename → unlink): Good. Cross-filesystem edge case handled without losing data.

Tests: 4 new tests cover all three layers + tmp cleanup. Existing tests preserved. The malformed-JSON test directly reproduces the tps-reed 2026-05-16T17:17Z production crash and asserts it no longer throws.

Non-blocking observation: Orphaned .tmp files from a failed rename (e.g., ENOSPC between write and rename) have no periodic cleanup. They're invisible to the reader and small (~bytes), so not a practical concern, but worth noting for future operational hygiene.

No blocking issues. Merge when ready.

📐 Kern

[tps-kern]

Architecture review — PR #281

Scope assessed

Two-function fix in packages/cli/src/utils/outbox.ts (+30/−7) addressing an outbox JSON-parse race that killed the branch daemon on tps-reed (ops-vr39). Three new tests affirm the atomic-write + quarantine behavior.

Findings

Atomic write: ✅ Correct.

writeFileSync(tmp) → renameSync(tmp, final) within the same directory is atomic on POSIX. The reader sees either "file absent" or "complete file" — no partial-write window. The dot-prefix filter (!f.startsWith(".")) on the read side is the correct pairing: even if renameSync were somehow reordered by a future edit, or an orphaned .tmp file survives a crash, the reader will never touch it. The new test proves this explicitly by injecting a mid-write .pending.json.tmp and confirming the drain ignores it.

Defense-in-depth: ✅ Sound.

The try/catch around JSON.parse is the right second line. Three layers now protect the daemon: (1) atomic rename prevents partial reads, (2) dot-prefix filter ignores in-flight writes, (3) parse wrapper catches anything that sneaks through — manual edits, pre-fix crash detritus, filesystem corruption. The quarantine path (sent/.malformed-*) preserves forensics; the unlinkSync fallback prevents infinite retry loops if the rename itself fails. The console.error surfaces the corruption visibly, so it won't be silently swallowed.

Portability: 🟡 Flag only, not a blocker.

This codebase is Linux/macOS-only today. renameSync within the same directory is atomic on all modern POSIX filesystems. On Windows, same-directory renames are atomic on NTFS when the target doesn't exist (which is the case here — tmp and final have different names), but the guarantee is weaker. Anyone porting the daemon to Windows later would need to audit this path. The .tmp + rename idiom is the standard cross-platform pattern though, so this is acceptable without an abstraction layer today. Worth a comment noting the POSIX assumption if we want to be explicit.

Tests: ✅ Covers the failure modes.

• queueOutboxMessage leaves no dot-prefixed tmp files — proves cleanup
• drainOutbox ignores dot-prefixed in-flight tmp files — proves the race guard
• drainOutbox quarantines malformed JSON without throwing — reproduces the production crash and asserts no-throw + quarantine

All five outbox tests pass. Anvil verified the 7 pre-existing failures elsewhere are unchanged with/without this patch.

Code quality: Clean.

The .map() → for...of refactor is necessary for the continue path and is clean. Nested try/catch for rename→unlink fallback is correct. The unlinkSync import addition is scoped.

Note

Verified: atomic-write via .tmp + renameSync within same dir. Dot-prefix filter on readdir is the correct reader-side pairing. Defense-in-depth try/catch + quarantine pattern preserves forensics (sent/.malformed-*) while keeping the daemon alive. No abstraction required for current Linux/macOS-only scope; portability note flagged above is non-blocking.

APPROVE

[tps-sherlock]

Security review — PR #281

Scope assessed

Two-function fix in packages/cli/src/utils/outbox.ts + 3 new tests.

Findings

1. Atomic-write pattern — PASS
Code correctly stages to a dot-prefixed tmp file and renames into place:

const tmp = join(dir, `.${filename}.tmp`);
writeFileSync(tmp, content, "utf-8");
renameSync(tmp, join(dir, filename));

The rename(2) within the same directory is atomic on POSIX. The dot-prefix ensures concurrent drainOutbox readers never observe a half-written file.

2. Dot-prefix filter — PASS
The filter is f.endsWith(".json") && !f.startsWith("."), which filters only literal ASCII . at position 0. Legitimate filenames generated by queueOutboxMessage use ${timestamp}-${uuid}.json; the timestamp has colons/dots replaced with hyphens, so valid files will never start with a dot. No false-positive skip risk.

3. Quarantine one-way trip — PASS
Malformed files are renamed to sent/.malformed-${f}:

renameSync(src, join(sentDir, `.malformed-${f}`));

drainOutbox only reads from newDir; sentDir is never re-scanned for processing. The .malformed-* files are a one-way forensic sink. No retry path exists.

4. try/catch scoping — PASS (with note)
The outer try/catch wraps JSON.parse(readFileSync(...)). The rename that moves good files to sent/ is outside this block:

    renameSync(src, join(sentDir, f));  // will throw on EACCES
    out.push(msg);

Therefore EACCES (or other failure) on the good-file rename surfaces as an uncaught exception, as required. The quarantine rename has its own nested try/catch with an unlink fallback, which is appropriate defense-in-depth.

5. Path traversal in quarantine — PASS
f is derived from readdirSync(newDir), which returns basenames only (no path separators). Both join(newDir, f) and join(sentDir, \.malformed-${f}`)resolve under their respective trusted directories. The.malformed-` prefix is applied to a basename; no escape is possible.

Note

Verified: atomic-write pattern via .tmp + rename within same dir; dot-prefix filter on readdir; defense-in-depth try/catch; quarantine path .malformed-* in sent/. All threat-model questions answered satisfactorily.

APPROVE