fix(runtime): a pnpm install elsewhere on the host no longer fails a launch

[aviggiano]

Problem

A launch fails with WORKFLOW_SUBMISSION_FAILED: workflow execution file <path> changed while reading if a pnpm install runs anywhere else on the host while the launch seals its execution closure. Agents have reported this as a recurring flake. On this host, one package.json I checked under node_modules/.pnpm has 27 hard links: every checkout that shares the pnpm store links the same inode. An install in any worktree therefore touches the inodes a concurrent launch is reading.

Refs #921.

Root cause

At launch, every execution source (most of them are node_modules files) is read through readOpenedRegularFile in packages/runtime/src/workflow-integrity.ts, once to seal and once to verify. The dependency manifests, schema files, ajv/package.json (read when the validator build identity is computed) and run documents are read through readRegularFileSnapshot in packages/artifacts/src/schema-registry.ts. Both functions compared nlink and ctime before and after the read.

pnpm hard-links node_modules files from its content-addressed store. An install in another project therefore adds a link to the same inode, which changes nlink and ctime and leaves every byte as it was. The runtime reader's own comment already accepted hard-linked package content ("require a stable regular inode here without rejecting that layout"); its stability check still rejected it. trusted-cli-closure.ts has left this comparison out since it was written (#900), for the same reason.

Change

1. fix(runtime)
   • Both readers now compare only identity (dev/ino), size and mtime.
   • An ordinary write updates mtime at the same time as ctime, so it is still caught.
   • The sealed sources are still digest-checked at verify.
   • One side effect in readRegularFileSnapshot: it no longer detects a rename over a path it has open. The descriptor stays on the complete original file, so the read returns the document as it was when opened. That rename detection is what Detect an atomic path replacement by link count, not by ctime #887 added nlink for.
   • The workflow-control reader is unchanged here: its lstat identity check still rejects a path that names a different inode after the read.
2. refactor(runtime)
   • retryTransientSnapshotRead existed to re-read run.json and state.json when that rename was reported (status fails on transient state snapshot replacement during live execution #1054).
   • Both documents are only ever republished whole, by rename. Its three call sites (readRunListEntry, getRunHealth, readPendingRunHealth) could therefore no longer retry anything.
   • They now read once, and the function is deleted.
3. docs(runtime): the comments on the two kept retries (readLinkedWorkflowEvidence, synchronizeObservedWorkflowRun) now name what still triggers each one, instead of the rename the artifacts reader no longer reports.
4. test(runtime): the run-document replacement helper and the new launch test share one mid-read harness, actDuringStrictRead, instead of two copies of the fs.openSync/readSync/closeSync wrapping. Test-only; runtime.test.ts is 11 lines shorter than before this commit.

Deliberately not built

• Kept retryTransientSnapshotObservation (observe-only sync) and the re-derivation retry in readLinkedWorkflowEvidence. Each one still has a trigger.
  • The re-derivation retry handles the workflow-control reader's path check after a rename.
  • The sync retry re-runs the pass after an in-place append under one of the sync's own strict reads of the event journal: appendEvent's tail read, and the replays once the workflow has stopped. The pass catches its node-attempt and usage ledger reads and reports them as warnings, so those never reach this retry.
• Kept the CLI stats retry (stats-snapshot.ts). stats reads JSONL ledgers that the controller appends to in place, and that can still race.
• Left operator-npm.ts's reader alone, although it also compares nlink and ctime. pnpm installs the patched [email protected] as copies: every file I checked there has nlink 1, so this failure does not reach it.
• Left the single-link readers alone: safe-paths' singly linked snapshot, the workflow-sync authority snapshot, verified-output's publication snapshot and the snapshot-copy verification. They require nlink 1 on run-owned files, which pnpm never links, and they keep their own lstat replacement checks.
• Left templates/smithers/agents/strict-json.tsx alone, although it compares ctime. It reads run-owned files. Editing a stock adapter template would also change the controller source digest that every project's .smithers/agents must match.
• Did not remove the workflow-control reader's lstat identity check or any size/mtime comparison.
• Did not fix an event that the sync retry loses. This happens on main too; this PR does not touch workflow-sync.ts.
  • The pass persists each change before it appends the matching event: for example the controller transition written at workflow-sync.ts:899, or a node patch at 3109.
  • If the append's tail read of events.jsonl is raced, the retry finds the change already persisted and records nothing. The event is lost, and no warning is reported. I reproduced this for the controller transition (see Verification); the node case is from reading the code.
  • A stopped workflow's workflow-synced event is not lost this way: the pass compares it against the journal, so the retry records it.
  • The concurrent appender has to be a lifecycle command, because syncs hold an exclusive lock. This needs its own issue.

Verification

Discriminating tests were run against origin/main at 2cacf4c with only the branch's test files copied in. The two runtime.test.ts tests were re-run that way with the final test file (head d9fdb42), and both failed as described below. I also confirmed the launch failure on the earlier base, beaf2c3.

• New launch test: a hard link added to an execution source while the launch seals it does not fail the launch (runtime.test.ts).
  • The test hard-links an execution source each time the launch's stable read has consumed its bytes and before the read's closing stat.
  • On origin/main the launch fails with WORKFLOW_SUBMISSION_FAILED / workflow execution file tsconfig.json changed while reading, which is the reported failure. With this PR the launch succeeds.
  • The second half of the test is a guard: an in-place append at the same point must still fail the launch with that exact message.
  • Mutation check (re-run at head d9fdb42): with all four size/mtime comparisons in readOpenedRegularFile also removed, the guard fails. The seal accepts the torn read, so verify re-reads the file and acted becomes 2 instead of 1.
• a runtime document read ignores a hard link or an atomic replacement made during its snapshot (artifacts run-documents.test.ts) replaces the Detect an atomic path replacement by link count, not by ctime #887 test that pinned rename rejection.
  • On origin/main it fails with file changed while it was read; with this PR it passes.
  • The existing runtime document reads detect same-file mutation and refuse symlinks still passes.
• listRuns, observers and status survive live run documents replaced while they were read (renamed from …re-read…).
  • The ps part now races every read of state.json and run.json three times. On origin/main this exhausts the retry and lists the run as unreadable (replaced: 3). With this PR each document is read once and the run is listed as running.
  • The observer re-derivation parts are unchanged and pass.
  • In the status synchronization part, renaming state.json under the sync's own reads is no longer reported as a race. The retry budget and the WORKFLOW_STATE_SYNC_RACED warning are now exercised by an in-place write instead. That write stands in for the event-journal appends that can trigger the retry in production. state.json itself is only ever replaced by rename.
• observation-snapshot.test.ts: the three tests of the deleted synchronous wrapper are removed. The async test already covers retry-until-success, budget exhaustion and pass-through. The exhausted-message test now drives the async wrapper.
• Targeted runs on the rebased branch (head 91cab2a, base 2cacf4c):
  • artifacts: full suite, 324/324.
  • runtime supporting files: observation-snapshot, workflow-execution-snapshot-capability, pinned-submodules (seals and verifies a real closure) and controller-source, 11/11.
  • runtime.test.js --test-name-pattern='listRuns|getRunHealth|[Pp]ending|hard link added|survive live run documents|startRun|launch observation|reads as incomplete': 51/52.
    • The one failure is startRun injects the configured Forge guard into the workflow environment and metadata. It fails the same way on origin/main 2cacf4c: it expects the run's safe-bin/forge and resolves the fake-bin one.
    • This host also has a real forge in ~/.local/bin. I suspect that is the cause but have not confirmed it. The test does not touch the code changed here.
  • runtime.test.js, sealed-evidence and observer tests (divergent control file, diverged execution files, manifest that stops re-deriving, validator rebuild after launch, extra snapshot generations, leftover controller-generation journal, repeated observations): 9/9.
  • cli, with --test-concurrency=1: run, ps, status, inspect, report, … end to end, ps text prefers linked workflow terminal status…, status observes an incomplete launch… (the pending-run-health path), status surfaces a terminal product…, and the stats and statistics-snapshot tests: 10/10.
• Re-run at head d9fdb42. The two commits after 91cab2a change only comments in src and the shared test harness.
  • runtime.test.js --test-name-pattern='listRuns|getRunHealth|[Pp]ending|hard link added|survive live run documents|launch observation|reads as incomplete|diverged execution|divergent control|stops re-deriving': 21/21.
  • observation-snapshot: 3/3.
• Greptile follow-up (head 291f158, comments only):
  • Keep ctime for single-link reads? Declined. Every file the single-link path returns is digest- or byte-checked afterwards, or anchored by the seal's generation. The exceptions are state.json and the seal-time read of plan.json. Only their identity fields are used, and each is compared: at seal time against values derived from the control files being sealed, and at verify time against the seal. Nothing in the repo restores an mtime. On this host (ext4, 6.14) an ordinary same-size in-place write moved mtime in 2000/2000 trials.
  • Race the attempt ledger instead of state.json? Tried and declined. The sync catches every attempt-ledger read and reports it as a warning (WORKFLOW_ATTEMPT_INSPECT_FAILED or NODE_ATTEMPT_LEDGER_WRITE_FAILED), so the race never reached the retry: replaced was 1, not 3. An unchanged live pass never reads events.jsonl. A pass that records events reads it once per event, in appendEvent's tail read. If one of those reads is raced, the retry finds the change already persisted and does not read the journal again. Only a stopped workflow's pass replays the journal on every attempt (workflow-sync.ts:919), so only that pass could exhaust the budget through it. The synchronizeObservedWorkflowRun comment had named the ledger as a trigger; it and the test comment are corrected.
  • Re-ran survive live run documents and hard link added (2/2), prettier, eslint, lint:strict:ci, pnpm -w lint, runtime tsc (src and tests) and knip.
• Greptile follow-up, second round (no code change; head is still 291f158).
  • The two claims above are corrected: which reads are compared, and when the pass reads events.jsonl.
  • A probe on this branch (a temporary test, not committed) counted the observe-only pass's own reads of events.jsonl on a live fake-Smithers run. An unchanged pass read it 0 times. A pass that records a controller-lease lapse (lease shortened to 1 s) read it once.
  • Then I raced that one read with an in-place append. The pass reported no warning, and state.json held the expired lease. No workflow-synced event was recorded, either by that pass or by the two passes after it. That is the pre-existing loss listed under Deliberately not built.
  • Re-ran survive live run documents and hard link added at 291f158: 2/2.
• Static checks, all clean at 91cab2a and again at d9fdb42:
  • prettier --check, pnpm -w lint, CI=1 ESLINT_PLUGIN_DIFF_COMMIT=origin/main pnpm -w lint:strict:ci and pnpm -w knip.
  • tsc --noEmit: artifacts, runtime and cli at 91cab2a. At d9fdb42, runtime tsconfig.json and tsconfig.test.json only, since artifacts and cli did not change after 91cab2a.
• Complexity: readOpenedRegularFile drops from 25 to 21 and readRegularFileSnapshot from 17 to 15. Other functions set the global ceiling of 83, so it stays.

Risk / compatibility

• Validator build identity rotates. schema-registry.js is one of the modules VALIDATOR_BUILD_IDENTITY digests. Since fix: a validator rebuild no longer strands in-flight runs (validator build becomes provenance) #1188 that identity is provenance only, and no test pins its value.

• Strict artifacts reads now tolerate a rename. A read that overlaps a rename over its path returns the file it opened instead of throwing. The durable writer only renames a complete, fsynced temp file into place (writeFileDurable, safe-paths.ts:219-238), so the returned document is a complete earlier version. A write into the opened file during the read is still caught by size and mtime. Readers that need "the path still names these bytes" have their own checks:

  • The workflow-sync authority snapshot, verified-output's publication snapshot and the workflow-control reader re-check the path with lstat.
  • verified-output's run authority capture is protected differently: readAuthoritySnapshot has no lstat check of its own. Instead the capture compares state.json and the graph against a fresh parse right after reading them. At the end it re-reads those two, the task manifest, the seal and the graph fingerprint, and byte-compares each (verified-output.ts:235-244 and 289-316).
  • A republish that changes bytes during the capture therefore still fails it with a VERIFIED_OUTPUT_CHANGED error (… changed while its exact bytes were being captured or … changed while run outputs were being read). Before, a republish that landed inside one of those reads failed first with the reader's own file changed while it was read. A republish between reads was already caught only by these comparisons. I verified this by reading the code, not by running it.

• Two cases are no longer rejected mid-read by these two readers:

  • a hard link added to a file while it is read;
  • a write that also resets mtime inside the read window.

  Sealed sources are still digest-checked at verify. Readers that enforce single-link policy check nlink === 1 themselves and are unchanged.

• Not verified end to end. I did not run a real pnpm install concurrently with a real ultrafuzz run. The tests reproduce it by linking the file at the exact point inside the read.

Changelog entry

A pnpm install anywhere on the host during a campaign launch no longer fails the launch with "workflow execution file … changed while reading". Launch-time and run-document reads no longer treat a changed link count or ctime as a changed file. A strict run-document read that overlaps an atomic rename now returns the complete pre-rename document, so ps, status and pending-run health no longer need to retry it.

🤖 Generated with Claude Code

Confidence Score: 5/5

The PR appears safe to merge with two previously reported, non-blocking concerns still open.

Findings

1.  Control files lose ctime protection ▶
2.  Retry test races wrong file ▶

Fix with agent prompt

### Issue 1
packages/runtime/src/workflow-integrity.ts:2161-2174
Execution sources allow hard links, but run control files still require a single link. Removing the ctime check for both means a same-size in-place write followed by restoring the original mtime can pass the control-file stability check. Keeping the ctime check for single-link reads would preserve that protection without bringing back the pnpm launch failure.

### Issue 2
packages/runtime/test/runtime.test.ts:12861-12866
This test appends to `state.json`, but the runtime publishes that document by rename. The in-place appends that still justify the synchronization retry occur in the event journal and node-attempt ledger. Racing one of those ledgers would cover the production behavior; as written, the test could pass even if the retry stopped handling ledger appends.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

---

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

Summary

The PR stops launch-time and run-document reads from treating hard-link count and ctime changes as file-content changes. It also removes retries for run documents that are published by atomic rename and updates the associated tests and comments.

_{Reviews (2) · Last reviewed commit: "docs(runtime): name only the event journ..."}

[greptile-apps]

Control files lose ctime protection

Execution sources allow hard links, but run control files still require a single link. Removing the ctime check for both means a same-size in-place write followed by restoring the original mtime can pass the control-file stability check. Keeping the ctime check for single-link reads would preserve that protection without bringing back the pnpm launch failure.

Prompt To Fix With AI

This is a comment left during a code review.
Path: packages/runtime/src/workflow-integrity.ts
Line: 2161-2174

Comment:
**Control files lose ctime protection**

Execution sources allow hard links, but run control files still require a single link. Removing the ctime check for both means a same-size in-place write followed by restoring the original mtime can pass the control-file stability check. Keeping the ctime check for single-link reads would preserve that protection without bringing back the pnpm launch failure.

---

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

[greptile-apps]

Retry test races wrong file

This test appends to state.json, but the runtime publishes that document by rename. The in-place appends that still justify the synchronization retry occur in the event journal and node-attempt ledger. Racing one of those ledgers would cover the production behavior; as written, the test could pass even if the retry stopped handling ledger appends.

Prompt To Fix With AI

This is a comment left during a code review.
Path: packages/runtime/test/runtime.test.ts
Line: 12861-12866

Comment:
**Retry test races wrong file**

This test appends to `state.json`, but the runtime publishes that document by rename. The in-place appends that still justify the synchronization retry occur in the event journal and node-attempt ledger. Racing one of those ledgers would cover the production behavior; as written, the test could pass even if the retry stopped handling ledger appends.

---

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

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!