feat(query): add lifecycle identity and terminal reasons - #1682
kevincodex1 merged 10 commits into
Conversation
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughIntroduces a query lifecycle tracking system. A new ChangesQuery Lifecycle Tracking
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 3
🤖 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/screens/REPL.tsx`:
- Line 1485: The single queryLifecycleTrackerRef tracker is being injected into
all ToolUseContext instances around line 2593, but it should be scoped only to
the specific foreground query that owns it. Currently, multiple execution
contexts (background sessions, immediate commands, permission UI, agent resume,
compaction) share the same tracker, causing their operations to interfere when
the tracker is cleared at line 3065. Remove the tracker injection from the
general ToolUseContext creation and instead pass
queryLifecycleTrackerRef.current only in the specific code path where the
guarded foreground query context is constructed in onQueryImpl that belongs to
startResult.context. This ensures each query type has proper isolation of its
lifecycle operations.
- Around line 1771-1776: The warning log for orphaned API calls in the condition
checking timeout.activeOperations.apiCalls.length is premature because it fires
before the abort cleanup has actually run, causing normal timeout/cancel cases
to be incorrectly flagged as warnings. Instead of logging this as a 'warn' level
message about orphaned calls, change it to log at a non-warning level (such as
debug or info) with a diagnostic message that reflects these are active calls
during abort rather than truly orphaned calls, or defer this orphan detection
logic until after the query cleanup has actually completed.
In `@src/utils/QueryGuard.test.ts`:
- Around line 301-323: The test for snapshot security is redundant because the
string-based assertions on lines 320-322 check for absence of values that were
never provided to the startToolUse method call, so they don't actually validate
filtering behavior. Either remove the expect statements checking for '/home/',
'ANTHROPIC_API_KEY', and 'tool output' in the JSON stringified snapshot (since
the exact-keys assertion already validates the snapshot structure), or
restructure the test to pass sensitive data (like file paths or environment
variables) within the startToolUse parameters and verify they are actually
filtered out from the snapshot.
🪄 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 Plus
Run ID: fbe5aef3-fa90-43fa-8a47-1a783c61e5ca
📒 Files selected for processing (8)
src/Tool.tssrc/query.tssrc/screens/REPL.tsxsrc/services/api/claude.tssrc/services/tools/toolExecution.tssrc/utils/QueryGuard.test.tssrc/utils/QueryGuard.tssrc/utils/queryLifecycle.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx,js,jsx,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Keep comments useful and concise in code
Files:
src/query.tssrc/utils/queryLifecycle.tssrc/utils/QueryGuard.test.tssrc/services/api/claude.tssrc/Tool.tssrc/services/tools/toolExecution.tssrc/utils/QueryGuard.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/query.tssrc/utils/queryLifecycle.tssrc/utils/QueryGuard.test.tssrc/services/api/claude.tssrc/Tool.tssrc/services/tools/toolExecution.tssrc/utils/QueryGuard.tssrc/screens/REPL.tsx
**
⚙️ CodeRabbit configuration file
**: # Contributing to OpenClaudeThanks for contributing.
OpenClaude is a fast-moving open-source coding-agent CLI with support for multiple providers, local backends, MCP, and a terminal-first workflow. The best contributions here are focused, well-tested, and easy to review.
Before You Start
- Search existing issues and discussions before opening a new thread.
- Check open pull requests for work that overlaps with your contribution. If a PR already exists that addresses the same change, open an issue or discussion first to align on direction — duplicate PRs may be closed without review.
- Use issues for confirmed bugs and actionable feature work.
- Use discussions for setup help, ideas, and general community conversation.
- For larger changes, open an issue first so the scope is clear before implementation.
- For security reports, follow SECURITY.md.
Pull Requests
Every PR needs a reason. Your PR description must include:
- what changed and why
- the user or developer impact
- the exact checks you ran
- a linked issue when one exists, using
Fixes#123, `Closes `#123, or another clear link- screenshots when the PR touches UI, terminal presentation, or the VS Code extension
- which provider path was tested when the PR changes provider behavior
The PR author is responsible for ensuring their PR is merge-ready. PRs with merge conflicts will not be reviewed or approved until the conflicts are resolved.
Issues are the recommended starting point for anything non-trivial — opening one first helps avoid wasted effort if the change is out of scope or already being worked on. Small fixes, doc corrections, and obvious improvements can stand on their own without a linked issue, as long as the PR description explains the intent.
What Gets Closed Without Review
PRs may be closed without review...
Files:
src/query.tssrc/utils/queryLifecycle.tssrc/utils/QueryGuard.test.tssrc/services/api/claude.tssrc/Tool.tssrc/services/tools/toolExecution.tssrc/utils/QueryGuard.tssrc/screens/REPL.tsx
{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/utils/QueryGuard.test.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.ts
🔇 Additional comments (16)
src/utils/queryLifecycle.ts (2)
1-65: LGTM!
66-106: LGTM!src/utils/QueryGuard.ts (6)
32-47: LGTM!
55-58: LGTM!
87-105: LGTM!
112-144: LGTM!
158-164: LGTM!Also applies to: 195-242
248-270: LGTM!src/utils/QueryGuard.test.ts (3)
73-82: LGTM!
86-235: LGTM!
257-299: LGTM!src/Tool.ts (1)
63-63: LGTM!Also applies to: 284-284
src/query.ts (1)
903-905: LGTM!src/services/api/claude.ts (1)
173-173: LGTM!Also applies to: 720-720, 1542-1542, 1855-1865, 1884-1888, 2863-2866
src/services/tools/toolExecution.ts (2)
1226-1236: LGTM!
1718-1718: LGTM!
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/utils/queryLifecycle.ts`:
- Around line 97-101: The updateApiCall method merges the update parameter
directly into the current object, which can propagate undefined values for
optional fields like startedAt, violating the type contract. Before merging
update with current on line 100, filter out any properties with undefined values
from the update object to ensure that only explicit, defined updates are
applied. This ensures that optional fields with number types like startedAt
cannot be accidentally overwritten with undefined during the spread operation.
- Around line 66-84: Add comment documentation above both toSafeApiCallSnapshot
and toSafeToolUseSnapshot functions to explain the purpose of the whitelisting
pattern. Document that these functions prevent accidental property leakage by
deliberately rebuilding the objects with only explicitly whitelisted properties
rather than spreading all properties from the input object, which protects
against unexpected properties being added at runtime.
🪄 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 Plus
Run ID: 308f8444-3d98-4038-bbd0-64e075411bf0
📒 Files selected for processing (3)
src/screens/REPL.tsxsrc/utils/QueryGuard.test.tssrc/utils/queryLifecycle.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (10)
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript with strict mode and ESM imports
Files:
src/utils/queryLifecycle.tssrc/utils/QueryGuard.test.tssrc/screens/REPL.tsx
{src/services/**/*.ts,src/utils/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
execafor child processes
Files:
src/utils/queryLifecycle.tssrc/utils/QueryGuard.test.ts
**/*.{ts,tsx,js,jsx,py,json,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow the existing code style in the touched files
Files:
src/utils/queryLifecycle.tssrc/utils/QueryGuard.test.tssrc/screens/REPL.tsx
**/*.{ts,tsx,js,jsx,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Keep comments useful and concise
Files:
src/utils/queryLifecycle.tssrc/utils/QueryGuard.test.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/utils/queryLifecycle.tssrc/utils/QueryGuard.test.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/utils/queryLifecycle.tssrc/utils/QueryGuard.test.tssrc/screens/REPL.tsx
**
⚙️ CodeRabbit configuration file
**: # Contributing to OpenClaudeThanks for contributing.
OpenClaude is a fast-moving open-source coding-agent CLI with support for multiple providers, local backends, MCP, and a terminal-first workflow. The best contributions here are focused, well-tested, and easy to review.
Before You Start
- Search existing issues and discussions before opening a new thread.
- Check open pull requests for work that overlaps with your contribution. If a PR already exists that addresses the same change, open an issue or discussion first to align on direction — duplicate PRs may be closed without review.
- Use issues for confirmed bugs and actionable feature work.
- Use discussions for setup help, ideas, and general community conversation.
- For larger changes, open an issue first so the scope is clear before implementation.
- For security reports, follow SECURITY.md.
Pull Requests
Every PR needs a reason. Your PR description must include:
- what changed and why
- the user or developer impact
- the exact checks you ran
- a linked issue when one exists, using
Fixes#123, `Closes `#123, or another clear link- screenshots when the PR touches UI, terminal presentation, or the VS Code extension
- which provider path was tested when the PR changes provider behavior
The PR author is responsible for ensuring their PR is merge-ready. PRs with merge conflicts will not be reviewed or approved until the conflicts are resolved.
Issues are the recommended starting point for anything non-trivial — opening one first helps avoid wasted effort if the change is out of scope or already being worked on. Small fixes, doc corrections, and obvious improvements can stand on their own without a linked issue, as long as the PR description explains the intent.
What Gets Closed Without Review
PRs may be closed without review...
Files:
src/utils/queryLifecycle.tssrc/utils/QueryGuard.test.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/utils/QueryGuard.test.ts
**/*.test.{ts,tsx,js}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Test the exact provider/model path you changed when possible
Files:
src/utils/QueryGuard.test.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/utils/QueryGuard.test.ts
🔇 Additional comments (6)
src/utils/queryLifecycle.ts (1)
1-64: LGTM!src/screens/REPL.tsx (3)
1485-1485: LGTM!
1771-1773: LGTM!Also applies to: 2325-2327
2568-2589: LGTM!Also applies to: 2852-2938, 3104-3104
src/utils/QueryGuard.test.ts (2)
86-108: LGTM!Also applies to: 110-171, 203-215, 217-235, 256-299
301-346: Snapshot safety test now properly validates filtering behavior.The updated test addresses the prior concern by passing sensitive data (
prompt,apiKey,cwd,command,output) through type assertions (lines 303-323), then verifying those fields are excluded from snapshots via exact equality (lines 327-335), exact keys (lines 336-342), and string absence checks (lines 343-345). The type assertions intentionally bypass TypeScript's excess-property checking to simulate runtime scenarios where extra fields might be present, making the filtering verification meaningful.
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
I found one issue that needs to be addressed before this is ready.
Findings
- [P3] Track tool uses before hook and permission waits
src/services/tools/toolExecution.ts:876
The lifecycle tracker only callsstartToolUse()after schema validation,validateInput, allPreToolUsehooks, andresolveHookPermissionDecision()finish. If the query watchdog fires while a hook, classifier-backed permission decision, or interactive permission prompt is the thing blocking the turn, the timeout snapshot still reportsactiveToolUses=0, so the new lifecycle debug output loses the specific tool id/name it was added to correlate. Please start the safe tool-use tracking before those awaited pre-execution phases and end it on the existing early-return paths as well as the final tool execution path.
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/services/tools/toolExecution.test.ts`:
- Around line 116-156: Add a new focused test case for Bash tool lifecycle
tracking. Create a test that uses BASH_TOOL_NAME instead of 'LifecycleTestTool'
when calling createToolFixture, and follow the same pattern as the existing test
that validates the snapshot during async input validation. In the new test, add
assertions to verify that the in-flight snapshot includes isBash: true and
timeoutMs properties in the captured toolUses array, ensuring the Bash-specific
lifecycle branch is exercised and properly tested.
🪄 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 Plus
Run ID: 53802fad-25d2-4ac6-9908-7fcd732c6809
📒 Files selected for processing (2)
src/services/tools/toolExecution.test.tssrc/services/tools/toolExecution.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (12)
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript with strict mode and ESM imports
Files:
src/services/tools/toolExecution.test.tssrc/services/tools/toolExecution.ts
{src/commands/**/*.ts,src/services/**/*.ts,src/entrypoints/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
chalkfor terminal color in CLI code
Files:
src/services/tools/toolExecution.test.tssrc/services/tools/toolExecution.ts
{src/services/**/*.ts,src/utils/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
execafor child processes
Files:
src/services/tools/toolExecution.test.tssrc/services/tools/toolExecution.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/tools/toolExecution.test.tssrc/services/tools/toolExecution.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/tools/toolExecution.test.tssrc/services/tools/toolExecution.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add or update tests when the change affects behavior
Files:
src/services/tools/toolExecution.test.ts
**/*.test.{ts,tsx,js}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Test the exact provider/model path you changed when possible
Files:
src/services/tools/toolExecution.test.ts
**/*.{ts,tsx,js,jsx,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Keep comments useful and concise
Files:
src/services/tools/toolExecution.test.tssrc/services/tools/toolExecution.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow TypeScript strict mode and type safety practices by running typecheck before submitting
Files:
src/services/tools/toolExecution.test.tssrc/services/tools/toolExecution.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/services/tools/toolExecution.test.tssrc/services/tools/toolExecution.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/tools/toolExecution.test.ts
**
⚙️ CodeRabbit configuration file
**: # Contributing to OpenClaudeThanks for contributing.
OpenClaude is a fast-moving open-source coding-agent CLI with support for multiple providers, local backends, MCP, and a terminal-first workflow. The best contributions here are focused, well-tested, and easy to review.
Before You Start
- Search existing issues and discussions before opening a new thread.
- Check open pull requests for work that overlaps with your contribution. If a PR already exists that addresses the same change, open an issue or discussion first to align on direction — duplicate PRs may be closed without review.
- Use issues for confirmed bugs and actionable feature work.
- Use discussions for setup help, ideas, and general community conversation.
- For larger changes, open an issue first so the scope is clear before implementation.
- For security reports, follow SECURITY.md.
Pull Requests
Every PR needs a reason. Your PR description must include:
- what changed and why
- the user or developer impact
- the exact checks you ran
- a linked issue when one exists, using
Fixes#123, `Closes `#123, or another clear link- screenshots when the PR touches UI, terminal presentation, or the VS Code extension
- which provider path was tested when the PR changes provider behavior
The PR author is responsible for ensuring their PR is merge-ready. PRs with merge conflicts will not be reviewed or approved until the conflicts are resolved.
Issues are the recommended starting point for anything non-trivial — opening one first helps avoid wasted effort if the change is out of scope or already being worked on. Small fixes, doc corrections, and obvious improvements can stand on their own without a linked issue, as long as the PR description explains the intent.
What Gets Closed Without Review
PRs may be closed without review...
Files:
src/services/tools/toolExecution.test.tssrc/services/tools/toolExecution.ts
🔇 Additional comments (1)
src/services/tools/toolExecution.ts (1)
758-775: LGTM!Also applies to: 1731-1733
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
Thanks for the update. I rechecked the previously discussed paths and found a couple of remaining issues.
Findings
-
[P3] Track API lifecycle per attempt/fallback request
activeApiCallKeyis started immediately before streaming.withResponse(), but it is only ended when the next streaming attempt begins or whenqueryModel()exits, andexecuteNonStreamingRequest()never starts its own lifecycle entry. That means two of the slow paths this PR is trying to diagnose report the wrong active operation: ifwithRetry()catches a retryable stream-creation error and sleeps forRetry-After/persistent retry, the failed request remains active through the backoff even though there is no network call; if streaming falls back to non-streaming, the snapshot either keeps showing the stale streaming request or shows no active non-streaming request at all. A query watchdog or user cancel during those windows will makeapi.call.active_on_abortpoint at the wrong phase of the hang. Please scope API lifecycle entries to each concrete request attempt, ending failed attempts before retry/backoff handling and starting/ending entries around each non-streaming fallback attempt as well. -
[P3] Refresh Bash lifecycle metadata after hook/permission input rewrites
The tool lifecycle entry is started from
parsedInput.databeforerunPreToolUseHooks()andresolveHookPermissionDecision()can replace the input that will actually be executed. Those existing paths supportupdatedInput: passthrough hooks assignprocessedInput = result.updatedInput, hookallow/askdecisions passhookPermissionResult.updatedInput, and PermissionRequest/SDK permission flows can also return updated input. For Bash, the new active-operation snapshot storestimeoutMsonly once here, so a query hard-timeout or abort diagnostic can report the model-provided timeout even though a hook or permission handler raised/lowered the timeout for the command that is now waiting. That makes the new lifecycle debugging misleading exactly in the permission/hook wait and execution paths this PR is trying to illuminate. Please update the tracked tool-use metadata after the finalprocessedInputis chosen, or delay the Bash timeout snapshot until after hook/permission resolution while still starting the operation early enough to cover the waits.
1baf29d to
24ac83a
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/services/api/claude.ts (1)
2733-2750:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve provider routing and lifecycle source in the 404 fallback.
Line 2735 drops
options.providerOverride, so provider-override requests that hit the 404 streaming fallback can retry non-streaming against the default client. The same call also omitsquerySource, so the new lifecycle snapshot records this fallback API call without its source.Proposed fix
// Fall back to non-streaming mode endActiveApiCall() const result = yield* executeNonStreamingRequest( - { model: options.model, source: options.querySource, effortValue: effort }, + { + model: options.model, + source: options.querySource, + providerOverride: options.providerOverride, + effortValue: effort, + }, { model: options.model, fallbackModel: options.fallbackModel, thinkingConfig, ...(isFastModeEnabled() && { fastMode: isFastMode }), signal, + querySource: options.querySource, },As per coding guidelines, “Review provider routing, model selection, env precedence, auth/token handling, OpenAI-compatible shims, retries, proxy behavior, and outbound HTTP behavior with high scrutiny.”
🤖 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/services/api/claude.ts` around lines 2733 - 2750, The second parameter object passed to executeNonStreamingRequest is missing two critical properties that preserve the API routing context. Add providerOverride: options.providerOverride to maintain provider-override routing when the 404 streaming fallback retries with non-streaming, and add querySource: options.querySource to ensure the lifecycle snapshot captures the source of this fallback API call. Update the options object in the executeNonStreamingRequest call (the one containing model, fallbackModel, thinkingConfig, and signal) to include both providerOverride and querySource properties from the options parameter.Source: Coding guidelines
🤖 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/services/api/claude.lifecycle.test.ts`:
- Around line 54-81: The importFreshClaudeModule function uses global module
mocks via mock.module() which leak between concurrent test files and interfere
with src/services/api/client.test.ts. Replace the global mocking approach for
./client.js and ../vcr.js with local injection seams such as fetchOverride or
dependency injection parameters that only affect the current test scope. This
will isolate the test assertions so that mocks in claude.lifecycle.test.ts
cannot affect concurrent test files, preventing the smoke-and-tests check
failures caused by the fallback failed stub being invoked across multiple
unrelated client tests.
---
Outside diff comments:
In `@src/services/api/claude.ts`:
- Around line 2733-2750: The second parameter object passed to
executeNonStreamingRequest is missing two critical properties that preserve the
API routing context. Add providerOverride: options.providerOverride to maintain
provider-override routing when the 404 streaming fallback retries with
non-streaming, and add querySource: options.querySource to ensure the lifecycle
snapshot captures the source of this fallback API call. Update the options
object in the executeNonStreamingRequest call (the one containing model,
fallbackModel, thinkingConfig, and signal) to include both providerOverride and
querySource properties from the options parameter.
🪄 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 Plus
Run ID: e6060392-3c7a-4d6b-b3d1-a501f5ea856e
📒 Files selected for processing (4)
src/services/api/claude.lifecycle.test.tssrc/services/api/claude.tssrc/services/tools/toolExecution.test.tssrc/services/tools/toolExecution.ts
📜 Review details
🧰 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/tools/toolExecution.test.tssrc/services/api/claude.lifecycle.test.tssrc/services/tools/toolExecution.tssrc/services/api/claude.ts
{src/commands/**/*.ts,src/services/**/*.ts,src/entrypoints/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
chalkfor terminal color in CLI code
Files:
src/services/tools/toolExecution.test.tssrc/services/api/claude.lifecycle.test.tssrc/services/tools/toolExecution.tssrc/services/api/claude.ts
{src/services/**/*.ts,src/utils/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
execafor child processes
Files:
src/services/tools/toolExecution.test.tssrc/services/api/claude.lifecycle.test.tssrc/services/tools/toolExecution.tssrc/services/api/claude.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/tools/toolExecution.test.tssrc/services/api/claude.lifecycle.test.tssrc/services/tools/toolExecution.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/tools/toolExecution.test.tssrc/services/api/claude.lifecycle.test.tssrc/services/tools/toolExecution.tssrc/services/api/claude.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add or update tests when the change affects behavior
Files:
src/services/tools/toolExecution.test.tssrc/services/api/claude.lifecycle.test.ts
**/*.test.{ts,tsx,js}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Test the exact provider/model path you changed when possible
Files:
src/services/tools/toolExecution.test.tssrc/services/api/claude.lifecycle.test.ts
**/*.{ts,tsx,js,jsx,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Keep comments useful and concise
Files:
src/services/tools/toolExecution.test.tssrc/services/api/claude.lifecycle.test.tssrc/services/tools/toolExecution.tssrc/services/api/claude.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow TypeScript strict mode and type safety practices by running typecheck before submitting
Files:
src/services/tools/toolExecution.test.tssrc/services/api/claude.lifecycle.test.tssrc/services/tools/toolExecution.tssrc/services/api/claude.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/services/tools/toolExecution.test.tssrc/services/api/claude.lifecycle.test.tssrc/services/tools/toolExecution.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/tools/toolExecution.test.tssrc/services/api/claude.lifecycle.test.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.
- Python exists for legacy/local-provider helper code. Do not add new Python code or expand Python-based features unless a maintainer explicitly approves that direction.
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/integration...
Files:
src/services/tools/toolExecution.test.tssrc/services/api/claude.lifecycle.test.tssrc/services/tools/toolExecution.tssrc/services/api/claude.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.lifecycle.test.tssrc/services/api/claude.ts
🪛 GitHub Check: smoke-and-tests
src/services/api/claude.lifecycle.test.ts
[failure] 26-26: error: 400 fallback failed
at makeApiError (/home/runner/work/openclaude/openclaude/src/services/api/claude.lifecycle.test.ts:26:10)
at <anonymous> (/home/runner/work/openclaude/openclaude/src/services/api/claude.lifecycle.test.ts:243:15)
at <anonymous> (/home/runner/work/openclaude/openclaude/src/services/api/client.test.ts:744:30)
[failure] 26-26: error: 400 fallback failed
at makeApiError (/home/runner/work/openclaude/openclaude/src/services/api/claude.lifecycle.test.ts:26:10)
at <anonymous> (/home/runner/work/openclaude/openclaude/src/services/api/claude.lifecycle.test.ts:243:15)
at <anonymous> (/home/runner/work/openclaude/openclaude/src/services/api/client.test.ts:687:30)
[failure] 26-26: error: 400 fallback failed
at makeApiError (/home/runner/work/openclaude/openclaude/src/services/api/claude.lifecycle.test.ts:26:10)
at <anonymous> (/home/runner/work/openclaude/openclaude/src/services/api/claude.lifecycle.test.ts:243:15)
at <anonymous> (/home/runner/work/openclaude/openclaude/src/services/api/client.test.ts:606:47)
[failure] 26-26: error: 400 fallback failed
at makeApiError (/home/runner/work/openclaude/openclaude/src/services/api/claude.lifecycle.test.ts:26:10)
at <anonymous> (/home/runner/work/openclaude/openclaude/src/services/api/claude.lifecycle.test.ts:243:15)
at <anonymous> (/home/runner/work/openclaude/openclaude/src/services/api/client.test.ts:472:30)
[failure] 26-26: error: 400 fallback failed
at makeApiError (/home/runner/work/openclaude/openclaude/src/services/api/claude.lifecycle.test.ts:26:10)
at <anonymous> (/home/runner/work/openclaude/openclaude/src/services/api/claude.lifecycle.test.ts:243:15)
at <anonymous> (/home/runner/work/openclaude/openclaude/src/services/api/client.test.ts:420:30)
[failure] 26-26: error: 400 fallback failed
at makeApiError (/home/runner/work/openclaude/openclaude/src/services/api/claude.lifecycle.test.ts:26:10)
at <anonymous> (/home/runner/work/openclaude/openclaude/src/services/api/claude.lifecycle.test.ts:243:15)
at <anonymous> (/home/runner/work/openclaude/openclaude/src/services/api/client.test.ts:364:47)
[failure] 26-26: error: 400 fallback failed
at makeApiError (/home/runner/work/openclaude/openclaude/src/services/api/claude.lifecycle.test.ts:26:10)
at <anonymous> (/home/runner/work/openclaude/openclaude/src/services/api/claude.lifecycle.test.ts:243:15)
at <anonymous> (/home/runner/work/openclaude/openclaude/src/services/api/client.test.ts:298:47)
🔇 Additional comments (3)
src/services/api/claude.ts (1)
688-721: LGTM!Also applies to: 832-924, 1557-1569, 1880-1915, 2632-2651, 2891-2893
src/services/tools/toolExecution.ts (1)
758-784: LGTM!Also applies to: 938-943, 1019-1030, 1210-1215, 1744-1745
src/services/tools/toolExecution.test.ts (1)
256-312: LGTM!
24ac83a to
0d2019c
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/services/api/claude.lifecycle.test.ts (2)
99-106: ⚡ Quick winConsider adding error handling to prevent test hangs.
The
drainGeneratorhelper uses an infinite loop without error handling. If the generator throws or never completes, the test could hang indefinitely. Consider wrapping the await in a try-catch to provide better error messages.♻️ Proposed enhancement
async function drainGenerator<T>( generator: AsyncGenerator<unknown, T>, ): Promise<T> { while (true) { - const result = await generator.next() + try { + const result = await generator.next() - if (result.done) return result.value + if (result.done) return result.value + } catch (error) { + // Clean up generator on error + await generator.return(undefined).catch(() => {}) + throw error + } } }🤖 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/services/api/claude.lifecycle.test.ts` around lines 99 - 106, The drainGenerator function lacks error handling and could cause test hangs if the generator throws an error or fails to complete. Wrap the await generator.next() call inside a try-catch block in the drainGenerator function to catch and handle any errors thrown by the generator, ensuring the test fails with a clear error message rather than hanging indefinitely.
108-114: 💤 Low valueType assertion in test helper could hide incompleteness.
The
makeParamsfunction uses a type assertion on line 113 that forces the minimal test object intoBetaMessageStreamParams. While acceptable for test fixtures, this may hide missing required fields if the type definition changes.🤖 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/services/api/claude.lifecycle.test.ts` around lines 108 - 114, Remove the type assertion on the return object in the makeParams function and instead explicitly define all required fields from the BetaMessageStreamParams type definition. Check what fields are required by BetaMessageStreamParams and add them to the returned object rather than forcing it with the as keyword. This ensures that if BetaMessageStreamParams changes in the future, the test helper will catch any missing required fields at compile time rather than hiding them with the assertion.
🤖 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/services/api/claude.lifecycle.test.ts`:
- Around line 227-265: The double type cast on makeBetaMessage() as unknown as
Record<string, unknown> when calling makeJsonResponse indicates a type mismatch
and hides potential type errors. Modify the makeJsonResponse function signature
to accept unknown as its parameter type instead of Record<string, unknown>, then
update the call in the test to pass makeBetaMessage() directly without any type
assertions. Since JSON.stringify can handle any type, this change will allow the
function to be more flexible while removing the unnecessary intermediate cast.
---
Nitpick comments:
In `@src/services/api/claude.lifecycle.test.ts`:
- Around line 99-106: The drainGenerator function lacks error handling and could
cause test hangs if the generator throws an error or fails to complete. Wrap the
await generator.next() call inside a try-catch block in the drainGenerator
function to catch and handle any errors thrown by the generator, ensuring the
test fails with a clear error message rather than hanging indefinitely.
- Around line 108-114: Remove the type assertion on the return object in the
makeParams function and instead explicitly define all required fields from the
BetaMessageStreamParams type definition. Check what fields are required by
BetaMessageStreamParams and add them to the returned object rather than forcing
it with the as keyword. This ensures that if BetaMessageStreamParams changes in
the future, the test helper will catch any missing required fields at compile
time rather than hiding them with the assertion.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 191d9f0e-2ca6-4fab-b407-e3ae10b1f815
📒 Files selected for processing (4)
src/services/api/claude.lifecycle.test.tssrc/services/api/claude.tssrc/services/tools/toolExecution.test.tssrc/services/tools/toolExecution.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/services/tools/toolExecution.test.ts
- src/services/tools/toolExecution.ts
- src/services/api/claude.ts
0d2019c to
b5288a5
Compare
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
|
kindly rebase and fix confclits |
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
Thanks for the update. I took another skeptical pass over the changed paths, prior reviewer threads, and neighboring query/tool/subagent flows. I still found one issue that needs to be addressed:
Findings
- [P2] Preserve lifecycle tracking for foreground subagent calls
src/utils/forkedAgent.ts:381
The foreground REPL now only attachesqueryLifecyclewhenonQueryImplbuilds the guarded context, andquery.tsonly forwardstoolUseContext.queryLifecycleintocallModel. However, when an Agent tool launches a synchronous foreground subagent,createSubagentContext()rebuilds the child context without copying that field, so the child query loop runs its API calls and nested tool uses with no lifecycle tracker or child identity. The parent Agent tool also does not get a QueryGuard lease becausecreateToolQueryLeaseInput()only leases Bash/PowerShell tools, so a timeout/cancel during a hung subagent API request reports no active child API call even though this PR is adding lifecycle identity and active-operation snapshots for exactly that debugging path. Please carry an appropriate lifecycle tracker into foreground child contexts, or create an explicit child tracker/parent linkage so subagent API/tool activity is visible in timeout and cancel diagnostics without reintroducing tracking for unrelated background contexts.
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
Findings
-
[P2] Emit timeout
query.endonly after abort cleanup is acknowledged
src/screens/REPL.tsx:1770
On the timeout path,abortTimedOutQuery()logsquery.endimmediately after callingabort('query-timeout'), before the queuedabort_acknowledgedcallback runs and beforeQueryGuard._handleTimeout()reaches itsfinallyblock and callsforceEnd(). That means the new lifecycle log can report anendevent whileactiveApiCalls/activeToolUsesare still nonzero and before the abort has actually unwound, producing an event order liketimeout -> abort_requested -> end -> abort_acknowledged. For the lifecycle diagnostics this PR adds,endshould be the terminal/cleanup point, not a pre-cleanup snapshot. Please move the timeoutquery.endemission to the acknowledged/post-force-end side, or use a different event name for the pre-cleanup active-operation snapshot so parsers and humans do not treat the timed-out query as completed before cleanup has run. -
[P2] Clear discarded streaming-tool lifecycle entries before fallback continues
src/query.ts:954
When streaming fallback occurs after streaming tool execution has already started a tool, this path discards the oldStreamingToolExecutorand immediately creates a replacement with the sametoolUseContext.queryLifecycle. HoweverStreamingToolExecutor.discard()only setsdiscarded = true; it does not abort in-flightrunToolUse()calls or end theirqueryLifecycle.startToolUse()entries. Those abandoned tools keep appearing in the shared tracker until the underlying tool eventually exits, so the fallback request's timeout/cancel snapshots can include active tool uses from the discarded streaming attempt, and long-running Bash tools can make the new lifecycle diagnostics point at the wrong work. Please either end/clear lifecycle entries for discarded tools or abort the discarded executor's in-flight tools before continuing with the fallback executor.
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
Thanks for the update. I found one issue that still needs to be addressed.
Findings
- [P2] Complete the timeout cleanup request before emitting
query.end
src/screens/REPL.tsx:1776
The latest timeout path queuesabort_acknowledgedandquery.endin a microtask immediately afterabort('query-timeout'), but that microtask is not tied to the model/tool cleanup path.QueryGuardinvokes the timeout handler beforeforceEnd(), and API lifecycle cleanup still happens later in the aborted query generator'sfinallyblock, so a timed-out request can still emitquery.endwhilequeryLifecycleTrackerRef.current.snapshot()contains active API calls or tool uses. Please complete the prior cleanup request by moving the terminalquery.endemission to the actual post-cleanup path, or log this microtask snapshot as a non-terminal pre-cleanup event instead.
|
Addressed the latest timeout cleanup finding in 72d88a5. What changed:
Verification:
Remote checks on the current head are green: CodeRabbit, smoke-and-tests, typecheck, and web. There are no unresolved current review threads. |
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
Thanks for the update. I rechecked the previously discussed paths and do not see any remaining actionable issues from my side.
@kevincodex1 LGTM
* feat(query): add lifecycle identity and terminal reasons * fix(query): isolate lifecycle tracking context * fix(query): guard lifecycle metadata updates * fix(query): track lifecycle during tool waits * test(query): cover bash lifecycle metadata * fix(query): scope lifecycle tracking to request attempts * fix(query): disambiguate lifecycle abort log reason * fix(query): preserve foreground subagent lifecycle tracking * fix(query): clean up timeout and fallback lifecycle events * fix(query): emit timeout end after cleanup
Port provider-agnostic timeout abort mechanism to OpenCC. The upstream `[codex]` tag is misleading — the change hooks QueryGuard's timeout to abort in-flight work, which OpenCC needs as substrate for the Twigpine#1682 lifecycle port. Codex-specific paths are excluded per provider policy. Upstream SHA: 174ebd5
Port activity-aware leases to OpenCC. Required substrate for the Twigpine#1682 lifecycle subsystem port (QueryGuard wiring depends on the lease / activity / timeout-handler surface added here). Upstream SHA: 23cfc24
…for Twigpine#1682 lifecycle port) # Conflicts: # src/utils/QueryGuard.test.ts # src/utils/QueryGuard.ts
Port the type module + tracker from Twigpine#1682 to OpenCC. Zero callers yet; later commits in this 4-part chain will wire it into QueryGuard, context types, and the streaming executor. Upstream SHA: 23bc49a (commit 1 of 4, types-only slice)
Port the lifecycle-aware delta from Twigpine#1682 on top of the modernized QueryGuard (substrate Twigpine#1686 already merged). Adds tryStart(metadata) / end(gen, terminalReason) / forceEnd(reason, abort) / activeContext / lastContext. Preserves setTimeoutHandler(generation, reason) API. Upstream SHA: 23bc49a (commit 2 of 4, QueryGuard slice) Prereq: 1e57836c (commit 1 of 4) + 5b05ced (substrate Twigpine#1686)
…f 4) Port the 5-file context-types delta from Twigpine#1682 to OpenCC. Adds queryLifecycle to ToolUseContext, instantiates the tracker in MainLoopContext, plumbs through forkedAgent / runAgent / REPL. Upstream SHA: 23bc49a (commit 3 of 4, context-types slice) Prereq: 73e2101 (commit 2 of 4) + 5b05ced (substrate Twigpine#1686) + 1e57836c (commit 1)
3 participants
Summary
Validation
bun run typecheckbun test src/utils/QueryGuard.test.tsbun test tests/sdk/query-methods.test.ts tests/sdk/query-lifecycle.test.ts tests/sdk/query-concurrency.test.ts tests/sdk/query-happy-path.test.ts src/queryEngine.goal.test.ts src/query/autoCompactCooldown.test.ts src/query/stopHooks.goal.test.ts src/query/goalContinuation.test.ts src/query/providerMaxTokensCapRetry.test.ts src/query/toolFailureLoopGuard.test.tsbun test src/services/tools/toolExecution.test.ts src/services/tools/toolHooks.test.tsgit diff --checkSummary by CodeRabbit
New Features
Refactor
Tests