Repository navigation
fix(agent-hooks): register forward-slash Windows hook paths for Grok, Devin, ZCode and Antigravity - #27713
Merged
Conversation
… Devin, ZCode and Antigravity These four agents registered their native-Windows status hook as a bare backslash path. A Git Bash hook host strips the backslashes, the command exits 127, and no status event reaches Orca. Use the same direct forward-slash form Claude, Codex and Muse already register, keeping the cmd-safe gate and the encoded PowerShell fallback. Existing backslash entries still match the managed-command matcher, so the next install replaces them and keeps user hooks.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ➖ Normal Merge Risk: 🔵 Low · up to The change moves Windows hook paths to forward slashes and adds test coverage. No new defects were found in the reviewed files. One earlier test-isolation concern could not be re-checked here, so owners should confirm it before merging. Pre-merge checks ·
  |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Description check |  |
The description explains the problem, implementation, testing, and compatibility impact. However, it does not provide the required linked issue and leaves required checklist and review details incompl… | Link the actual issue addressed by this PR, or document the maintainer exception if applicable. Complete the Review, AI Disclosure, Agent skill upstream boundary, Notes, and remaining checklist sections. State whether the required lint, typ… |
| Docstring Coverage | |
Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 18 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly identifies the main change: registering forward-slash Windows hook paths for Grok, Devin, ZCode, and Antigravity. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
Full details: Description check
-
Explanation
The description explains the problem, implementation, testing, and compatibility impact. However, it does not provide the required linked issue and leaves required checklist and review details incomplete.
Resolution
Link the actual issue addressed by this PR, or document the maintainer exception if applicable. Complete the Review, AI Disclosure, Agent skill upstream boundary, Notes, and remaining checklist sections. State whether the required lint, typecheck, test, and build checks pass or are covered by CI, and clearly separate pre-existing failures from PR failures.
✨ Finishing Touches 💡 2
-
📝 Generate docstrings 💡
-
- Commit to this branch
- Create a new PR
🛠️ Fix failing CI checks 💡
-
- 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.
Comment @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
4a3663fa-7f0f-49ab-b1f6-e84ba66fbcd5
📒 Files selected for processing (11)
.github/workflows/pr.ymlconfig/scripts/pr-code-change-scope.mjssrc/main/agent-hooks/windows-direct-cmd-agent-hook-hosts.test.tssrc/main/antigravity/hook-service.test.tssrc/main/antigravity/hook-service.tssrc/main/devin/hook-service.test.tssrc/main/devin/hook-settings.tssrc/main/grok/grok-hook-script.tssrc/main/grok/windows-hook-launcher-chain.test.tssrc/main/zcode/hook-service.test.tssrc/main/zcode/hook-settings.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review.
|
|
||
| const { homedirMock } = vi.hoisted(() => ({ homedirMock: vi.fn<() => string>() })) | ||
|
|
||
| vi.mock('os', async (importOriginal) => { |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
set -eu
printf '%s\n' '--- diff ---'
git diff --no-ext-diff 83056e5edb4d8eadccb93cdd9937091d8d7cb641 5ee1dff2cce5eb50245298ef0319a79835f94ea5 -- src/main/agent-hooks/windows-direct-cmd-agent-hook-hosts.test.ts
printf '%s\n' '--- test imports and mock ---'
nl -ba src/main/agent-hooks/windows-direct-cmd-agent-hook-hosts.test.ts | sed -n '1,220p'
printf '%s\n' '--- agent hook imports ---'
rg -n -F --glob '*.ts' -- 'from "node:os"' src/main src | head -80 || true
rg -n -F --glob '*.ts' -- "from 'node:os'" src/main src | head -80 || true
rg -n -F --glob '*.ts' -- "from 'os'" src/main src | head -80 || true
printf '%s\n' '--- vitest declarations ---'
rg -n -F --glob 'package.json' -- '"vitest"' . | head -40 || true
rg -n -F --glob '*lock*' -- 'vitest@5.0.3' . | head -20 || trueRepository: stablyai/orca
Length of output: 22768
🌐 Web query:
Vitest 5.0.3 vi.mock os node:os separate module mock identity
💡 Result:
If you mean **“does `vi.mock('node:os')` create a separate mock identity from `vi.mock('os')`?”**: **don’t assume it does.** They are two specifiers for Node’s OS builtin, and Vitest’s mock matching depends on how the importer’s specifier is resolved. Use the exact specifier your code imports, consistently—for example, if production code imports `node:os`, mock `node:os`. Vitest documents that mocks are registered for module imports and handled through its module resolver; it doesn’t establish that `os` and `node:os` are distinct mock identities. ([vitest.dev](https://vitest.dev/guide/mocking/modules))
```ts
vi.mock('node:os', () => ({
cpus: vi.fn(() => []),
}))
```
If you’re seeing duplicate module identities or a mock not applying under **Vitest 5**, there is a related report for **5.0.1** involving `vmThreads`, `isolate: false`, and dependency optimization—but that report does not establish a `node:os`-specific issue or confirm behavior in **5.0.3**. (GitHub issue 11289 in vitest-dev/vitest (link omitted to avoid creating a cross-reference))
Citations:
- 1: https://vitest.dev/guide/mocking/modules
- 2: GitHub issue 11289 in vitest-dev/vitest (link omitted to avoid creating a cross-reference)
🏁 Script executed:
set -eu
printf '%s\n' '--- relevant homedir bindings ---'
for f in \
src/main/grok/hook-service.ts \
src/main/grok/hook-settings.ts \
src/main/devin/hook-service.ts \
src/main/devin/hook-settings.ts \
src/main/zcode/hook-service.ts \
src/main/zcode/hook-settings.ts \
src/main/antigravity/hook-service.ts \
src/main/antigravity/hook-settings.ts
do
if test -f "$f"; then
printf '\n--- %s ---\n' "$f"
rg -n -C 4 -F -- 'homedir' "$f" || true
fi
done
printf '%s\n' '--- local Vitest source candidates ---'
find node_modules src/packages -path '*vitest*' -type f -maxdepth 8 2>/dev/null | head -80 || true
printf '%s\n' '--- Vitest mock resolver references if present ---'
rg -n -F --glob '*.{js,ts,mjs,cjs}' -- 'node:' node_modules/.pnpm/vitest@5.0.3* 2>/dev/null | head -80 || true
rg -n -F --glob '*.{js,ts,mjs,cjs}' -- 'mockPath' node_modules/.pnpm/vitest@5.0.3* 2>/dev/null | head -80 || trueRepository: stablyai/orca
Length of output: 16663
🏁 Script executed:
set -eu
VITEST=$(find node_modules/.pnpm -path '*/node_modules/@vitest/mocker/dist' -type d -print -quit)
printf '%s\n' "--- mocker directory: $VITEST ---"
printf '%s\n' '--- mock registration/resolution symbols ---'
rg -n -C 5 -F -- 'mock:' "$VITEST" node_modules/.pnpm/vitest@5.0.3*/node_modules/vitest/dist 2>/dev/null | head -160 || true
rg -n -C 5 -E 'resolveId|moduleId|normalize.*id|node:' "$VITEST" node_modules/.pnpm/vitest@5.0.3*/node_modules/vitest/dist 2>/dev/null | head -240 || true
printf '%s\n' '--- hook settings/service imports and path calls ---'
for f in src/main/grok/hook-service.ts src/main/grok/hook-settings.ts src/main/devin/hook-service.ts src/main/devin/hook-settings.ts src/main/zcode/hook-service.ts src/main/zcode/hook-settings.ts src/main/antigravity/hook-service.ts; do
if test -f "$f"; then
printf '\n--- %s ---\n' "$f"
sed -n '1,180p' "$f"
fi
doneRepository: stablyai/orca
Length of output: 42587
🏁 Script executed:
set -eu
MOCKER=$(find node_modules/.pnpm -path '*/node_modules/@vitest/mocker/dist/chunk-mocker.js' -print -quit)
REGISTRY=$(find node_modules/.pnpm -path '*/node_modules/@vitest/mocker/dist/chunk-registry.js' -print -quit)
NODE=$(find node_modules/.pnpm -path '*/node_modules/@vitest/mocker/dist/node.js' -print -quit)
printf '%s\n' "--- $NODE ---"
rg -n -C 14 -E 'resolveMockId|resolveMock\\(|resolveId\\(' "$NODE" | head -260
printf '%s\n' "--- $REGISTRY ---"
rg -n -C 10 -E 'registryBy(Id|Url)|add\\(|register\\(' "$REGISTRY" | head -220
printf '%s\n' "--- $MOCKER ---"
rg -n -C 8 -E 'resolveMock\\(|resolveMockId|mockedIds|queueMock' "$MOCKER" | head -260Repository: stablyai/orca
Length of output: 1057
🏁 Script executed:
set -eu
MOCKER=$(find node_modules/.pnpm -path '*/node_modules/@vitest/mocker/dist/chunk-mocker.js' -print -quit)
REGISTRY=$(find node_modules/.pnpm -path '*/node_modules/@vitest/mocker/dist/chunk-registry.js' -print -quit)
NODE=$(find node_modules/.pnpm -path '*/node_modules/@vitest/mocker/dist/node.js' -print -quit)
printf '%s\n' "--- $NODE ---"
rg -n -E 'resolveMockId|resolveMock\(|resolveId\(' -C 14 "$NODE" | head -260
printf '%s\n' "--- $REGISTRY ---"
rg -n -E 'registryBy(Id|Url)|add\(|register\(' -C 10 "$REGISTRY" | head -220
printf '%s\n' "--- $MOCKER ---"
rg -n -E 'resolveMock\(|resolveMockId|mockedIds|queueMock' -C 8 "$MOCKER" | head -260Repository: stablyai/orca
Length of output: 1052
🏁 Script executed:
set -eu
MOCKER=$(find node_modules/.pnpm -path '*/node_modules/@vitest/mocker/dist/chunk-mocker.js' -print -quit)
REGISTRY=$(find node_modules/.pnpm -path '*/node_modules/@vitest/mocker/dist/chunk-registry.js' -print -quit)
NODE=$(find node_modules/.pnpm -path '*/node_modules/@vitest/mocker/dist/node.js' -print -quit)
printf '%s\n' "--- $NODE ---"
rg -n -C 12 'resolveMockId|resolveMock\(|resolveId\(' "$NODE" | head -260
printf '%s\n' "--- $REGISTRY ---"
rg -n -C 10 'registryBy|add\(|register\(' "$REGISTRY" | head -220
printf '%s\n' "--- $MOCKER ---"
rg -n -C 8 'resolveMock\(|resolveMockId|mockedIds|queueMock' "$MOCKER" | head -260Repository: stablyai/orca
Length of output: 13324
🏁 Script executed:
set -eu
printf '%s\n' '--- resolver packages ---'
find node_modules/.pnpm -maxdepth 1 -type d \( -name 'rolldown-vite@*' -o -name 'vite@*' \) -print | head -20
printf '%s\n' '--- builtin resolution references ---'
rg -n -C 6 'builtin|builtins|node:' node_modules/.pnpm/rolldown-vite@*/node_modules/rolldown-vite node_modules/.pnpm/vite@*/node_modules/vite 2>/dev/null | head -260 || trueRepository: stablyai/orca
Length of output: 26584
🏁 Script executed:
set -eu
VITE=$(find node_modules/.pnpm -path '*/node_modules/rolldown-vite/dist' -type d -print -quit)
printf '%s\n' "--- $VITE ---"
find "$VITE" -maxdepth 3 -type f | head -80
printf '%s\n' '--- resolver terms ---'
rg -n -C 5 'builtinModules|isBuiltin|nodeBuiltins|builtins|external|resolveId' "$VITE" | head -300 || trueRepository: stablyai/orca
Length of output: 41330
🏁 Script executed:
set -eu
VITE=$(find node_modules/.pnpm -path '*/node_modules/rolldown-vite/dist/node/chunks/node.js' -print -quit)
printf '%s\n' "--- $VITE ---"
rg -n -C 12 'isBuiltin|createIsBuiltin|resolveExternal|externalize|builtin' "$VITE" | head -320Repository: stablyai/orca
Length of output: 17077
Mock node:os explicitly.
The installers import node:os, but this test mocks only os. These specifiers resolve as distinct module IDs, so the installers can use the real home directory while cleanup removes only the temporary directory.
Suggested fix
--- "a/src/main/agent-hooks/windows-direct-cmd-agent-hook-hosts.test.ts"
+++ "b/src/main/agent-hooks/windows-direct-cmd-agent-hook-hosts.test.ts"
@@ -14,7 +14,7 @@
const { homedirMock } = vi.hoisted(() => ({ homedirMock: vi.fn<() => string>() }))
-vi.mock('os', async (importOriginal) => {
+vi.mock('node:os', async (importOriginal) => {
const actual = await importOriginal<typeof osModule>()
return { ...actual, homedir: homedirMock.mockImplementation(actual.homedir) }
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| vi.mock('os', async (importOriginal) => { | |
| vi.mock('node:os', async (importOriginal) => { |
added 6 commits
October 11, 2026 19:00
…and Command Code on Windows On a cmd-safe profile path these agents now register the managed .cmd path itself instead of a `powershell.exe -EncodedCommand` launcher, so each hook event no longer pays a second PowerShell start-up on top of the runner's own shell. Profiles the bare path cannot carry (spaces, non-ASCII, UNC) keep the encoded launcher. Same pattern as Claude (#18875) and Muse (#26755). Refs #20913
…nicode upgrade ASCII profiles now register the direct forward-slash wrapper path, so the upgrade-delivery test expects that form and also runs it through Git Bash. Unicode profiles keep the encoded launcher.
…home Pins that the os mock reaches the agents' node:os imports, so the host test never installs into the real profile.
…ry table - Add wrapWindowsBatchHookCommand (bare .cmd path, encoded launcher otherwise) and use it for Muse, Gemini, DSH, Kiro, Droid and Command Code. - Fold the five agents into windows-hook-payload-delivery.test.ts as rows run under each vendor's shell, from a plain and a spaced profile, with a stale encoded registration seeded per row; drop the separate runner suite and its CI list entries. - Replace the Windows-only per-agent registration tests with ones that stub process.platform, so they run on every CI OS. - Fix stale launcher, helper and DSH comments. Refs #20913
…ershell-runners' into OrcaWin/hooks-forward-slash-cmd-paths
…rough wrapWindowsBatchHookCommand Use the shared batch-hook helper from #27717 instead of an inline direct-or-cmd fallback. A UNC home is cmd-safe but has no drive letter; the old fallback registered it as a bare backslash path that Git Bash cannot start, while the helper sends it to the encoded launcher. wrapWindowsCmdHookCommand has no production callers left, so it is removed and its tests point at the helper. Grok, Devin and ZCode join the Windows payload delivery table with a legacy backslash entry to sweep, replacing the separate host test, and the Antigravity migration test now runs on every platform.
OrcaWin
changed the base branch from
main
to
OrcaWin/hooks-direct-cmd-for-powershell-runners
October 12, 2026 18:40
…-slash-cmd-paths # Conflicts: # src/main/agent-hooks/windows-direct-cmd-hook-command.test.ts # src/main/agent-hooks/windows-direct-cmd-hook-command.ts # src/main/agent-hooks/windows-hook-payload-delivery.test.ts
OrcaWin
changed the base branch from
OrcaWin/hooks-direct-cmd-for-powershell-runners
to
main
October 12, 2026 18:44
added 3 commits
October 12, 2026 13:11
…-slash-cmd-paths # Conflicts: # src/main/agent-hooks/windows-hook-payload-delivery.test.ts
…-slash-cmd-paths # Conflicts: # src/main/agent-hooks/windows-hook-payload-delivery.test.ts
7 of 9 tasks
trs-80
added a commit
to trs-80/orca
that referenced
this pull request
Oct 21, 2026
…returning detection Main (stablyai#27713) replaced wrapWindowsCmdHookCommand with wrapWindowsBatchHookCommand, which also sends UNC homes to the encoded launcher; Bob registers through it like ZCode, Kiro and Droid. Main's install-dir fallback tests (stablyai#27819) mock the fallback detection calls, which on this branch is resolveCommandsInInstallDirs (paths kept for the identity probe), so the mocks follow the rename; the PATH assertions are unchanged. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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.
ELI5
On Windows, Orca tells Grok, Devin, ZCode and Antigravity to run a small script whenever the agent's status changes. If the agent happens to run that script through Git Bash, the backslashes in the path are stripped, the script isn't found, and the status never updates. The path now uses forward slashes, which every Windows shell understands.
What Changed
wrapWindowsBatchHookCommand(added in perf(agent-hooks): drop the extra PowerShell per hook event for Gemini, dsh, Kiro, Droid and Command Code #27717). For an ordinary profile that's a forward-slash path such asC:/Users/x/.orca/agent-hooks/grok-hook.cmd, the form Claude, Codex and Muse already use.devin/hook-settings.ts,zcode/hook-settings.ts,grok/grok-hook-script.ts,antigravity/hook-service.ts. For Antigravity only the wrapper path changes; its.cjsand Electron runtime are untouched.\\server\share\...). The old code registered a UNC home as a bare backslash path, which Git Bash can't start either.wrapWindowsCmdHookCommandhas no callers left, so it's deleted and its tests now usewrapWindowsBatchHookCommand. One name now covers one policy.createManagedCommandMatcher, so they're replaced on the next install. Users' own hooks are kept.Why
Git Bash treats
\as an escape character, soC:\Users\x\...\grok-hook.cmdbecomesC:Usersx...grok-hook.cmd, and the hook exits 127 with no status event. Grok's docs list Git Bash as a shell it may use; for Devin, ZCode and Antigravity this is a latent risk. The forward-slash form runs the same under cmd.exe, PowerShell 5.1, Git Bash, and when the agent starts the script directly as a program (Devin does this). It reuses the shared helper from #27717 rather than a second copy of the same fallback, and it starts no extra interpreter.Found by an audit of every agent's Windows hook registration.
Linked Issue
None. This was found by a Windows hook audit.
Visual Proof
N/A: no UI change; only the hook command text in each agent's config changes.
Testing
Real run on Windows 11: a temp profile per agent, a mock hook server, and cmd's AutoRun active.
Starting the script directly with a forward-slash path (no shell,
UseShellExecute=false, as Devin does) also runs it: exit 0.Grok, Devin and ZCode are new rows in the Windows payload-delivery table from perf(agent-hooks): drop the extra PowerShell per hook event for Gemini, dsh, Kiro, Droid and Command Code #27717 (
windows-hook-payload-delivery.test.ts, on the Windows CI lane). Each row seeds an old encoded entry and an old backslash entry and requires exactly one managed entry after install. It then pipes a multi-KB non-ASCII payload through cmd.exe, Git Bash and Windows PowerShell for a plain profile and one with a space. It checks the posted payload byte for byte, empty stderr and the expected stdout, and that the hook exits without posting outside an Orca pane. Reverting Grok to the bare backslash path fails the Git Bash leg with "command not found".Antigravity's upgrade and delivery are covered by
windows-cmd-hook-command-unicode.test.ts(on the lane), which now also runs the forward-slash command through Git Bash.Unit tests on every platform cover the forward-slash form, a UNC home falling back to the encoded launcher, and sweeping an old backslash entry while keeping a user hook (Devin, ZCode, Grok, Antigravity).
pnpm run typecheck:node,oxlint, the anti-slop lint step,check:code-quality:changedand the Windows lane-registration test pass.Some tests fail on this machine on
maintoo (a Devin test that expects the non-Windows script, Grok and installer-utils symlink checks, a Grok file-mode check), so they predate this change.I manually tested these changes locally
Automated tests added/updated
Checklist
N/Awith reason