Skip to content

fix(recorder): keep late progress updates from rewriting the state being sealed - #158

Merged
abrichr merged 3 commits into
mainfrom
fix/control-concurrent-stop-flake
Oct 17, 2026
Merged

abrichr merged 3 commits into
mainfrom
fix/control-concurrent-stop-flake

Conversation

@abrichr

@abrichr abrichr commented Oct 17, 2026

Copy link
Copy Markdown
Member

Problem

tests/test_control.py::test_concurrent_repeated_stop_is_idempotent fails intermittently on test-macos:

  • 2026-10-14 run 38601787804: both stop callers get CaptureControlError('Capture control failed: capture_seal_failed')
  • 2026-08-27, on older code: the same test fails with recording_or_finalization_failed

Root cause

This is a real race in Recorder, not just a slow test. A timeout bump would not fix it.

  1. After record() returns, _run_record writes the final capture-state.json (_stage_completed_control_state). It then calls seal_capture, which inventories every file in the capture directory, including that file, and verifies the inventory.
  2. _transition_control only refused updates once the in-memory phase was terminal. That phase becomes complete only after the seal. During the seal, any non-terminal transition still rewrote capture-state.json:
    • the status-drain thread handling a late record.stopping → finalizing, which happens when that thread is scheduled late on a loaded runner, or
    • a stop caller's finalization_timeout transition.
  3. If the rewrite lands during the seal, an inventoried artifact changes (or its atomic-write temp file appears), and the stop returns capture_seal_failed. If it lands just before the seal, the capture seals a stale finalizing state.

Reproduced deterministically: delivering the finalizing update right after build_artifact_manifest returns gives exactly capture_seal_failed.

Fix

Once the final state is staged, _transition_control ignores non-terminal transitions. That file now belongs to the seal. A terminal outcome, such as a seal failure marking the capture failed, can still replace it. A stop caller that times out still gets its finalization_timeout response; the timeout just no longer rewrites a sealed artifact.

Test

Added test_late_progress_update_cannot_rewrite_the_state_being_sealed. It runs an in-process recorder and delivers late finalizing and finalization_timeout updates in the middle of the seal.

  • Without the fix it fails with capture_seal_failed.
  • With it, the stop completes, verify_capture_artifacts passes, and the sealed state is complete.

Locally on macOS: tests/test_control.py passed 5 runs in a row, and the full non-slow suite passed (871 passed).

…ing sealed

The worker writes the final capture-state.json and then seals the
capture directory, inventorying that file. A non-terminal control
transition that lands after staging still rewrote it: the status-drain
thread's late "record.stopping" (finalizing) on a loaded host, or a stop
caller's finalization timeout. A write during the seal changed an
inventoried artifact, so stop returned capture_seal_failed
(test_concurrent_repeated_stop_is_idempotent on test-macos). A write
just before it sealed a stale finalizing state.

Once the final state is staged, ignore non-terminal transitions; a
terminal outcome such as a seal failure can still replace it. Add a
deterministic regression test that delivers the late update mid-seal.
@abrichr
abrichr merged commit af945e2 into main Oct 17, 2026
14 checks passed
@abrichr
abrichr deleted the fix/control-concurrent-stop-flake branch October 17, 2026 22:50
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