Repository navigation
fix: dispatch Muse Windows hooks directly on safe profile paths - #26755
Merged
Merged
Conversation
Reuse the existing Claude batch launcher and upgrade registered managed hook files asynchronously without rewriting identical settings. Co-authored-by: Naveen Chatlapalli <naveen.aifanatic@gmail.com>
Contributor
📝 WalkthroughWalkthroughMuse hook command generation now uses direct Priority: ➖ Normal Severity of issue fixed: Medium Merge Risk: 🔵 Low · up to The Windows hook change looks sound. One rare case remains: a zero-byte Muse managed-hooks file is not repaired by the automatic refresh, so it stays broken until hooks are reinstalled. This is low risk and can be fixed as a follow-up. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
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[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
🧹 Nitpick comments (1)
src/main/muse/hook-service.test.ts (1)
170-173: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake the upgrade test require an actual command change.
If the temporary home contains a space or another unsafe character,
getMuseManagedCommand(scriptPath())returns the same encoded command seeded at Line 168. The test then passes without testing the Windows upgrade. Use a cmd-safe fixture path and assert that the upgraded command differs from the seeded command.
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
ed0203db-1e36-44a7-8871-ab0a0036af85
📒 Files selected for processing (5)
src/main/agent-hooks/windows-hook-payload-delivery.test.tssrc/main/muse/hook-service.test.tssrc/main/muse/hook-service.tssrc/main/muse/hook-settings.test.tssrc/main/muse/hook-settings.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.
| readFile(managedHooksPath, 'utf8').catch(() => null) | ||
| ]) | ||
| const config = settings === null ? null : parseMuseSettingsText(settings, 'Muse settings.json') | ||
| if (!existing || config?.managed_hooks_path !== managedHooksPath) { |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Refresh an empty managed-hooks file.
If muse-hooks.json exists but contains zero bytes, !existing returns before the refresh. Muse hooks remain unavailable until another install repairs the file. Use existing === null to distinguish a failed read from an empty file.
This was referenced Oct 9, 2026
7 tasks done
OrcaWin
added a commit
that referenced
this pull request
Oct 12, 2026
…i, dsh, Kiro, Droid and Command Code (#27717) * perf(hooks): register direct .cmd hooks for Gemini, DSH, Kiro, Droid 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 * refactor(hooks): one Windows batch-hook helper and one payload-delivery 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 --------- Co-authored-by: Orca Worker <orca-worker@localhost>
OrcaWin
added a commit
that referenced
this pull request
Oct 12, 2026
…27714) * perf(hooks): register direct .cmd hooks for Gemini, DSH, Kiro, Droid 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 * fix(kimi): register a cmd.exe-runnable hook on Windows Kimi runs hooks with asyncio.create_subprocess_shell, which is cmd.exe on Windows. Orca registered a POSIX `if [ -f ... ]` command there, so cmd failed with "-f was unexpected at this time" before reaching the script and, since Kimi fails open, no status event was ever sent. On win32 write a cmd+curl kimi-hook.cmd and register it the way the other direct .cmd agents do: the bare forward-slash path on a cmd-safe profile, the encoded launcher otherwise. The ownership matcher already covers every extension of the stem, so the old kimi-hook.sh entry is replaced on the next install. SSH remotes keep the POSIX script. Kimi joins the direct .cmd runner suite as a row run the way CPython's shell=True does, and every row now also runs from a profile with a space (the encoded launcher) and outside an Orca pane. * refactor(hooks): one Windows batch-hook helper and one payload-delivery 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 --------- Co-authored-by: Orca Worker <orca-worker@localhost>
OrcaWin
added a commit
that referenced
this pull request
Oct 12, 2026
…nd UTF-8 safe (#27718) * perf(hooks): register direct .cmd hooks for Gemini, DSH, Kiro, Droid 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 * fix(kimi): register a cmd.exe-runnable hook on Windows Kimi runs hooks with asyncio.create_subprocess_shell, which is cmd.exe on Windows. Orca registered a POSIX `if [ -f ... ]` command there, so cmd failed with "-f was unexpected at this time" before reaching the script and, since Kimi fails open, no status event was ever sent. On win32 write a cmd+curl kimi-hook.cmd and register it the way the other direct .cmd agents do: the bare forward-slash path on a cmd-safe profile, the encoded launcher otherwise. The ownership matcher already covers every extension of the stem, so the old kimi-hook.sh entry is replaced on the next install. SSH remotes keep the POSIX script. Kimi joins the direct .cmd runner suite as a row run the way CPython's shell=True does, and every row now also runs from a profile with a space (the encoded launcher) and outside an Orca pane. * fix(copilot): run the Windows status hook as a direct curl .cmd Copilot runs a Windows hook as `pwsh|powershell -nop -nol -c <command>`. Orca registered an encoded `powershell.exe -EncodedCommand` launcher there that started a second PowerShell to run copilot-hook.ps1, which posted with Invoke-WebRequest: two PowerShell starts per event (~850-900 ms median), and the .ps1 read stdin through the console code page, so non-ASCII prompt text arrived as "caf?? ...". On win32 write copilot-hook.cmd (answer `{}`, refresh the endpoint, env guard before stdin, curl form POST with hookEventName) and register its bare path with the event as its argument; non-cmd-safe profile paths keep the encoded launcher, which now sets ORCA_COPILOT_HOOK_EVENT for the .cmd. The managed matcher covers every extension of the stem, so old .ps1 entries are replaced on the next install while user hooks stay. SSH remotes keep the POSIX script. Copilot joins the direct .cmd runner suite as a row run under both PowerShells, checking the byte-exact payload and the posted hookEventName. Refs #20913 * refactor(hooks): one Windows batch-hook helper and one payload-delivery 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 --------- Co-authored-by: Orca Worker <orca-worker@localhost>
OrcaWin
added a commit
that referenced
this pull request
Oct 12, 2026
… Devin, ZCode and Antigravity (#27713) * fix(agent-hooks): register forward-slash Windows hook paths for Grok, 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. * perf(hooks): register direct .cmd hooks for Gemini, DSH, Kiro, Droid 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 * test(agent-hooks): expect forward-slash Antigravity hooks after the Unicode 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. * test(agent-hooks): assert the registered hook lives under the mocked home Pins that the os mock reaches the agents' node:os imports, so the host test never installs into the real profile. * refactor(hooks): one Windows batch-hook helper and one payload-delivery 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 * refactor(agent-hooks): register Grok, Devin, ZCode and Antigravity through 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. --------- Co-authored-by: Orca Worker <orca-worker@localhost>
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
Muse status hooks on ordinary Windows profile paths now run their existing batch script directly, removing a PowerShell startup on every event—even outside Orca.
What Changed
Reuse Claude's existing direct batch launcher for safe drive-letter paths, preserving its cross-shell spelling and the existing quoting fallback. Refresh registered Muse hook files asynchronously so existing installations pick up the command once; repeat refresh leaves their bytes and timestamps unchanged. Keep the existing stdin transport, environment guard and user-owned settings pointers.
Why
The batch script already exits early without Orca context and posts with system curl. Reusing its proven launcher removes the redundant interpreter without creating another path-quoting mechanism. Adapted community PR #26734, with Naveen Chatlapalli credited from commit metadata.
Linked Issue
Fixes #26717
Visual Proof
N/A — background hook execution only; no rendered UI change.
Before product edits, actual generated commands on Windows launched twelve observed PowerShell children across twelve executions with Orca variables absent. On the same host/profile/shell after the fix: zero observed PowerShell children, median exit time 923 ms → 19 ms, maximum 1431 ms → 59 ms. Process snapshots filtered descendants by parent lineage to exclude diagnostic CIM scans; this is a small instrumented sample, not a production latency guarantee.
Testing
Windows
awin, Node 24.18.0, using bundles of the actual source in an isolated checkout and temporary profiles:macOS: generated SSH-install POSIX command executed with live payload/authentication/pane checks and quoted path; in-memory SFTP tests retain POSIX installation even from Windows clients.
ORCA_BACKGROUND_LAUNCH=1used throughout. Final focused run: 201 passed / 10 skipped, 14 files.pnpm tc:node, focused Oxlint, Oxfmt, changed-code quality gate and diff whitespace checks passed. Windows-only Vitest legs were skipped on macOS; Windows behavior was verified through standalone assertions against generated commands.Review
No new dependencies, wire fields, status stores, Git assumptions, secure-write changes or watchdog changes. Startup registration refresh reuses the existing asynchronous atomic writer. SSH execution remains POSIX and uses the execution host's existing endpoint contract; folder attribution remains unchanged.
Agent skill upstream boundary
Notes
Profiles needing special quoting, Unicode or UNC retain the established encoded PowerShell fallback and its startup cost. UNC network execution, a second Windows host, a real SSH connection and a full packaged build remain unverified. A deleted direct shim returns code 1, matching Claude's existing launcher; fallback missing-shim exits 0. Muse's hook contract records code 1 as a failed hook and continues the turn; only code 2 with stderr can block an action. This preserves session progress, though a missing file is now recorded as a hook failure rather than a successful no-op.
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)