Skip to content

fix(wsl): Codex WSL hook installs and host hook checks never start a stopped distro (E9) - #35827

Merged
nwparker merged 4 commits into
mainfrom
nwparker/wsl-e9-hook-gaps
Oct 29, 2026
Merged

nwparker merged 4 commits into
mainfrom
nwparker/wsl-e9-hook-gaps

Conversation

@nwparker

@nwparker nwparker commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 7 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​432 $\color{#cf222e}{\Huge{\mathbf{−}}}$​104 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​328
Prod 11 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​116 $\color{#cf222e}{\Huge{\mathbf{−}}}$​34 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​82

ELI5

Two background jobs could still start a stopped WSL Linux: the job that installs Orca's Codex hooks into a WSL Codex home, and the check for a project's orca.yaml when that project lives on a remote Orca host. Now both check first whether the distro is running and leave it alone if it isn't. Opening a terminal or pane in that distro still installs the hooks as before.

What Changed

(a) Codex WSL hook install and reconcile (codexHookService.installForRuntimeHome, reached in the background from the WSL hook relay):

  • Before: it wrote hooks.json, .orca/agent-hooks/codex-hook.sh and config.toml under \\wsl.localhost\<distro>\…. The canonical-path probe was a raw execFile('wsl.exe', … readlink) with no running check. If the distro stopped during the ≤3 s hash wait, or before the readlink settled, the next UNC write started it again.
  • After:
    • Install: installManagedHooksIntoWslRuntime asks the shared settled check blockedWslDistroForPath('', distro) inside the per-file queue, right before its first UNC read. If the distro is stopped or unverifiable then, it skips and returns null, the existing "no WSL plan" result. It asks again before the post-hash fallback trust write. By that point hooks.json is already written, so a skip there returns an error status saying the trust is pending, not null. A user act on that distro passes the check.
    • Reconcile: a reconcile that settles later (reinstall, or revoking trust for a missing home) asks the same check before touching any file.
    • Probe: the canonical-path probe now runs through @orca/process-host (runProcess with wslExeProcessSpec) instead of raw execFile, behind blockedWslDistroForExec(distro). A refused probe settles as unavailable, which keeps the last known-good trust identity, exactly like a failed wsl.exe.
    • User acts stay ungated: the guest shell RPC caller (prepareManagedWslCodexHomeBeforeShellLaunch) runs as a user act on its distro.
    • A pane launch is not a user act here. Launch preparation is also reached by passive restores and remounts, which must never start the distro (fix(wsl): a passive restore never starts a stopped WSL distro; its bridge panes wait with Start #35813). So on a stopped distro it skips. A skipped WSL home returns not_installed; it no longer falls through to the host writer, which would have written the WSL home over UNC. The hook relay installs the hooks once the distro runs.

(b) Runtime-RPC hook check:

  • Before: checkRuntimeHooks(…, { background: true }) dropped background on the environment path, so the host's repo.hooksCheck → checkRepoHooks read orca.yaml over UNC with no check.
  • After: repo.hooksCheck takes an optional background (RepoHooksCheck = RepoSelector + background?). The renderer sends it, and the host refuses exactly as the desktop's hooks:check IPC does: status: 'error' with unavailable: 'wsl-distro-stopped' | 'wsl-distro-unverifiable', without reading any file.

Why

Both are E9 gaps found by the preflight. They reuse the shared gate (wsl-unc-access-gate / wsl-background-exec-gate) instead of adding another rule. Checking inside the trust queue, and again after the hash wait, closes the windows the preflight named; a single check at the start would not.

Linked Issue

WSL re-write, gate E9 (wsl-e9-preflight findings).

Visual Proof

N/A: no UI change.

Testing

  • codex-wsl-hook-install-plan.test.ts: rewritten for process-host. New: a stopped distro is never probed (runProcess not called), the settlement is unavailable, and the cached identity is kept.
  • codex-hook-wsl-stopped-distro.test.ts:
    • a stopped distro's files are not read or written and no grant runs;
    • a stop during the hash wait skips the fallback trust write and reports the trust as pending, while a running distro still writes it;
    • a later "missing" settlement doesn't revoke trust on a stopped distro.
  • codex-wsl-hook-user-acts.test.ts: the shell preflight runs as a user act on its own distro and no other; a pane launch does not run as one.
  • A launch on a stopped distro returns not_installed and never calls the host writer.
  • runtime-repository-hooks-commands.test.ts: a background check of a stopped or unverifiable distro reads nothing and says why; a user or older-client check reads as before.
  • repo.test.ts (the RPC passes background) and runtime-hooks-client.test.ts (the renderer sends it to the host).
  • pnpm run typecheck:ci, oxlint, check:code-quality:changed and verify:rpc-params-catalog pass.

Review

Agent skill upstream boundary

  • Not applicable, or this change copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

  • Flag-off: not behind the WSL hosts flag; both gates apply to every Windows WSL user, as fix(wsl): background file reads and watches never start a stopped WSL distro (E9) #34222's do. Off Windows, or for a non-WSL Codex home or repo, both checks answer "not blocked" with no probe, so nothing changes. The hook relay that drives (a) serves every WSL bridge user regardless of the flag.
  • User risk: low. For WSL users, a background Codex hook install, or a host's background hook check, now waits for the distro to run instead of starting it. Opening a WSL terminal still installs hooks immediately through the guest preflight. A Codex pane launched into a stopped distro starts without them, and the relay installs them once the distro runs.
    • Probes: each background install or probe adds at most one shared running-list read, which falls under the existing 60 s lease.
    • Mixed versions: an old host ignores background and behaves as before. An old client never sends it.
  • Wire compatibility: background is a new optional param on repo.hooksCheck. The response can carry the existing optional unavailable field.

@coderabbitai

coderabbitai Bot commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Walkthrough

WSL hook path probing and installation now check whether a distro is blocked. Installation can skip work when a distro is stopped, and launch preparation returns a skipped-WSL status instead of falling back to host preparation. Managed-home shell preflight runs installation as a WSL user act. Repository hook checks now accept a background option through the RPC contract, renderer, and runtime. Background checks return stopped or unverifiable WSL status before reading the hooks file.

Priority: Normal

Merge Risk: Moderate · up to f3ffa

Pane preparation can still boot stopped WSL distros when hooks are disabled, and unavailable-distro diagnostics can be misleading; both should be fixed before merging.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 42.86% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title clearly identifies the main change: preventing Codex WSL hook installs and host hook checks from starting stopped WSL distros.
Description check Passed The description is detailed and covers the user impact, implementation, rationale, testing, compatibility, and visual proof. The linked issue entry is not an actual issue link, and the full checklist …
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description is detailed and covers the user impact, implementation, rationale, testing, compatibility, and visual proof. The linked issue entry is not an actual issue link, and the full checklist is not included, but the description is otherwise mostly complete and on topic.

Full details: Docstring Coverage

Explanation

Docstring coverage is 42.86% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 7 functions across 16 files.

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 4/5

[Medium impact] The changes affect background Codex hook work and repository hook checks on Windows WSL.

The PR still needs the callback race fixed before merging; an older reconciliation can undo newer hook changes.

Findings

  1. P1 Old callbacks undo newer changes ▶
  2. P2 Setup guide saves temporary absence ▶

Summary

Background Codex hook installs and remote-host hook checks now check whether a WSL distro is running before touching its files.

  • Background Codex hook work leaves stopped WSL distros untouched.
  • Background repository hook checks leave stopped WSL projects unread.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Background hook install or check] --> B{Distro running?}
  B -->|No or unknown| C[Skip without opening WSL files]
  B -->|Yes| D[Read or update hooks]
  E[User shell preflight] --> F[Allow its named distro]
  F --> D
Loading

Reviews (2) · Last reviewed commit: "chore: drop the stale process-host direc..." · Reviewed by Greptile

Comment on lines +108 to +112
// The probe settled after the install; the distro may have stopped since (E9).
const distro = resolvedPlan?.wslDistro ?? wslPlan?.wslDistro
if (!distro || (await isWslHookDistroBlocked(distro))) {
return
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Old callbacks undo newer changes

The new await isWslHookDistroBlocked(distro) lets a newer launch or hooks-off refresh supersede this callback after isCurrentGeneration was checked. The old callback then still removes trust or reinstalls hooks using its earlier decision. Turning hooks off during this wait can therefore be undone when the old callback resumes.

Recheck the generation after the wait and before the queued mutation.

target,
'repo.hooksCheck',
{ repo: repoId },
{ repo: repoId, ...(options.background ? { background: true } : {}) },

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Setup guide saves temporary absence

Forwarding background makes runtime-host checks return unavailable for stopped or unverifiable WSL distros. But useSetupGuideProgress treats that reply as a missing setup script and saves hasSetupScript: false. Its effect does not watch distro availability, so a configured remote project can keep showing an incomplete setup step after its distro starts.

Preserve the unavailable state and retry when the host becomes available instead of recording absence.

…stopped distro (E9)

(a) installManagedHooksIntoWslRuntime checks the settled running gate inside
its trust queue right before the first UNC read, and again before the
post-hash fallback trust write; a later reconcile checks before revoking or
reinstalling. A stopped or unverifiable distro skips (null, as with no WSL
plan). The canonical-path probe runs through process-host behind
blockedWslDistroForExec; a refused probe settles as unavailable and keeps the
known identity. The guest shell preflight and a pane launch's home preparation
run as user acts on their distro.

(b) repo.hooksCheck takes an optional background flag (older hosts ignore it);
the renderer sends it, and the host refuses a stopped distro's background
check the same way the desktop's hooks:check does.
A passive restore or remount reaches launch preparation too (#35813), so it no
longer runs as a user act; a stopped distro skips the install and the relay
installs once the distro runs. A skipped WSL home now returns not_installed
instead of falling through to the host writer, which would have written the
WSL home over UNC.
hooks.json is written before the hash wait, so a skip after it is not a clean
skip: return an error status saying the trust is pending instead of null.
@nwparker
nwparker force-pushed the nwparker/wsl-e9-hook-gaps branch from aa135f0 to f3ffaf5 Compare October 29, 2026 09:36

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to GitHub limitations.

⚠️ Outside diff range comments (2)

Minor · Gate the hooks-disabled WSL refresh before UNC… · codex-hook-service-implementation.ts:198-210

src/main/codex/codex-hook-service-implementation.ts:198-210
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Gate the hooks-disabled WSL refresh before UNC access.

The hooks-disabled pane path calls refreshWslRuntimeUserHooks without checking isWslHookDistroBlocked. That helper reads and writes hooks.json and trust files. The gate contract states that any UNC touch can boot a stopped distro. An unverifiable distro is also blocked. This violates the PR's no-start/no-access invariant.

Make the wrapper await the existing asynchronous gate before calling refreshWslRuntimeUserHooks. Return skippedWslHookInstallStatus for a blocked WSL plan so the caller does not fall through to host preparation.

Suggested fix
-      this.refreshRuntimeUserHooksForRuntimeHome(runtimeHomePath, target) ??
+      (await this.refreshRuntimeUserHooksForRuntimeHome(runtimeHomePath, target)) ??
       (await this.refreshRuntimeUserHooksForLaunchPrep(runtimeHomePath ?? undefined))
     )
   }
 
-  refreshRuntimeUserHooksForRuntimeHome(
+  async refreshRuntimeUserHooksForRuntimeHome(
     runtimeHomePath: string | null | undefined,
     target?: CodexWslRuntimeHookTarget
-  ): AgentHookInstallStatus | null {
+  ): Promise<AgentHookInstallStatus | null> {
     this.supersedeWslReconciliation(runtimeHomePath)
     const wslPlan = createCodexWslRuntimeHookInstallPlan(runtimeHomePath, target)
+    if (
+      runtimeHomePath &&
+      wslPlan &&
+      (await isWslHookDistroBlocked(wslPlan.wslDistro))
+    ) {
+      return skippedWslHookInstallStatus(runtimeHomePath)
+    }
     return wslPlan ? refreshWslRuntimeUserHooks(wslPlan) : null
   }
Minor · Preserve the unverifiable WSL state in the skipped-install… · codex-hook-wsl-runtime.ts:41-53

src/main/codex/codex-hook-wsl-runtime.ts:41-53
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Preserve the unverifiable WSL state in the skipped-install status.

blockedWslDistroForPath('', distro) returns separate stopped and unverifiable states, but isWslHookDistroBlocked converts both to true. The skipped-install path then returns an AgentHookInstallStatus whose detail says the distro is not running. An unverifiable distro can therefore produce a false operator diagnostic.

Preserve the state when constructing the status, or use state-neutral wording:

Suggested fix
-/** A WSL home whose install was skipped because its distro is stopped; the relay installs later. */
+/** A WSL home whose install was skipped because its distro is unavailable; the relay installs later. */
...
-    detail: 'The WSL distro is not running; Orca installs these hooks once it runs.'
+    detail: 'The WSL distro is unavailable; Orca installs these hooks once it is available.'

Review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 4618a8ad-12f0-478c-a503-2c1aa2f2c7b6
Commits

Reviewing files that changed from the base of the PR and between aa135f0 and f3ffaf5.

Files ignored due to path filters: 1
  • src/shared/rpc-contract/rpc-params-catalog.generated.ts is excluded by !**/*.generated.*
Files selected for processing: 5
  • config/process-host-direct-import-baseline.txt
  • src/main/runtime/rpc/methods/repo.test.ts
  • src/main/runtime/rpc/methods/repo.ts
  • src/renderer/src/runtime/runtime-hooks-client.ts
  • src/shared/rpc-contract/repo-params.ts
Files with no reviewable changes: 1
  • config/process-host-direct-import-baseline.txt

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review.

@nwparker
nwparker merged commit e0f05c3 into main Oct 29, 2026
52 checks passed
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.

1 participant