Repository navigation
fix(query): configure hard max and abort reasons - #1850
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📜 Recent review details⏰ Context from checks skipped due to timeout. (2)
🧰 Additional context used📓 Path-based instructions (9)src/**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{ts,tsx,js,jsx,py,json,md}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*.test.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*.test.{ts,tsx,js}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*.{ts,tsx,js,jsx,py}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**⚙️ CodeRabbit configuration file
Files:
**/*⚙️ CodeRabbit configuration file
Files:
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}⚙️ CodeRabbit configuration file
Files:
🔇 Additional comments (1)
📝 WalkthroughWalkthroughThis PR adds reason-aware abort messaging across query, tool, and Claude streaming paths, extends abort-reason normalization, and introduces ChangesAbort Reason Classification and Wiring
Query Hard-Max Timeout Configuration
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 7✅ Passed checks (7 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/screens/REPL.tsx (1)
564-581: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing direct test coverage for the new terminal-reason categorization.
The switch collapses
background/parent-ended/side-task-cancelledinto'parent-ended'— a lifecycle-log/messaging behavior change.REPL.queryLifecycle.test.tsin this cohort only assertsQueryGuardconstruction wiring, not this function's branch behavior.As per path instructions, tests should "add/update tests to cover the new behavior" for changed user-facing/lifecycle-logging behavior.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/screens/REPL.tsx` around lines 564 - 581, Add direct test coverage for getQueryTerminalReason in REPL.tsx, since the new abort-reason mapping is user-facing lifecycle behavior. Update REPL.queryLifecycle.test.ts (or the closest REPL query lifecycle test suite) to assert each branch: non-aborted signals returning ok/unknown, query-timeout and hard-max-query-timeout passthrough, user-abort and interrupt mapping to user-abort, and background/parent-ended/side-task-cancelled mapping to parent-ended. Use the existing getQueryTerminalReason and normalizeAbortReason behavior as the target for the new assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/query.abortClassification.test.ts`:
- Around line 1-65: This test is asserting implementation text in query.ts
instead of the real abort behavior, so replace or supplement it with a
behavioral regression test. Use the existing generator flow in query.ts (and
mirror stopHooks.goal.test.ts) to mock an abortController, trigger
query-timeout/background/hard_max reasons, and assert the yielded
system/tool-result messages. Keep coverage focused on user-visible output, not
string presence like getQueryAbortSystemMessage or the removed Interrupted by
user literal.
In `@src/query.ts`:
- Around line 51-55: Fix the shared abort-reason gap by updating
normalizeAbortReason in utils/abortReasons so a legacy bare
AbortController.abort() still normalizes to the user-interruption text used by
getMissingToolResultAbortMessage. In query.ts, keep the abort classification
flow around abortReason, getQueryAbortSystemMessage, and
shouldCreateUserInterruptionMessage unchanged aside from relying on the
corrected normalization. The repeated abort-message block in query.ts and the
matching logic in stopHooks.ts can be left for a follow-up shared helper
refactor since the root-cause fix is in abortReasons.
In `@src/services/api/claude.abortClassification.test.ts`:
- Around line 1-13: The current grep-based test only verifies source substrings
and does not exercise the real abort-message behavior in claude.ts. Replace or
supplement it with a behavioral test that mocks the streaming abort path,
triggers an actual abort, and asserts on the resulting logForDebugging/logEvent
output and gating behavior. Use the claude.ts abort handling flow around
getStreamingAbortMessage and signal.reason to locate the code, and verify the
user-visible message rather than implementation text.
In `@src/utils/abortReasons.test.ts`:
- Around line 82-123: The abortReasons test currently covers the legacy
bare-abort signal only for shouldCreateUserInterruptionMessage, so extend the
same test to assert getMissingToolResultAbortMessage and
getStreamingAbortMessage with defaultAbort.signal.reason as well. Use the
existing abortReasons helper names in src/utils/abortReasons.test.ts to add
expectations that the bare-abort case stays consistent across all three
functions.
In `@src/utils/abortReasons.ts`:
- Around line 102-155: The abort reason normalization is missing the legacy bare
AbortController.abort() / AbortError case, causing
getMissingToolResultAbortMessage and getStreamingAbortMessage to classify it as
unknown instead of user abort. Update normalizeAbortReason so the DOMException
name === 'AbortError' path maps to the same user-abort signal used elsewhere,
then simplify shouldCreateUserInterruptionMessage to rely on
normalizeAbortReason only. This will make getMissingToolResultAbortMessage and
getStreamingAbortMessage return the correct user-facing messages consistently.
---
Outside diff comments:
In `@src/screens/REPL.tsx`:
- Around line 564-581: Add direct test coverage for getQueryTerminalReason in
REPL.tsx, since the new abort-reason mapping is user-facing lifecycle behavior.
Update REPL.queryLifecycle.test.ts (or the closest REPL query lifecycle test
suite) to assert each branch: non-aborted signals returning ok/unknown,
query-timeout and hard-max-query-timeout passthrough, user-abort and interrupt
mapping to user-abort, and background/parent-ended/side-task-cancelled mapping
to parent-ended. Use the existing getQueryTerminalReason and
normalizeAbortReason behavior as the target for the new assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2ceb6eb6-59d7-4797-bd11-987248bcc561
📒 Files selected for processing (20)
docs/advanced-setup.mdsrc/query.abortClassification.test.tssrc/query.tssrc/query/stopHooks.goal.test.tssrc/query/stopHooks.tssrc/query/toolFailureLoopGuard.test.tssrc/query/toolFailureLoopGuard.tssrc/screens/REPL.queryLifecycle.test.tssrc/screens/REPL.tsxsrc/services/api/claude.abortClassification.test.tssrc/services/api/claude.tssrc/services/tools/StreamingToolExecutor.test.tssrc/services/tools/StreamingToolExecutor.tssrc/services/tools/toolExecution.test.tssrc/services/tools/toolExecution.tssrc/utils/QueryGuard.test.tssrc/utils/abortReasons.test.tssrc/utils/abortReasons.tssrc/utils/queryGuardConfig.test.tssrc/utils/queryGuardConfig.ts
📜 Review details
⚠️ CI failures not shown inline (2)
GitHub Actions: PR Checks / smoke-and-tests: fix(query): configure hard max and abort reasons
Conclusion: failure
ored tip catalog > all tips have unique sponsor-prefixed ids [1.00ms]
(pass) sponsored tip catalog > rendered content embeds sponsor name, tip body, and URL
(pass) sponsored tip catalog > Xiaomi MiMo tips render through sponsored tip chrome [1.00ms]
(pass) sponsored tip catalog > isRelevant follows sponsoredTipsEnabled [1.00ms]
(pass) sponsored history tracking > records lastShownAt and increments totalShown [1.00ms]
(pass) sponsored history tracking > getSessionsSinceLastSponsored returns Infinity when never shown [1.00ms]
(pass) sponsored history tracking > getSessionsSinceLastSponsored returns delta from current startups
##[endgroup]
##[group]src/services/compact/microCompact.test.ts:
(pass) microCompact MCP tool compaction > module exports load correctly [1.00ms]
(pass) microCompact MCP tool compaction > estimateMessageTokens counts MCP tool_use blocks
(pass) microCompact MCP tool compaction > microcompactMessages processes MCP tools without error
(pass) microCompact MCP tool compaction > microcompactMessages processes mixed built-in and MCP tools [1.00ms]
(pass) microCompact MCP tool compaction > time-based microcompact clears native toolUseResult for old compacted results
##[endgroup]
##[group]src/services/compact/snipProjection.test.ts:
(pass) isSnipBoundaryMessage > returns true for message with snipMetadata
(pass) isSnipBoundaryMessage > returns false for compact_boundary without snipMetadata
(pass) isSnipBoundaryMessage > returns false for regular message
(pass) isSnipBoundaryMessage > returns false for null/undefined
(pass) projectSnippedView > returns original array when no snip boundaries present
(pass) projectSnippedView > removes messages whose UUIDs appear in snipMetadata.removedUuids
(pass) projectSnippedView > accumulates removedUuids from multiple snip boundaries
(pass) projectSnippedView > handles boundaries with no removedUuids gracefully
(pass) projectSnippedView > prunes earlier messages when the boundary is appended...
GitHub Actions: PR Checks / 2_smoke-and-tests.txt: fix(query): configure hard max and abort reasons
Conclusion: failure
truncated structured Bash JSON in streaming responses [2.00ms]
(pass) does not normalize incomplete streamed Bash commands when finish_reason is length [1.00ms]
(pass) repairs truncated JSON objects even without command field [1.00ms]
(pass) preserves raw input for unknown plain string tool arguments [1.00ms]
(pass) preserves parsed string input for unknown JSON string tool arguments [2.00ms]
(pass) sanitizes malformed MCP tool schemas before sending them to OpenAI [1.00ms]
(pass) optional tool properties are not added to required[] — fixes Groq/Azure 400 tool_use_failed [2.00ms]
(pass) coalesces consecutive user messages to avoid alternation errors (issue `#202`) [1.00ms]
(pass) coalesces consecutive assistant messages preserving tool_calls (issue `#202`) [1.00ms]
(pass) non-streaming: reasoning_content emitted as thinking block only when content is null [1.00ms]
(pass) non-streaming: empty string content does not fall through to reasoning_content as text [2.00ms]
(pass) non-streaming: real content takes precedence over reasoning_content [1.00ms]
(pass) non-streaming: preserves response body when usage parsing fails [1.00ms]
(pass) non-streaming: preserves response.url routing metadata after body read [1.00ms]
(pass) non-streaming: strips <think> tag block from assistant content [1.00ms]
(pass) streaming: thinking block closed before tool call [1.00ms]
(pass) streaming: strips <think> tag block from assistant content deltas [2.00ms]
(pass) streaming: strips <think> tag split across multiple content chunks [1.00ms]
(pass) streaming: preserves prose without tags (no phrase-based false positive) [1.00ms]
(pass) strips credentials and query params from URL in fetch network error message [1.00ms]
(pass) classifies localhost transport failures with actionable category marker [2.00ms]
(pass) transport failures are not labeled with HTTP status 503 [1.00ms]
(pass) propagates AbortError without wrapping it as transport failure [1.00ms]
(pass) classifies...
🧰 Additional context used
📓 Path-based instructions (15)
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript with strict mode and ESM imports
Files:
src/query.abortClassification.test.tssrc/services/api/claude.abortClassification.test.tssrc/query/stopHooks.goal.test.tssrc/utils/queryGuardConfig.tssrc/screens/REPL.queryLifecycle.test.tssrc/utils/queryGuardConfig.test.tssrc/utils/QueryGuard.test.tssrc/services/tools/toolExecution.tssrc/services/tools/toolExecution.test.tssrc/services/tools/StreamingToolExecutor.test.tssrc/query/stopHooks.tssrc/utils/abortReasons.test.tssrc/services/api/claude.tssrc/query/toolFailureLoopGuard.tssrc/query/toolFailureLoopGuard.test.tssrc/query.tssrc/screens/REPL.tsxsrc/services/tools/StreamingToolExecutor.tssrc/utils/abortReasons.ts
**/*.{ts,tsx,js,jsx,py,json,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow the existing code style in the touched files
Files:
src/query.abortClassification.test.tsdocs/advanced-setup.mdsrc/services/api/claude.abortClassification.test.tssrc/query/stopHooks.goal.test.tssrc/utils/queryGuardConfig.tssrc/screens/REPL.queryLifecycle.test.tssrc/utils/queryGuardConfig.test.tssrc/utils/QueryGuard.test.tssrc/services/tools/toolExecution.tssrc/services/tools/toolExecution.test.tssrc/services/tools/StreamingToolExecutor.test.tssrc/query/stopHooks.tssrc/utils/abortReasons.test.tssrc/services/api/claude.tssrc/query/toolFailureLoopGuard.tssrc/query/toolFailureLoopGuard.test.tssrc/query.tssrc/screens/REPL.tsxsrc/services/tools/StreamingToolExecutor.tssrc/utils/abortReasons.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add or update tests when the change affects behavior
Files:
src/query.abortClassification.test.tssrc/services/api/claude.abortClassification.test.tssrc/query/stopHooks.goal.test.tssrc/screens/REPL.queryLifecycle.test.tssrc/utils/queryGuardConfig.test.tssrc/utils/QueryGuard.test.tssrc/services/tools/toolExecution.test.tssrc/services/tools/StreamingToolExecutor.test.tssrc/utils/abortReasons.test.tssrc/query/toolFailureLoopGuard.test.ts
**/*.test.{ts,tsx,js}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Test the exact provider/model path you changed when possible
Files:
src/query.abortClassification.test.tssrc/services/api/claude.abortClassification.test.tssrc/query/stopHooks.goal.test.tssrc/screens/REPL.queryLifecycle.test.tssrc/utils/queryGuardConfig.test.tssrc/utils/QueryGuard.test.tssrc/services/tools/toolExecution.test.tssrc/services/tools/StreamingToolExecutor.test.tssrc/utils/abortReasons.test.tssrc/query/toolFailureLoopGuard.test.ts
**/*.{ts,tsx,js,jsx,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Keep comments useful and concise
Files:
src/query.abortClassification.test.tssrc/services/api/claude.abortClassification.test.tssrc/query/stopHooks.goal.test.tssrc/utils/queryGuardConfig.tssrc/screens/REPL.queryLifecycle.test.tssrc/utils/queryGuardConfig.test.tssrc/utils/QueryGuard.test.tssrc/services/tools/toolExecution.tssrc/services/tools/toolExecution.test.tssrc/services/tools/StreamingToolExecutor.test.tssrc/query/stopHooks.tssrc/utils/abortReasons.test.tssrc/services/api/claude.tssrc/query/toolFailureLoopGuard.tssrc/query/toolFailureLoopGuard.test.tssrc/query.tssrc/screens/REPL.tsxsrc/services/tools/StreamingToolExecutor.tssrc/utils/abortReasons.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow TypeScript strict mode and type safety practices by running typecheck before submitting
Files:
src/query.abortClassification.test.tssrc/services/api/claude.abortClassification.test.tssrc/query/stopHooks.goal.test.tssrc/utils/queryGuardConfig.tssrc/screens/REPL.queryLifecycle.test.tssrc/utils/queryGuardConfig.test.tssrc/utils/QueryGuard.test.tssrc/services/tools/toolExecution.tssrc/services/tools/toolExecution.test.tssrc/services/tools/StreamingToolExecutor.test.tssrc/query/stopHooks.tssrc/utils/abortReasons.test.tssrc/services/api/claude.tssrc/query/toolFailureLoopGuard.tssrc/query/toolFailureLoopGuard.test.tssrc/query.tssrc/screens/REPL.tsxsrc/services/tools/StreamingToolExecutor.tssrc/utils/abortReasons.ts
**
⚙️ CodeRabbit configuration file
**: # AGENTS.md - AI Agent Coding GuideThis guide is for AI coding agents working in the OpenClaude repository. Read it before changing code, and also follow CONTRIBUTING.md for contributor policy, PR expectations, review follow-up, and project scope.
Project Snapshot
OpenClaude is a coding-agent CLI for cloud and local model providers. It supports OpenAI-compatible APIs, Anthropic, Gemini, DeepSeek, Ollama, MCP, local backends, slash commands, tools, agents, and a React/Ink terminal UI.
The installed CLI runs on Node.js
>=22.0.0. Bun is used for source builds, scripts, dependency management, and tests.Work Style
- Keep changes focused on one problem.
- Prefer existing patterns in the file or nearby module.
- Avoid unrelated formatting, renames, dependency changes, or broad rewrites.
- Add or update tests when behavior changes.
- Update docs when setup, commands, provider behavior, or user-facing behavior changes.
- For new features, larger refactors, dependencies, or runtime changes, follow the issue-first guidance in CONTRIBUTING.md.
Stack And Conventions
- TypeScript with strict mode and ESM imports.
- React + Ink for terminal UI.
- Bun lockfile and Bun scripts for development workflows.
- Node runtime for the built CLI.
Common libraries and patterns:
chalkfor terminal color.commanderfor CLI argument parsing.execafor child processes.- Existing service, provider, settings, permission, and UI patterns over new abstractions.
Repository Map
src/commands/- slash and CLI command implementations.src/components/- React/Ink UI components.src/services/- API, MCP, OAuth, wiki, voice, and other service integrations.src/tools/- tool implementations.src/utils/- shared utilities.src/integrations/- provider and model integration metadata.src/entrypoints/- CLI, MCP, SDK, and generated public types.src/tasks/- local, remote, workflow, and monitor tas...
Files:
src/query.abortClassification.test.tsdocs/advanced-setup.mdsrc/services/api/claude.abortClassification.test.tssrc/query/stopHooks.goal.test.tssrc/utils/queryGuardConfig.tssrc/screens/REPL.queryLifecycle.test.tssrc/utils/queryGuardConfig.test.tssrc/utils/QueryGuard.test.tssrc/services/tools/toolExecution.tssrc/services/tools/toolExecution.test.tssrc/services/tools/StreamingToolExecutor.test.tssrc/query/stopHooks.tssrc/utils/abortReasons.test.tssrc/services/api/claude.tssrc/query/toolFailureLoopGuard.tssrc/query/toolFailureLoopGuard.test.tssrc/query.tssrc/screens/REPL.tsxsrc/services/tools/StreamingToolExecutor.tssrc/utils/abortReasons.ts
**/*
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
src/query.abortClassification.test.tsdocs/advanced-setup.mdsrc/services/api/claude.abortClassification.test.tssrc/query/stopHooks.goal.test.tssrc/utils/queryGuardConfig.tssrc/screens/REPL.queryLifecycle.test.tssrc/utils/queryGuardConfig.test.tssrc/utils/QueryGuard.test.tssrc/services/tools/toolExecution.tssrc/services/tools/toolExecution.test.tssrc/services/tools/StreamingToolExecutor.test.tssrc/query/stopHooks.tssrc/utils/abortReasons.test.tssrc/services/api/claude.tssrc/query/toolFailureLoopGuard.tssrc/query/toolFailureLoopGuard.test.tssrc/query.tssrc/screens/REPL.tsxsrc/services/tools/StreamingToolExecutor.tssrc/utils/abortReasons.ts
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
src/query.abortClassification.test.tssrc/services/api/claude.abortClassification.test.tssrc/query/stopHooks.goal.test.tssrc/screens/REPL.queryLifecycle.test.tssrc/utils/queryGuardConfig.test.tssrc/utils/QueryGuard.test.tssrc/services/tools/toolExecution.test.tssrc/services/tools/StreamingToolExecutor.test.tssrc/utils/abortReasons.test.tssrc/query/toolFailureLoopGuard.test.ts
docs/**/*.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Update docs when setup, commands, or user-facing behavior changes
Files:
docs/advanced-setup.md
{README.md,CONTRIBUTING.md,docs/**,.github/pull_request_template.md}
⚙️ CodeRabbit configuration file
{README.md,CONTRIBUTING.md,docs/**,.github/pull_request_template.md}: Review docs for accuracy against current code behavior. Flag security or provider claims that overpromise, stale install commands, missing setup caveats, and instructions that could push users toward unsafe credential handling. Keep purely wording-level suggestions non-blocking.
Files:
docs/advanced-setup.md
{src/commands/**/*.ts,src/services/**/*.ts,src/entrypoints/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
chalkfor terminal color in CLI code
Files:
src/services/api/claude.abortClassification.test.tssrc/services/tools/toolExecution.tssrc/services/tools/toolExecution.test.tssrc/services/tools/StreamingToolExecutor.test.tssrc/services/api/claude.tssrc/services/tools/StreamingToolExecutor.ts
{src/services/**/*.ts,src/utils/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
execafor child processes
Files:
src/services/api/claude.abortClassification.test.tssrc/utils/queryGuardConfig.tssrc/utils/queryGuardConfig.test.tssrc/utils/QueryGuard.test.tssrc/services/tools/toolExecution.tssrc/services/tools/toolExecution.test.tssrc/services/tools/StreamingToolExecutor.test.tssrc/utils/abortReasons.test.tssrc/services/api/claude.tssrc/services/tools/StreamingToolExecutor.tssrc/utils/abortReasons.ts
{src/integrations/**/*.ts,src/services/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Test the exact provider/model path you changed when possible for provider modifications
Files:
src/services/api/claude.abortClassification.test.tssrc/services/tools/toolExecution.tssrc/services/tools/toolExecution.test.tssrc/services/tools/StreamingToolExecutor.test.tssrc/services/api/claude.tssrc/services/tools/StreamingToolExecutor.ts
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}
⚙️ CodeRabbit configuration file
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}: Review provider routing, model selection, env precedence, auth/token handling, OpenAI-compatible shims, retries, proxy behavior, and outbound HTTP behavior with high scrutiny. Block on silent default changes, hidden fallback expansion, credential reuse mistakes, hardcoded provider assumptions, or new network reach that is not intentional and documented.
Files:
src/services/api/claude.abortClassification.test.tssrc/services/api/claude.ts
🔇 Additional comments (21)
src/utils/queryGuardConfig.ts (1)
1-67: LGTM!src/utils/queryGuardConfig.test.ts (1)
1-79: LGTM!src/utils/QueryGuard.test.ts (1)
2-2: LGTM!Also applies to: 350-391
src/screens/REPL.tsx (2)
38-40: LGTM!Also applies to: 1011-1015
1817-1822: 🎯 Functional CorrectnessNo issue here: timeout reasons already map to canonical abort reasons. The timeout path only produces
query-timeoutorhard-max-query-timeout, and both are accepted bynormalizeAbortReason, so this does not degrade tounknown-abort.> Likely an incorrect or invalid review comment.src/screens/REPL.queryLifecycle.test.ts (1)
26-32: LGTM!docs/advanced-setup.md (1)
397-397: 🗄️ Data Integrity & IntegrationNo change needed The documented default matches
DEFAULT_QUERY_HARD_MAX_MS(30 * 60 * 1000).> Likely an incorrect or invalid review comment.src/query/stopHooks.ts (1)
20-23: Logic mirrors the (already flagged) query.ts pattern; same optional dedup suggestion applies here, not repeating separately.Also applies to: 323-334
src/utils/abortReasons.ts (2)
1-73: LGTM!
134-155: 🗄️ Data Integrity & IntegrationNo change needed
tool-timeoutis already excluded fromSYNTHETIC_ABORT_TOOL_RESULT_PREFIXES, so real tool timeouts still count toward repeated-failure detection.> Likely an incorrect or invalid review comment.src/query/stopHooks.goal.test.ts (1)
247-352: LGTM!src/services/api/claude.ts (2)
60-63: LGTM!Also applies to: 2629-2641
2664-2664: 🩺 Stability & AvailabilityNo issue here: the SDK-timeout path already logs separately, and
getStreamingAbortMessageonly runs whensignal.abortedis true.> Likely an incorrect or invalid review comment.src/query/toolFailureLoopGuard.ts (1)
4-18: LGTM!Also applies to: 343-345
src/query/toolFailureLoopGuard.test.ts (1)
4-4: LGTM!Also applies to: 177-183, 202-244
src/services/tools/StreamingToolExecutor.ts (3)
12-15: LGTM (aside from the narrowing issue flagged above).Also applies to: 38-41, 221-252
257-283: LGTM!
182-220: 🎯 Functional CorrectnessNo change needed here
reason === 'parent_abort'still narrowssyntheticReason, sosyntheticReason.abortReasonis valid.> Likely an incorrect or invalid review comment.src/services/tools/StreamingToolExecutor.test.ts (1)
28-44: LGTM!Also applies to: 245-307
src/services/tools/toolExecution.ts (1)
65-68: LGTM!Also applies to: 139-145, 486-488, 517-521
src/services/tools/toolExecution.test.ts (1)
20-20: LGTM!Also applies to: 38-54, 527-577
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/query.abortClassification.test.ts`:
- Around line 65-142: Extract the repeated QueryParams setup shared by
makeParams and makeToolAbortParams into a common helper so the duplicated
messages, systemPrompt, canUseTool, querySource, agentStepLimit, and deps
boilerplate live in one place. Keep the per-test differences localized by
letting the helper accept the varying pieces, especially deps.callModel and the
tool/toolUseContext setup used by makeToolAbortParams. Use the existing
makeParams and makeToolAbortParams symbols as the main entry points and preserve
the current test behavior while removing the repeated scaffolding.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c4d4f1c7-4d0d-4547-9f0e-e8d9452cfd70
📒 Files selected for processing (8)
src/query.abortClassification.test.tssrc/screens/REPL.tsxsrc/services/api/claude.abortClassification.test.tssrc/services/api/claude.tssrc/utils/abortReasons.test.tssrc/utils/abortReasons.tssrc/utils/queryLifecycle.test.tssrc/utils/queryLifecycle.ts
📜 Review details
⚠️ CI failures not shown inline (2)
GitHub Actions: PR Checks / smoke-and-tests: fix(query): configure hard max and abort reasons
Conclusion: failure
Xiaomi MiMo tips [1.00ms]
(pass) sponsored tip catalog > all tips have unique sponsor-prefixed ids [1.00ms]
(pass) sponsored tip catalog > rendered content embeds sponsor name, tip body, and URL [1.00ms]
(pass) sponsored tip catalog > Xiaomi MiMo tips render through sponsored tip chrome
(pass) sponsored tip catalog > isRelevant follows sponsoredTipsEnabled [1.00ms]
(pass) sponsored history tracking > records lastShownAt and increments totalShown [1.00ms]
(pass) sponsored history tracking > getSessionsSinceLastSponsored returns Infinity when never shown [1.00ms]
(pass) sponsored history tracking > getSessionsSinceLastSponsored returns delta from current startups
##[endgroup]
##[group]src/services/compact/microCompact.test.ts:
(pass) microCompact MCP tool compaction > module exports load correctly
(pass) microCompact MCP tool compaction > estimateMessageTokens counts MCP tool_use blocks
(pass) microCompact MCP tool compaction > microcompactMessages processes MCP tools without error
(pass) microCompact MCP tool compaction > microcompactMessages processes mixed built-in and MCP tools
(pass) microCompact MCP tool compaction > time-based microcompact clears native toolUseResult for old compacted results [1.00ms]
##[endgroup]
##[group]src/services/compact/snipProjection.test.ts:
(pass) isSnipBoundaryMessage > returns true for message with snipMetadata
(pass) isSnipBoundaryMessage > returns false for compact_boundary without snipMetadata
(pass) isSnipBoundaryMessage > returns false for regular message
(pass) isSnipBoundaryMessage > returns false for null/undefined
(pass) projectSnippedView > returns original array when no snip boundaries present
(pass) projectSnippedView > removes messages whose UUIDs appear in snipMetadata.removedUuids
(pass) projectSnippedView > accumulates removedUuids from multiple snip boundaries
(pass) projectSnippedView > handles boundaries with no removedUuids gracefully [1.00ms]
(pass) projectSnippedView > prunes earlier...
GitHub Actions: PR Checks / 1_smoke-and-tests.txt: fix(query): configure hard max and abort reasons
Conclusion: failure
rguments that begin with bracket syntax [2.00ms]
(pass) normalizes streaming Bash arguments when the first chunk is only an opening brace [1.00ms]
(pass) repairs truncated structured Bash JSON in streaming responses [3.00ms]
(pass) does not normalize incomplete streamed Bash commands when finish_reason is length
(pass) repairs truncated JSON objects even without command field [2.00ms]
(pass) preserves raw input for unknown plain string tool arguments
(pass) preserves parsed string input for unknown JSON string tool arguments [2.00ms]
(pass) sanitizes malformed MCP tool schemas before sending them to OpenAI [2.00ms]
(pass) optional tool properties are not added to required[] — fixes Groq/Azure 400 tool_use_failed [1.00ms]
(pass) coalesces consecutive user messages to avoid alternation errors (issue `#202`) [2.00ms]
(pass) coalesces consecutive assistant messages preserving tool_calls (issue `#202`) [2.00ms]
(pass) non-streaming: reasoning_content emitted as thinking block only when content is null
(pass) non-streaming: empty string content does not fall through to reasoning_content as text [1.00ms]
(pass) non-streaming: real content takes precedence over reasoning_content [2.00ms]
(pass) non-streaming: preserves response body when usage parsing fails [1.00ms]
(pass) non-streaming: preserves response.url routing metadata after body read [1.00ms]
(pass) non-streaming: strips <think> tag block from assistant content [2.00ms]
(pass) streaming: thinking block closed before tool call [3.00ms]
(pass) streaming: strips <think> tag block from assistant content deltas [1.00ms]
(pass) streaming: strips <think> tag split across multiple content chunks [1.00ms]
(pass) streaming: preserves prose without tags (no phrase-based false positive) [3.00ms]
(pass) strips credentials and query params from URL in fetch network error message
(pass) classifies localhost transport failures with actionable category marker [3.00ms]
(pass) transport failures are not labeled ...
🧰 Additional context used
📓 Path-based instructions (13)
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript with strict mode and ESM imports
Files:
src/services/api/claude.abortClassification.test.tssrc/utils/queryLifecycle.test.tssrc/utils/abortReasons.test.tssrc/utils/queryLifecycle.tssrc/query.abortClassification.test.tssrc/services/api/claude.tssrc/utils/abortReasons.tssrc/screens/REPL.tsx
{src/commands/**/*.ts,src/services/**/*.ts,src/entrypoints/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
chalkfor terminal color in CLI code
Files:
src/services/api/claude.abortClassification.test.tssrc/services/api/claude.ts
{src/services/**/*.ts,src/utils/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
execafor child processes
Files:
src/services/api/claude.abortClassification.test.tssrc/utils/queryLifecycle.test.tssrc/utils/abortReasons.test.tssrc/utils/queryLifecycle.tssrc/services/api/claude.tssrc/utils/abortReasons.ts
{src/integrations/**/*.ts,src/services/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Test the exact provider/model path you changed when possible for provider modifications
Files:
src/services/api/claude.abortClassification.test.tssrc/services/api/claude.ts
**/*.{ts,tsx,js,jsx,py,json,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow the existing code style in the touched files
Files:
src/services/api/claude.abortClassification.test.tssrc/utils/queryLifecycle.test.tssrc/utils/abortReasons.test.tssrc/utils/queryLifecycle.tssrc/query.abortClassification.test.tssrc/services/api/claude.tssrc/utils/abortReasons.tssrc/screens/REPL.tsx
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add or update tests when the change affects behavior
Files:
src/services/api/claude.abortClassification.test.tssrc/utils/queryLifecycle.test.tssrc/utils/abortReasons.test.tssrc/query.abortClassification.test.ts
**/*.test.{ts,tsx,js}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Test the exact provider/model path you changed when possible
Files:
src/services/api/claude.abortClassification.test.tssrc/utils/queryLifecycle.test.tssrc/utils/abortReasons.test.tssrc/query.abortClassification.test.ts
**/*.{ts,tsx,js,jsx,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Keep comments useful and concise
Files:
src/services/api/claude.abortClassification.test.tssrc/utils/queryLifecycle.test.tssrc/utils/abortReasons.test.tssrc/utils/queryLifecycle.tssrc/query.abortClassification.test.tssrc/services/api/claude.tssrc/utils/abortReasons.tssrc/screens/REPL.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow TypeScript strict mode and type safety practices by running typecheck before submitting
Files:
src/services/api/claude.abortClassification.test.tssrc/utils/queryLifecycle.test.tssrc/utils/abortReasons.test.tssrc/utils/queryLifecycle.tssrc/query.abortClassification.test.tssrc/services/api/claude.tssrc/utils/abortReasons.tssrc/screens/REPL.tsx
**
⚙️ CodeRabbit configuration file
**: # AGENTS.md - AI Agent Coding GuideThis guide is for AI coding agents working in the OpenClaude repository. Read it before changing code, and also follow CONTRIBUTING.md for contributor policy, PR expectations, review follow-up, and project scope.
Project Snapshot
OpenClaude is a coding-agent CLI for cloud and local model providers. It supports OpenAI-compatible APIs, Anthropic, Gemini, DeepSeek, Ollama, MCP, local backends, slash commands, tools, agents, and a React/Ink terminal UI.
The installed CLI runs on Node.js
>=22.0.0. Bun is used for source builds, scripts, dependency management, and tests.Work Style
- Keep changes focused on one problem.
- Prefer existing patterns in the file or nearby module.
- Avoid unrelated formatting, renames, dependency changes, or broad rewrites.
- Add or update tests when behavior changes.
- Update docs when setup, commands, provider behavior, or user-facing behavior changes.
- For new features, larger refactors, dependencies, or runtime changes, follow the issue-first guidance in CONTRIBUTING.md.
Stack And Conventions
- TypeScript with strict mode and ESM imports.
- React + Ink for terminal UI.
- Bun lockfile and Bun scripts for development workflows.
- Node runtime for the built CLI.
Common libraries and patterns:
chalkfor terminal color.commanderfor CLI argument parsing.execafor child processes.- Existing service, provider, settings, permission, and UI patterns over new abstractions.
Repository Map
src/commands/- slash and CLI command implementations.src/components/- React/Ink UI components.src/services/- API, MCP, OAuth, wiki, voice, and other service integrations.src/tools/- tool implementations.src/utils/- shared utilities.src/integrations/- provider and model integration metadata.src/entrypoints/- CLI, MCP, SDK, and generated public types.src/tasks/- local, remote, workflow, and monitor tas...
Files:
src/services/api/claude.abortClassification.test.tssrc/utils/queryLifecycle.test.tssrc/utils/abortReasons.test.tssrc/utils/queryLifecycle.tssrc/query.abortClassification.test.tssrc/services/api/claude.tssrc/utils/abortReasons.tssrc/screens/REPL.tsx
**/*
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
src/services/api/claude.abortClassification.test.tssrc/utils/queryLifecycle.test.tssrc/utils/abortReasons.test.tssrc/utils/queryLifecycle.tssrc/query.abortClassification.test.tssrc/services/api/claude.tssrc/utils/abortReasons.tssrc/screens/REPL.tsx
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}
⚙️ CodeRabbit configuration file
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}: Review provider routing, model selection, env precedence, auth/token handling, OpenAI-compatible shims, retries, proxy behavior, and outbound HTTP behavior with high scrutiny. Block on silent default changes, hidden fallback expansion, credential reuse mistakes, hardcoded provider assumptions, or new network reach that is not intentional and documented.
Files:
src/services/api/claude.abortClassification.test.tssrc/services/api/claude.ts
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
src/services/api/claude.abortClassification.test.tssrc/utils/queryLifecycle.test.tssrc/utils/abortReasons.test.tssrc/query.abortClassification.test.ts
🔇 Additional comments (9)
src/services/api/claude.ts (1)
60-63: LGTM!Also applies to: 808-814, 2636-2645, 2668-2668
src/services/api/claude.abortClassification.test.ts (1)
1-58: LGTM! The previously flagged grep-based test concern is resolved — these tests now exercise realgetClaudeStreamingAbortLogMessage/shouldCreateUserInterruptionMessagebehavior against actual reason values and a realAbortController, rather than asserting on source text.src/screens/REPL.tsx (1)
40-40: LGTM!src/utils/abortReasons.ts (2)
42-56: LegacyAbortErrorcase now handled — resolves prior gap.
normalizeAbortReasonnow maps a bareAbortController.abort()(name === 'AbortError') to'user-abort', andshouldCreateUserInterruptionMessagecorrectly derives from it. This fixes the previously flagged mismatch wheregetMissingToolResultAbortMessage/getStreamingAbortMessagefell through to'unknown-abort'for this path.Also applies to: 111-113
62-153: LGTM!src/utils/abortReasons.test.ts (1)
31-46: Test coverage gap from prior review now closed.Added assertions for
getMissingToolResultAbortMessage/getStreamingAbortMessageagainst the legacy bare-abort() signal, matching what was requested previously.Also applies to: 95-146
src/query.abortClassification.test.ts (1)
1-260: Behavioral rewrite resolves prior "source-text pattern matching" concern.Tests now drive the real
query()generator and assert on yielded tool-result/system/interruption messages, mirroringstopHooks.goal.test.ts. This is a solid improvement over the previous string-matching approach.src/utils/queryLifecycle.test.ts (1)
32-96: LGTM!src/utils/queryLifecycle.ts (1)
1-2: LGTM!Also applies to: 71-91
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
Thanks for the contribution. I do not see any actionable issues from my review.
@kevincodex1 LGTM
Configure hard max query duration via OPENCLAUDE_QUERY_HARD_MAX_MS env var, classify abort signals (AbortError/TimeoutError) into specific reasons, and wire abort-reason system messages into stop hooks. Co-Authored-By: Claude <noreply@anthropic.com>
Configure hard max query duration via OPENCLAUDE_QUERY_HARD_MAX_MS env var, classify abort signals (AbortError/TimeoutError) into specific reasons, and wire abort-reason system messages into stop hooks. Co-Authored-By: Claude <noreply@anthropic.com>
3 participants
Summary
OPENCLAUDE_QUERY_HARD_MAX_MSso long foreground query sessions can raise the QueryGuard hard maximum without editing source.Impact
Testing
bun run buildbun run smokebun run check(not run; focused tests, typecheck, smoke/build, and PR intent scan were run instead)bun test src/utils/queryGuardConfig.test.ts src/screens/REPL.queryLifecycle.test.ts src/utils/QueryGuard.test.ts src/utils/abortReasons.test.ts src/query.abortClassification.test.ts src/query/stopHooks.goal.test.ts src/query/toolFailureLoopGuard.test.ts src/services/api/claude.abortClassification.test.ts src/services/api/claude.lifecycle.test.ts src/services/api/claude.streamWatchdog.test.ts src/services/tools/StreamingToolExecutor.test.ts src/services/tools/toolExecution.test.tsenv ZDOTDIR=/tmp/openclaude-empty-zdotdir bun test src/services/tools/queryActivityLease.test.ts src/tools/BashTool/BashTool.errorOutput.test.ts src/utils/ShellCommand.test.ts src/utils/queryLifecycle.test.tsbun run typecheckbun run security:pr-scangit diff --checkNotes
Summary by CodeRabbit
OPENCLAUDE_QUERY_HARD_MAX_MS.OPENCLAUDE_QUERY_HARD_MAX_MS, including defaults and invalid-value handling.