fix(executor): capture Claude CLI stderr via SDK callback - #105
Conversation
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR adds two observable properties to the Claude Agent SDK execution path: an ChangesExecutor Query Options Enhancement
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
There was a problem hiding this comment.
Pull request overview
This PR improves daemon/operator observability by wiring the Claude Agent SDK’s stderr streaming callback into the request-scoped pino logger, so Claude Code CLI failures surface their real root-cause text (instead of only the SDK’s generic “process exited” wrapper error).
Changes:
- Add an
stderrcallback to the SDKquery()options inexecuteAgent(). - Trim and skip whitespace-only stderr chunks, logging non-empty output as
warnwith structured{ stderr }.
Apply Copilot review feedback on PR #105: - length-cap stderr at 500 chars + truncated flag (matches updater.ts / scoped-rebase-executor.ts convention) - swap trim() for trimEnd() so leading indentation in multi-line stack traces stays readable - add 5 unit tests covering forwarding, logging, indentation preservation, whitespace-skip, and truncation
CLI stderr can echo bearer tokens, OAuth tokens, or DB URLs (e.g. a failed HTTP call dumping its Authorization header). The new pino log path would surface those into pod logs and any cluster-wide log aggregator, undoing the protection that buildProviderEnv's allowlist provides on the subprocess side. Pipe each chunk through redactSecrets() before truncation/logging and surface a structured redactedSecretCount + redactedSecretKinds log field so operators can attribute redactions without ever seeing the matched bytes. If the chunk becomes empty after redaction it is skipped, matching the existing whitespace-only behaviour. Comment also corrected: SDK forwards stderr in stream chunks, not strictly line-by-line.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
## [1.10.1](v1.10.0...v1.10.1) (2026-05-06) ### Bug Fixes * **executor:** capture Claude CLI stderr via SDK callback ([#105](#105)) ([3482443](3482443))
|
🎉 This PR is included in version 1.10.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Summary
When the bundled Claude Code CLI subprocess exits non-zero, the Agent SDK throws a content-free
Error("Claude Code process exited with code N")and the daemon logs only that wrapper — the CLI's actual stderr (auth failure, 429 rate-limit, model rejection, OOM, missing binary, etc.) is never surfaced. Today's pod failure (github-app-playground-daemon-default-5b89f6dc4-twmdkon issue #93,durationMs: 248ms) was diagnosable only bykubectl execinto the pod and reproducing the call by hand. The SDK already exposes astderr: (data: string) => voidcallback hook inquery()options — we just weren't wiring it.This change subscribes the daemon's request-scoped pino logger to that callback. No new deps, no env-var changes, no API surface changes.
Diagram
flowchart LR subgraph Flow["Failure surfacing path"] direction TB BeforeCLI["Claude CLI exits 1<br/>writes to stderr"]:::before BeforeSDK["SDK wraps in<br/>Error 'exited with code 1'"]:::before BeforePino["pino logs<br/>err.message only<br/>stderr SWALLOWED"]:::beforeBad BeforeOps["Operator must kubectl exec<br/>and reproduce by hand"]:::beforeBad BeforeCLI --> BeforeSDK --> BeforePino --> BeforeOps AfterCLI["Claude CLI exits 1<br/>writes to stderr"]:::after AfterSDK["SDK pipes stderr chunks<br/>to callback hook"]:::after AfterPino["pino logs each line as<br/>warn 'Claude CLI stderr'"]:::afterGood AfterOps["Operator reads pod log<br/>root cause visible"]:::afterGood AfterCLI --> AfterSDK --> AfterPino --> AfterOps end classDef before fill:#fde8e8,stroke:#9b1c1c,color:#1a1a1a classDef beforeBad fill:#9b1c1c,stroke:#1a1a1a,color:#ffffff classDef after fill:#def7ec,stroke:#03543f,color:#1a1a1a classDef afterGood fill:#03543f,stroke:#1a1a1a,color:#ffffffChanges
src/core/executor.ts: addstderrcallback toqueryOptionspassed to the SDKquery()call. Each non-empty stderr chunk is trimmed and logged atwarnlevel on the existing request-scoped logger as{ stderr: <line> }, "Claude CLI stderr". Empty/whitespace-only chunks are skipped to avoid log spam.Related Issues
5b89f6dc4-twmdkfailure on issue feat(resolve): gate handler success on post-fix CI state (handler-side CI re-check) #93.Test plan
bun run typecheckcleanbun test test/core/executor.test.ts5/5executeAgentcomplexity/length warnings are unchanged in kind, count went 146 → 152 lines on a function already over the 120 limit).Summary by CodeRabbit