Repository navigation
T102 + T103: Windows: proofmerge on PATH, and the app finds Claude Code - #85
Merged
Merged
Conversation
After installing, `proofmerge` did not run in a terminal, so the Agent Skill could not run `proofmerge verify`. On a yes, the installer adds %LOCALAPPDATA%\ProofMerge\bin (one proofmerge.cmd, never the venv's Scripts folder) to the user PATH, keeping %VAR% entries as they are, and the uninstaller takes it off again.
Setup did not detect Claude Code that worked in a terminal: starting a program by name on Windows finds only .exe files, and an npm install is claude.cmd. Claude Code is now resolved to a full path on Windows, and an npm shim is replaced by the claude.exe (or node cli.js) it starts, so cmd.exe never re-reads the JSON arguments.
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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two fixes from testing on a Windows PC.
T102: the installer can put
proofmergeon the terminal's PATHAfter installing,
proofmergedid not run in a terminal. The installer left PATH untouched, so only the Start menu app worked, and the Agent Skill could not runproofmerge verify.%LOCALAPPDATA%\ProofMerge\bin\proofmerge.cmdand adds that one folder to the user PATH. The venv'sScriptsfolder never goes on PATH, because itspython.exeandpipwould hide the user's own.%VAR%entries survive. It never touches the machine PATH, and it tells Windows that the environment changed.%%stands for one%.T103: the app finds Claude Code on Windows
Setup did not detect Claude Code, even though it worked in a terminal. On Windows, starting a program by name finds only
.exefiles, and an npm install of Claude Code isclaude.cmd.claude_command()resolves Claude Code on Windows only. It tries PATH first, then%USERPROFILE%\.local\bin\claude.exe, then%APPDATA%\npm\claude.cmd.bin/claude.exe(npm 2.1.318 lists that file as its bin), ornode cli.json older releases. So cmd.exe never re-reads the JSON arguments.Checked
tests/test_windows_installer.pypins the PATH rules. Before this, it pinned "PATH is never written".tests/test_backends.pycovers the npm shim, the 2.1.xclaude.exe, the fallbacks, and the case where nothing is found.Not checked