Skip to content

test: keep the both-arms-dry walkthrough off real kernel subprocesses on either route - #1622

Merged
xiaofei-zheng merged 2 commits into
mainfrom
fix/walkthrough-hermetic-kernel-entry
Sep 23, 2026
Merged

xiaofei-zheng merged 2 commits into
mainfrom
fix/walkthrough-hermetic-kernel-entry

Conversation

@lishuoshuo-amd

@lishuoshuo-amd lishuoshuo-amd commented Sep 23, 2026 •

Copy link
Copy Markdown
Collaborator
  • Description: what and why

test_optimize_loop_walkthrough.py::test_both_arms_dry_walks_the_rest_of_the_chain failed twice on the #1619 CI (py3.10 shard 1): assert 'KERNEL_AGENT' == 'CLOSE', and teardown logged _on_phase_entered hook was cancelled.

Since #1605, KERNEL entry runs out of band, and _advance_phase_if_needed holds the phase while _kernel_entry_in_flight(). The test ticks eleven times without waiting on that hook, and the hook launched a real subprocess. So whether the run reached CLOSE depended on how fast that process exited on the CI host.

The fix in #1619 only covered the GEAK route (geak_runner.py). The failing CI log shows kernel-entry reprofile produced no new snapshot (kernel.py:362), which appears only on the forge route. The route comes from KERNEL_OPT_BACKEND_ORDER, and on a shared xdist worker that variable can already be forge. The cause is test_atom_kernel_backend_default.py::test_unset_backend_defaults_to_forge and test_the_default_lands_before_the_session_records_it: each calls monkeypatch.delenv on an unset key, which monkeypatch does not record, and _apply_atom_auto_tighten then writes os.environ["KERNEL_OPT_BACKEND_ORDER"] = "forge", which nothing restores. On the forge route the hook ran python -m kernelforge.cli kernel-rewrite-controller through controller_submit.run_controller_subprocess.

The test now covers both routes and is hermetic on each. It also takes #1623's placement: the stubs live in the session_dir fixture, so no test in the file can reach either subprocess.

  • It is parametrized over the backend order (geak, forge) and sets KERNEL_OPT_BACKEND_ORDER itself, so a leaked value no longer picks the route.

  • Both process boundaries are replaced in the fixture. request_handlers._kernel_agent_tool_path refuses only backends/geak_runner.py, so GEAK takes its existing runner_not_found branch while every other tool path still resolves. controller_submit.run_controller_subprocess returns no_opportunity, following the stub already used in test_controller_patch_integration.py.

  • After each tick the walkthrough awaits the out-of-band KERNEL entry hook (coord._await_kernel_entry_task()). With the subprocesses stubbed, the hook has returned by the next tick, so the wait is immediate. It removes the last dependency on the hook finishing within the fixed tick count.

  • Each route pins the terminal result it reached (geak_result.error_class == "runner_not_found" and kernel_rewrite_controller_result.status == "no_opportunity"). A new subprocess step on either route then fails the assertion instead of racing it.

  • Linked issue(s): close/fix refs

Follow-up to #1619. Supersedes #1623: this PR also stubs the Controller in the fixture, and adds the GEAK route, route pinning and the entry-hook wait.

  • Tests: added/updated? commands run?

Each scenario was run on main before the change and on this branch after it:

Scenario main This branch
test_unset_backend_defaults_to_forge then the walkthrough, same process (the CI leak) 1 failed ('KERNEL_AGENT' == 'CLOSE') 3 passed
KERNEL_OPT_BACKEND_ORDER=forge 1 failed ('KERNEL_AGENT' == 'CLOSE') 2 passed
Subprocess trace over both params spawns geak_runner.py or kernel-rewrite-controller only the import-time Magpie probe from executors/baseline.py, outside KERNEL
KERNEL entry hook delayed 20s in-process (the "main" column is 801e28d6b, before the wait) 2 failed ('KERNEL_AGENT' == 'CLOSE') 2 passed (the walk waits for the hook)

pytest src/hyperloom/inference_optimizer/tests/test_optimize_loop_walkthrough.py gives 3 passed; the two walkthrough params take 7.1s and 6.6s. ruff check and ruff format --check are clean.

  • Size/complexity triggers crossed: split, or why the shape is right

One test file, two commits.

  • If this simplifies or refactors: which mechanism was removed, which contract was preserved, and the tests that show it:

N/A.

  • Observable effect: what an operator will now see, and what the old behaviour cost them (if none, why the change is unobservable):

None. This is test-only; CI stops failing this test depending on host speed and worker ordering.

  • Breaking changes: yes/no (details if yes)

No.

  • PR addresses single concern: yes/no (details if no):

Yes. The env leak in test_atom_kernel_backend_default.py is left for a separate change; this test no longer depends on it.

  • Root cause is upstream (Magpie/TraceLens/GEAK/IntelliKit/AgentKernelArena), ticket filed:

No.

Made with Cursor

… on either route

KERNEL entry runs out of band and holds the phase until the hook returns. The
previous fix only covered the GEAK route, but the route is read from
KERNEL_OPT_BACKEND_ORDER, and an earlier test on the same xdist worker leaves it
at "forge" (test_atom_kernel_backend_default deletes an unset key, which
monkeypatch does not record, and the code under test then writes it). On the
forge route the hook launched `kernelforge.cli kernel-rewrite-controller`, so
reaching CLOSE within eleven ticks again depended on how fast a subprocess
exited on the CI host.

The test now sets the backend order itself and runs once per route, with both
process boundaries replaced: the GEAK runner path is unresolvable and the
Controller run returns no_opportunity. Each route pins the terminal result it
reached, so a new subprocess step on either route fails the test instead of
racing it.

Co-authored-by: Cursor <cursoragent@cursor.com>
@lishuoshuo-amd
lishuoshuo-amd requested a review from a team as a code owner September 23, 2026 07:00
…on KERNEL entry

Both process boundaries now live in the session_dir fixture, as #1623 does for
the Controller, so no test in the file can reach a real GEAK runner or
Controller process. Only geak_runner.py is refused; every other kernel-agent
tool path still resolves.

The walkthrough also waits on the out-of-band KERNEL entry hook after each
tick. With the subprocesses stubbed the hook returns within a tick, so the wait
is immediate; it removes the remaining dependency on the hook finishing inside
the fixed tick count.

Co-authored-by: Cursor <cursoragent@cursor.com>
@xiaofei-zheng
xiaofei-zheng merged commit 146c113 into main Sep 23, 2026
33 checks passed
@xiaofei-zheng
xiaofei-zheng deleted the fix/walkthrough-hermetic-kernel-entry branch September 23, 2026 07:48
zengleixin-amd pushed a commit that referenced this pull request Sep 23, 2026
Bring in #1622 without changing coverage thresholds or runtime phase behavior.

Co-Authored-By: Claude <noreply@anthropic.com>
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.

2 participants