…and the refs name the spec that owns the gauge
Three review findings on the unparseable-body byte accounting.
`fs.rm(file, { force: true })` RESOLVES for a path that is already gone, so
"sized only once the unlink succeeded" did not hold: two reads of the same
`body_ref` in flight at once (the receive handler is not serialized, so an
exporter retry overlaps the original it retries) both read the file, both
call it unparseable, and both added `raw.length` - `spool_bytes` came down by
twice one deletion. `fs.unlink` makes the guard the comment describes real:
the second call rejects with ENOENT and reports nothing. New regression case
proves it, 30 bytes before and 15 after for one 15-byte file.
The comment claimed the sizing matched the projected arm. It does not on this
head: the projected arm subtracts `spooled.consumedBytes` unconditionally at
the call site whether or not `deleteSpooledBodies` removed anything. It also
said an undeleted file is "occupying the cap", but `state.spoolBytes` is a
published gauge only - the cap is enforced by `enforceClaudeBodySpoolCap`
re-scanning the directory. Both claims replaced with what the code does.
Both `@ref`s cited LLP 0253#byte-cap with a gloss reading "whichever arm
removed the file". The published gauge is LLP 0257 S16's duty
(#status-and-health), not the cap's, and the "whichever arm" claim is not
true: the policy-drop arm still deletes bodies without subtracting, because
the delete-unread path reports no bytes. Anchors moved to 0257 and the
overclaim replaced with a note of the arm that still lacks it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Triage of PR #851 recorded five deferred findings on #904. One of them is reachable from
mastertoday; the other four are not, and they are listed below with the reason.Fixed: finding 3, the unparseable-body arm leaves
spool_bytesdrifted highWhat was wrong.
loadSpooledBodiesdeletes a body file that fails to parse (an undeleted body is a raw prompt sitting on disk) and counts it inunparseable, but reported no size for it. The receive handler subtractsspooled.consumedBytesfor the projected arm only, so an unparseable body leftstate.spoolBytesat its pre-batch value:hyp statuspublished bytes for a file that was already off the disk, for up to the 60s spool-sweep interval, until the sweep restated the gauge.Root cause. The byte accounting is caller-side (
state.spoolBytes = Math.max(0, state.spoolBytes - spooled.consumedBytes)inhypaware-core/plugins-workspace/claude/src/telemetry/source.js), andloadSpooledBodiesreturned no field the caller could subtract for the deletion it performed itself. The.catch(() => {})on the unlink also hid whether the file actually left the disk.Fix.
loadSpooledBodiesnow returnsunparseableBytes, accumulated only oncefs.rmresolves, so a file that could not be removed (EPERM, a read-only spool) keeps occupying the cap rather than being subtracted out from under it. The receive handler deducts it beside the existingstate.bodiesUnparseable += spooled.unparseable.Regression test. New
test/plugins/claude-telemetry-unparseable-body.test.js, two cases:api_request_bodynaming that file, then asserts the file is gone and the publishedspool_bytesis0.loadSpooledBodiesreportsunparseableBytesequal to the file's size whileconsumedBytesstays0.Before the fix (at
b1afa7c2):After the fix:
# tests 2 / # pass 2 / # fail 0. Full suitenpm test:# tests 4486 / # pass 4485 / # fail 0 / # skipped 1.npm run typecheckclean.Gate met: behavioural finding, failing-then-passing regression test.
Still deferred
Findings 1, 2 and 4 are not reachable from
master. All three describe code that PR #851 introduces and that PR is still open (fix/issue-843, head1378a82c, state OPEN). Verified againstorigin/master:telemetry/source.js. Onmasterthe write-failure catch carries no comment about usage at all; the sentence to correct is added by Deferred #843 findings: listener counters, the refusal log, and the usage a failed batch consumed #851's diff. There is nothing onmasterto edit, and no test can fail on it.usageBeforeProjectionsnapshot andrestoreUnclaimedUsage. Neither symbol exists onmaster;projectClaudeTelemetryEventsis synchronous there, so a batch's mutation of the shared index is already atomic with respect to another awaited request. There is no reproduction to write until Deferred #843 findings: listener counters, the refusal log, and the usage a failed batch consumed #851 lands.restoreUnclaimedUsage(telemetry/projection.js), which does not exist onmaster. The issue also records it as a deliberate tradeoff, bounded by the cap, to revisit only if an age-aware structure is introduced.These three should be picked up on top of #851, either as review comments on that PR or as a follow-up once it merges. Fixing them here would have meant re-implementing #851's changes on a second branch.
Finding 5 needs a maintainer decision and is out of an autonomous worker's authority. It asks whether "a retried batch re-projects from the same inputs, so those inputs must survive a failed write" belongs in LLP 0257, which is
Accepted. Under CLAUDE.md an Accepted LLP is a record: the change is a new request extending 0257, plus anExtended-by:forward-ref on the old doc. Whether that clause belongs in the spec at all, and what it should say, is the judgement call the issue explicitly routes to a maintainer, so no LLP was minted.Note on merge order
This branch touches
telemetry/bodies.jsandtelemetry/source.js, which #851 also touches. The hunks are disjoint (this one is insideloadSpooledBodiesand at its call site; #851's are indeleteSpooledBodies, the refusal log, the write-failure catch, and the timestamp max), so a textual conflict is unlikely but possible. If #851 merges first, rebase this branch onto it.Fixes #904