outflux-v1-playback #1

Merged
yash merged 22 commits from outflux-v1-playback into main 2026-09-01 03:26:13 +00:00
Owner
No description provided.
yash added 22 commits 2026-09-01 03:26:07 +00:00
- Add null/falsy guard for technical parameter to prevent destructuring crash
- Add totalBytes finitude guard in clampOffset to prevent NaN propagation
- Add regression tests for both findings

Fixes issue where garbage metadata caused TypeError and over-long reads could
silently produce NaN offsets, violating the core invariant that playback must
degrade gracefully rather than crashing or hanging.
bare-http1's writeHead() merges caller headers without lowercasing while
its internal setHeader() lowercases, so a Title-Case key emitted both
'Content-Length' and 'content-length' and the parser rejected every
response. Lowercasing the keys makes the library's recompute overwrite
our key rather than add a second one, fixing the cause rather than the
symptom.

Removes the previous res.flushHeaders() workaround, which additionally
committed the status line and a promised Content-Length before the
transport had produced a first chunk -- once the swarm transport lands, a
peer vanishing between available() and the first read would surface as a
truncated 200 rather than an honest error status.
Adds the Phase 5 acceptance test (test/integration/playback.test.js) per a
controller ruling that overrides Task 10's brief: instead of a t.fail(...)
placeholder, the test body is real -- it imports the corpus, dynamically
selects the longest container==='mp3' record with technical.vbr === true
(no hardcoded hash), seeks to 3:00 (or 75% of duration with the substitution
logged, if no mp3 is that long), and checks time-to-first-byte, the decoded
timestamp via ffprobe against ground truth built from the full file's own
packet positions, and that x-outflux-seek reports method=toc/cbr rather than
estimate.

On this machine corpus/ does not exist and ffprobe is not on PATH (both
verified directly, including that bare-subprocess's spawnSync throws
synchronously on ENOENT rather than returning a nonzero status -- the
try/catch in test/helpers/ffmpeg.js is load-bearing for that reason). The
test therefore skips; a skip is not evidence, and TODO-V1.md's Phase 5
acceptance line is left unticked with the exact command that would
demonstrate it. The mechanism ticks (Range endpoint, seek mapping, state
machine, instrumentation) are each backed by an existing passing test.

Also: added bare-subprocess to devDependencies; recorded two bare-http1
findings (writeHead not lowercasing header keys vs. setHeader lowercasing,
and req._socket being nulled synchronously before a handler's first await)
plus the HTTPAgent.global keepAlive default, each verified against library
source, in docs/superpowers/notes/bare-dependency-audit.md; and added a
Phase 4 note that failover should subscribe to playstate's `stalled`
transition rather than re-deriving stall detection in the swarm transport.

Full test-body logic (corpus track selection, ffprobe CSV parsing, the
packet-position ground-truth match) is unexercised on this machine -- see
.superpowers/sdd/2026-09-01-outflux-v1-playback/task-10-report.md for what
was and was not verified.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Six joins between separately-reviewed pieces, all in the playback server.

I1 - nothing ever emitted `idle`, so `stalled` was unreachable and
`stallMs` was dead configuration. The streaming loop now arms a timer
across every wait on the transport's iterator and applies 'idle' when the
wait outlasts stallMs, re-arming until bytes move again. Cleared on
chunk, end, abort and error, and again in a finally, so no response can
leave a timer holding the loop open.

I3 - `res.write()`'s return value was ignored, so a slow player made the
whole remainder of a track accumulate in the response write buffer:
invariant 5's failure mode reached from the write side. Now awaits drain.
Invisible before because every test body was 4096 bytes against a 64KB
blob chunk, so every test streamed in exactly one iteration; a 200KB
fixture makes the multi-chunk path real.

I4 - the catch destroyed the socket claiming "headers are already out".
They are not: nothing here calls flushHeaders(). A failure before any
byte has been written now answers 503 with the stale content-range and
content-length cleared first; only a failure after a body has started
tears the socket. Branches on a bodyStarted flag rather than
res.headersSent, since bare-http1 flushes headers from inside _write().

I6/M4 - 'requested' was dated after resolveHash, available() and size(),
which under Plan 4's swarm transport are exactly the terms that dominate
time-to-first-audio. The arrival timestamp is captured at the top of
onRequest and threaded down. Sessions now open only once the request is
actually going to serve bytes, so 400 and 416 no longer leave phantom
sessions stuck in `buffering` for the CLI to print on shutdown.

M2 - the comment claimed Bare has no global Buffer. It does; reworded to
say what is actually true, that b4a is the cross-runtime byte shim.

M3 - the 409 body reported `matches.length` as a track count, but
resolveHash stops at 2. Says "at least 2".

M5 - `?t=` equal to durationSec yielded an empty 200. The seekable
interval is now half-open [0, duration), matching every other range API
here, with the reasoning recorded at the boundary.

M10 - the ID3 probe read was the one unclamped read path. Clamped, via a
small clampAsFetch helper shared with the frame window.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
I5 - both files that called createServer used memoryBlobs, so spec ss10's
"Integration, real server, real drive" was never delivered, the server
never streamed more than one chunk, and it never saw an abort. Worse, the
existing "a range at the very end of the blob does not hang" test -- whose
own comment says a timeout there IS the failure -- ran against memoryBlobs,
where the shape it names is trivially safe and cannot hang.

Adds test/integration/serve-drive.test.js over a real Corestore-backed
drive with a 512KB body: a full read and a deep range checked byte-for-byte
against the source, the end-of-blob range where an unclamped Hyperdrive
read really does wait forever (audit Q5), and a client that destroys its
socket after the first chunk, asserting endedAs 'aborted' and nothing
logged at error level. Drive and Corestore are closed and the temp
directory removed in a finally; a leaked handle hangs the suite.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
M9 - a Title-Case key handed to bare-http1's writeHead emits the header
twice and the client's parser rejects the response, on every response the
server makes. Until now the only thing standing between the codebase and
that outage was a comment asking people not to tidy the keys.

Adds a guard to test/source-hygiene.test.js: it strips comments (the
explanatory comment itself contains 'Content-Length'), walks each
writeHead( ... ) call by balancing parentheses, and fails on any uppercase
letter in a quoted header key -- plus a file-wide check on hyphenated
quoted keys, which catches the header object server.js builds in a
variable and passes to writeHead indirectly. Verified by temporarily
reintroducing 'Accept-Ranges', which the guard fails on.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
I2 - refineSeek was dead code and the spec's +/-1s "bounded scan" was
never wired in. It could not be: it corrects by a caller-supplied
deltaSec, and deltaSec is exactly the unknown. Knowing the true time at a
byte offset needs a frame count from audioStart -- O(file), and the end of
the sparse-fetch property the content layer exists to provide. The plan
already named the real remedy, an import-time frame index, as a separate
plan.

Deletes refineSeek, its four unit tests, and its export. snapToFrame
stays: it is called and it does real work, since frame alignment is what
stops a decoder emitting noise at the seek point.

The descope is recorded where someone would look for the guarantee rather
than only in a commit message:
- spec ss5 no longer says "the scan still ships as the guarantee"; it says
  V1 accuracy is TOC interpolation plus snapping, that snapping fixes
  alignment and not absolute error, and that an import-time frame index in
  its own plan is the remedy if the acceptance run misses +/-1s. The
  seek.js signature block is also corrected to what was actually built.
- TODO-V1 Phase 5 says the same in that file's voice.
- The tocOffset comment in seek.js no longer points at a function that
  does not exist, and states plainly that nothing in V1 corrects the TOC's
  absolute error.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
C1 - the acceptance test handed ffprobe a http://127.0.0.1:<port>/ URL
served by the same process, then called it through bare-subprocess's
spawnSync. spawnSync runs its own fresh uv loop and does not pump the main
one, so the in-process server serves nothing while it blocks, and ffmpeg's
HTTP protocol has no default read timeout -- so it waits forever for a
response that cannot arrive until it returns. The suite would have hung,
permanently, on the first machine that actually had a corpus and ffprobe:
the only machine where this test ever runs for real.

The packet map is a property of the FILE, so ground truth no longer goes
over HTTP. The selected track's bytes are pulled back out through the
ContentTransport into a scratch file under test/.tmp-playback and ffprobe
is pointed at that path; the file and its directory are removed in a
finally. The HTTP path is still exercised by the time-to-first-byte
assertion and by the x-outflux-seek header the probed offset comes from.

ffprobe is also given an explicit -rw_timeout, so a future variant that
reaches for a URL again fails loudly instead of hanging.
bare-subprocess's spawnSync accepts no timeout option of its own, so the
timeout has to be handed to ffprobe.

Still SKIPS here: this machine has neither corpus/ nor ffprobe, so the
change is verified by reading and by lint/suite, not by a run.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
M6, which the review deliberately capped at a clarifying comment rather
than a behaviour change: a ?t= response promises the length of the
remainder of the track with no content-range explaining the truncation.
The reasoning (the client sent no Range, so answering 206 would confuse
players that do speak Range) is now written at the site instead of living
only in a review thread.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two residuals from the final whole-branch review, both comment-only:

M2 -- the comment in send() asserted b4a was needed because Bare has no
global Buffer. That is false: `bare -e "typeof Buffer"` prints `function`,
and bare-http1 itself calls Buffer.from. Reworded so it no longer asserts a
falsehood: b4a stays (correct house style, one byte API identical under
Node and Bare), and the comment now says exactly that.

M6 -- the known-and-deliberate truncated-200 note on `?t=` responses now
sits at the site where the trade is made (the headers built for writeHead)
rather than below the content-range line, and says plainly that the strict
RFC 7233 reading (partial body => 206) is declined on purpose. Behaviour
unchanged; the review capped M6 at a clarifying comment.

No test changes: 337/337 pass, lint clean.

Co-Authored-By: openhands <openhands@all-hands.dev>
yash merged commit 5bf262910e into main 2026-09-01 03:26:13 +00:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
yash/OutFlux!1
No description provided.