Skip to content

fix(artifacts): event appends and materialize keep working after the host clock steps back - #1211

Merged
aviggiano merged 3 commits into
mainfrom
claude/g3-artifacts-greptile-fixes
Sep 29, 2026
Merged

aviggiano merged 3 commits into
mainfrom
claude/g3-artifacts-greptile-fixes

Conversation

@aviggiano

@aviggiano aviggiano commented Sep 29, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

Change

  • appendEvent (packages/artifacts/src/events.ts). It still builds the record before reading the journal, as before. Once the read has found the journal's final event, the record is rebuilt one millisecond after that event when two things hold: the caller passed no timestamp, and the record's time is earlier than the final event's. Otherwise the prebuilt record is appended unchanged.
    • The record's time is earlier after a backward clock step. It is also earlier when another process appended an event between this build and the read. origin/main rejected that append as not ordered (see Verification).
    • Timestamps still never decrease, so the journal's ordering rule and its readers do not change.
    • The stamped event is later than every recorded one, so an event whose content matches the final event's still gets a new ID. Reusing the final timestamp would make it a rejected duplicate, because the event ID hashes the timestamp.
    • A timestamp the caller passes explicitly is still rejected when it is earlier. No production caller passes one. All 19 appendEvent calls in package sources omit it, checked by walking their argument objects with the TypeScript parser. The one spread argument is typed Omit<…, "nodeId" | "runId" | "timestamp">.
  • Still one read per append, and the same fenced window. The final event comes from the read that already fences the write. refactor(artifacts): delete never-dispatched gates, production-dead code, and the unread event index #1181 found that this byte read is the part of an append's cost that still grows with the journal, so a second read would add to it.
    • appendStrictJsonlRecordsAfterTail becomes appendStrictJsonlRecordAfterTail (packages/artifacts/src/strict-jsonl.ts). It takes a builder in place of the records array, calls it with the final record, and returns the record it appended. appendEvent was its only caller (git grep).
    • The duplicate window is unchanged: the trailing records that share the prebuilt record's timestamp. A rebuilt record's time is later than the final event's, so the window then holds only the final event, and no recorded event can share the new time.
    • Building a record redacts its payload. That costs about 4 ms for a 1.8 KB materialize-selection payload. The canonicalization origin/main already runs between the read and the fenced write costs 0.06 ms for the same payload. Because the record is prebuilt, redaction stays out of that window, as on origin/main. Only a rebuilt record is built inside it.
  • Docs. docs/reference/artifacts-reports.md says that event times can run ahead of the wall clock after a backward clock step.
  • materialize.ts is unchanged.

Size: 2 source files +20/−11, 2 test files +48, 1 doc +6/−4.

Deliberately not fixed

  • Explicit earlier timestamps. appendEvent still rejects a caller-supplied timestamp that is earlier than the final event. That timestamp is the caller's statement of when something happened, and rewriting it would record a false time. Only tests pass one, and an event append refuses a repeated or out-of-order event without changing the journal still pins the rejection.
  • Materialize's write order. materializeSelection still copies, then appends the audit record, then appends the event. This change removes the finding's trigger. An event append that fails for another reason, such as a full disk or a torn journal, still leaves the copied files and the audit record behind. Making the three steps atomic is a separate change.
  • Concurrent appends can lose or tear records (pre-existing). appendBytesDurableAt checks the file size, then writes at that offset, with no lock. Two processes can both pass the check and write at the same offset. The result is a lost record, or a leftover tail that makes every later append and replay of that journal fail. The entry points take different locks or none. Sync takes its .workflow-sync lock, and launch takes the control lock. Resume, replay and fork take the lifecycle lock, and cancelRun and materializeSelection take none. So their appends to one events.jsonl can overlap.
    • In the two-writer harness below, the journal tore in 5 of 7 rounds on origin/main and in 3 of 7 here. In the rounds that did not tear, 0 to 6 records per round were missing although both writers had reported success.
    • The same helper backs the other strict JSONL journals. The fix is a per-journal lock held across the read, validation and write, and belongs in its own change.
  • The evals recovery-equivalence clock comparison. See Risk.

Verification

Both new tests fail on origin/main and pass on this branch. For the origin/main runs, the two source files came from origin/main and the @ultrafuzz/artifacts dist was rebuilt.

Test On origin/main Here
artifacts event appends take the wall clock and keep working after it steps back behind the journal Error: event journal timestamps are not ordered at record 2 passes. An append after an hour-old final event takes the current time. The two new, identical events after a future-dated one are stamped 1 ms and 2 ms after it, and the whole journal replays.
runtime materializeSelection records its event after the host clock steps back behind the run's events Error: event journal timestamps are not ordered at record 2, thrown by materializeSelection passes: ok: true, and the journal's last event is the one the result names

Ablations, each run on this branch with one line changed:

  • Dropping record.timestamp < final.timestamp from the condition, so every append after the first is stamped 1 ms after the final event: the artifacts test fails with stamped 2026-09-29T12:54:00.806Z, wall clock was 2026-09-29T13:54:00.805Z.
  • Stamping the final event's own timestamp instead of 1 ms after it: the artifacts test fails with event journal contains a duplicate identity "evt-…" at record 2.

The verifier's repro script calls materializeSelection after a future-dated event. Against this branch's build, the first call returns ok: true and records its event, and a retry returns MATERIALIZE_DESTINATION_EXISTS. On origin/main, the first call throws event journal timestamps are not ordered at record 2 after copying.

Two-writer contention. Each round, two processes each appended 300 materialize-selection events with 1.8 KB payloads to one journal. There were 7 rounds per build, alternating builds.

  • Fence failures (append path changed size) per round: 9 to 51 on origin/main, 15 to 52 here.
  • timestamps are not ordered failures: every origin/main round had them (3 to 28 in the 4 rounds that counted them). No round here had any.

Suites run on this branch after merging origin/main 142ba80:

  • artifacts: the full suite, 325/325.
  • runtime:
    • materialize.test.js and audit-contracts.test.js: 9/9.
    • 18 runtime.test.js tests of sync, status, health, resume, launch and workflow linking. 15 of them name events.jsonl, replayEvents or appendEvent directly: 18/18.
    • verified-output.test.js, whose fixtures append events, and clean.test.js: 54/54.
    • lifecycle-inspection.test.js, including the cancelRun and getRunTimeline tests: 52/52.
    • From dynamic-lifecycle.test.js, the two tests that read events.jsonl: 2/2.
  • Checks, all passing:
    • npx prettier --check and npx eslint on the changed files.
    • CI=1 ESLINT_PLUGIN_DIFF_COMMIT=origin/main pnpm -w lint:strict:ci.
    • pnpm -w lint.
    • pnpm --filter @ultrafuzz/artifacts --filter @ultrafuzz/runtime typecheck.
    • pnpm -w knip.
    • node scripts/docs-check.mjs.

Risk / compatibility

Changelog entry

Run event appends no longer fail after the host clock steps back behind a run's last event. Until the clock catches up, a new event is stamped one millisecond after the last one. materialize --confirm now records its event instead of failing after it has copied files, and sync, resume and cancel keep working.

Greptile follow-up

  • Kept: an event stamped after the final one can be ahead of the wall clock (comment). This happens only after the host clock steps back behind the journal. Without this change, the same append throws timestamps are not ordered, so the command fails outright. Greptile's scenario also needs an operator to resume an eval's run by hand in that window, because the eval runner never resumes runs (packages/evals/src has no resumeRun or replayRun). The recovery-equivalence risk is listed under Risk.
  • Kept: final timestamps with a UTC offset (comment). Not reachable. Every production appendEvent call omits timestamp, so every recorded event carries a toISOString() value in Z form. The only explicit timestamps come from tests, and an earlier explicit timestamp is still rejected.

🤖 Generated with Claude Code

aviggiano and others added 3 commits September 29, 2026 12:41
… 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 <[email protected]>
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 <[email protected]>
Brings in #1202 (modal) and #1203 (runtime tests). Neither touches the
files this branch changes, and the merge had no conflicts.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@aviggiano
aviggiano requested a review from a team as a code owner September 29, 2026 14:40
Comment on lines +1192 to +1193
input.timestamp === undefined && final !== undefined && record.timestamp < final.timestamp
? createEventRecord(layout, { ...input, timestamp: new Date(Date.parse(final.timestamp) + 1).toISOString() })

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Future timestamps reject valid attempts

When a watched eval run resumes after the host clock steps back, this branch can stamp the controller invocation ahead of the wall clock. Attempt started_at values still come from Smithers NodeStarted events. If an attempt starts before the clock catches up, the recovery-equivalence check treats it as preceding its controller submission, rejects valid lineage, and prevents the eval row from being recorded as terminal.

Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/artifacts/src/events.ts
Line: 1192-1193

Comment:
**Future timestamps reject valid attempts**

When a watched eval run resumes after the host clock steps back, this branch can stamp the controller invocation ahead of the wall clock. Attempt `started_at` values still come from Smithers `NodeStarted` events. If an attempt starts before the clock catches up, the recovery-equivalence check treats it as preceding its controller submission, rejects valid lineage, and prevents the eval row from being recorded as terminal.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fix in Claude Code

// identical event still gets a new ID.
(final) =>
input.timestamp === undefined && final !== undefined && record.timestamp < final.timestamp
? createEventRecord(layout, { ...input, timestamp: new Date(Date.parse(final.timestamp) + 1).toISOString() })

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Offset timestamps still block appends

If the journal ends with a valid timestamp using a positive UTC offset, this conversion can produce a UTC string that sorts before the final timestamp even though it represents a later instant. For example, 2026-09-30T16:00:00+05:00 becomes 2026-09-30T11:00:00.001Z. Event history compares the strings, so an append after a backward clock step still throws timestamps are not ordered.

Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/artifacts/src/events.ts
Line: 1193

Comment:
**Offset timestamps still block appends**

If the journal ends with a valid timestamp using a positive UTC offset, this conversion can produce a UTC string that sorts *before* the final timestamp even though it represents a later instant. For example, `2026-09-30T16:00:00+05:00` becomes `2026-09-30T11:00:00.001Z`. Event history compares the strings, so an append after a backward clock step still throws `timestamps are not ordered`.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fix in Claude Code

@aviggiano
aviggiano merged commit c47c892 into main Sep 29, 2026
17 checks passed
@aviggiano
aviggiano deleted the claude/g3-artifacts-greptile-fixes branch September 29, 2026 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant