fix(record): replay the finished export from a retried record stop - #2534
Conversation
A remote record stop can outlive its client window while the daemon is still exporting. The finished manifest was then read as no active recording, and its metadata carried no client output path, so the caller had no way to collect the file. A repeated record stop now serves the completed export and says so in the timeout hint.
A mapped codec per completion property drives encoding and decoding from one declaration, and the declaration fails to typecheck if a property has no codec.
|
Size Report
Startup median (7 runs, lower is better):
|
Session teardown reaches the recording resource definition while it loads, and that eager closure takes no new module. Writing a completion is property reads only, so the field map and writers now live with the resource definition; reading one back needs the recording vocabulary and stays behind the stop path.
The hint assertions had outgrown the aggregate client test past its size ratchet; they mirror src/daemon-client/daemon-client-timeout.ts, so they move rather than shrink.
… key The manifest now holds the completion as the one object record stop returned, so a replay cannot lose a field between an encoder and a decoder, and the reader lives with the stop path that needs it. Recovery still refuses a response whose served path or caller-side paths are not whole.
|
No actionable code findings on 8139aeb. The simplified completion envelope validates stored metadata and the matching session/device before recovering an existing artifact. Current checks pass, including coverage, and there are no conflicts. Readiness still needs the live remote-export case: an export lasting beyond 90 seconds, a completed manifest after client timeout, and a retry that downloads the completed recording without starting another export. |
|
Live remote-export evidence on
Method note: to make the export deterministically exceed the 90 s window I ran this verification build with a temporary 120 s delay in the recording finalizer host adapter, gated on an environment variable. That patch is not part of this PR — Cleanup: session closed, proxy and host daemon stopped with |
…declare A stored scope is checked by isRecordingScope next to the vocabulary it validates, and a session's durable record path comes from the factory that names it instead of being re-derived at each read.
Command, session, and action all come from the same request, so they are passed as one request instead of three more positional arguments.
The client timeout handler stays off the daemon request shape: R10 daemon-modularity holds external importers of that module at the merge-base count, so the fields arrive as named properties instead of the request object.
|
No actionable findings on 811e4a3. The reported live remote-HTTP run at 8139aeb proves timeout, completed manifest, retry and SHA-matched download without a second export; the temporary finalizer delay tests recovery, not ordinary export latency. The later refactors preserve that behavior. Required live evidence is satisfied; merge readiness follows the current checks. |
thymikee
added
the
ready-for-human
…nsports A socket timeout and an HTTP timeout described the same request with two copies of the same mapping.
|
No actionable findings on 0cb272a. The shared timeout-context helper preserves the same fields for socket and HTTP requests, so the earlier live timeout/retry/download proof still applies. Current checks pass and there are no conflicts; ready for human review and merge. |
1 participant
Summary
record stopagainst a remote daemon can run out of its 90 s client window while the daemon is still exporting. The manifest then reachescompleted, but a repeatedrecord stopread it asno active recording, and the stored completion carried no caller-side output path — the recording existed on the daemon host with no CLI way to collect it (#2281).A repeated
record stopnow serves that export from the durable manifest: session and device guards still apply, the file must still exist, and the caller gets the ordinary stop response, including the caller-side paths that make it downloadable. The manifest stores the finished response as the one objectrecord stopreturned, under a single key, so a replay cannot lose a field, and recovery refuses a response whose served or caller-side paths are not whole. The replay records no second session stop action, and a remote timeout now names that retry.16 files touched. No new command, flag, or timeout change; the 90 s envelope is untouched. Addresses #2281 — its
needs-inforequest for customer evidence is still open.Validation
pnpm check:affected --runpassed at0cb272ae553a9b48c2189a8b8ce2dda8d63e9bcd(format, lint, typecheck, layering, fallow, build, related vitest, daemon wire compat, command docs), and all PR checks including Coverage pass on that head.Regression proof: removing the completed-manifest replay fails
record stop returns the export whose response never reached the callerand its downloadable-path sibling insrc/daemon/handlers/__tests__/record-runtime-stop-recovery.test.ts.Two gates shaped the code: session teardown's eager-closure budget keeps the completion writers inside the resource definition, and the test-size ratchet moved hint assertions to
src/daemon-client/__tests__/daemon-client-timeout.test.ts.Live evidence for the readiness case is in a comment on this PR, captured at
8139aeb1ea; the reuse commits after it declare no behavior change. On an iPhone 16 Pro simulator behindagent-device proxy,record stoptimed out at 90 s, the manifest turnedcompleted97 s later, and the retriedrecord stopreturned in under a second and downloaded a recording whose SHA-256 matches the host file, with one export in the host log.