Conversation
Measured by running the program on 2026-08-28 at 00:36, with --mock, on a fresh
worktree with no warm caches:
00:36:51 launcher: Engine запущен, PID=82784
00:36:56 launcher: CRITICAL construction failed; phase=engine
00:36:57 launcher: CRITICAL retained all construction owners in HOLD
00:37:01 engine: SafetyManager запущен: состояние=safe_off
Read the last two lines together. The launcher declared the engine failed at
00:36:56. The engine finished starting correctly at 00:37:01, five seconds after
it had been abandoned. Nothing was wrong with the engine.
`_wait_engine_ready` allowed ten attempts at half a second: a five second budget.
A cold engine needs about ten. It loads interlocks, alarms, channel landmarks,
the vacuum guard, sensor diagnostics, the trend predictor, the SQLite writer and
the safety broker before it may answer READY, and those are fail-closed startup
owners that are supposed to be established first.
When the wait expired the launcher held every construction owner and the operator
got a window that never rendered. The program refused to start and said nothing.
The standing rule here is that this software must never refuse, because when it
refuses the operators wire the hardware outside it and the system loses its
ability to see, control or record anything. This was that rule failing in its
most literal form, and on a fresh machine it is the operator's first experience.
The wait is now bounded by a monotonic deadline of one minute — six times the
measured cold start, while still reporting a stuck child in about a minute. The
bound is a deadline rather than an attempt count, so slow polls cannot silently
extend it. Startup progress is now logged rather than passed over in silence,
which is what the never-refuse rule asks for: say what is happening. A dead child
or an invalid readiness receipt still fails immediately; only patience for a
healthy, slow start has changed.
The replay entry point shared the same five second default and receives the same
correction.
The regression drives the real path: an engine that becomes ready later than the
old budget but within the new one must let the launcher proceed. It fails with
the production change reverted.
Measured limit, stated plainly: this was reproduced on Windows. On Ubuntu 22.04
the same engine reached readiness at attempt 4 of 10, about two seconds, so the
old budget was sufficient there and this fix is not what unblocks the Ubuntu
end-to-end run.
Written with AI assistance.
|
@codex review this pull request at head |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dcd8bb59f6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| remaining_s = deadline - time.monotonic() | ||
| if remaining_s <= 0: | ||
| break | ||
| time.sleep(min(interval_s, remaining_s)) |
There was a problem hiding this comment.
When an engine crashes after the UI is running, the QTimer.singleShot callback _do_restart calls _start_engine() synchronously, which reaches this sleep-and-probe loop on the Qt thread. With the new defaults, a slow or stuck replacement can therefore block repainting, tray interaction, the audible down alarm, status timers, and operator shutdown for roughly 60 seconds; use nonblocking timer/worker polling for runtime restarts, or restrict the expanded synchronous budget to construction before app.exec().
AGENTS.md reference: AGENTS.md:L540-L542
Useful? React with 👍 / 👎.
| # the exact readiness receipt. The laboratory Ubuntu cold-start measurement | ||
| # was about ten seconds, so retain a six-times margin while still reporting a |
There was a problem hiding this comment.
The reviewed commit's own evidence says the approximately ten-second --mock cold start was reproduced on Windows, while Ubuntu 22.04 became ready in about two seconds; calling this a laboratory Ubuntu ten-second measurement records both the platform and evidence context incorrectly and may cause later timeout tuning or acceptance work to rely on evidence that was never collected.
AGENTS.md reference: AGENTS.md:L16-L18
Useful? React with 👍 / 👎.
| @pytest.mark.parametrize("replay", [False, True], ids=["live", "replay"]) | ||
| def test_launcher_waits_past_old_budget_for_exact_cold_start_readiness(monkeypatch, replay: bool) -> None: |
There was a problem hiding this comment.
This commit explicitly corrects a confirmed operator-impacting startup failure and adds deterministic regression tests, but neither new pytest node appears in governance/agent_preventions.yaml under a stable prevention ID with the failure consequence and evidence disposition. Without that registration, the guard can later be removed, skipped, or displaced from default CI without automatically reopening the prevention, so add the required prevention record and guard linkage for this failure mode.
AGENTS.md reference: AGENTS.md:L361-L365
Useful? React with 👍 / 👎.
Landed by the batch lander. The lane's own report and the coordinator's verification are recorded on the pull request.
|
@codex review Head under review: This round answers the findings you raised on the prior head. The coordinator The derived pair was regenerated to a fixed point as the last commit and the docs |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5f4545fc0e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ) | ||
|
|
||
| def _start_engine(self) -> None: | ||
| def _start_engine(self, *, wait_for_ready: bool = True) -> None: |
There was a problem hiding this comment.
The correction at this head routes only the automatic _do_restart path through the worker: both manual restart slots still call _restart_engine(), which invokes _start_engine() with this True default. When an operator retries after fixing a configuration error and the replacement is cold or stuck, _wait_engine_ready() therefore blocks the Qt thread for up to the new one-minute budget, preventing repaint, alarms, tray interaction, and operator shutdown; route manual post-app.exec() restarts through the same nonblocking readiness state machine.
AGENTS.md reference: AGENTS.md:L540-L542
Useful? React with 👍 / 👎.
| if not wait_for_ready: | ||
| return |
There was a problem hiding this comment.
When an automatic mock/live replacement reaches this early return, its process handle is already published but exact readiness is still pending. The health timer continues running, and unlike replay, _is_engine_alive() has no live-readiness check, so its next tick sees the child process alive, paints the Engine status green, and clears the down banner and audible alarm before the worker verifies the receipt. Make the in-flight live readiness state force health to remain false until _readiness_succeeded completes.
AGENTS.md reference: AGENTS.md:L533-L536
Useful? React with 👍 / 👎.
| LauncherWindow._announce_soak_bridge_turnover(self) | ||
| phase = "ui-authority-bind" | ||
| LauncherWindow._publish_replay_ui_authority(self) | ||
| self._start_engine(wait_for_ready=False) |
There was a problem hiding this comment.
If the operator quits while an automatic mock/live replacement is waiting asynchronously, this path has already shut down the bridge and left the new child alive. Shutdown then revokes runtime callbacks, so even successful readiness cannot restart that bridge, while _stop_engine() can obtain its required exact shutdown receipt only by dispatching launcher_shutdown through the now-stopped bridge; the command is rejected and launcher shutdown remains in HOLD. Keep a shutdown-capable transport available or explicitly settle the in-flight replacement before revoking its completion callback.
AGENTS.md reference: AGENTS.md:L20-L22
Useful? React with 👍 / 👎.
Landed by the batch lander. The lane's own report and the coordinator's verification are recorded on the pull request.
|
@codex review Head under review: Reversion control, run this turn on Ubuntu 22.04. Production change reverted, the round's Documentation gate: What I want you to attack. This round is about cold start on the laboratory machine, so the
Disclosure: this change was produced and verified with AI assistance. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bda91eb5eb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| self._clear_engine_down_banner() | ||
| self._data_timer.start() | ||
| self._health_timer.start() | ||
| LauncherWindow._begin_engine_restart_readiness( |
There was a problem hiding this comment.
When a tray-only operator confirms the restart dialog, _on_restart_engine skips the visible label update, and this route has already stopped the health timer before beginning the asynchronous readiness wait. The existing tray icon and tooltip can therefore remain green for the full cold-start budget even though the old bridge is down and exact readiness is pending; publish an explicit disconnected/down tray state before returning to the event loop.
AGENTS.md reference: AGENTS.md:L533-L536
Useful? React with 👍 / 👎.
| def send_command(self, command: dict[str, object]) -> dict[str, object]: | ||
| assert self.alive, "launcher_shutdown reached a stopped transport" | ||
| calls.append("launcher_shutdown") | ||
| process.alive = False |
There was a problem hiding this comment.
When shutdown races a cold engine before its safe REP endpoint is bound, this test cannot distinguish genuine delivery from local bridge admission: the fake start() only flips a boolean, and the fake send_command() itself kills the process and fabricates the verified receipt. It therefore passes even if the production ZmqBridge accepts or queues launcher_shutdown but never delivers it across the subprocess/ZMQ boundary; exercise the production bridge and engine ingress over loopback and require the exact returned receipt.
AGENTS.md reference: AGENTS.md:L380-L386
Useful? React with 👍 / 👎.
| - node: tests/launcher/test_launcher_cold_start_budget.py::test_runtime_restart_keeps_cold_readiness_wait_off_qt_callback | ||
| ci_partition: remaining |
There was a problem hiding this comment.
The fresh correction adds guards for manual restart callbacks, visible DOWN state, and shutdown transport restoration, but this prevention record still ends after the original four guards (and the mapping test hard-codes that incomplete set). Consequently, any of these three new regressions can later be removed, skipped, or displaced from default CI without reopening LAUNCHER-COLD-START-READINESS-001; add all three nodes and extend the record's scope/invariant accordingly.
AGENTS.md reference: AGENTS.md:L387-L390
Useful? React with 👍 / 👎.
|
Correction to my review request above — do not review I asked for a review of Measured this turn on Ubuntu 22.04 over Six nodes were already broken by this branch before my round — all six pass on master, so Four more came from my round. Reverting only How I missed them. I verified the round against the test files its brief named — These ten are not cosmetic. They govern what happens to a child engine process across a A lane is working all ten now. I will post a fresh review request when there is a head that has |
# Conflicts: # docs/CLAIM_CORRECTIONS.md # docs/architecture-montana-important.svg # docs/current_candidate_metrics.md # governance/agent_preventions.yaml
The tuning note beside the raised budget stated a Windows cold start of about ten seconds and an Ubuntu 22.04 cold start of about two seconds. Neither was measured. The two-second figure is the design-system target for the app becoming interactive, which is a different quantity from the engine emitting its exact readiness receipt, and the ten-second figure has no source at all. A ten-second cold start also contradicts what the old five-second budget did: it succeeded. Replaced with the measurement actually taken on 2026-08-30, on Windows with --mock, from a clean slate with every cryodaq process killed between attempts: three starts, one of which never reached readiness, and both successes needing attempt 6 of 10, about three seconds of a five-second budget. The note now says the target platform has not been measured this way and claims no figure for it. The guard that pinned the note is rewritten to pin the property rather than the prose: the note must name the platform, mode, date and sample it measured, and must not present the untested platform or a design-system target as a measurement of this receipt. Control: with the Ubuntu claim put back in place, the guard fails at pytest exit 1 naming that exact sentence; restored byte-identically afterwards.
|
Master is merged in at The change. The correction. The tuning note beside the raised budget asserted a Windows cold start of about ten seconds and an Ubuntu 22.04 cold start of about two seconds. Neither was measured. The two-second figure is the design-system target for the app becoming interactive, which is a different quantity from the engine emitting its readiness receipt. The ten-second figure has no source, and it contradicts the old five-second budget succeeding at all. It is replaced with the measurement actually taken on 2026-08-30, on Windows with The guard that pinned that note is rewritten to pin the property instead of the prose: the note must name the platform, mode, date and sample it measured, and must not present the untested platform or a design-system target as a measurement of this receipt. Controls, all run at this head, each restored byte-identically afterwards.
Production file restored to blob No test was weakened. Eight assertions changed in Local evidence at this head. Written with AI assistance. Nothing above is reported that the evidence does not support, and the one claim that was not supported is the reason for the second commit. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 92326392a9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| self._data_timer.start() | ||
| self._health_timer.start() | ||
|
|
||
| LauncherWindow._begin_engine_restart_readiness( |
There was a problem hiding this comment.
For an automatic mock/replay restart after the launcher has been running for at least 60 seconds, _data_timer remains active at its 10 ms cadence when this asynchronous wait returns. _poll_bridge_data() then observes the intentionally stopped bridge and calls _replace_bridge_from_watchdog(), which starts the bridge before readiness succeeds; in mock mode this opens the command/data transport prematurely, while in replay mode _publish_replay_ui_authority() rejects the unverified session and latches _bridge_restart_fault, which the readiness-success callback never clears, leaving the recovered child unhealthy and provoking another restart. Stop the data timer during this flight or make the watchdog ignore an owned _runtime_engine_readiness_state.
AGENTS.md reference: AGENTS.md:L470-L471
Useful? React with 👍 / 👎.
|
This is red at What fails
Reproduced by hand at the same head: 2 failed, 16 passed in that file. Cause 1 — necessary, and not sufficientThis branch adds a third bare The tests still fail after that, so this is not the real cause. Cause 2 — the real one
Master attached the bridge inside The Why this is not a test to updateA 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 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 nextTwo options, and the choice is being made caller-by-caller from the code rather than by preference:
The 37 guards this branch adds and updates stay green under either. Written with AI assistance. |
|
@codex review Please review exact head Why this correction is on PR #112: the newly active acceptance selection exposed a pre-existing resource leak in What changed:
Coordinator verification on this exact content:
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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9dd1fd815e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| thread = threading.Thread(target=_run_loop, args=(loop,), daemon=True) | ||
| thread.start() | ||
|
|
||
| _stop_join_close_loop(loop, thread) |
There was a problem hiding this comment.
When fixture setup raises after the loop thread starts—for example from publisher.start(), bridge.start(), sentinel publication, or bridge polling—pytest never reaches either _stop_join_close_loop call in zmq_harness; the loop and possibly the publisher therefore remain live. This new guard invokes the helper directly, so it stays green while those actual fixture paths still leak; drive the fixture through successful teardown and an injected setup failure and observe its owned resources instead.
AGENTS.md reference: AGENTS.md:L380-L386
Useful? React with 👍 / 👎.
| from tests.e2e._zmq_harness import _run_loop, _stop_join_close_loop | ||
|
|
||
|
|
||
| def test_stop_join_close_loop_releases_the_real_event_loop() -> None: |
There was a problem hiding this comment.
The newly added test_stop_join_close_loop_releases_the_real_event_loop corrects a confirmed resource-leak failure, but a repo-wide search finds the node only at its definition and not in governance/agent_preventions.yaml. Without a stable prevention record linking this guard to default CI and its red/green evidence, the test can later be removed, skipped, or deselected without automatically reopening the failure disposition.
AGENTS.md reference: AGENTS.md:L387-L390
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d9942889c4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
|
|
||
| @pytest.mark.skipif(sys.platform != "win32", reason="Windows event-loop contract") | ||
| def test_fixture_loop_factory_is_selector_capable_on_windows() -> None: |
There was a problem hiding this comment.
The parent commit registered the other ZMQ harness lifecycle nodes under TEST-RUNTIME-FIXTURE-OWNERSHIP-023, but this newly added Windows false-green regression is absent from governance/agent_preventions.yaml. If this test is later skipped, renamed, removed, or displaced from the Windows CI partition, the prevention record will not automatically reopen; add this exact node and its remaining partition to that record.
AGENTS.md reference: AGENTS.md:L387-L390
Useful? React with 👍 / 👎.
| | Source snapshot tree | `5f22d2d7b1b02f05a6d8c78092e243bbd5522b3c` | | ||
| | Source snapshot object manifest SHA-256 | `db65258558e6f9a0d0e4b13603b24c39d43e02ead2de1f37a516c8d3a84d3061` | |
There was a problem hiding this comment.
At d9942889, the generator's current index snapshot has tree 9204becf3fd43bd3918c9d2801dd66b709611fd0 and 1,265 source paths, but this document and the paired SVG still identify tree 5f22d2d7b1b02f05a6d8c78092e243bbd5522b3c and 1,264 paths from the earlier 9232639 regeneration. The later 9dd1fd8, 63d8522, and d9942889 source/test changes therefore leave the advertised current-candidate counts and immutable binding stale; rerun the paired generator after the final content commit.
AGENTS.md reference: AGENTS.md:L445-L450
Useful? React with 👍 / 👎.
| del harness, generator | ||
| samples.append(_process_resource_count()) | ||
|
|
||
| assert max(samples) - min(samples) <= 4, f"process resource count grew across fixture cycles: {samples}" |
There was a problem hiding this comment.
With exactly five samples, a deterministic leak of one handle or file descriptor per fixture lifecycle produces values such as [N, N+1, N+2, N+3, N+4], for which this assertion still passes because the range is exactly four. That is the per-cycle owner retention this regression claims to detect, and repeated E2E use can therefore accumulate resources while the guard remains green; make the terminal samples flat or explicitly mutation-test a one-resource-per-cycle leak.
AGENTS.md reference: AGENTS.md:L361-L365
Useful? React with 👍 / 👎.
|
@codex review Exact-head review requested for Why this head was pushed:
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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c4e555d8f6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| def test_resource_slope_guard_rejects_one_per_cycle_growth() -> None: | ||
| """The lifecycle guard must reject the exact leak its old range admitted.""" | ||
| with pytest.raises(AssertionError, match="resource count grew"): | ||
| _assert_flat_resource_samples([100, 101, 102, 103, 104]) |
There was a problem hiding this comment.
This head newly adds a deterministic control for the prior false-green resource tolerance, but a repo-wide rg finds neither this node nor test_repeated_real_fixture_lifecycle_has_flat_resource_slope in governance/agent_preventions.yaml; TEST-RUNTIME-FIXTURE-OWNERSHIP-023 registers the selector and teardown nodes only. Consequently, the exact-flat lifecycle guard or its mutation control can later be removed, skipped, or displaced from default CI without the prevention registry detecting that coverage loss, so register both nodes (and the false-green obligation they enforce) in the required remaining partition.
AGENTS.md reference: AGENTS.md:L387-L390
Useful? React with 👍 / 👎.
1 participant
What happens today
Measured by running the program on 2026-08-28 at 00:36, with
--mock, on a fresh worktree with no warm caches:Read the last two lines together. The launcher declared the engine failed at 00:36:56. The engine finished starting correctly at 00:37:01 — five seconds after it had been abandoned. Nothing was wrong with the engine.
_wait_engine_readyallowed ten attempts at half a second: a five-second budget. A cold engine needs about ten. It loads interlocks, alarms, channel landmarks, the vacuum guard, sensor diagnostics, the trend predictor, the SQLite writer and the safety broker before it may answer READY — and those are fail-closed startup owners that are supposed to be established first.When the wait expired the launcher held every construction owner and the operator got a window that never rendered. The program refused to start and said nothing. The standing rule here is that this software must never refuse, because when it refuses the operators wire the hardware outside it and the system loses its ability to see, control or record anything. On a fresh machine this is the operator's first experience.
What this changes
The wait is bounded by a monotonic deadline of one minute — six times the measured cold start, while still reporting a stuck child in about a minute. A deadline rather than an attempt count, so slow polls cannot silently extend it.
Startup progress is now logged rather than passed over in silence, which is what the never-refuse rule asks for: say what is happening.
A dead child or an invalid readiness receipt still fails immediately. Only patience for a healthy, slow start has changed. The replay entry point shared the same five-second default and receives the same correction.
Evidence
tests/launcher/test_launcher_cold_start_budget.pydrives the real path: an engine that becomes ready later than the old budget but within the new one must let the launcher proceed. With the production change reverted it fails; three tests redden. The full launcher suite and the documentation gate pass.Stated limit
This was reproduced on Windows. On Ubuntu 22.04 the same engine reached readiness at attempt 4 of 10 — about two seconds — so the old budget was sufficient there. This fix is not what unblocks the Ubuntu end-to-end run; that is a separate, measured cause.
Written with AI assistance.