Skip to content

fix(agent-hooks): cap Windows managed-hook stdin stalls at 2s host budget - #13560

Open
brennanb2025 wants to merge 6 commits into
mainfrom
brennanb2025/issue-13285-windows-hook-stdin
Open

fix(agent-hooks): cap Windows managed-hook stdin stalls at 2s host budget#13560
brennanb2025 wants to merge 6 commits into
mainfrom
brennanb2025/issue-13285-windows-hook-stdin

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Summary

Why not replace the reader?

Real Windows probing found no lower-risk batch-only replacement:

Keeping the established readers and narrowing the host backstop avoids routine 10–12 second Claude stalls without adding hundreds of milliseconds to every healthy hook or changing other providers.

Windows evidence

Case Existing 10s host budget Final Windows Claude budget
more.com, held-open stdin still running at 8,012 ms host kill ~2,035 ms
curl payload@-, held-open stdin still running at 8,007 ms host kill ~2,032 ms
curl, closed stdin / unavailable endpoint ~723 ms ~703 ms
missing Orca env, held-open stdin ~51 ms ~53 ms
more.com, closed stdin 86–129 ms ~63–98 ms

Independent final-head probing also confirmed form success at 79–150 ms, unavailable-endpoint completion at 561–588 ms, exact UTF-8 body fidelity, no writer error for the retained form path, and writer EOF for the rejected curl -T - alternative.

Tests

  • Windows low-spec, exact head: 22 passed / 1 intentional skip across the Windows stdin-bound, all-provider timeout, and Claude install suites.
  • Local focused run: 20 passed / 3 platform skips.
  • pnpm tc passed.
  • Touched-file oxlint/oxfmt passed.
  • Windows install regeneration asserts Claude becomes 2 seconds while OpenClaude remains 10 seconds; global provider/SSH timeout coverage remains unchanged.

AI Review Report

  • Orca-native same-model high review audited the complete diff, issue fit, stdin behavior, cross-provider/SSH scope, performance, and Windows evidence.
  • It found one scope leak where OpenClaude inherited the 2-second timeout; fixed in dc6826665c with an explicit Windows regression assertion.
  • No other actionable findings remained.

Security Audit

  • No new executable lookup, endpoint, token, auth, remote-wire, or payload-format surface.
  • Existing fully-qualified Windows system binaries and hook token header are unchanged.

Screenshots

No visual change.

Closes #13285 · STA-3729

Windows managed hooks block until stdin EOF on both more.com drain and
curl payload@-; --max-time does not cover body assembly. Batch has no
bounded reader without PowerShell startup cost, so lower the host
timeout from 10s to 2s near the curl budget while keeping the #8419
more.com full-drain contract and missing-env exit path.
@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

The change documents Windows stdin-draining and timeout behavior for managed hooks. It adds a two-second timeout for Windows Claude hooks and passes the optional timeout through managed-hook installation. Windows tests cover timeout values, environment handling, stdin draining, curl command construction, installed scripts, Devin handling, and temporary-resource cleanup.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes lower the Windows host timeout to 2 seconds, preserve stdin draining and hook behavior, and add regression coverage for the linked issue objectives.
Out of Scope Changes check ✅ Passed The code, documentation, timeout wiring, fixtures, and tests are directly related to the Windows managed-hook stdin stall objectives.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check ✅ Passed The title clearly identifies the Windows managed-hook stdin stall fix and the new two-second host budget.
Description check ✅ Passed The description covers the summary, testing, AI review, security audit, screenshots, platform scope, risks, and linked issue; Notes is not explicit but key details are included.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant