[yzxcj797]

Summary

The generated Codex hooks previously contained only Unix shell commands. On Windows, Codex attempted to run cat, /dev/null, and Unix shell chaining, causing PostToolUse hook failures.

Both generated hooks now include commandWindows overrides using PowerShell:

• drain stdin with [Console]::In.ReadToEnd();
• check whether the current directory is a Git repository;
• run the graph update or status command;
• always exit successfully, matching the Unix || true behavior;
• print Not a git repo, skipping for the startup hook outside Git repositories.

Fixes #620.

Testing

• Added regressions asserting both Windows overrides drain stdin, perform the Git check, invoke the correct graph command, and end successfully.
• pytest tests/test_skills.py::TestGenerateCodexHooksConfig tests/test_skills.py::TestInstallCodexHooks -q — 12 passed
• ruff check code_review_graph/skills.py tests/test_skills.py — passes
• git diff --check — passes

[github-actions]

code-review-graph review

Overall risk: 0.30 (LOW) — 3 changed function(s)/class(es), 1 affected flow(s), 1 test gap(s)

Risk-scored changes

Risk	Level	Symbol	Location	Tested
0.30	low	code_review_graph/skills.py::generate_codex_hooks_config	code_review_graph/skills.py:1451	yes
0.05	low	tests/test_skills.py::TestGenerateCodexHooksConfig	tests/test_skills.py:523	no
0.05	low	tests/test_skills.py::TestGenerateCodexHooksConfig.test_windows_commands_drain_stdin_and_exit_successfully	tests/test_skills.py:587	(test)

Affected execution flows

• install_hermes_skills — criticality 0.26, 3 node(s) across 1 file(s)

Test gaps

• tests/test_skills.py::TestGenerateCodexHooksConfig (tests/test_skills.py:523)

Token savings: this graph-backed report used ~57,446 fewer tokens (~98%) than reading every changed file in full (estimated, chars/4 approximation).

---

Powered by code-review-graph — local-first analysis; no code leaves the CI runner.

[tirth8205]

Changes required: reinstall still fails to add Windows overrides to existing Codex hooks. Generate a config with generate_codex_hooks_config, remove its inner commandWindows fields, write it to the hooks.json location used by install_codex_hooks, and call that installer again; the fields remain absent because the existing Unix commands match. Consolidate the migration with the related hook PRs and verify actual execution in the released Windows client.

[tirth8205]

Checks pass and the code looks right. Missing: tests for the edge cases it introduces.

• Upgrade path: install_codex_hooks must add (or refresh) commandWindows on existing entries whose command already matches, plus a test that writes an older Unix-only hooks.json, reinstalls, and asserts both hooks gain commandWindows and no duplicate entries are appended.
• Windows execution tests behind @pytest.mark.skipif(sys.platform != 'win32') that run each commandWindows string through os.environ['COMSPEC'] /C exactly as Codex does: (a) in a git init tmp repo with a multi-MB piped stdin , returncode 0, no hang/BrokenPipe; (b) outside a git repo , returncode 0 a...
• Add tests/test_skills.py (or the new Windows tests) to the windows-native job in .github/workflows/ci.yml so the Windows-only tests actually execute in CI.
• CHANGELOG.md [Unreleased] entry for the fix referencing Bug: Codex installer generates Unix-only hooks on Windows #620 (repo convention; the PR touches no changelog).

Every new branch in the code should have a test, including the failure branch. Disagree with any of these? Say so here first.

PRs now target staging, not main. Yours was retargeted already, so nothing to do there.

[tirth8205]

The upgrade path works because of _merge_hook_entries, which landed on staging in d39f9fa on 2026-09-16 from a different PR, not because of anything on this branch. The only content commit here is still c883e50 from 2026-08-19. Installing with staging and then reinstalling with this branch into the same HOME does add commandWindows to both existing Codex entries with no duplicate appended, but nothing pins that coupling. If _is_generated_hook_command or the merge rule changes, Windows users keep a Unix-only hooks.json forever and no test notices.

Four items from my 2026-09-15 comment are still open and neither review got a reply: the reinstall test over an older Unix-only hooks.json, the Windows-only execution tests behind skipif(sys.platform != 'win32'), the CI wiring, and the CHANGELOG entry for #620.

The new test never runs on a Windows runner. It sits in TestGenerateCodexHooksConfig, and the windows-native job enumerates only:

tests/test_skills.py::TestInstallCodexHooks
tests/test_skills.py::TestInstallCursorHooks

It runs only in the cross-platform test job, where it checks string shape and proves nothing about Windows.

Nothing here shows Codex honours a commandWindows key at all. The installer writes that key into every user's ~/.codex/hooks.json on every OS. If Codex ignores unknown keys it is harmless. If it validates the hooks schema strictly, every Codex hook breaks for everyone.

The two forms also diverge on the failure branches, untested. The Unix form sends git's stderr to /dev/null and the PowerShell form does not, so outside a git repo it prints a red error record on every hook fire. If git is missing from PATH, PowerShell raises CommandNotFoundException, skips the if body, and runs code-review-graph anyway, where the Unix form takes the skip branch.

The claim does reproduce with the real CLI. Staging writes Codex hooks with no commandWindows, this branch writes both. The new test fails on staging with KeyError: 'commandWindows' and passes here, so it is a real guard. Gates are clean, and uninstall is unaffected because uninstall._commands() matches on command only.

To merge: the reinstall test, the Windows execution tests, TestGenerateCodexHooksConfig added to windows-native, the CHANGELOG entry, and evidence that a released Codex build actually runs commandWindows.

[tirth8205]

Thank you for this PR.

I tested reinstalls, the installer tests, and both hooks with PowerShell 7 on a Mac.

Before I can merge it, please fix these:

1. Add a test for reinstalling over an old Unix-only hooks.json. No test covers this today. If _merge_hook_entries (skills.py:1924) keeps the old hook, all tests still pass.
2. Add Windows-only tests that run each commandWindows hook. The only test now (tests/test_skills.py:587) checks the string shape. Please cover large stdin, a non-repo folder, missing git, and a failing CLI. With git missing, the Windows form still runs update --skip-flows. The Unix form does not.
3. Add the new tests to the windows-native job in ci.yml. It now runs only TestInstallCodexHooks and TestInstallCursorHooks.
4. Add a CHANGELOG.md entry under [Unreleased] for Bug: Codex installer generates Unix-only hooks on Windows #620.
5. Test it on a real Windows machine with Codex 0.131.0 or newer. Both hooks should run at startup and after Write, Edit and Bash.

Thank you for your patience.

[Copilot]

🟡 Changes recommended

The outer PowerShell launcher expands $null out of the hook conditions, producing invalid syntax that causes execution to fail before reaching exit 0.

1 open issue

• Avoid $null expansion in PowerShell launcher conditions · New

What changed in this PR

Adds Windows-specific Codex hook commands to address the Unix-only hook failures reported in #620, while preserving existing Unix commands.

Changes:

• Adds PowerShell overrides for graph updates and startup status checks.
• Includes stdin draining, Git checks, and explicit successful exits.
• Adds regression assertions for both generated Windows commands.

File	Description
tests/​test_skills.py	Adds assertions for Windows hook command generation.
code_review_graph/​skills.py	Adds commandWindows overrides to both Codex hooks.

_{Review effort: Balanced}