fix(coding-agent): resolve kernel venv interpreter for Windows layout - #663
fix(coding-agent): resolve kernel venv interpreter for Windows layout#663skulitom wants to merge 1 commit into
Conversation
bootstrapVenv and ensureKernelPythonUncached built the interpreter path as <venv>/bin/python, which only exists on POSIX. On Windows the interpreter is at <venv>/Scripts/python.exe, so uv pip install --python got a path that does not exist and exited 2, leaving the kernel unusable. The same path feeds kernelReady, so on Windows it was always false: every launch fell through to bootstrapVenv, which runs uv venv --seed and recreates the venv. A manually provisioned venv was therefore discarded on the next run. Add venvPython to pick the layout from process.platform, matching the existing win32 branch in findExecutable, and use it at both sites. Update the two integration tests that hardcoded the POSIX candidate path. Fixes PrimeIntellect-ai#660 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b8fac4b972
ℹ️ 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".
| const candidates = [ | ||
| process.env.PRIME_AGENT_KERNEL_PYTHON, | ||
| join(homedir(), ".prime", "agent", "kernel-venv", "bin", "python"), | ||
| venvPython(join(homedir(), ".prime", "agent", "kernel-venv")), |
There was a problem hiding this comment.
Reuse the platform-aware path in the recursion test
On Windows, test/agent-session-recursion.test.ts:2141 still falls back to kernel-venv/bin/python, so its spawnSync targets a nonexistent interpreter even though the managed environment now uses Scripts/python.exe. The repository-wide search for kernel-venv and bin.*python found this remaining executable-path construction; update it to use venvPython as these two changed tests do.
Useful? React with 👍 / 👎.
| expect(getKernelVenvDir()).toBe(venv); | ||
| }); | ||
|
|
||
| it("resolves the venv interpreter using the host platform layout", () => { |
There was a problem hiding this comment.
Run each modified test file to passing
The commit's Testing section reports only a filtered invocation of this new case and explicitly says the complete file still has 17 failures; it also does not report complete runs of the other two modified test files. The root AGENTS.md requires every modified test file to be run and iterated until it passes, so the full affected files need validation before this change is accepted.
AGENTS.md reference: AGENTS.md:L30-L31
Useful? React with 👍 / 👎.
| expect(getKernelVenvDir()).toBe(venv); | ||
| }); | ||
|
|
||
| it("resolves the venv interpreter using the host platform layout", () => { |
There was a problem hiding this comment.
Put the issue regression in the required suite directory
This is explicitly the regression for issue #660, but it is added to test/kernel-bootstrap.test.ts rather than test/suite/regressions/660-<slug>.test.ts. The root AGENTS.md requires issue-specific regressions in that directory and requires the suite harness with the faux provider, so move the regression to the prescribed location and structure.
AGENTS.md reference: AGENTS.md:L32-L33
Useful? React with 👍 / 👎.
|
|
||
| 1. `PRIME_AGENT_KERNEL_PYTHON`, when it can import `ipykernel`; | ||
| 2. `~/.prime/agent/kernel-venv/bin/python`, bootstrapped with `uv`; or | ||
| 2. `~/.prime/agent/kernel-venv/bin/python` (`Scripts\python.exe` on Windows), bootstrapped with `uv`; or |
There was a problem hiding this comment.
Record the Windows fix in the coding-agent changelog
This is a user-visible fix for Windows kernel startup, but packages/coding-agent/CHANGELOG.md remains unchanged with an empty [Unreleased] section. Add a one-line past-tense entry there so the fix is included in the next package release notes, as required by the root AGENTS.md.
AGENTS.md reference: AGENTS.md:L105-L112
Useful? React with 👍 / 👎.
|
Thank you for the report and proposed work. This root cause is now covered by maintainer-owned stacked PR #1163, authored independently from We did not inspect or reuse this PR's diff, branch, commits, implementation code, or tests; its public description/comments were used only as a bug report. To keep one review surface, this PR is superseded by #1163 and is being closed. The complete review stack is #1158–#1165. It is being left unmerged for human review after CI and review-bot findings are cleared. |
Fixes #660.
Problem
bootstrapVenvandensureKernelPythonUncachedboth built the kernel interpreter path aspath.join(venv, "bin", "python"). That is the POSIX venv layout; on Windows the interpreter is at<venv>\Scripts\python.exe. The path never exists there, souv pip install --python <path>exits 2 and the kernel never starts.Since the IPython kernel is the agent's only tool, the practical effect on Windows is an agent with no ability to read files, grep, or run commands. It still answers from context files, which makes this read as a model problem rather than a bootstrap failure.
The second site compounds it.
kernelReady(python, ...)is checked against the same non-existent path, so it is always false on Windows: every launch falls through tobootstrapVenv, which callsuv venv --seedand recreates the directory. A venv provisioned by hand is discarded on the next run, so the failure looks intermittent when it is in fact deterministic.Change
venvPython(venv), which selects the layout fromprocess.platform. This mirrors thewin32branch already present infindExecutableandensureUvin the same file.test/ipython-bootstrap.test.tsandtest/kernel-state-roundtrip.test.ts, which hardcoded the POSIX candidate path and so could never locate the venv on Windows.docs/rlm-runtime.md.No behavior change on macOS or Linux:
venvPythonreturns exactly the previous path there.Testing
npm run checkpasses (biome,tsgo --noEmit, installer render, browser smoke).The new assertion in
test/kernel-bootstrap.test.tsstubsprocess.platformand pins both layouts, so it exercises the Windows branch regardless of host OS:I could not run the rest of that file meaningfully, and want to be explicit about it: its fake
uvand fake interpreter are#!/bin/shscripts, so the file is POSIX-only by construction and fails wholesale on a Windows host. I baselined instead — on the unmodified tree it is 17 failed / 4 passed, and with this change 17 failed / 5 passed. Same pre-existing failures, plus the new test. Please confirm against CI on Linux.Verified manually on Windows 11 with prime-agent 0.7.0: with the interpreter provisioned at
Scripts\python.exeandPRIME_AGENT_KERNEL_PYTHONpointed at it, the kernel starts and the agent completes a real repo search, returning a function's defining file and line number that matched an independent grep.Two notes
AGENTS.mdasks for issue regressions undertest/suite/regressions/<issue>-<slug>.test.ts. I put the assertion intest/kernel-bootstrap.test.tsinstead, since this is a pure path unit test with no agent interaction and that directory is built around the faux-provider harness. Happy to move it if you would rather keep the convention uniform.venvPythonis exported so the test can call it directly, consistent withgetKernelVenvDirandresolveRuntimeIdentityin the same module. If you would rather it stay private, the test can assert throughensureKernelPythonon POSIX only, at the cost of not covering the Windows branch in CI.Not addressed here
Two adjacent items from the issue, left out to keep this reviewable:
ipykerneland runtime first, default packages second), so discovering the full set takes three launches.uv venv --seedruns before the interpreter path is validated. That is harmless once the path is correct, but it is the mechanism that destroyed a working venv.Generated with Claude Code
Note
Low Risk
Small, localized path helper with POSIX parity; main risk is incorrect platform detection, covered by a new unit test.
Overview
Fixes Windows IPython kernel bootstrap by resolving the managed venv interpreter with
venvPython(venv)(Scripts\python.exeonwin32,bin/pythonelsewhere) instead of always using the POSIX path.bootstrapVenvandensureKernelPythonUncachednow pass that path intouv pip installandkernelReadychecks, so bootstrap and reuse work when the interpreter actually lives underScripts. POSIX behavior is unchanged.Tests that locate the default kernel venv use
venvPython, andkernel-bootstrap.test.tsstubsprocess.platformto lock both layouts.rlm-runtime.mddocuments the Windows path.Reviewed by Cursor Bugbot for commit b8fac4b. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Fix kernel venv interpreter path resolution for Windows in coding agent
Adds a
venvPythonhelper inbootstrap.tsthat returnsScripts/python.exeonwin32andbin/pythonotherwise. All venv interpreter references in bootstrap and resolver functions are updated to use this helper, replacing the hardcoded Unix-only path. Tests are updated to usevenvPythoninstead of hardcoded paths, and a new test case asserts the correct path per platform.Macroscope summarized b8fac4b.