Repository navigation
fix(artifacts): concurrent ultrafuzz commands can no longer tear or lose journal records - #1221
Conversation
… back After the wall clock stepped back behind the last event in a run's events.jsonl (an NTP step, a VM snapshot restore, a manual correction), every appendEvent call failed with "event journal timestamps are not ordered at record N" until the clock caught up. #1195 let the clean, materialize and dashboard audit journals accept such a step but kept this journal's rule, so `ultrafuzz materialize --confirm` still copied its files and wrote its audit record and then threw, and a rerun failed with MATERIALIZE_DESTINATION_EXISTS. Sync, resume, replay, fork, pause, cancel and the workflow-link commit append events the same way. When the current time is earlier than the journal's final event, appendEvent now stamps the new event one millisecond after that event. Timestamps stay nondecreasing, so the journal's ordering rule and its readers are unchanged. The stamped event is later than every recorded one, so an event identical to the final one still gets a new ID; reusing the final timestamp would have made it a rejected duplicate. A timestamp the caller passes explicitly is still rejected when it is earlier. The final event comes from the same read that fences the write: appendStrictJsonlRecordsAfterTail becomes appendStrictJsonlRecordAfterTail, which builds its one record from the journal's final record. Its window is now the trailing records that share the final record's timestamp rather than the new record's. That still holds every record a new ID can repeat, because a new record either shares the final timestamp or is later than every recorded one. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The first version built every event record inside appendStrictJsonlRecordAfterTail, between the journal read that sets the fence size and the fenced write. Building a record redacts its payload, which costs about 4 ms for a 1.8 KB materialize-selection payload against 0.06 ms for the canonicalization origin/main already runs there, so more concurrent appends failed the size fence. appendEvent again builds the record before the read, as origin/main does. The builder rebuilds it 1 ms after the final event only when the record's time is earlier than that event's: after the wall clock steps back, or when another process appended since the record was built, which origin/main rejected as out of order. The duplicate window stays keyed on the prebuilt record's timestamp; when the record is rebuilt, the window holds only the final event and no recorded event shares the new time. parseStrictJsonlTail and the inWindow signature are origin/main's again. The artifacts test now also checks that an append after an older final event takes the current time. The event journal reference says event times can run ahead of the wall clock after a backward clock step. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Every strict JSONL journal (events.jsonl, usage.jsonl, attempts.jsonl and the .ultrafuzz materialize, clean and dashboard audits) appended by reading the journal, validating the new records against it, and then writing at the size it had read, with no lock. ultrafuzz commands take different locks or none (sync takes .workflow-sync, launch the control lock, resume/replay/fork the lifecycle lock, cancel and materialize none), so `status --watch` syncing while `cancel` runs could append to one events.jsonl at once. Both writers could pass the size check and write at the same offset: one record was lost, or a leftover tail made every later append and replay of the journal fail and stranded the run. With two processes appending 300 events each, the journal tore in 3 of 7 rounds; the other rounds refused 28 to 81 appends, and 3 of them lost 2 records each. Both strict JSONL append paths now hold <journal>.lock from the read through the write. The lock is an O_EXCL file recording the holder's host, pid and (on Linux) process start time. Waiters poll every 5 ms with Atomics.wait, since every caller is synchronous, and give up after 30 s with an ArtifactPathError coded "journal-locked" that names the holder. A lock whose holder has died on this host (the pid is gone, or reused by a process that started later) is broken by the next waiter; an empty lock, as a power loss can leave, once it is 30 s old. Breaking holds <lock>.break so two waiters cannot both break it, which let the second remove the first one's new lock in 3 of 10 stress rounds without it. The lock is taken inside the append and nothing else is locked while it is held, so it cannot deadlock with the sync, control or lifecycle locks. It sits next to the journal, where no reader, report bundle or directory inventory looks. The new tests fail on the base branch: three processes appending 50 events each lose records every time, an append does not wait for a holder, and a SIGKILLed holder's lock is never cleared. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
#1211 merged into main as a squash (c47c892), so this branch takes main to keep its diff against main to the journal lock alone. The two conflicts, in strict-jsonl.ts and artifacts-reports.md, are #1211's own hunks, which main carries unchanged; this branch's versions of both files are kept. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…used PID no longer blocks appends The lock's first revision broke a lock only when its record named this host and that process had provably exited. Review probes found four ways a holder killed mid-append could still block a journal: - A lock recorded by another host was never broken, however old. A new container on the same bind-mounted project, a rescheduled pod or a replacement sandbox on the same volume can report another hostname, so every later append waited 30 s and failed with journal-locked until someone removed the lock by hand. Such a lock, like an empty or unparseable one, is now abandoned once its mtime is 10 s old. A live append holds the lock for milliseconds, and 10 s is well below the 30 s wait, so an append already waiting takes over within its wait. - kill(pid, 0) failing with EPERM, meaning another user's process now has the ID, counted as held without the start-time check. Only ESRCH now means gone; otherwise the start time in /proc/<pid>/stat decides. - A killed holder that is not reaped yet is a zombie with the recorded start time, so its lock stayed held until its parent reaped it, which a parent blocked in an append, or a container's PID 1 without an init, may never do. The process state Z or X now counts as gone. - A waiter that had just removed an abandoned lock still checked its deadline before trying again, and an empty lock aged out exactly at the wait's length. An append already waiting when a holder died between creating the lock and recording itself therefore failed at 30 s with the lock already gone. A waiter now tries again at once after freeing a lock and checks the deadline only before sleeping. The holder's record is now built before the lock file is created, which narrows the window in which a kill leaves the lock empty. Nothing imports JOURNAL_LOCK_WAIT_MS any more, so it is no longer exported, and the tests remove their temporary projects. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
| const lock = readJournalLock(lockPath); | ||
| if (lock === undefined) return "free"; | ||
| const { holder, mtimeMs } = lock; | ||
| if (holder !== undefined && holder.host === os.hostname()) return holderIsGone(holder) ? "abandoned" : "held"; |
There was a problem hiding this comment.
If two containers share a journal directory and report the same hostname but use different PID namespaces, this check can mistake the other container’s live holder for a missing or reused process. It then removes that holder’s lock. Both commands can read the same journal length and write at the same offset, losing a record or tearing the journal.
Knowledge Base Used: Artifact management
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/artifacts/src/journal-lock.ts
Line: 137
Comment:
**Live container lock removed**
If two containers share a journal directory and report the same hostname but use different PID namespaces, this check can mistake the other container’s live holder for a missing or reused process. It then removes that holder’s lock. Both commands can read the same journal length and write at the same offset, losing a record or tearing the journal.
**Knowledge Base Used:** [Artifact management](https://app.greptile.com/monad-foudnation/-/custom-context/knowledge-base/monad-developers/ultrafuzz/-/docs/artifact-management.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| if (holder !== undefined && holder.host === os.hostname()) return holderIsGone(holder) ? "abandoned" : "held"; | ||
| // Only a crash right after creating the lock leaves it without a record, and | ||
| // another host's process cannot be checked here. Age is by the wall clock. | ||
| return Date.now() - mtimeMs > JOURNAL_LOCK_STALE_MS ? "abandoned" : "held"; |
There was a problem hiding this comment.
If hosts sharing a journal have clocks more than 10 seconds apart, this check can mark another host’s live lock as abandoned during an ordinary append. A live append that lasts past the threshold has the same problem. Removing that lock lets two writers read the same journal length and write at the same offset, so records can be lost or the journal torn.
Knowledge Base Used: Artifact management
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/artifacts/src/journal-lock.ts
Line: 140
Comment:
**Clock skew breaks live locks**
If hosts sharing a journal have clocks more than 10 seconds apart, this check can mark another host’s live lock as abandoned during an ordinary append. A live append that lasts past the threshold has the same problem. Removing that lock lets two writers read the same journal length and write at the same offset, so records can be lost or the journal torn.
**Knowledge Base Used:** [Artifact management](https://app.greptile.com/monad-foudnation/-/custom-context/knowledge-base/monad-developers/ultrafuzz/-/docs/artifact-management.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Stacked on #1211 and must merge after it. #1211 has since merged into
mainas a squash (c47c892), so this branch mergesorigin/main(7728e7b) on top of #1211's commits; its diff againstmainis this change alone, and a trial merge into the currentmain(13af837) is clean.Problem
.workflow-sync, launch takes the control lock, and resume, replay and fork take the lifecycle lock.cancelRunandmaterializeSelectiontake none. Soultrafuzz status --watchsyncing while an operator runsultrafuzz cancelappends to oneevents.jsonlfrom two processes.event journal record N is invalid strict JSONorcontains a blank record. When it is not shorter, one record is gone although its writer reported success.usage.jsonl,attempts.jsonl, and the.ultrafuzz/materialize, clean and dashboard audit journals.materialize-selectionevents each, 1.8 KB payloads), the journal tore in 11 of 21 rounds without the lock. The other rounds refused 23 to 81 appends and lost up to 4 records each (Verification).Root cause
appendStrictJsonlRecordsandappendStrictJsonlRecordAfterTail(packages/artifacts/src/strict-jsonl.ts) read, validate and callappendBytesDurableAtwithout mutual exclusion between processes. The size fence inappendBytesDurableAtis anfstatfollowed by a positional write, so two writers can both pass it. The validation (timestamp order, duplicate IDs) also holds only against the tail that was read, so even an atomic append could not stop a concurrent record from breaking the journal's order.Change
packages/artifacts/src/journal-lock.ts, new, not exported from the package index).withJournalLock(journalPath, append)holds<journal>.lockwhileappendruns.strict-jsonl.tsrun their existing bodies, moved unchanged into…Lockedfunctions, inside it: from the read, through validation and fix(artifacts): event appends and materialize keep working after the host clock steps back #1211'sbuild, to the fenced write. The size fence stays as a second check.O_CREAT | O_EXCL | O_NOFOLLOWfile, mode 0600, holding{"host", "pid", "started"}.startedis the process start time from/proc/<pid>/staton Linux, so a later process given the same PID is not taken for the holder. The record is built before the file is created, so a holder killed right after creating it has the least time to leave it empty.Atomics.wait, the idiomeval-durable.tsandworkspace-handoff.tsalready use, because every caller is synchronous. After 30 s by the monotonic clock it throwsArtifactPathErrorwith codejournal-locked, naming the lock and its holder, and the append does not run. A waiter that has just removed an abandoned lock, or seen it go, tries again at once and checks its deadline only before sleeping, so it never gives up on a lock that is free.kill(pid, 0)fails with EPERM while/proc/<pid>/statstays readable.JOURNAL_LOCK_STALE_MS). That covers an empty or unparseable lock, which a kill between the create and the record's write can leave, and a lock recorded by another host, whose PID cannot be checked here, such as one an earlier sandbox or container left on the same volume. A live append holds the lock for milliseconds. 10 s is well below the 30 s wait, so an append that was already waiting when such a holder died takes over within its own wait.<lock>.break, taken with the same primitive, and removes the lock only if it is still abandoned when re-read. Without that, two waiters that both judged the lock abandoned could both remove it, and the second would remove the lock the first had just created. A stress run shows it: with the.breakstep removed, 5 of 10 rounds had two holders at once.events.jsonl.lock,usage.jsonl.lockandattempts.jsonl.lockin the run root, andmaterialize-audit.jsonl.lock,clean-audit.jsonl.lockanddashboard-audit.jsonl.lockin.ultrafuzz/. It exists for the milliseconds of one append. I checked every directory walk that could reach those directories:artifacts/,review/,events.index/andsmithers/logs;clean, private eval evidence and the Modal runner enumerateruns/entries, andlistSafeFilescallers walk node artifact directories;.ultrafuzz/.docs/reference/artifacts-reports.mddescribes the lock after the event journal paragraph.Size:
journal-lock.ts+228,strict-jsonl.ts+33/−5, one test file +287, docs +15.Deliberately not built
replayEvents,status, report bundles andstatsstill read without the lock. A read that races a write already fails withfile changed while it was readrather than returning a torn snapshot. A reader lock would also stop read-only run directories from being readable.appendUsageEventsandappendNodeAttemptspick the new entries from a read outside the lock. Both run only inside a sync pass, which holds.workflow-sync. A racing duplicate would still be rejected by the locked validation (contains a duplicate identity), not written.proper-lockfile(runtime's sync, control and lifecycle locks) throwsESYNCwhen its synchronouslockSyncis given retries. Its staleness is an mtime age for every holder, so it depends on the wall clock, the clock that fix(artifacts): event appends and materialize keep working after the host clock steps back #1211 had to survive stepping back, and it would take over a live holder on this host stalled in a slow fsync. It is also not a dependency of@ultrafuzz/artifacts.withModalLaunchStateLockinpackages/modal/src/launch-state.ts, also anO_EXCLfile recording the PID and/procstart ticks. It is async, and@ultrafuzz/modaldepends on artifacts, not the other way round.processStathere is therefore a second copy of itsparseProcessStartTicks.mkdirlock ineval-durable.tsbelongs to@ultrafuzz/evals, which depends on artifacts; it records no holder and never breaks a stale lock. A possible follow-up is foracquireEvalJournalLockto callwithJournalLock.O_EXCLcreate and its record's write leaves an empty lock, and create-then-write cannot close that window, because the kill can land insideopen()itself. Creating the lock with its record already in place (writing a temporary file, thenlink) would leave temporary files behind after a crash and needs hard links, which some network filesystems lack. What the window costs is a wait of up to 10 s for the next append, not a failure: in the SIGKILL stress below, 2 of 6 rounds stalled on an empty lock.appendEventstill builds its record before the read. Under contention the final event is often newer than the prebuilt one, so the record is rebuilt under the lock, which takes about 4 ms for a 1.8 KB payload. Without contention nothing changes.Verification
New tests are in
packages/artifacts/test/journal-lock.test.ts. Two bases were built from this branch's test file:strict-jsonl.tsfromorigin/claude/g3-artifacts-greptile-fixes(origin/maincarries the same file), with this branch'sjournal-lock.tsin place so that the tests compile. Tests 4 to 7 exercise the lock alone, so they pass there.journal-lock.tsfrom this branch's first commit (a5d0134), before the takeover rules were widened, with an exportedJOURNAL_LOCK_STALE_MSadded for the test's import.appends from concurrent processes all land in the event journal, which still replays: 3 processes × 50 appendsAssertionError: events reported as appended are missing, listing event IDs thatappendEventreturned but the journal does not contain, and 3 torn, withevent journal contains a blank record at line 24(twice) orline 20eatmydataan append waits while another process holds the journal's lock, then appends after its record[waited, holder]instead of[holder, waited]a journal lock left by a process killed while holding it is taken over:appendStrictJsonlRecordsafter a SIGKILLed holdertrue !== false: the append ignores the lock, which is still there afterwardsa journal lock whose holder was killed but is not reaped yet is taken over: the test waits without turning its event loop, so its SIGKILLed child stays a zombiejournal-locked, held by the zombiea journal lock whose process ID was reused, even by another user, is taken over: PID 1 (EPERM unless root) and the test's own PID, each recorded with a start time one tick earlyjournal-locked, held by process 1an append that cannot take the journal's lock within its wait fails without runningArtifactPathErrorcodejournal-locked, and the append never ranan empty journal lock, or one recorded on another host, is taken over once it is stale: each is held while new, then taken over when 11 s old by an append withwaitMs: 0The file takes about 3 s (3.0–3.1 s with
eatmydata, 3.3–3.9 s without). Tests 4 and 5 skip where/proc/self/statdoes not exist.The review's probes against this build:
Harness: the #1211 fixer's
contend.mjs, unchanged. Each round, two processes each append 300materialize-selectionevents with 10 copies (1.8 KB) to one journal. "Refused" counts appends that threw.origin/main7728e7b, whosepackages/artifacts/srcis identical, 7 rounds each in the first run; the unlocked base above, 7 more in this runcontains a blank record at line 222,31and201, and records 82, 179, 69, 87, 427, 388, 171 and 76 invalid strict JSON. In this run's torn rounds, every later append failed: 378 to 565 of them per round.A round takes 5.0–5.9 s here against 3.1–4.0 s for the unlocked rounds of this run that did not tear, under the same background load, because the appends now run one at a time.
Stress, with scratch scripts:
.lockor.breakfile was left. With the.breakstep removed from the build, 5 of 10 rounds had two holders at once: 4 refused 1 or 2 appends withappend path changed size before repair, and 1 lost a record that its writer had reported as appended.Suites on this branch:
materialize,clean,audit-contracts,data-governance,source-revision,verified-outputandtrusted-cli: 97/97.trusted-cli.test.js's closures copy the artifactsdist/.lifecycle-inspection, the whole file: 55/55.dynamic-lifecycletests that readevents.jsonl: 2/2.runtime.test.jstests whose names match synchronization,syncRun, resume, cancel, replay, fork, materialize, pause, lifecycle or event journal: 136 pass and none fail. The other 9 areBun adapter contracttests, which skip under node; the package script runs them underbun test.contracts.test.js5/5, and the two audit tests indashboard.test.js, includingdashboard audit append refuses a final-component symlink: 2/2.report bundle,report renderandstatstests incli.test.js, whose runs append events and ledgers and are then bundled: 57/57.dist/that differed from the final build only in still exportingJOURNAL_LOCK_WAIT_MS. The artifacts suite and the checks below ran on the final commit.npx prettier --checkandnpx eslinton the changed files.CI=1 ESLINT_PLUGIN_DIFF_COMMIT=origin/claude/g3-artifacts-greptile-fixes pnpm -w lint:strict:ci, and the same with the merge base withorigin/main.pnpm -w lint.pnpm --filter @ultrafuzz/artifacts typecheck.pnpm -w knip, and also with ci: fail on unused exports, types and duplicates #1219'sexports,types,duplicatesadded, which reports nothing in these files.node scripts/docs-check.mjs.Risk / compatibility
.ultrafuzz/are created writable. In the read-only run root ofa synchronization that cannot take its lock is reported instead of thrown, a pass stops at the sync lock before it appends anything.strict-jsonl.tsorjournal-lock.ts. fix(runtime): a renderer or projection change no longer strands runs whose dynamic groups expanded #1216, feat(topology)!: a failed property lens no longer skips the rest of the campaign #1198, refactor!: remove per-node cloud execution (execution.mode = "cloud") #1197 and fix(runtime): record final-report producer selections in the run instead of querying smithers #1183 editdocs/reference/artifacts-reports.mdelsewhere and merge with this branch cleanly in that file. A trialgit merge-treewith each open PR conflicts in none of this branch's files; the conflicts with feat(runtime)!: run lifecycle commands from the pnpm-patched install instead of per-command npm installs (#921 step 1) #1201, refactor!: remove per-node cloud execution (execution.mode = "cloud") #1197, fix(runtime): record final-report producer selections in the run instead of querying smithers #1183 and feat(runtime): let agents record roadblocks in a run-local friction log #1162 are in other files and appear againstorigin/mainalone. ci: fail on unused exports, types and duplicates #1219 turns on knip's unused-export check, which this branch passes.Changelog entry
Concurrent ultrafuzz commands, such as
ultrafuzz status --watchwhileultrafuzz cancelruns, no longer lose or tear records in a run'sevents.jsonl,usage.jsonlorattempts.jsonl, or in the.ultrafuzz/audit journals. Each append now holds a short-lived<journal>.locknext to the journal. A later append takes over at once a lock left by a killed process on the same host, and after 10 seconds one it cannot check, such as a lock an earlier sandbox left on the same volume.Greptile follow-up
Both comments describe limits this PR already lists under Risk, and neither makes anything worse than before it.
🤖 Generated with Claude Code