docs(adr): ADR 0024 — screen recording as one coordinator with two facts (proposed) - #2588
Conversation
… two facts Records the design for record start/stop after the last three months of fixes: one coordinator, the manifest as the only lifecycle store, backend-owned signalling, and the export-exists / recorder-stopped split. Amends ADR 0019 section 5 for the screen-recording kind by pointer. Adds the index row and the Recorder observation glossary term.
…s by generation Checkpoints now mark durable, valid artifacts (collected after the playability sniff, finalized after the export is written), never an attempted stop; an unconfirmed stop is re-run on retry until confirmed or lost, or until an export is committed. A replacement start archives an unsettled manifest under its fence generation before adoption so recovery can still enumerate it; the next generation is the maximum across the session directory. Drops the CONTEXT.md glossary addition: the file sits one byte under its 12,000-byte budget on main, and the ADR defines the term.
|
One ownership gap in the proposal at 423c8c6: section 2.3 removes the raw capture after commit even when recorder is unconfirmed, while the matrix describes local/Web collection as “none.” A playable export does not prove that the native writer has stopped using that path. Define collect as producing a separate immutable copy when termination is unconfirmed; retain the native path and identity until the backend proves cleanup safe. Only the collected copy may be finalized or removed after commit. A lost PID identity alone must not permit deleting the native path. Add the local WebM/simulator case where a playable export exists while the writer remains unconfirmed. Please also remove trimming from the finalization/retry examples and account for #2586 in the rollout. The fence, archived-manifest and recovery separation otherwise looks coherent. Documentation checks pass with no merge conflicts; this needs a contract correction before implementation. |
…nfirmed stop A playable export never proves the native writer stopped using its path, so collect always copies into a sibling of the export path on every backend, local ones included, and only that copy is finalized or removed after commit. Native-path disposition is a backend proof recorded at commit: confirmed or proven gone; a lost pid identity alone never permits deletion. Drops trimming from the finalization examples (PR #2586) and records that PR in the rollout.
|
The separate collected copy fixes the original ownership gap at cefc7ec, and the trimming update is consistent with #2586. One contradiction remains in section 2.4: it still removes an archived manifest when recorder is lost, although section 2.3 now requires retaining the native path and identity until safe disposition is proven. That deletes the only recovery record while the native artifact still needs it. Retire the archive only after native-path disposition is complete, not merely because recorder is confirmed or lost. Keep manifests with pending native disposal in recovery’s enumeration, including confirmed recorders, and cover a lost recorder whose native path must remain retained. The latest wording-only commit does not change this gap; checks for that new revision are still settling. |
|
Resolved in 207c000. Archive retention and recovery now depend on completed native-path disposition, including confirmed or lost recorders with pending cleanup. Failed retirement preserves the manifest for retry; acceptance cases cover both paths. The final design review is clean. Formatting, git diff --check, and pnpm check:affected --run pass on this commit; this is documentation-only, so no runtime tests were selected. No merge conflicts; new-head CI is queued. |
thymikee
added
the
ready-for-human
|
1 participant
Summary
Proposes ADR 0024 for
record start/record stop. Nine record bugs since July; five were the ownership machinery refusing on its own uncertainty (#2549, #2550, #2476, #2281, #1117), not the native recorders. Reattachment is hard-codedunreattachableon four of five backends today, so a post-daemon-lossrecord stopthrows on Apple, HarmonyOS, and web even with a finalized file on disk.The design: one recording coordinator, the session manifest as the only lifecycle store, signalling owned entirely by each backend (
start/stop(handle | descriptor, budget)/collect), and two independent facts on the stop response: whether a playable export exists, and what the backend observed of the recorder (confirmed | unconfirmed | lost). Recovery enumerates abandoned or unsettled manifests and asks the coordinator to settle the recorder under the fence; it never signals or deletes on its own. A failed collect, finalize, or commit preserves the raw capture and the manifest phase for retry instead of running forced cleanup.Also records, so they are not re-litigated: the fence stays on every destructive step; native markers (
owned-processes.json, the Android device marker) stay for the crash windows before a manifest exists; chunk rotation stays below API 34;--time-limit 0on API 34+ is a separate later step.Files: the ADR, its index row in
docs/adr/README.md, and one glossary term inCONTEXT.md. No code.Evidence in the ADR
adbon an API 36 emulator: clip ends at the last screen change, not atSIGINT; PR fix(android): report the clip an Android recording really captured #2566's disclosure is the whole fix.finishDurableCaptureHandlerunsforceCleanupwhen finish throws; on Android that removes the remote artifact after a failed pull (transitions.ts→cleanupVerifiedAndroidEvidence).exportProcessedVideotrims and overlays in place, so a retry after a commit failure needs the raw capture preserved.Review history folded in
Two review rounds and one design challenge shaped the text. Rejected and recorded: retirement that deletes unresolved evidence; a backend interface that exposes probe sequencing to the coordinator; a live-handle bypass of identity checks;
owned-processes.jsonas a second lifecycle store; recovery that replays a committed export instead of settling the recorder; a promise that stop fails for exactly two reasons.Relation to open PRs
#2564, #2565, #2566 are rollout step 1: their tests are the "keep" set in section 5. #2377 (compiled
pshelper for sandboxed macOS) should wait for this decision; under rule 1 a blockedpsno longer wedges a stop.