fix: let a cold engine finish starting before the launcher gives up - #112

@codex review this pull request at head dcd8bb59f6a47ecaf85a9318c787979c2f40a5e7.

@codex review

Head under review: 5f4545fc0e978a40fcd31698aa7b74a111fd1fa8. Please bind your verdict to that exact SHA — the
branch moved with this correction round, so a verdict against the previous head
does not carry.

This round answers the findings you raised on the prior head. The coordinator
verified the lane's claim by reverting the production change and confirming the
new assertions redden, and checked each deleted assertion against its
replacement rather than trusting a net-positive count.

The derived pair was regenerated to a fixed point as the last commit and the docs
gate is green at this head. If a finding is unaddressed rather than fixed, please
name it rather than treating silence as agreement.

@codex review

Head under review: bda91eb5eb3415a6a08d136fa451a85c922f08bf (verified equal to
origin/fix/cold-engine-startup-budget-20260828 at the moment of writing).

Reversion control, run this turn on Ubuntu 22.04. Production change reverted, the round's
tests kept:

PHASE 1 tests only, no production change -> 5 failed, 121 passed
 test_manual_restart_keeps_cold_readiness_wait_off_qt_callback[confirmed-dialog]
 test_live_replacement_health_stays_visibly_down_until_exact_readiness
 test_shutdown_during_runtime_readiness_restores_command_transport
PHASE 2 with production -> 126 passed

Documentation gate: tests/docs 68 passed. No test was deleted, skipped, weakened or
restated; the control applies the production half only and keeps every test.

What I want you to attack. This round is about cold start on the laboratory machine, so the
failure it must not introduce is a start that looks ready before it is. Two questions:

1. Does health stay visibly DOWN until exact readiness on every path, including the one where
   the operator confirms a dialog — or is there a window where the interface reports ready
   while the runtime is still coming up?
2. On shutdown during runtime readiness, is the command transport genuinely restored, or
   merely re-created in a state that would accept a command it cannot deliver?

Disclosure: this change was produced and verified with AI assistance.

Correction to my review request above — do not review bda91eb5e yet.

I asked for a review of bda91eb5eb3415a6a08d136fa451a85c922f08bf and presented it as
verified. The remaining partition is red on both platforms, and part of that is my push.

Measured this turn on Ubuntu 22.04 over tests/test_launcher_backoff.py and
tests/e2e/test_d7_4_phase2_acceptance.py:

origin/master (d5a0f315e) 46 passed, 0 failed
this branch, last round reverted 6 failed, 40 passed
this branch as pushed 10 failed, 36 passed

Six nodes were already broken by this branch before my round — all six pass on master, so
the branch causes them:

e2e::test_t3_static_all_five_invalidate_before_start
backoff::test_replay_bridge_attach_failure_settles_verified_child_before_retry
backoff::test_replay_readiness_failure_settles_child_before_scheduling_next_generation
backoff::test_restart_shot_fires_when_still_pending
backoff::test_stale_restart_generation_cannot_consume_a_new_crash_restart
backoff::test_start_engine_has_no_readiness_bypass_and_health_does_not_call_it_directly

Four more came from my round. Reverting only src/cryodaq/launcher.py makes exactly
these four pass again:

e2e::test_t3_real_restart_via_restart_engine
backoff::test_clean_pre_spawn_live_restart_recovery_stays_operator_retryable
backoff::test_manual_live_restart_startup_failure_settles_new_child_before_backoff
backoff::test_manual_replay_bridge_attach_failure_settles_new_child_before_backoff

How I missed them. I verified the round against the test files its brief named —
test_launcher_cold_start_budget.py and test_d7_1b_repair.py — and the documentation gate.
I did not run tests/test_launcher_backoff.py, although the round modified
src/cryodaq/launcher.py, which that file exists to cover. Running the tests a change names
is not the same as running the tests a change can reach. This is the second time today I have
made that mistake; it is now written into the working rules as a pre-push gate over every file
a round touches.

These ten are not cosmetic. They govern what happens to a child engine process across a
restart: settling a failed child before the next attempt, refusing a stale restart generation
that would swallow a new crash's restart, and keeping the operator able to retry. A launcher
that mishandles those does not lose a test — over a week-long run it loses the engine, and
with it the measurement.

A lane is working all ten now. I will post a fresh review request when there is a head that has
actually passed. Please disregard the previous request rather than reviewing a head I have
since found red.

This is red at 92326392, and the cause is the design change, not the test harness. Recording it here before it is fixed, because the finding is worth more than the fix.

What fails

remaining fails on Ubuntu in both workflows, one node per invocation, taken from the failure receipts rather than from a summary line:

invocation 1 tests/e2e/test_d7_4_phase2_acceptance.py::test_t3_static_all_five_invalidate_before_start
invocation 2 tests/e2e/test_d7_4_phase2_acceptance.py::test_t3_real_restart_via_restart_engine

Reproduced by hand at the same head: 2 failed, 16 passed in that file.

Cause 1 — necessary, and not sufficient

This branch adds a third bare restart_epoch = self._runtime_callback_epoch in the manual restart path, earlier in the flow than the two reads master already had. The module's own idiom everywhere else — including _runtime_callback_is_current, which compares against getattr(self, "_runtime_callback_epoch", -1) — is the defaulted read, and this same commit two lines later already uses vars(self).get("_restart_generation", 0). Making the new read consistent removes the AttributeError.

The tests still fail after that, so this is not the real cause.

Cause 2 — the real one

test_t3_real_restart_via_restart_engine calls the real LauncherWindow._restart_engine and asserts bridge.start is called exactly once during that call.

Master attached the bridge inside _restart_engine. This branch defers it: the readiness wait moves to a worker, and bridge attachment, the replay-session bind and the UI authority bind all happen later from a QTimer.singleShot callback. So _restart_engine returns with bridge.start not yet called, and the assertion fails at 0 calls. The observed sequence is:

phase=readiness failure=TypeError -> bounded retry
phase=readiness-reschedule failure=AttributeError -> HOLD

The TypeError appears because the new path calls self._wait_engine_ready() directly, while master reached it only through _start_engine. Readiness waiting moved out from behind that call.

Why this is not a test to update

A restart now completes only if the Qt event loop is pumping. In any context where it is not, the engine comes back up and the bridge never attaches — and the only signal is a log line while _restart_pending stays true. For an unattended week that is a restart that half-completes, which is the first failure mode on this project's list.

Whether the loop is always running when restart is invoked was not established either way. The acceptance suite asserts the opposite contract and is the only thing that noticed. It will not be edited to accommodate production.

What happens next

Two options, and the choice is being made caller-by-caller from the code rather than by preference:

• A. Keep the off-thread wait and make completion independent of the loop, so _restart_engine still attaches the bridge before returning when no loop is running. The acceptance contract must hold again.
• B. Take the off-thread wait out of this branch and keep only the budget raise, the monotonic deadline and the progress and attempt logging — the part that was asked for, is proven by controls, and does not touch this contract. The off-thread work then gets its own round with its own evidence.

The 37 guards this branch adds and updates stay green under either.

Written with AI assistance.

@codex review

Please review exact head 9dd1fd815e6641ac26859b5921ded609024a10c2.

Why this correction is on PR #112: the newly active acceptance selection exposed a pre-existing resource leak in tests/e2e/_zmq_harness.py. The failing test_t3_real_restart_via_restart_engine and the fixture are byte-identical on the previous PR head and on the baseline that was compared. The fixture created an asyncio event loop, stopped its thread, but never called loop.close(). The remaining-suite resource observer therefore found one loop and its two self-pipe sockets after teardown. This is test-harness cleanup, not a weakening of the PR #112 startup contract.

What changed:

• successful fixture teardown now stops the loop, performs a bounded thread join, fails if the thread remains alive, and then closes the loop;
• the sentinel setup-failure path uses the same cleanup sequence;
• a direct lifecycle regression test uses a real event loop and thread and observes the side effects: the thread is dead, the loop is not running, and loop.is_closed() is true.

Coordinator verification on this exact content:

• Windows focused lifecycle guard plus the real ZMQ loopback: 2 passed;
• removing only loop.close() was asserted to change the file and made the lifecycle guard fail with pytest exit 1 on loop.is_closed() is True;
• the file was restored byte-for-byte to blob d24ef6c5ee68ccca04a3c91ed5605fcabce7d3f9, after which the same focused set passed again;
• Ruff lint, Ruff format check, and git diff --check passed;
• a standalone privacy scan found no new private-layer names or internal identifiers in the tree delta, commit message, or PR metadata.

An Ubuntu 22.04 implementation run also passed the lifecycle guard plus real ZMQ loopback. Its broader local candidate runner stopped before pytest because that WSL clone lacked an immutable defective commit referenced by an existing prevention receipt; this comment does not claim a green remaining partition.

Please focus on exception paths during fixture setup, double-stop races, Windows Proactor versus Ubuntu selector-loop behavior, and whether the direct guard could remain green while the actual fixture still leaks.

@codex review d994288

@codex review

Exact-head review requested for c4e555d8f69357c64cfd10744fbaedde0e2881c0.

Why this head was pushed:

• It closes the three findings on the previous head: the Windows selector-loop guard is now registered in the default remaining partition; the resource guard rejects any per-cycle growth; the generated metrics/SVG pair was regenerated after the content commit.
• Independent coordinator mutation control changed the exact-flat assertion back to the previous max - min <= 4 tolerance. The new +1 resource per cycle regression then failed because no AssertionError was raised. The mutation was removed and the scratch tree returned clean.
• Exact-head Windows checks: the three corrected lifecycle nodes passed; the complete prevention-registry test file passed 55 passed with PYTHONPATH explicitly bound to this worktree. The first attempt without that binding imported production modules from a different checkout and was discarded as invalid evidence.
• Native Ubuntu 22.04 qualification on this exact candidate reported the lifecycle, prevention, and freshness closure green, with the Windows-only selector test skipped as designed.
• A second generator run on a clean Ubuntu exact-head checkout produced no tracked diff. Ruff check/format, changed-path diff, and publication privacy scans are clean.

This is still a draft. Hosted CI and a clean Codex verdict bound to this exact head remain open gates. Please review this exact head for correctness, especially lifecycle ownership and whether the new exact-flat resource assertion can false-green under any platform-specific counter behaviour.