fix(gateway): use respawn_executable when restarting after update

[bolichen97]

Problem / Motivation

After an auto-update replaces a managed install, the gateway tries to restart itself and dies with ENOENT instead of coming back. The three restart-after-update paths in GatewayOrchestrator (_restart_after_update, _auto_apply_update, _auto_apply_wheel_update) exec sys.executable, and after the update pruned the old install that path no longer exists.

Why it matters

Every operator on a managed (wheel/installer) install who has auto-update on: the update applies, the gateway exits to restart, and nothing comes back up until someone starts it by hand. An unattended host stays down.

What changed (motivation → approach → change)

Symptom: the gateway process is gone after a successful auto-update, with an os.execv ENOENT in the log.

Root cause, in two halves. The gateway sites called platform_compat.reexec_python_module("kiro_crew", sys.argv[1:]) with no executable=, so they fell back to the cached sys.executable in the tree the update just removed. But the resolver they should have called could not answer correctly either: respawn_executable() decided "is this a managed venv?" by resolving the interpreter FILE, and python -m venv writes bin/python3 as a symlink to the base interpreter — so the resolved path landed outside every venv and the predicate said "not managed" for every real cli.sh install. Wiring the sites up alone would still have ENOENT'd.

The change fixes both halves. Each of the three sites resolves the interpreter through wheel_engine.respawn_executable() (via asyncio.to_thread, since it stats the filesystem) and passes it as executable=, matching what the dashboard update handler already does. The resolver is imported before the apply step and called after it — the module is deliberately off the gateway boot path, so a deferred import would read a package the migration has already deleted, while the answer is only correct once the stable link has been repointed. In wheel_engine.py, respawn identity is now read from the interpreter's bin/ directory (symlink-correct), the retired in-data-home venv counts as a respawn identity so a process it serves can restart through the stable link after the migration deletes it, and the fallback reaches the validated legacy tree when the stable link is unusable and sys.executable is already gone.

This changes behaviour for the existing dashboard caller too (handlers/updates.py), not just the three new ones: both identity rules widen what respawn_executable() answers for symlinked-interpreter and legacy-nested-venv installs. running_from_managed_venv() keeps its own file-resolving rule — it gates shadow-build dispatch rather than a restart, so correcting it moves a different mechanism's behaviour and is tracked in #8938 instead of being folded in here.

Tests

test/test_gateway_restart_uses_respawn_executable.py (new) — one test per restart site. Each drives the real method to its exec line with the download/apply/breadcrumb steps stubbed, patches respawn_executable to return a sentinel path and reexec_python_module to record the call, and asserts the exec received executable=<sentinel>. TestResolverIsLoadedBeforeTheApply parses gateway.py and pins, per method, exactly one respawn_executable import whose line precedes the first apply / create_subprocess_exec call.

test/test_wheel_engine.py — the identity rules and the fallback. Two migration shapes that must reach the legacy tree (a real directory at the stable name, so cli.sh skips the repoint; and no stable link at all, so the repoint failed non-fatally) and two that must NOT (sys.executable still present, so a corrupt link cannot displace a healthy gateway; no usable legacy tree, so the answer stays sys.executable).

Each production hunk was reverted in place to confirm the matching tests go red. Local run on the rebased tree: 753 passed, 23 skipped across test_wheel_engine.py, test_gateway_restart_uses_respawn_executable.py, test_update_check_install_aware.py, test_platform_compat.py, test_slack_gateway.py. isort / flake8 7.1.0 / black / mypy (1302 files) all clean.

Manual verification

N/A — unit coverage sufficient: the change is which interpreter path is handed to execv, and the tests assert exactly that argument on every affected site. The one premise that is not unit-testable — that cli.sh creates venvs with a symlinked interpreter, skips the stable-link repoint when a real directory occupies the stable name, and deletes the nested venv on the new tree's own import check rather than on the repoint — was read off cli.sh on this PR's rebased base, and the symlink behaviour was measured on a real python3 -m venv.

Why no screenshot: backend-only change to the gateway restart path; nothing under website/ and no rendered surface.

Related Issues

no linked issue: found while tracing a post-update restart failure on a managed install. Follow-up filed as #8938.

Pattern harvest

Rule candidate: review-prompt

Two concretely-checkable shapes, both grep-able:

1. reexec_python_module(...) / an os.exec* call with no executable= on a code path that runs after an install or update step — the cached sys.executable may name a pruned tree.
2. Path(sys.executable).resolve() (or is_managed_tree(Path(sys.executable))) used as a "which venv am I in?" predicate. python -m venv symlinks bin/python3 to the base interpreter, so resolving the FILE escapes the venv; the containment question must be asked of Path(sys.executable).parent. Grep resolve() applied to sys.executable — the sibling this PR left is running_from_managed_venv() answers False on every real cli.sh install (interpreter symlink resolved past the venv) #8938, found exactly this way.

Not generalizable: nothing here is a tuning constant.

Checklist

• At most two commits (one is the norm), with a Conventional Commits title (feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)
• Existing tests pass and new tests added for new functionality
• Self-review completed; code follows project style guidelines
• Documentation updated (if applicable) — N/A, no documented behaviour changed
• No secrets, credentials, or internal references in the diff

Backport: please include in release/0.6.0.

[github-actions]

Design Review (Fable 5) — ✅ PASS

Design-level review of 5cf5721be02cf47158612986cab9940c4df11b4d — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: PASS

Root cause fixed at both halves — call sites and the symlink-blind identity predicate — with exec-target validation preserved and the sibling predicate deliberately deferred to #8938.

Suggestions

• _restart_after_update takes respawn as a parameter while the other two sites import-early-and-call-inline; converging on one shape (parameter injection everywhere) would let the AST-based import-ordering test pin a single pattern instead of three.

[DESIGN-REVIEWED] 5cf5721

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed 5cf5721be02cf47158612986cab9940c4df11b4d — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 5cf5721

Verdict parsed from the review's SHA-scoped output markers for commit 5cf5721be02cf47158612986cab9940c4df11b4d.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 5cf5721be02cf47158612986cab9940c4df11b4d: <one-sentence reason>

[github-actions]

First Principles Review (Fable 5) — ✅ PASS

Premise-level review of 5cf5721be02cf47158612986cab9940c4df11b4d — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All claims verified. The dashboard caller's own comment names the recorded convention (no-new-work-on-gateway-boot-path, updates.py:1419), which makes the deferred-import + callable-parameter apparatus derived rather than a shape to subtract. Grep counts: zero remaining reexec_python_module calls without executable= in src/; one remaining file-resolving venv predicate (running_from_managed_venv, declared and tracked in #8938). The cli.sh premises (repoint skipped on a real directory at the stable name, non-fatal repoint failure, delete gated on the new tree's import check) all read true at cli.sh:669-729.

First-Principles-Verdict: PASS

A FIX that lands at cause level: both halves of the ENOENT (unwired sites and a wrong predicate) are fixed, siblings counted, one declared and tracked.

What this change ships

Intent: after an auto-update prunes a managed install, the gateway restarts itself instead of staying down — a FIX.

1. Three gateway restart paths exec the resolved interpreter, not the cached path — justified (the fix).
2. Managed-venv identity read from bin/ directory, symlink-correct; also changes the dashboard caller — declared, justified.
3. Retired in-data-home venv now counts as a respawn identity — declared, verified against cli.sh:583,715-729.
4. Fallback to the import-verified legacy tree only when sys.executable is already gone — declared, justified.
5. Resolver imported before the apply; _restart_after_update takes it as a parameter — derived (no-new-work-on-gateway-boot-path, updates.py:1419, plus the delete-under-process hazard).
6. AST test polices any future reexec_python_module without executable= in gateway.py — justified (guards the shape, not a list).

Watch

• The dashboard restart handler's own deferred import (updates.py:1422) is the one remaining post-apply import of the resolver: after an out-of-band installer re-run deletes the nested venv, that from ... import can raise instead of restarting. The new AST guard parses only gateway.py, so nothing pins it. Same root cause, count: 1; fold into running_from_managed_venv() answers False on every real cli.sh install (interpreter symlink resolved past the venv) #8938 or its own follow-up.
• running_from_managed_venv (wheel_engine.py:243) keeps the file-resolving rule — declared, deferred as running_from_managed_venv() answers False on every real cli.sh install (interpreter symlink resolved past the venv) #8938 with a stated reason; accepted.

[FIRST-PRINCIPLES-REVIEWED] 5cf5721

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 5cf5721be02cf47158612986cab9940c4df11b4d and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 5cf5721

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 5cf5721be02cf47158612986cab9940c4df11b4d: <one-sentence reason>

[github-actions]

UX Review (Fable 5) — ⏭️ skipped

Revision 5cf5721be02cf47158612986cab9940c4df11b4d touches no user-facing surface (no changes under website/ or committed screenshots), so the UX review was skipped. Advisory — does not block merge.

[bolichen97]

Disposition: fixed in ef76748 — span=e51cd2330ac2

• legacy managed-venv migration execs the deleted interpreter (src/kiro_crew/slack/gateway.py, the respawn_executable resolution in the update-restart path)

Legitimate: yes. respawn_executable() answered "not managed" for a process still served by the retired in-data-home venv (<data home>/venv, cli.sh's _OLD_VENV), because that tree lives inside the data home while ManagedVenvLayout lives beside it. The installer re-run that migrates it repoints the stable link and then rm -rfs the nested venv, so a later restart exec'd a sys.executable that no longer exists. Proportional: yes — the finding's own fix ("recognize the retired nested venv and select the new stable-link interpreter") is a resolver-side identity check, not a new mechanism.

What changed in src/kiro_crew/platform/wheel_engine.py:

• legacy_nested_venv() names the retired tree (data_home() / "venv"), mirroring cli.sh exactly.
• _respawn_tree_is_managed(layout) is the respawn identity: the interpreter's bin/ directory is a tree of this layout, OR resolves to (or under) the legacy nested venv. Identity is read from bin/, not the interpreter file, because python -m venv writes bin/python3 as a symlink to the base interpreter — resolving the file lands outside every venv and would answer "not managed" for every real install.
• respawn_executable() dispatches on _respawn_tree_is_managed instead of running_from_managed_venv. The stable-link target must still resolve inside the layout's own trees before it is trusted; with no stable link yet (pre-migration) the fallback keeps the restart on the nested venv, unchanged. running_from_managed_venv keeps its own rule for the shadow-build dispatch.

Tests (test/test_wheel_engine.py): test_retired_nested_venv_routes_through_stable_link (nested venv + stable link → the link's interpreter), test_nested_venv_before_migration_keeps_sys_executable (nested venv, no link → sys.executable), test_symlinked_interpreter_still_routes_through_stable_link (a symlinked bin/python3 inside a managed tree still routes). 77 tests in test_wheel_engine.py + test_gateway_restart_uses_respawn_executable.py pass locally.

[bolichen97]

Disposition: rebutted — the executable= kwarg on reexec_python_module stays, and the three call sites keep passing respawn_executable() explicitly.

• The fix patches all callers while keeping the unsafe sys.executable default; fold respawn_executable() into reexec_python_module and drop the kwarg

The observation is accurate (4/4 callers now pass the same value), but folding the resolution into reexec_python_module is disproportional for this PR and changes a contract that is not this PR's to change:

• reexec_python_module lives in src/kiro_crew/platform_compat.py; respawn_executable() lives in src/kiro_crew/platform/wheel_engine.py, which itself imports from kiro_crew.platform_compat (line 55). Making platform_compat reach back into wheel_engine — even lazily — turns a one-way dependency into a cycle for a bug fix whose stated scope is "the restart execs the wrong interpreter".
• The executable= kwarg pre-existed on platform_compat.py:68, with a docstring describing exactly this caller, and the dashboard's own restart handler (updates.py:1465) already used it before this PR. This PR brings the three Slack-gateway restart sites into line with the existing pattern; it does not introduce the pattern.
• respawn_executable() does filesystem work (Path.resolve, is_managed_tree, stat of the stable link) and the three sites run on the event loop, so the asyncio.to_thread hop is the same off-loop discipline the surrounding code applies to every other blocking call in the restart path, not a "dance" specific to this fix. It costs one line per site.
• Deleting the default would be a wider change: every future caller that legitimately wants sys.executable (a non-gateway process, a test harness) would have to opt out, and the resolver's own sys.executable fallback would then be reached through two layers rather than one.

The "Pattern harvest" note in the PR body stands as the proportional response: the review rule ("a reexec_python_module(...) call in gateway code without executable= is suspect") catches the next caller at zero risk to the layering, and a follow-up that moves the default can be argued on its own merits once this fix has shipped. No new capability was added here.

[bolichen97]

Disposition: fixed in 827a058 — span=e51cd2330ac2 (2nd finding in this span: round 1 was the resolver's identity rule at the wheel-update restart site; this round is the import ORDER at the provider restart site)

• Provider update can delete the module before it is imported (src/kiro_crew/slack/gateway.py, _restart_after_update — the deferred from kiro_crew.platform.wheel_engine import respawn_executable after provider.apply())

Legitimate: yes. wheel_engine is deliberately kept off the gateway boot path (cli_server.py, the no-new-work-on-boot note), so at _restart_after_update it is not in sys.modules; the deferred import reads the package from disk, and the legacy-nested-venv migration that the resolver exists for has just deleted that tree. Sessions were already closed, so the ModuleNotFoundError left an un-restarted gateway. Proportional: yes — the fix is the one the finding names: load the resolver before the apply, call it after.

What changed (all three restart paths, so the invariant is uniform):

• _check_for_updates_via_provider imports respawn_executable at the top, before either provider.apply() call, and passes it to _restart_after_update(respawn_executable).
• _restart_after_update(self, respawn: Callable[[], str]) now takes the pre-loaded resolver and imports nothing from wheel_engine; the CALL is still deferred to after the apply, because the answer (stable link vs sys.executable) is only correct once the link has been repointed.
• _auto_apply_update (git path) imports the resolver before its first create_subprocess_exec, i.e. before pip install -e . rewrites the package; _auto_apply_wheel_update imports it before cli.sh runs. Both keep the deferred call at the exec.
• The module stays off the boot path: all three methods run from the update timer / apply branch, never from gateway start.

Tests (test/test_gateway_restart_uses_respawn_executable.py): TestResolverIsLoadedBeforeTheApply parses gateway.py and pins, per method, exactly one respawn_executable import whose line precedes the first apply / create_subprocess_exec call, and that _restart_after_update contains no wheel_engine import at all. The existing exec-seam tests still pass (7 in that file, 81 across it and test_wheel_engine.py); isort/flake8 clean on the touched files.

Coverage of the whole family, since this span has now been hit twice: (1) the interpreter identity — resolver recognises the retired nested venv (round 1, wheel_engine.py); (2) the import order — resolver is in memory before the venv can be deleted (this round, all three call sites). The one remaining post-apply disk read in _restart_after_update is the pre-existing save_all_slots_to_history import from kiro_crew.dashboard.chat, a module the running dashboard has already loaded and which only runs when dashboard_state is set; it predates this PR and is unchanged.

[bolichen97]

Disposition: fixed in 3e88e15 — span=dd36ca1a6f5b (3rd finding in this PR's GPT sequence, 1st in this span: rounds 1-2 were the resolver's identity rule and the import order, both at slack/gateway.py; this one is the resolver's own fallback in platform/wheel_engine.py)

• legacy migration still execs a deleted interpreter when stable-link promotion fails (src/kiro_crew/platform/wheel_engine.py:312, the if not _respawn_tree_is_managed(layout): fallback chain)

Legitimate: yes, and I reproduced the window on this PR's rebased base rather than taking the trace on faith. Proportional: yes — the fix is the one the finding names, validate the new layout.legacy interpreter before falling back, and it adds no mechanism.

Why it is reachable. The three conditions are independent, which is what makes the combination ordinary rather than exotic:

• cli.sh:669 guards the stable-link repoint with if [ -L "$_VENV_CURRENT" ] || [ ! -e "$_VENV_CURRENT" ], so a real directory at the stable name means the repoint never runs; cli.sh:689 makes an actual repoint failure non-fatal (|| echo "WARNING: ... continuing.").
• The _OLD_VENV delete at cli.sh:725 is gated on the NEW tree's own "$VENV/bin/python" -c 'import kiro_crew' check at cli.sh:723 — it does not depend on the repoint having succeeded.
• So the nested venv this process is running from is deleted while the stable link is unusable, _respawn_tree_is_managed still answers True (nested identity), is_managed_tree(stable_link) answers False, and the old code handed back a sys.executable naming nothing.

What changed. _respawn_fallback(layout) replaces the two bare return sys.executable exits after the identity gate. It returns sys.executable whenever that path still exists — a broken link must never take the restart away from a process whose own interpreter is fine — and only when it is gone does it validate layout.legacy (inside the layout, interpreter present, executable) and use it. The legacy fixed tree is exactly the tree that re-run just built and import-verified, and this engine never prunes it. _interpreter_in(tree) factors out the own-name-then-python3 selection the stable-link branch already used, so both branches validate identically instead of by two copies of the rule.

Tests (test/test_wheel_engine.py, TestRespawnExecutable). Two shapes that must reach the legacy tree: test_unusable_stable_link_after_migration_uses_legacy_tree (a real directory at the stable name, i.e. the skipped repoint) and test_failed_stable_link_repoint_after_migration_uses_legacy_tree (no link at all, i.e. the non-fatal failure). Two that must NOT, because the risk of this fix is displacing a healthy process: test_live_interpreter_is_never_displaced_by_the_legacy_tree (corrupt link but sys.executable present → unchanged) and test_deleted_interpreter_with_no_usable_legacy_tree_keeps_sys_executable (nothing to validate → unchanged). Reverting _respawn_fallback to a bare return sys.executable fails exactly the first two and leaves the guard pair green.

On the adjudication's FLAG: agreed that the consequence is loud and launcher-recoverable rather than silent, and that the PR narrows the pre-existing fallback. That is why the fix is a validated last resort rather than a new recovery mechanism — it removes the one case where the returned path could not possibly work.

[bolichen97]

Disposition: fixed in 3e88e15 — description and commit message now declare the wheel_engine half and its behaviour change for the existing dashboard caller.

• The description's stated root cause is incomplete and never mentions wheel_engine.py changing, though items 3 and 4 change behaviour for the existing dashboard consumer too

Legitimate: yes, and the premise checks out on measurement, not just on reading cli.sh:556. On a plain python3 -m venv, Path(sys.executable).resolve() lands at the base interpreter outside the venv while Path(sys.executable).parent.resolve() stays inside it — so the identity rewrite is load-bearing exactly as stated, and the declared wiring fix alone would still have ENOENT'd. Proportional: yes, the remedy is prose.

What the description now says instead of "mirrors what the dashboard handler already does":

• The root cause is stated in two halves — the missing executable= at the call sites, AND the resolver that could not have answered correctly anyway because it resolved the interpreter FILE through a venv symlink.
• A paragraph names all three wheel_engine changes (bin/-directory identity, the retired nested venv as a respawn identity, the validated legacy fallback).
• A sentence in bold states that this changes behaviour for handlers/updates.py, the pre-existing caller, and not only for the three new sites — which was the part a reader could previously miss entirely.
• The commit message carries the same declaration, so the record survives independently of the PR body.

The ## Pattern harvest section also gained the second, more valuable shape: Path(sys.executable).resolve() used as a "which venv am I in?" predicate is grep-able, and it is how the remaining sibling was found.

[bolichen97]

Disposition: accepted-and-deferred to #8938 (label deferred-finding, assignee bolichen97, Due: 2026-09-20).

• The named root cause has 1 unfixed sibling: is_managed_tree(Path(sys.executable)) at wheel_engine.py:243 (running_from_managed_venv), consumed at cli_server.py:1567, updates.py:1051, updates.py:2274

Legitimate: yes, and I verified the premise rather than inheriting it. Measured on a real python3 -m venv: sys.executable is <venv>/bin/python3, Path(sys.executable).resolve() is <base>/bin/python3.12 — outside the venv — while .parent.resolve() stays inside. cli.sh:632 creates the venv with a plain "$PY" -m venv "$VENV" and no --copies, so the predicate answers False on every real managed install. The reviewer is right that "keeps its own rule" recorded the split without arguing the sibling is correct; it is not correct, it is a real defect.

Deferred rather than fixed here, and the reason is blast radius, not effort:

• The two predicates gate different mechanisms. _respawn_tree_is_managed decides which interpreter a restart execs; running_from_managed_venv decides whether an install takes the shadow-build wheel-update path at all. Correcting the second changes which installs start building versioned trees — a change to the update path, inside a PR whose stated scope is the restart path.
• It fails safe, which is what makes it deferrable at all: a False answer means an update is not dispatched, not that a wrong one is. There is no crash, data loss, or corruption on this side, so it is not in the never-deferrable class.
• It needs evidence this PR does not carry. Before flipping it, someone has to establish what those three consumers actually do today when it is False — whether another update route compensates, or wheel updates simply never dispatch there. Landing the flip blind would be a bigger risk than the bug.

#8938 carries the measurement, the three consumer sites, the fail-safe reasoning, and three concrete steps (confirm on a cli.sh host, establish current consumer behaviour, then either change it with a symlinked-interpreter test or document why dispatch legitimately wants the file-resolving rule).

[bolichen97]

Disposition: fixed in 3e88e15 — the function is now _legacy_nested_venv and the export is gone.

• Subtraction: drop "legacy_nested_venv" from __all__ and underscore the function — grep counted exactly 1 consumer, in the same module, and the public export has zero

Legitimate: yes, and I re-counted before removing it rather than trusting the review. Across src/ the name appeared 4 times: the definition, the single call inside _respawn_tree_is_managed, one docstring cross-reference, and the __all__ entry. Nothing in test/ referenced it, and no test pins wheel_engine.__all__ (the modules that freeze their exports — workflows, dashboard.types, governance — do so in their own tests; this one does not). Proportional: yes, and it is a subtraction, so it removes public surface instead of adding any.

Applied: def legacy_nested_venv → def _legacy_nested_venv, the __all__ entry deleted, and the one :func: cross-reference (in respawn_executable's docstring) retargeted to the private name so the docs do not point at a symbol that no longer exists. The name now appears three times in the module and nowhere else: the definition, the call in _respawn_tree_is_managed, and that docstring reference. Verified by re-running the module's suite plus the restart tests (753 passed, 23 skipped) and mypy src/kiro_crew/ over 1302 files, which is what would have caught a missed reference.

[bolichen97]

Disposition: fixed in 5cf5721 — the fourth-site gap is now closed by a mechanical check rather than by the review rule, though not with the helper as proposed.

• The eager-import + to_thread(respawn) + reexec ritual is copy-pasted at three sites; a single gateway helper capturing the resolver before apply() would encode the ordering constraint once, instead of relying on the "pattern harvest" review rule to police the fourth site

Legitimate: yes on the diagnosis, and the concrete worry was correct in a way worth stating plainly — I checked, and the existing TestResolverIsLoadedBeforeTheApply uses a hardcoded parametrize list of the three known methods, so a fourth restart path genuinely would not have been examined by it. Relying on a review-prompt rule was the weak part.

On the proposed remedy specifically, a helper is disproportional here, and it would not actually have closed the gap:

• A helper can only encode the ordering constraint if it owns both sides of it — the import and the apply. The three sites apply differently: provider.apply(), pip install -e . via create_subprocess_exec, and cli.sh via create_subprocess_exec. Encoding the order would mean passing each apply in as a callback so the helper could sandwich it, which restructures three control flows to remove roughly two lines of duplication each.
• It polices a fourth site only if the fourth site calls it. A new restart path written without the helper is exactly as unguarded as one written without the executable= kwarg — the suggestion moves the convention, it does not enforce it.

What was done instead, test_every_reexec_in_the_gateway_passes_an_explicit_executable: it walks the AST of gateway.py for every reexec_python_module call rather than a list of known methods, and requires each to pass executable=. That is the defect's own shape, findable without knowing where it lives, so a fourth restart path trips it on the commit that adds it. Mutation-checked by dropping executable= from one of the three call sites, which fails the test; restored after.

The enumerated order check is kept as-is alongside it. It asserts something the discovered check cannot — that the import precedes the apply within each method — and the two are complementary: one is precise about the three paths that exist, the other is total over the invariant that matters most.

The second suggestion (state the _respawn_tree_is_managed behaviour change on the record) is fixed separately: the description and commit message now declare the wheel_engine half and call out that it changes what the existing handlers/updates.py caller answers for symlinked-interpreter and legacy-nested-venv installs.

[bolichen97]

Disposition: accepted-and-deferred to #8953 (label deferred-finding, assignee bolichen97, Due: 2026-09-20).

• The dashboard restart handler's own deferred import (updates.py:1422) is the one remaining post-apply import of the resolver, and the new AST guard parses only gateway.py so nothing pins it

Legitimate: yes, and it is reachable on a narrower path than the reviewer stated and a broader one. _restart_gateway performs no apply itself, but I traced its three callers and two apply before calling it: api_update_apply runs apply_policy_update() at :1481 then restarts at :1669, and api_update_approve runs apply_wheel_update then restarts at :2481. Only api_gateway_restart (:2215) is apply-free. So the import is post-apply in-band, not only after an out-of-band re-run. The consequence matches this PR's own bug: _restart_gateway has already saved history and called state.sessions.close_all() before that line, so a raising import leaves a closed-session gateway that never comes back.

Deferred rather than fixed here, and the honest reason is that the fix is a design call on a subsystem this PR does not touch:

• Reachability is narrower than the call graph implies, which is what makes it deferrable. The in-band applies do not delete the tree this process imports from — apply_wheel_update builds a shadow sibling and promotes a symlink, and the engine's stated invariant is that no tree a live process might be using is moved or deleted, with pruning skipping the stable link's target, the serving tree, and the legacy directory. The window that actually bites is an out-of-band cli.sh re-run, which is what rm -rfs the retired nested venv at cli.sh:715-729, racing a dashboard restart.
• It is not a one-line hoist. _restart_gateway is a shared helper whose callers disagree about whether they applied, so the options are: thread the resolver in as a parameter from each applying caller (what this PR did for _restart_after_update), or warm a module-level lazy cache before any apply. Module scope is not available — it breaks the no-new-work-on-gateway-boot-path convention the existing comment at :1421 explicitly cites, which is the same convention that makes the deferred import correct in the first place.
• Choosing between those is a dashboard-updater decision, and folding a fifth file with a 3-caller refactor into a gateway restart-path fix would widen the diff past what the change is for.

#8953 carries the caller table, the reachability split (in-band preserves the importing tree; out-of-band cli.sh is the real window), the three options with module scope marked rejected and why, and the second step the reviewer's point implies: extend the AST guard to parse dashboard/handlers/updates.py too, so the import-order invariant is pinned wherever a restart path lives rather than in the one file this PR touched.

[bolichen97]

Disposition: rebutted — the two shapes track two different roles, so converging them would add a parameter to functions that have no injector.

• _restart_after_update takes respawn as a parameter while the other two sites import-early-and-call-inline; converging on one shape (parameter injection everywhere) would let the AST test pin a single pattern instead of three

The observation is accurate and the motive is right — one pattern is easier to pin than three. But the asymmetry is not stylistic drift; it follows from which function owns the apply:

• _auto_apply_update and _auto_apply_wheel_update are the appliers. They are entered from the update timer, so there is no caller above them to inject a resolver from. Parameter injection there would mean adding a parameter that only their own callers could fill, and those callers are the timer — which would have to import wheel_engine on the boot path to fill it, the exact thing the no-new-work-on-gateway-boot-path convention forbids.
• _restart_after_update is a helper invoked by an applier that has already imported the resolver. Taking it as a parameter is what lets it import nothing from wheel_engine at all, which is the property test_restart_helper_does_not_import_the_resolver pins.

So the three sites already do express one rule: whoever owns the apply imports before it; whoever is called after an apply receives the result. Making the appliers take a parameter would either push the import up onto the boot path or introduce a parameter with no legitimate filler.

On the underlying goal — one thing for the test to pin — that is already addressed, and by a stronger check than shape convergence would give. test_every_reexec_in_the_gateway_passes_an_explicit_executable walks the AST for every reexec_python_module call in gateway.py rather than a list of methods, so it is indifferent to which of the two shapes a future site picks and still catches the defect. Shape convergence would make the enumerated ordering test simpler; it would not make a fourth site safer, because a new site can decline the convention either way.

Related and tracked, since it is the real remaining gap in what that guard covers: the guard parses gateway.py only, and dashboard/handlers/updates.py:1422 is a post-apply import of the same resolver. Deferred as #8953, which includes extending the guard to that file.

[iamwhatever]

Tier 1 auto-approve: fix (4 files). Criteria: no conflict, no requested changes, security path denylist clean, design-doc gate clean, SAST annotations clean, security checklist all-NO, AI reviewers green. Category: fix -- a gateway restart after a wheel auto-update exec'd sys.executable, which the same installer re-run had just deleted along with the retired in-data-home venv, so a closed-session gateway was left un-restarted; respawn_executable now recognises that tree as restartable and falls back to a validated in-layout interpreter, and the resolver is imported before provider.apply() so the deferred import cannot fail after the venv is gone. Exec targets stay gated by is_managed_tree, so no new path is trusted with an exec.

[chenmingwei23]

Tier 1 auto-approve: fix (4 files). Criteria: no conflict, no requested changes, security path denylist clean, design-doc gate clean, SAST annotations clean, security checklist all-NO, AI reviewers green. Category: fix(gateway) - resolves stale cached sys.executable at the restart-after-update sites via respawn_executable(), clear single root cause, 4 files (cap 5).