Skip to content

Fix #2053: hold loadSpooledBodies' shared read through the owner's unlink - #2717

Merged
HypForge merged 1 commit into
masterfrom
steward/fix-2053
Oct 18, 2026
Merged

HypForge merged 1 commit into
masterfrom
steward/fix-2053

Conversation

@HypForge

Copy link
Copy Markdown
Contributor

Refs #2053

What was wrong

loadSpooledBodies shares one in-flight readFile per body path between overlapping calls (the reading map). The entry was removed as soon as the read settled, which is before the owning call unlinks an unparseable body. A second call that arrived while that unlink was in flight no longer found the entry, read the file itself, got ENOENT, and counted the body as missing instead of unparseable.

Fix

Each reading entry now counts its users. A caller joins it before its first await and leaves in a finally around its whole classification, so the owner holds the entry until its unlink has settled. The last caller out deletes it. That covers every exit path, including the removing.has early continue and the missing arm. One file, 24 lines in and 23 out, ignoring reindentation.

Tests (deterministic, explicit ordering)

Two new tests in test/plugins/claude-telemetry-unparseable-body.test.js:

  • A caller arriving during the owner's unlink shares the read and never counts missing. Gated promises hold the owner inside unlink while the second call starts. Expected: unparseable 2, missing 0, bytes reported once, one readFile. Then the test writes valid JSON at the same path, and a fresh call parses it with a second readFile. That proves the entry was released, not left in the map.
  • The shared read is released after overlapping callers on the missing and projected paths. Two overlapping calls on a missing file give missing 2. Two overlapping calls on a valid body project it. A rewrite of the body is then read fresh.

Mutation check:

  • The master bodies.js fails the first test (unparseable 1, expected 2).
  • Removing the release from the fix fails both new tests, because a stale entry would serve later calls.

Timing probe: 400 trials, with the second call delayed by N setImmediate ticks. "Miscounted" means the second call counted the body as missing.

Delay (ticks) Master: miscounted Fix: miscounted
0 0/400 0/400
5 1/400 0/400
20 238/398 0/394
50 8/11 0/16

At 20 and 50 ticks the calls rarely overlapped, so most trials did not reach the race. The denominators count only trials where they did.

Single caller is unchanged

  • Same results. A base-versus-fix differential, one call at a time, gives identical output. The cases cover a valid body, a missing body, an unparseable body, an unparseable body named twice, an out-of-spool ref, and a mixed batch, and each compares bodies, consumed files and bytes, missing, unparseable, unparseable bytes, refused, and files left in the spool.
  • Same capture-health line. The hyp status capture-health line (captureHealth in src/core/daemon/status.js) reads event and transcript timestamps only. It never reads bodiesMissing or bodiesUnparseable. With identical single-caller counters, the line is unchanged.
  • Smokes. claude_telemetry_capture passes, including its bodies_missing === 1 check. status_capture_health fails at its first step ("Daemon is not running") with byte-identical output on master's bodies.js and on this branch, so that failure is pre-existing in this environment and unrelated.

Checks

  • npm run typecheck: pass
  • npm test: 8656 tests, 8653 pass, 0 fail, 3 skipped

CPU and memory

  • Memory. One small { read, users } object per body path with a read in flight, the same count as before. It is now released after classification instead of when the read settles, and always by the last user, so it cannot grow across ticks.
  • CPU. No extra reads. The fix removes the duplicate readFile that the race used to cause.

Server-suite gate: not triggered (no CLI command, contract, MCP codec, tool schema or error code change).

🤖 Generated with Claude Code

…link

The shared `reading` entry was dropped as soon as the read settled, before
the owning caller unlinked an unparseable body. A caller that arrived during
the unlink read the file again, got ENOENT, and counted the body as missing
instead of unparseable. The entry now counts its users and the last one out
drops it in a finally, on every exit path, so it covers the unlink and never
outlives the reads in flight.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@HypForge
HypForge added this pull request to the merge queue Oct 18, 2026
Merged via the queue into master with commit c5bdc85 Oct 18, 2026
8 checks passed
@HypForge
HypForge deleted the steward/fix-2053 branch October 18, 2026 02:35
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