fix(rerun): shut down live worker on eval start to prevent blueprint …

[Adityakk9031]

Closes #267.

Problem

When an eval ends without on_eval_end (e.g. Ctrl-C or crash path) and the caller reuses the RerunSink instance, the still-running worker thread from the previous eval can interleave with bind_spaces and on_eval_start blueprint resets on the subsequent eval. This could cause torn blueprint payloads or suppressed blueprint warnings.

Fix

In on_eval_start, check if a worker thread is still alive and shut it down before resetting _blueprint_prefix and _blueprint_warned.

Note on residual race: The disowned-worker case (a worker wedged in send_blueprint past flush_timeout) can still set _blueprint_warned after the reset, so cosmetic race 2 from #267 is narrowed rather than fully eliminated.

Verification

• ruff check . — Passed.
• ruff format --check . — Passed.
• pytest tests/test_rerun_sink.py — 66 passed, 4 skipped, 0 failures.

[Adityakk9031]

@aris-zhu and @jeqcho have a look

[jeqcho]

Thanks for picking up #267 so quickly — this lands exactly the candidate fix the issue sketched, and it's in the right place: stopping a leftover worker before the blueprint reset (and before rr.init re-points the global recording) closes the reset interleave and bounds cross-eval step contamination. The is_alive() guard is a nice touch too, since it keeps the normal start path from paying the _probe_recording_flush timeout. I also verified the new test fails without the source change and passes with it, so it genuinely guards the fix.

A few small asks before merge, all in the test:

1. The hand-rolled FiringRR fake is missing Scalars/Scalar, so the worker's emit actually raises AttributeError inside the test and triggers the "RerunSink failed to emit a step" warning (you can see it in the pytest output), and FiringRR.events never receives anything. Please use the file's existing _install_fake_rerun(monkeypatch) + _log_one(sink, t) helpers instead — they simulate a working SDK surface, avoid the stray warning, and will roughly halve the test.
2. Placement: the test currently sits between test_real_rerun_accepts_the_blueprint and test_real_rerun_process_exits_when_tcp_peer_never_reads, which are the real-SDK group at the end of the file. It fits better next to test_eval_end_shuts_down_worker_and_log_step_restarts_it with the other lifecycle tests.
3. This module comments its non-obvious moves pretty thoroughly — a one-liner above the new guard (e.g. "a crash path can skip on_eval_end; stop the previous eval's worker before resetting blueprint state so its writes can't land after the reset") would help future readers.
4. Optional, but worth a sentence in the PR description: the disowned-worker case (a worker wedged in send_blueprint past flush_timeout) can still set _blueprint_warned after the reset, so #267's cosmetic race 2 is narrowed rather than fully eliminated. Fine to still close the issue given how it was framed — just good to state the residual explicitly.

The core change is solid; happy to approve once the test is reworked.

[Adityakk9031]

Thanks for the detailed review @jeqcho!

I have addressed all the feedback and rebased the branch onto the latest main:

• Test helpers: Refactored the test to use _install_fake_rerun(monkeypatch) and _log_one(sink, 0), eliminating the stray warning.
• Test placement: Moved test_sink_reuse_after_interrupted_eval_shuts_down_previous_worker to sit alongside the lifecycle test group next to test_eval_end_shuts_down_worker_and_log_step_restarts_it.
• In-code documentation: Added the explanatory comment above the _shutdown() check in on_eval_start.
• Merge conflict & PR description: Resolved the conflict with upstream in on_eval_start and updated the PR description to explicitly document the disowned-worker residual note.