Skip to content

fix(hooks): exit Windows vendor-replay hook skips without draining stdin - #33470

Merged
nwparker merged 2 commits into
mainfrom
nwparker/agint-windows-hook-moredrain
Oct 25, 2026
Merged

nwparker merged 2 commits into
mainfrom
nwparker/agint-windows-hook-moredrain

Conversation

@nwparker

@nwparker nwparker commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 2 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​26 $\color{#cf222e}{\Huge{\mathbf{−}}}$​24 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​2
Prod 4 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​11 $\color{#cf222e}{\Huge{\mathbf{−}}}$​23 $\color{#cf222e}{\Huge{\mathbf{−}}}$​12

ELI5

On Windows, when Grok, Devin, or Cursor runs Orca's Claude hook (these agents import Claude's hooks), Orca is supposed to skip quietly. Instead of returning, the skip handed stdin to more.com and waited for stdin to close. If the caller kept stdin open, the hook just hung: it held the agent's tool call until the hook timeout and left a child process holding the agent's stdout pipe. These skips now return right away without reading stdin, which is already how Orca handles the "not in an Orca pane" guards on Windows.

Root cause

What changed

  • Rebased @inforanger's fix(hooks): exit Windows Devin/Grok skip without draining stdin #24909 onto current main (their commit is kept). The Devin and Grok .cmd skips now exit /b 0.
  • The Cursor replay guard gets the same treatment.
  • native/windows-agent-hook/src/main.rs: Grok/Devin move from drain_vars to exit_vars, and the now-unused drain branch is removed.
  • Tests:
    • The Windows live test (managed-hook-stdin-lifecycle.test.ts) now runs the in-pane Devin/Grok/Cursor skips with stdin held open.
    • The structural check now rejects any goto into the drain from managed scripts. It was ORCA_*-only before.
    • The native exe host test now runs the Grok/Devin skip with stdin abandoned.

Verification

Reproduced on current main with a real Windows 11 host (win-lowspec). I generated claude-hook-impl.cmd with this repo's getManagedScript on Windows and ran it through cmd.exe /d /c with Orca pane env plus each vendor's variable. The writer sent a 256 KB payload:

skip stdin main this PR
Devin held open still running after 6 s exit 0 in ~55 ms
Grok held open still running after 6 s exit 0 in ~52 ms
Cursor held open still running after 6 s exit 0 in ~59 ms
any closed exit 0, ~80 ms exit 0, ~55 ms (writer sees EOF)

Native exe logic, from the same Rust source built for the host with the payload written and stdin left open: main was still running after 3 s for both GROK_HOOK_EVENT and DEVIN_PROJECT_DIR, and this PR exits 0 in 2–196 ms. cargo check is clean. The Windows-only exe/host tests run in Windows CI, because the exe build needs a Windows host.

I also ran the Windows-only vitest suites in a clean clone of this branch on win-lowspec. With this PR, managed-hook-stdin-lifecycle.test.ts and grok-replay-guard.test.ts pass, including the new held-open-stdin skip leg. With main's three source files restored, the new leg fails with hook did not finish with stdin abandond: cmd.exe /d /c ...\claude-hook-impl.cmd. The one remaining failure on that host is the orca-agent-hook.exe build, because it has no Rust toolchain. It fails the same way on main.

Also ran pnpm tc, check:code-quality:changed, and the agent-hooks / claude / cursor / grok / devin suites (296 files) on macOS.

Risk

  • The writer can see a broken pipe on these skips. A caller that writes a large payload after the hook has exited gets EPIPE/EOF, the same as Orca's missing-context guards today. I checked the callers against upstream:
    • Grok ignores the stdin write result (grok-build crates/codegen/xai-grok-hooks/src/runner/command.rs:259-266, let _ = stdin.write_all(..)) and kills the hook group on timeout.
    • Cursor feeds hooks through a shell heredoc (cursor-agent 2026.06.02 3880.index.js executeCommandScript: ${command} <<'CURSOR_HOOK_EOF').
    • Devin is closed source, so I couldn't confirm how it treats a hook stdin write failure. The hook still prints {} first and exits 0.
  • The unreachable :orca_agent_hook_drain_stdin epilogue is left in the Claude script. It's shared with other scripts and with the statusline and missing-script launchers.

Credit

This builds on #24909 by @inforanger: their commit is preserved, and the follow-up commit credits them as co-author. It supersedes #24909, which conflicts with current main and misses the Cursor guard and the native exe.

Fixes #24907

inforanger and others added 2 commits October 25, 2026 01:00
The Windows managed hook routes the Devin-imports-Claude and Grok-replay
skips to the `orca_agent_hook_drain_stdin` epilogue, which parks in
`more.com` until stdin reaches EOF. When the caller abandons the pipe
instead of closing it — the same #11549 class this file already guards
against for missing Orca env — the drain child never returns: the hook is
stranded until the caller's timeout, and because `more.com` inherits the
hook's stdout handle it keeps that pipe open after a timeout-kill, so the
agent's read never sees EOF.

Exit the skip paths without owning stdin instead. The payload is discarded
on those paths anyway, matching the rule the env guards already follow
(buildWindowsHookEnvironmentGuardLines), and removing the last in-script
jumps to `more.com`.

- hook-script.ts: DEVIN skip `goto :drain` -> `exit /b 0`
- grok-replay-guard.ts: Windows Grok skip `goto :drain` -> `exit /b 0`
- update the lifecycle + grok tests to pin exit-without-drain

The shared drain epilogue itself is left intact (the missing-managed-script
launcher case and the lifecycle contract still reference it); a bounded
Windows drain, mirroring the POSIX deadline reader, is a larger follow-up.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…reading stdin

Extends the Devin/Grok skip fix to the two remaining Windows paths that
still drain stdin to EOF for vendor-replayed Claude hooks:

- the Cursor replay guard (added after the original fix) now exits
  instead of jumping to the more.com drain;
- orca-agent-hook.exe, now the primary Windows Claude hook, treats
  GROK_HOOK_EVENT / DEVIN_PROJECT_DIR as exit vars instead of draining.

Adds a Windows regression that runs the in-pane skip paths with stdin
held open, and widens the structural check to reject any goto into the
drain from managed hook scripts.

Refs #24907

Co-authored-by: inforanger <inforanger@users.noreply.github.com>
@coderabbitai

coderabbitai Bot commented Oct 25, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: c9aa41ac-174d-4761-9a79-8d03ee089a49
Commits

Reviewing files that changed from the base of the PR and between 1e221ce and 4a69485.

Files selected for processing: 6
  • native/windows-agent-hook/src/main.rs
  • src/main/agent-hooks/cursor-replay-guard.ts
  • src/main/agent-hooks/grok-replay-guard.ts
  • src/main/agent-hooks/managed-hook-stdin-lifecycle.test.ts
  • src/main/agent-hooks/windows-native-hook-exe-hosts.test.ts
  • src/main/claude/hook-script.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.

Walkthrough

Windows native and managed hook skip paths now exit without draining stdin. The native hook reads stdin only after its exit checks pass. Tests cover skip paths with stdin left unwritten and verify successful exits.

Priority: Normal

Severity of issue fixed: Medium

Merge Risk: Minimal · up to 4a694

The Windows replay-hook skips can exit without waiting for abandoned stdin. No merge-blocking issue was identified.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 6 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title "fix(hooks): exit Windows vendor-replay hook skips without draining stdin" matches the main change. The PR makes Windows Devin, Grok, and Cursor replay-hook skips (and the native orca-agent-…
Description check Passed The description follows the repository template. It has ELI5, What Changed (with root cause and mechanism), Why (explains the drain behavior and the chosen approach over alternatives), Linked Issue (F…
Linked Issues check Passed Issue #24907 requires Windows skip paths to avoid an EOF-dependent stdin drain or to use a bounded reader. The PR changes the Devin and Grok managed-script skips, the Cursor replay skip, and the nativ…
Out of Scope Changes check Passed The changes stay within issue #24907. Production changes remove stdin ownership from vendor-replay skip paths. Test changes verify the no-drain structure and the required early exit behavior. The Curs…
Full details: Title check

Explanation

The title "fix(hooks): exit Windows vendor-replay hook skips without draining stdin" matches the main change. The PR makes Windows Devin, Grok, and Cursor replay-hook skips (and the native orca-agent-hook.exe Grok/Devin path) exit without reading stdin. It uses a conventional commit prefix and is specific. It does not name every file, which is expected.

Full details: Description check

Explanation

The description follows the repository template. It has ELI5, What Changed (with root cause and mechanism), Why (explains the drain behavior and the chosen approach over alternatives), Linked Issue (Fixes #24907), Visual Proof (behavior change with a detailed before/after table of measured timings instead of screenshots), Testing (Windows 11 and macOS runs listed with specifics), a Risk section, Credit, and a checklist. Gaps: the AI Disclosure section is blank, the Review and Agent skill upstream boundary checkboxes are not ticked or addressed, the Notes cross-platform/SSH/backwards-compatibility sweep is not explicitly confirmed, and the template's checklist items are not marked. The 'Visual Proof' section is a timing table rather than the requested BEFORE/AFTER attachments, which is acceptable for a non-UI hook change but should be stated as N/A with reason or supported with the table as is.

Resolution

Optionally fill in the AI Disclosure section (or mark it as not applicable for internal contributors), tick or address the Agent skill upstream boundary checkbox, and confirm the Notes checks for Security, cross-platform, SSH, and backwards compatibility. Mark the Visual Proof section as N/A with the reason, or keep the timing table and label it as the behavior evidence.

Full details: Linked Issues check

Explanation

Issue #24907 requires Windows skip paths to avoid an EOF-dependent stdin drain or to use a bounded reader. The PR changes the Devin and Grok managed-script skips, the Cursor replay skip, and the native Claude hook executable to exit before reading stdin. The tests cover held-open or abandoned stdin for the managed scripts and native executable. The shared drain remains only for other callers, which does not conflict with the issue requirement.

Full details: Out of Scope Changes check

Explanation

The changes stay within issue #24907. Production changes remove stdin ownership from vendor-replay skip paths. Test changes verify the no-drain structure and the required early exit behavior. The Cursor and native executable changes directly address the same Windows hang mechanism. No unrelated production behavior is introduced.

Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@nwparker
nwparker merged commit e804a02 into main Oct 25, 2026
45 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Windows managed hooks strand on the more.com stdin drain when the caller abandons stdin

2 participants