Repository navigation
fix(repl): make local interactive max-turns configurable - #2086
Conversation
📝 WalkthroughWalkthroughThe change adds configurable local interactive turn limits, shared budgets across resumed queries, provider-dispatch ownership checks, asynchronous background-session handoff, notification restoration, and validation. ChangesInteractive REPL turn limits
Estimated code review effort: 5 (Critical) | ~90+ minutes Suggested labels: 🚥 Pre-merge checks | ✅ 5 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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: 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/screens/replMaxTurnsProp.test.ts`:
- Around line 125-142: Replace the source-text assertions in the tests around
sessionConfig and --max-turns help with behavior-focused coverage: invoke the
actual Commander setup to verify rendered help output, and exercise the local
REPL handoff with --max-turns to assert the received maxTurns value. Preserve
coverage for both local interactive and remote-backed help wording without
asserting implementation text.
🪄 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: 26a18260-ab4c-41cc-bae1-e84a0af978be
📒 Files selected for processing (5)
docs/advanced-setup.mdsrc/main.tsxsrc/screens/REPL.tsxsrc/screens/replMaxTurns.tssrc/screens/replMaxTurnsProp.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: smoke-and-tests (22)
- GitHub Check: smoke-and-tests (24.11.x)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
TypeScript code in this repository must use strict mode and ESM imports.
**/*.{ts,tsx}: Add or update tests when a TypeScript or TSX change affects behavior.
Run the relevant TypeScript validation checks for changed code, includingbun run typecheckand, when applicable,bun run typecheck:type-tests.
Files:
src/screens/replMaxTurnsProp.test.tssrc/screens/replMaxTurns.tssrc/main.tsxsrc/screens/REPL.tsx
**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*: Preserve existing repository patterns unless intentionally refactoring them.
Keep changes small, readable, and focused; avoid broad rewrites or unrelated cleanup.
Do not reformat unrelated files, and keep comments useful and concise.
Update documentation when setup, commands, or user-facing behavior changes.
Review AI-generated changes for correctness, style consistency, unnecessary noise, and adherence to project architecture before submitting them.
Provider changes must follow the documented integration patterns indocs/integrations/overview.mdand the focused guides underdocs/integrations/how-to/.
When changing provider behavior, avoid breaking third-party providers and test the exact provider/model path changed when possible.
Provider pull requests must explicitly identify affected providers, limitations, and follow-up work.
Run the narrowest meaningful validation command for the touched area, and ensure relevant CI checks pass before merging.
Usebun installto install dependencies and the repository's Bun scripts for building, testing, smoke testing, and development.
Dependency changes require a concrete project benefit such as a bug fix, security issue, or approved feature; preference alone is insufficient.
Do not change the project's language, core runtime, or dependency stack, or introduce a new runtime, without prior maintainer agreement.
Keep each pull request focused on one issue or clearly scoped improvement and avoid bundling unrelated fixes, features, or refactors.
Files:
src/screens/replMaxTurnsProp.test.tssrc/screens/replMaxTurns.tssrc/main.tsxsrc/screens/REPL.tsxdocs/advanced-setup.md
⚙️ 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/screens/replMaxTurnsProp.test.tssrc/screens/replMaxTurns.tssrc/main.tsxsrc/screens/REPL.tsxdocs/advanced-setup.md
{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/screens/replMaxTurnsProp.test.ts
{bin/**,scripts/**,package.json,src/setup.ts,src/main.tsx,src/entrypoints/**}
⚙️ CodeRabbit configuration file
{bin/**,scripts/**,package.json,src/setup.ts,src/main.tsx,src/entrypoints/**}: Review install, launcher, build, packaging, startup, and entrypoint changes for cross-platform compatibility, tracked-source rewrites, env/config precedence, and release safety. Block on changes that can break Windows/macOS/Linux startup or publish unexpected artifacts.
Files:
src/main.tsx
{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
🪛 ast-grep (0.45.0)
src/screens/replMaxTurnsProp.test.ts
[error] 51-53: Recursive/iterative merge copies attacker-controllable keys from a source object into a target via a computed property assignment without rejecting dangerous keys, allowing prototype pollution. Skip or block "proto", "constructor", and "prototype" keys (e.g. if (key === "__proto__" || key === "constructor" || key === "prototype") continue;), use a null-prototype object (Object.create(null)), or use a safe merge utility instead.
Context: for (const key of ENV_KEYS) {
savedEnv[key] = process.env[key]
}
Note: [CWE-1321] Improperly Controlled Modification of Object Prototype Attributes ('Prototype Pollution').
(prototype-pollution-recursive-merge-typescript)
🔇 Additional comments (5)
src/screens/replMaxTurns.ts (1)
3-16: LGTM!Also applies to: 18-48
src/main.tsx (1)
948-948: LGTM!Also applies to: 3052-3053
src/screens/REPL.tsx (1)
575-577: LGTM!docs/advanced-setup.md (1)
481-481: LGTM!src/screens/replMaxTurnsProp.test.ts (1)
1-19: LGTM!Also applies to: 35-109
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/utils/replMaxTurns.ts`:
- Around line 99-101: Update the invalid-value debug record in the replMaxTurns
parsing logic to remove the interpolated openClaudeRaw value. Keep logging the
OPENCLAUDE_MAX_TURNS variable name and DEFAULT_REPL_MAX_TURNS fallback, without
exposing the raw environment input.
🪄 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: e12ea626-1459-4342-a8c2-11ab65fdc0ff
📒 Files selected for processing (10)
docs/advanced-setup.mdsrc/components/Settings/Config.tsxsrc/main.tsxsrc/query.tssrc/query/agentStepLimit.test.tssrc/screens/REPL.tsxsrc/screens/replMaxTurns.tssrc/screens/replMaxTurnsProp.test.tssrc/utils/config.tssrc/utils/replMaxTurns.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: typecheck
- GitHub Check: smoke-and-tests (24.11.x)
- GitHub Check: smoke-and-tests (22)
🧰 Additional context used
📓 Path-based instructions (6)
**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*: Preserve existing repository patterns unless intentionally refactoring them.
Keep changes small, readable, and focused; avoid broad rewrites or unrelated cleanup.
Do not reformat unrelated files, and keep comments useful and concise.
Update documentation when setup, commands, or user-facing behavior changes.
Review AI-generated changes for correctness, style consistency, unnecessary noise, and adherence to project architecture before submitting them.
Provider changes must follow the documented integration patterns indocs/integrations/overview.mdand the focused guides underdocs/integrations/how-to/.
When changing provider behavior, avoid breaking third-party providers and test the exact provider/model path changed when possible.
Provider pull requests must explicitly identify affected providers, limitations, and follow-up work.
Run the narrowest meaningful validation command for the touched area, and ensure relevant CI checks pass before merging.
Usebun installto install dependencies and the repository's Bun scripts for building, testing, smoke testing, and development.
Dependency changes require a concrete project benefit such as a bug fix, security issue, or approved feature; preference alone is insufficient.
Do not change the project's language, core runtime, or dependency stack, or introduce a new runtime, without prior maintainer agreement.
Keep each pull request focused on one issue or clearly scoped improvement and avoid bundling unrelated fixes, features, or refactors.
Files:
docs/advanced-setup.mdsrc/query/agentStepLimit.test.tssrc/components/Settings/Config.tsxsrc/screens/replMaxTurns.tssrc/screens/replMaxTurnsProp.test.tssrc/main.tsxsrc/utils/config.tssrc/query.tssrc/utils/replMaxTurns.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:
docs/advanced-setup.mdsrc/query/agentStepLimit.test.tssrc/components/Settings/Config.tsxsrc/screens/replMaxTurns.tssrc/screens/replMaxTurnsProp.test.tssrc/main.tsxsrc/utils/config.tssrc/query.tssrc/utils/replMaxTurns.tssrc/screens/REPL.tsx
{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
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
TypeScript code in this repository must use strict mode and ESM imports.
**/*.{ts,tsx}: Add or update tests when a TypeScript or TSX change affects behavior.
Run the relevant TypeScript validation checks for changed code, includingbun run typecheckand, when applicable,bun run typecheck:type-tests.
Files:
src/query/agentStepLimit.test.tssrc/components/Settings/Config.tsxsrc/screens/replMaxTurns.tssrc/screens/replMaxTurnsProp.test.tssrc/main.tsxsrc/utils/config.tssrc/query.tssrc/utils/replMaxTurns.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/query/agentStepLimit.test.tssrc/screens/replMaxTurnsProp.test.ts
src/components/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Use React + Ink for terminal UI components under
src/components/.
Files:
src/components/Settings/Config.tsx
{bin/**,scripts/**,package.json,src/setup.ts,src/main.tsx,src/entrypoints/**}
⚙️ CodeRabbit configuration file
{bin/**,scripts/**,package.json,src/setup.ts,src/main.tsx,src/entrypoints/**}: Review install, launcher, build, packaging, startup, and entrypoint changes for cross-platform compatibility, tracked-source rewrites, env/config precedence, and release safety. Block on changes that can break Windows/macOS/Linux startup or publish unexpected artifacts.
Files:
src/main.tsx
🔇 Additional comments (12)
src/utils/replMaxTurns.ts (2)
4-50: LGTM!
79-110: 📐 Maintainability & Code QualityValidate the resolver change before merge.
The supplied context has no validation output. Run
bun run typecheck, runbun run typecheck:type-testswhen configured, and report the exact focused REPL/query regression-test command and result.As per coding guidelines, run the relevant TypeScript validation checks; as per path instructions, validate focused REPL/query behavior and report exact commands.
Sources: Coding guidelines, Path instructions
src/utils/config.ts (1)
297-301: LGTM!Also applies to: 779-779
src/screens/replMaxTurns.ts (1)
1-8: LGTM!src/screens/replMaxTurnsProp.test.ts (1)
223-260: Test the production CLI-to-REPL path.
createMaxTurnsCliProgramduplicates the option definition. Themain.tsxchecks only inspect source text. These tests can pass if production Commander setup or the interactivesessionConfighandoff breaks.Exercise the production command setup, or extract it into a testable function. Assert that
--max-turns 200reaches the local REPL session configuration.As per path instructions, “Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.”
Sources: Coding guidelines, Path instructions
src/components/Settings/Config.tsx (1)
14-14: LGTM!Also applies to: 339-361, 1299-1301
src/main.tsx (2)
126-126: LGTM!Also applies to: 949-949, 3053-3054
3053-3054: 📐 Maintainability & Code QualityConfirm the required validation results.
No CI or command output is included in this review context. Provide the results for
bun install,bun run typecheck, and, when applicable,bun run typecheck:type-tests. Also provide the narrow build, smoke, and REPL/query test results required byAGENTS.md.Sources: Coding guidelines, Path instructions
src/screens/REPL.tsx (1)
11-11: LGTM!Also applies to: 42-42, 577-579, 662-672, 2847-2850, 2892-2893, 2905-2905, 3137-3140, 3166-3166, 3205-3205
docs/advanced-setup.md (1)
481-481: LGTM!src/query.ts (1)
447-449: LGTM!Also applies to: 623-623, 670-681, 3003-3004
src/query/agentStepLimit.test.ts (1)
135-158: LGTM!
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/main.tsx`:
- Line 949: Update the max-turns option’s parser around parseMaxTurnsCli so
validation failures are caught and rethrown as Commander’s InvalidArgumentError.
Preserve successful numeric parsing and ensure invalid --max-turns values
produce Commander-formatted option errors with exit code 1.
In `@src/query.ts`:
- Around line 1429-1466: Extract the duplicated abort handling in the query
function into a local generator helper, such as emitAbort, that accepts an
AbortSignal, emits the optional system and user-interruption messages, and
returns { reason: 'aborted_streaming' }. Replace all three abort checks,
including the entry preflight and both checks around onModelRequestStart, with
delegation to this helper so their behavior stays consistent.
In `@src/screens/replMaxTurnsProp.test.ts`:
- Around line 214-237: Replace the source-text assertions in the test named
“shares one resolved per-prompt budget with the background query” with
behavioral coverage: invoke the relevant background handoff flow, such as
handleBackgroundQuery, and verify that startBackgroundSession receives the same
turnBudget object as the foreground query while duplicate handoff remains
guarded. Remove assertions tied to exact identifiers, formatting, or
implementation details, and rely on the existing agentStepLimit coverage where
it already validates this behavior.
In `@src/utils/replMaxTurns.ts`:
- Around line 18-24: Update parseMaxTurnsCli to reject empty or whitespace-only
raw values before converting with Number, while preserving the existing
non-negative safe-integer validation and error behavior for other invalid
inputs.
🪄 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: 10c683f6-c2c2-4204-bf67-fb37c28deeb0
📒 Files selected for processing (7)
src/main.tsxsrc/query.tssrc/query/agentStepLimit.test.tssrc/screens/REPL.tsxsrc/screens/replMaxTurnsProp.test.tssrc/tasks/LocalMainSessionTask.tssrc/utils/replMaxTurns.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: smoke-and-tests (24.11.x)
- GitHub Check: typecheck
- GitHub Check: smoke-and-tests (22)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
TypeScript code in this repository must use strict mode and ESM imports.
**/*.{ts,tsx}: Add or update tests when a TypeScript or TSX change affects behavior.
Run the relevant TypeScript validation checks for changed code, includingbun run typecheckand, when applicable,bun run typecheck:type-tests.
Files:
src/tasks/LocalMainSessionTask.tssrc/query/agentStepLimit.test.tssrc/main.tsxsrc/screens/replMaxTurnsProp.test.tssrc/utils/replMaxTurns.tssrc/query.tssrc/screens/REPL.tsx
**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*: Preserve existing repository patterns unless intentionally refactoring them.
Keep changes small, readable, and focused; avoid broad rewrites or unrelated cleanup.
Do not reformat unrelated files, and keep comments useful and concise.
Update documentation when setup, commands, or user-facing behavior changes.
Review AI-generated changes for correctness, style consistency, unnecessary noise, and adherence to project architecture before submitting them.
Provider changes must follow the documented integration patterns indocs/integrations/overview.mdand the focused guides underdocs/integrations/how-to/.
When changing provider behavior, avoid breaking third-party providers and test the exact provider/model path changed when possible.
Provider pull requests must explicitly identify affected providers, limitations, and follow-up work.
Run the narrowest meaningful validation command for the touched area, and ensure relevant CI checks pass before merging.
Usebun installto install dependencies and the repository's Bun scripts for building, testing, smoke testing, and development.
Dependency changes require a concrete project benefit such as a bug fix, security issue, or approved feature; preference alone is insufficient.
Do not change the project's language, core runtime, or dependency stack, or introduce a new runtime, without prior maintainer agreement.
Keep each pull request focused on one issue or clearly scoped improvement and avoid bundling unrelated fixes, features, or refactors.
Files:
src/tasks/LocalMainSessionTask.tssrc/query/agentStepLimit.test.tssrc/main.tsxsrc/screens/replMaxTurnsProp.test.tssrc/utils/replMaxTurns.tssrc/query.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/tasks/LocalMainSessionTask.tssrc/query/agentStepLimit.test.tssrc/main.tsxsrc/screens/replMaxTurnsProp.test.tssrc/utils/replMaxTurns.tssrc/query.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/query/agentStepLimit.test.tssrc/screens/replMaxTurnsProp.test.ts
{bin/**,scripts/**,package.json,src/setup.ts,src/main.tsx,src/entrypoints/**}
⚙️ CodeRabbit configuration file
{bin/**,scripts/**,package.json,src/setup.ts,src/main.tsx,src/entrypoints/**}: Review install, launcher, build, packaging, startup, and entrypoint changes for cross-platform compatibility, tracked-source rewrites, env/config precedence, and release safety. Block on changes that can break Windows/macOS/Linux startup or publish unexpected artifacts.
Files:
src/main.tsx
🔇 Additional comments (16)
src/utils/replMaxTurns.ts (1)
107-109: LGTM!src/main.tsx (1)
126-126: LGTM!Also applies to: 3053-3054
src/screens/REPL.tsx (5)
662-673: LGTM!
2896-2896: LGTM!Also applies to: 2908-2908
2983-2983: LGTM!Also applies to: 3140-3140, 3166-3166
3205-3207: LGTM!Also applies to: 3267-3267, 3281-3287, 3446-3446
2846-2854: 🩺 Stability & AvailabilityNo change required.
SessionBackgroundHintdoes not invokeonBackgroundSessionunless the session-background gate andisLoadingare enabled. Foreground tasks usebackgroundAllinstead, sohandleBackgroundQueryis not reachable without its foreground budget.> Likely an incorrect or invalid review comment.src/query.ts (5)
448-452: LGTM!Also applies to: 467-477
617-622: LGTM!Also applies to: 642-642
684-714: LGTM!
1467-1475: LGTM!
2756-2761: LGTM!src/tasks/LocalMainSessionTask.ts (1)
402-412: LGTM!src/query/agentStepLimit.test.ts (2)
4-11: LGTM!Also applies to: 140-196, 238-295, 297-373
212-221: 📐 Maintainability & Code QualityRemove this review comment.
query.tscheckscompactionResultby truthiness, sonullandundefinedare equivalent. Both queries pass the initial abort and turn-limit checks and reach the barrier before model dispatch. The proposed changes do not address a current defect.> Likely an incorrect or invalid review comment.src/screens/replMaxTurnsProp.test.ts (1)
133-148: LGTM!Also applies to: 281-289
b9c7b4c to
ecf2449
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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.ts`:
- Around line 1538-1572: Update executeNonStreamingRequest to accept the
onProviderRequestStart callback and invoke it immediately before each
anthropic.beta.messages.create fallback dispatch. Pass the existing callback
from both fallback branches, preserving the callback’s boolean rejection
behavior so non-streaming fallbacks cannot dispatch when provider start is
denied.
In `@src/query/agentStepLimit.test.ts`:
- Around line 227-265: Update the concurrent query test’s preparationBarrier
wait inside autocompact to race against a short timeout that rejects with a
descriptive assertion error. Keep the existing releasePreparation
synchronization for two arrivals, while ensuring a query that exits before
autocompact cannot leave the other query hanging indefinitely.
In `@src/screens/REPL.tsx`:
- Around line 3181-3191: Wrap the manual `queryGenerator.next()` loop in a
`try`/`finally`, track whether iteration reached `done`, and call
`queryGenerator.return()` in the `finally` when it exits before completion.
Preserve the existing `registerActivity`, `onQueryEvent`, and `queryTerminal`
handling while ensuring abnormal exits close the generator and run its cleanup.
In `@src/screens/replMaxTurnsProp.test.ts`:
- Around line 279-283: Replace the source-text check in the “background tasks
retain a max-turn terminal attachment” test with behavioral coverage of
startBackgroundSession. Stub query to yield a max_turns_reached attachment, run
the background session, and assert that attachment appears in the task’s emitted
messages; remove the readSourceUp/literal-condition assertion while preserving
the existing task-layer setup.
🪄 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: f550beb7-3218-4b46-88f6-bc1b51c97a4e
📒 Files selected for processing (11)
src/main.tsxsrc/query.tssrc/query/agentStepLimit.test.tssrc/query/requestOnlyMessages.test.tssrc/screens/REPL.tsxsrc/screens/replMaxTurns.tssrc/screens/replMaxTurnsProp.test.tssrc/services/api/claude.lifecycle.test.tssrc/services/api/claude.tssrc/tasks/LocalMainSessionTask.tssrc/utils/replMaxTurns.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: smoke-and-tests (22)
- GitHub Check: smoke-and-tests (24.11.x)
- GitHub Check: typecheck
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
TypeScript code in this repository must use strict mode and ESM imports.
**/*.{ts,tsx}: Add or update tests when a TypeScript or TSX change affects behavior.
Run the relevant TypeScript validation checks for changed code, includingbun run typecheckand, when applicable,bun run typecheck:type-tests.
Files:
src/services/api/claude.lifecycle.test.tssrc/main.tsxsrc/query/requestOnlyMessages.test.tssrc/utils/replMaxTurns.tssrc/query/agentStepLimit.test.tssrc/tasks/LocalMainSessionTask.tssrc/screens/replMaxTurns.tssrc/screens/REPL.tsxsrc/screens/replMaxTurnsProp.test.tssrc/services/api/claude.tssrc/query.ts
**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*: Preserve existing repository patterns unless intentionally refactoring them.
Keep changes small, readable, and focused; avoid broad rewrites or unrelated cleanup.
Do not reformat unrelated files, and keep comments useful and concise.
Update documentation when setup, commands, or user-facing behavior changes.
Review AI-generated changes for correctness, style consistency, unnecessary noise, and adherence to project architecture before submitting them.
Provider changes must follow the documented integration patterns indocs/integrations/overview.mdand the focused guides underdocs/integrations/how-to/.
When changing provider behavior, avoid breaking third-party providers and test the exact provider/model path changed when possible.
Provider pull requests must explicitly identify affected providers, limitations, and follow-up work.
Run the narrowest meaningful validation command for the touched area, and ensure relevant CI checks pass before merging.
Usebun installto install dependencies and the repository's Bun scripts for building, testing, smoke testing, and development.
Dependency changes require a concrete project benefit such as a bug fix, security issue, or approved feature; preference alone is insufficient.
Do not change the project's language, core runtime, or dependency stack, or introduce a new runtime, without prior maintainer agreement.
Keep each pull request focused on one issue or clearly scoped improvement and avoid bundling unrelated fixes, features, or refactors.
Files:
src/services/api/claude.lifecycle.test.tssrc/main.tsxsrc/query/requestOnlyMessages.test.tssrc/utils/replMaxTurns.tssrc/query/agentStepLimit.test.tssrc/tasks/LocalMainSessionTask.tssrc/screens/replMaxTurns.tssrc/screens/REPL.tsxsrc/screens/replMaxTurnsProp.test.tssrc/services/api/claude.tssrc/query.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/api/claude.lifecycle.test.tssrc/main.tsxsrc/query/requestOnlyMessages.test.tssrc/utils/replMaxTurns.tssrc/query/agentStepLimit.test.tssrc/tasks/LocalMainSessionTask.tssrc/screens/replMaxTurns.tssrc/screens/REPL.tsxsrc/screens/replMaxTurnsProp.test.tssrc/services/api/claude.tssrc/query.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
{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.lifecycle.test.tssrc/query/requestOnlyMessages.test.tssrc/query/agentStepLimit.test.tssrc/screens/replMaxTurnsProp.test.ts
{bin/**,scripts/**,package.json,src/setup.ts,src/main.tsx,src/entrypoints/**}
⚙️ CodeRabbit configuration file
{bin/**,scripts/**,package.json,src/setup.ts,src/main.tsx,src/entrypoints/**}: Review install, launcher, build, packaging, startup, and entrypoint changes for cross-platform compatibility, tracked-source rewrites, env/config precedence, and release safety. Block on changes that can break Windows/macOS/Linux startup or publish unexpected artifacts.
Files:
src/main.tsx
🔇 Additional comments (31)
src/utils/replMaxTurns.ts (2)
18-24: LGTM!
108-108: LGTM!src/main.tsx (3)
126-126: LGTM!
3059-3060: LGTM!
949-955: 🗄️ Data Integrity & IntegrationNo change required for
--max-turns 0.Headless query guards use a truthiness check, so
0disables the cap and does not stop the first provider request.> Likely an incorrect or invalid review comment.src/screens/replMaxTurns.ts (1)
1-4: LGTM!Also applies to: 17-40, 42-62, 64-85
src/screens/REPL.tsx (6)
42-42: LGTM!Also applies to: 146-147, 663-674
2854-2924: LGTM!Also applies to: 2926-2941
3016-3016: LGTM!Also applies to: 3153-3180, 3207-3208
3247-3255: LGTM!Also applies to: 3311-3311
3325-3329: LGTM!Also applies to: 3488-3488
2925-2925: 🗄️ Data Integrity & IntegrationNo change needed: shared
QueryTurnBudgetownership is guarded. The foreground rejects stale or aborted reservations, while retries reusereservedTurnCount. Focused race tests cover this handoff path.> Likely an incorrect or invalid review comment.src/query.ts (4)
169-233: LGTM!
513-517: LGTM!Also applies to: 532-542, 682-687, 707-707
749-776: LGTM!
1784-1794: LGTM!Also applies to: 2021-2021, 2787-2793, 3113-3124
src/tasks/LocalMainSessionTask.ts (3)
51-55: LGTM!Also applies to: 95-102, 164-164
338-408: LGTM!
423-444: LGTM!Also applies to: 454-471, 536-547
src/services/api/claude.ts (3)
740-744: LGTM!Also applies to: 1935-1939
1960-1997: LGTM!
2065-2069: 🩺 Stability & AvailabilityNo lifecycle leak occurs on a null result. The callback returns
nullbeforestartActiveApiCall(), and the enclosingfinallystill callsendActiveApiCall()and releases stream resources.> Likely an incorrect or invalid review comment.src/query/agentStepLimit.test.ts (4)
4-11: LGTM!Also applies to: 20-20, 106-109, 120-120
145-201: LGTM!
203-225: LGTM!Also applies to: 267-368
370-446: LGTM!Also applies to: 448-545
src/query/requestOnlyMessages.test.ts (1)
295-299: LGTM!src/screens/replMaxTurnsProp.test.ts (3)
1-48: LGTM!Also applies to: 64-109
154-169: LGTM!Also applies to: 235-277
327-347: LGTM!src/services/api/claude.lifecycle.test.ts (1)
340-382: LGTM!
192dba9 to
56e3467
Compare
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/query/requestOnlyMessages.test.ts (1)
295-305: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCoverage gap: no case where
onModelRequestStartitself aborts.The mock returns early only when ownership is denied up front.
src/query.tsline 1556 documents that the lifecycle callback can abort synchronously, and that path currently emitsstartwithout a matchingend. Add a case whereparams.onModelRequestStartaborts the controller, then assert the recordedeventsarray.This depends on the fix proposed on
src/query.tslines 1555-1571.🤖 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/query/requestOnlyMessages.test.ts` around lines 295 - 305, Extend the lifecycle callback coverage in the test around requestOnlyMessages by making params.onModelRequestStart abort the controller, then assert the recorded events reflect the expected start-only behavior without a matching end. Keep the existing provider-dispatch denial case unchanged.Source: Path instructions
src/screens/replMaxTurnsProp.test.ts (1)
49-62: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftReplace source-text assertions with behavior tests. Tests still inspect
REPL.tsx,main.tsx,Config.tsx, andResumeConversation.tsxas text. These checks can pass while runtime wiring is broken. Add focused tests for warning scope, configuration rendering, CLI wiring, and resume propagation.🤖 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/replMaxTurnsProp.test.ts` around lines 49 - 62, The test helper objectBody and its source-text assertions should be replaced with behavior-focused tests. Add focused coverage that exercises warning scope, configuration rendering, CLI wiring, and resume propagation through the actual REPL, main, Config, and ResumeConversation behavior, ensuring tests fail when runtime wiring is broken rather than merely when source text changes.Source: Path instructions
🤖 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.ts`:
- Around line 1555-1571: The onModelRequestStart lifecycle can abort before
providerRequestStarted is set, causing the finally block to omit
onModelRequestEnd; update src/query.ts lines 1555-1571 to mark the request
started immediately after onModelRequestStart or explicitly end it in the abort
branch, and add coverage in src/query/requestOnlyMessages.test.ts lines 295-305
for a callback-triggered abort asserting every start event has a matching end.
In `@src/screens/REPL.tsx`:
- Around line 3186-3202: Preserve the generator cleanup in onQueryImpl while
avoiding the compiler-blocking try/finally shape: add a catch that rethrows the
original error around the existing loop and finally block, or extract the loop
and cleanup into a small helper outside the hook. Keep generatorDone handling
and queryTerminal assignment unchanged.
- Around line 2872-2880: Update restoreNotificationsIfUnsent in the
task-notification claim flow so pendingNotifications are restored ahead of
notifications enqueued after the claim, preserving FIFO order for equal
later-priority items. Use the queue’s existing prepend/front-insertion mechanism
rather than the current enqueue call, while retaining the ownership guard and
single-restoration behavior.
In `@src/services/api/claude.ts`:
- Around line 921-922: When next modifying executeNonStreamingRequest, replace
its nine positional parameters with a single named options object and update all
call sites to use explicit property names, preserving the existing request
behavior and callback handling.
- Around line 2067-2071: Update queryModelWithoutStreaming and the surrounding
onProviderRequestStart handling so a false dispatch result produces a typed
denial or dedicated error instead of generic “No assistant message found”
handling. Preserve normal API failure behavior while allowing callers to
distinguish ownership denial from provider errors.
In `@src/tasks/LocalMainSessionTask.test.ts`:
- Around line 44-46: Strengthen the asynchronous assertion in the test around
startBackgroundSession so it does not rely on a single zero-delay macrotask.
Wait for a short settling period or use the runner’s completion callback, then
re-check that state.tasks[taskId] remains undefined after the background
session’s full async flow.
- Around line 64-83: Update the test setup around startBackgroundSession to use
a temporary project/config directory, configure the task output storage there,
and await completion of pending disk operations before assertions. Add afterEach
cleanup that removes the temporary directory, using initTaskOutputAsSymlink for
output initialization so Windows symlink failures fall back to a regular file.
In `@src/tasks/LocalMainSessionTask.ts`:
- Around line 339-342: Update the doc comment for the task-starting method in
LocalMainSessionTask to describe that prepare completes before task
registration, matching the implementation and inline comment. Remove claims that
registration is synchronous or precedes preparation, and document that this
ordering avoids creating a phantom task when preparation fails.
- Around line 429-445: Move the assignment of
prepared.restoreNotificationsIfUnsent to restoreNotificationsIfUnsent
immediately after prepare returns successfully and before the
abortSignal.aborted check. Keep the existing early returns and task registration
flow unchanged so notifications can be restored if the abort occurs in that
window.
---
Outside diff comments:
In `@src/query/requestOnlyMessages.test.ts`:
- Around line 295-305: Extend the lifecycle callback coverage in the test around
requestOnlyMessages by making params.onModelRequestStart abort the controller,
then assert the recorded events reflect the expected start-only behavior without
a matching end. Keep the existing provider-dispatch denial case unchanged.
In `@src/screens/replMaxTurnsProp.test.ts`:
- Around line 49-62: The test helper objectBody and its source-text assertions
should be replaced with behavior-focused tests. Add focused coverage that
exercises warning scope, configuration rendering, CLI wiring, and resume
propagation through the actual REPL, main, Config, and ResumeConversation
behavior, ensuring tests fail when runtime wiring is broken rather than merely
when source text changes.
🪄 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: a7fbaaad-cf35-4589-8812-a46e7217d56c
📒 Files selected for processing (10)
src/query.tssrc/query/agentStepLimit.test.tssrc/query/requestOnlyMessages.test.tssrc/screens/REPL.tsxsrc/screens/replMaxTurns.tssrc/screens/replMaxTurnsProp.test.tssrc/services/api/claude.lifecycle.test.tssrc/services/api/claude.tssrc/tasks/LocalMainSessionTask.test.tssrc/tasks/LocalMainSessionTask.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: smoke-and-tests (22)
- GitHub Check: smoke-and-tests (24.11.x)
- GitHub Check: typecheck
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
TypeScript code in this repository must use strict mode and ESM imports.
**/*.{ts,tsx}: Add or update tests when a TypeScript or TSX change affects behavior.
Run the relevant TypeScript validation checks for changed code, includingbun run typecheckand, when applicable,bun run typecheck:type-tests.
Files:
src/tasks/LocalMainSessionTask.test.tssrc/query/requestOnlyMessages.test.tssrc/query/agentStepLimit.test.tssrc/screens/replMaxTurnsProp.test.tssrc/services/api/claude.lifecycle.test.tssrc/screens/replMaxTurns.tssrc/query.tssrc/services/api/claude.tssrc/tasks/LocalMainSessionTask.tssrc/screens/REPL.tsx
**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*: Preserve existing repository patterns unless intentionally refactoring them.
Keep changes small, readable, and focused; avoid broad rewrites or unrelated cleanup.
Do not reformat unrelated files, and keep comments useful and concise.
Update documentation when setup, commands, or user-facing behavior changes.
Review AI-generated changes for correctness, style consistency, unnecessary noise, and adherence to project architecture before submitting them.
Provider changes must follow the documented integration patterns indocs/integrations/overview.mdand the focused guides underdocs/integrations/how-to/.
When changing provider behavior, avoid breaking third-party providers and test the exact provider/model path changed when possible.
Provider pull requests must explicitly identify affected providers, limitations, and follow-up work.
Run the narrowest meaningful validation command for the touched area, and ensure relevant CI checks pass before merging.
Usebun installto install dependencies and the repository's Bun scripts for building, testing, smoke testing, and development.
Dependency changes require a concrete project benefit such as a bug fix, security issue, or approved feature; preference alone is insufficient.
Do not change the project's language, core runtime, or dependency stack, or introduce a new runtime, without prior maintainer agreement.
Keep each pull request focused on one issue or clearly scoped improvement and avoid bundling unrelated fixes, features, or refactors.
Files:
src/tasks/LocalMainSessionTask.test.tssrc/query/requestOnlyMessages.test.tssrc/query/agentStepLimit.test.tssrc/screens/replMaxTurnsProp.test.tssrc/services/api/claude.lifecycle.test.tssrc/screens/replMaxTurns.tssrc/query.tssrc/services/api/claude.tssrc/tasks/LocalMainSessionTask.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/tasks/LocalMainSessionTask.test.tssrc/query/requestOnlyMessages.test.tssrc/query/agentStepLimit.test.tssrc/screens/replMaxTurnsProp.test.tssrc/services/api/claude.lifecycle.test.tssrc/screens/replMaxTurns.tssrc/query.tssrc/services/api/claude.tssrc/tasks/LocalMainSessionTask.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/tasks/LocalMainSessionTask.test.tssrc/query/requestOnlyMessages.test.tssrc/query/agentStepLimit.test.tssrc/screens/replMaxTurnsProp.test.tssrc/services/api/claude.lifecycle.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.lifecycle.test.tssrc/services/api/claude.ts
🪛 React Doctor (0.9.1)
src/screens/REPL.tsx
[warning] 3188-3188: This component misses React Compiler's automatic memoization & re-renders more than it should. Rewrite the flagged code so the compiler can optimize it.
Todo: (BuildHIR::lowerStatement) Handle TryStatement without a catch clause
(todo)
🔇 Additional comments (33)
src/screens/replMaxTurns.ts (4)
1-4: LGTM!Also applies to: 17-21
23-40: LGTM!
42-62: LGTM!
64-85: LGTM!src/screens/REPL.tsx (6)
42-42: LGTM!Also applies to: 146-147
666-674: LGTM!
3021-3021: LGTM!Also applies to: 3158-3185, 3218-3219
3258-3266: LGTM!
3322-3322: LGTM!Also applies to: 3336-3340
3499-3499: LGTM!src/query.ts (5)
169-233: LGTM!
513-517: LGTM!Also applies to: 532-542
680-687: LGTM!Also applies to: 707-707, 749-784
1784-1794: LGTM!Also applies to: 2021-2021
2787-2793: LGTM!Also applies to: 3113-3124
src/services/api/claude.ts (4)
740-744: LGTM!
936-936: LGTM!Also applies to: 1010-1010
2891-2895: LGTM!Also applies to: 3014-3018
1937-1941: 📐 Maintainability & Code QualityDefine
logAPIQuerytelemetry scope.withRetryinvokes the callback for each retry, butapiQueryLoggedrecords only the first attempt. Retries can changemax_tokens,fastMode, and model-dependent settings. Model fallback runs in a new outer query iteration and receives a fresh guard. Iftengu_api_querymust describe every provider attempt, add attempt-level logging or document its query-level scope.src/tasks/LocalMainSessionTask.ts (5)
51-55: LGTM!
363-412: LGTM!
460-480: LGTM!
545-560: LGTM!
95-104: 🗄️ Data Integrity & IntegrationNo caller requires a return-shape update. The only call ignores the return value, so no caller reads
.abortSignalor gains abort ownership.> Likely an incorrect or invalid review comment.src/query/agentStepLimit.test.ts (4)
20-20: LGTM!Also applies to: 106-109, 120-120
203-225: LGTM!Also applies to: 244-258
340-382: LGTM!
462-559: LGTM!src/screens/replMaxTurnsProp.test.ts (1)
235-277: LGTM!src/services/api/claude.lifecycle.test.ts (2)
344-381: LGTM!Also applies to: 698-698, 709-739
341-343: 📐 Maintainability & Code QualityNo change needed:
afterEachrestoresOPENCLAUDE_MAX_RETRIESfromoriginalEnvafter each test.> Likely an incorrect or invalid review comment.src/tasks/LocalMainSessionTask.test.ts (2)
14-29: LGTM!
49-63: LGTM!
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/query/requestOnlyMessages.test.ts`:
- Around line 315-329: Update the test around the provider request generator and
its dispatch gate to track whether execution proceeds past
options.onProviderRequestStart?.(), then assert that marker remains false after
collect(params). Keep the existing lifecycle assertion and ensure the marker is
only set after the gate, so the test fails if provider dispatch resumes.
In `@src/screens/REPL.tsx`:
- Around line 2942-2968: Add terminalTitle to the dependency array of the
handleBackgroundQuery callback so later Ctrl+B actions use the current terminal
title when setting the background session description.
In `@src/utils/messageQueueManager.ts`:
- Around line 136-150: Add focused queue-level coverage for prepend, exercising
restoration followed by later enqueues and asserting restored commands are
delivered first in the supplied FIFO order, retain their priority, and trigger
subscriber notification exactly once.
🪄 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: a2f1770e-7205-4886-8d1f-e6b68f3039b2
📒 Files selected for processing (6)
src/query.tssrc/query/requestOnlyMessages.test.tssrc/screens/REPL.tsxsrc/tasks/LocalMainSessionTask.test.tssrc/tasks/LocalMainSessionTask.tssrc/utils/messageQueueManager.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: smoke-and-tests (24.11.x)
- GitHub Check: smoke-and-tests (22)
- GitHub Check: typecheck
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
TypeScript code in this repository must use strict mode and ESM imports.
**/*.{ts,tsx}: Add or update tests when a TypeScript or TSX change affects behavior.
Run the relevant TypeScript validation checks for changed code, includingbun run typecheckand, when applicable,bun run typecheck:type-tests.
Files:
src/query/requestOnlyMessages.test.tssrc/utils/messageQueueManager.tssrc/tasks/LocalMainSessionTask.test.tssrc/tasks/LocalMainSessionTask.tssrc/screens/REPL.tsxsrc/query.ts
**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*: Preserve existing repository patterns unless intentionally refactoring them.
Keep changes small, readable, and focused; avoid broad rewrites or unrelated cleanup.
Do not reformat unrelated files, and keep comments useful and concise.
Update documentation when setup, commands, or user-facing behavior changes.
Review AI-generated changes for correctness, style consistency, unnecessary noise, and adherence to project architecture before submitting them.
Provider changes must follow the documented integration patterns indocs/integrations/overview.mdand the focused guides underdocs/integrations/how-to/.
When changing provider behavior, avoid breaking third-party providers and test the exact provider/model path changed when possible.
Provider pull requests must explicitly identify affected providers, limitations, and follow-up work.
Run the narrowest meaningful validation command for the touched area, and ensure relevant CI checks pass before merging.
Usebun installto install dependencies and the repository's Bun scripts for building, testing, smoke testing, and development.
Dependency changes require a concrete project benefit such as a bug fix, security issue, or approved feature; preference alone is insufficient.
Do not change the project's language, core runtime, or dependency stack, or introduce a new runtime, without prior maintainer agreement.
Keep each pull request focused on one issue or clearly scoped improvement and avoid bundling unrelated fixes, features, or refactors.
Files:
src/query/requestOnlyMessages.test.tssrc/utils/messageQueueManager.tssrc/tasks/LocalMainSessionTask.test.tssrc/tasks/LocalMainSessionTask.tssrc/screens/REPL.tsxsrc/query.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/requestOnlyMessages.test.tssrc/utils/messageQueueManager.tssrc/tasks/LocalMainSessionTask.test.tssrc/tasks/LocalMainSessionTask.tssrc/screens/REPL.tsxsrc/query.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/requestOnlyMessages.test.tssrc/tasks/LocalMainSessionTask.test.ts
🔇 Additional comments (7)
src/screens/REPL.tsx (4)
3208-3224: React Compiler cannot memoize this callback (already flagged).The
try/finallywithout acatcharound the query-generator loop still blocks React Compiler's automatic memoization foronQueryImpl, the same finding raised previously on this pattern. It remains unaddressed (no confirmation in the prior thread) and is non-blocking.
663-674: LGTM!
2846-2941: LGTM!
3358-3362: 🗄️ Data Integrity & IntegrationNo change required.
releaseForegroundTurnBudgetuses the identity guard, and its synchronous cleanup cannot interleave withclaimBackgroundTurnBudget.> Likely an incorrect or invalid review comment.src/query.ts (1)
1538-1574: LGTM!src/tasks/LocalMainSessionTask.ts (2)
341-465: LGTM!
473-582: LGTM!
There was a problem hiding this comment.
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)
2907-2918: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftDo not use prompt text as the notification identity.
If two independent tasks emit the same prompt text,
uniqueNotificationsremoves the second notification. The removed notification is not restored after a successful background handoff.Carry a stable task or notification ID through the queued command, or deduplicate only when the source identity matches. Add a regression test with different task IDs and identical prompt text.
🤖 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 2907 - 2918, Replace the prompt-based deduplication in the notification handling around settledMessages and uniqueNotifications with stable task or notification identity, preserving both notifications when their IDs differ even if prompts match. Carry that identity through the queued command or require matching source identity before filtering, and add a regression test covering distinct task IDs with identical prompt text.Source: Coding guidelines
♻️ Duplicate comments (1)
src/screens/replMaxTurnsProp.test.ts (1)
337-346: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftReplace the source-text wiring assertion.
This test reads
src/main.tsxand asserts imports and literal statements. It does not prove that production Commander parsing passes--max-turnsinto localsessionConfig.Exercise the production command setup, or extract its registration into an executable helper. Assert that parsed input reaches the local REPL configuration.
As per coding guidelines, “Add or update tests when a TypeScript or TSX change affects behavior.” As per path instructions, “Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible 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/replMaxTurnsProp.test.ts` around lines 337 - 346, The test currently verifies source text instead of production behavior. Replace the assertions in “main uses the production Commander parser and wires sessionConfig” with an executable test of the production Commander setup (or an extracted registration helper), passing --max-turns and asserting the parsed value reaches the local REPL sessionConfig.Sources: Coding guidelines, Path instructions
🤖 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.
Outside diff comments:
In `@src/screens/REPL.tsx`:
- Around line 2907-2918: Replace the prompt-based deduplication in the
notification handling around settledMessages and uniqueNotifications with stable
task or notification identity, preserving both notifications when their IDs
differ even if prompts match. Carry that identity through the queued command or
require matching source identity before filtering, and add a regression test
covering distinct task IDs with identical prompt text.
---
Duplicate comments:
In `@src/screens/replMaxTurnsProp.test.ts`:
- Around line 337-346: The test currently verifies source text instead of
production behavior. Replace the assertions in “main uses the production
Commander parser and wires sessionConfig” with an executable test of the
production Commander setup (or an extracted registration helper), passing
--max-turns and asserting the parsed value reaches the local REPL sessionConfig.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 914ba7a5-34e2-4412-bc89-17426ff14aaf
📒 Files selected for processing (7)
src/main.tsxsrc/query/requestOnlyMessages.test.tssrc/screens/REPL.tsxsrc/screens/replMaxTurnsProp.test.tssrc/tasks/LocalMainSessionTask.test.tssrc/utils/messageQueueManager.test.tssrc/utils/replMaxTurns.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: smoke-and-tests (22)
- GitHub Check: smoke-and-tests (24.11.x)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
TypeScript code in this repository must use strict mode and ESM imports.
**/*.{ts,tsx}: Add or update tests when a TypeScript or TSX change affects behavior.
Run the relevant TypeScript validation checks for changed code, includingbun run typecheckand, when applicable,bun run typecheck:type-tests.
Files:
src/utils/messageQueueManager.test.tssrc/tasks/LocalMainSessionTask.test.tssrc/screens/replMaxTurnsProp.test.tssrc/main.tsxsrc/screens/REPL.tsxsrc/utils/replMaxTurns.tssrc/query/requestOnlyMessages.test.ts
**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*: Preserve existing repository patterns unless intentionally refactoring them.
Keep changes small, readable, and focused; avoid broad rewrites or unrelated cleanup.
Do not reformat unrelated files, and keep comments useful and concise.
Update documentation when setup, commands, or user-facing behavior changes.
Review AI-generated changes for correctness, style consistency, unnecessary noise, and adherence to project architecture before submitting them.
Provider changes must follow the documented integration patterns indocs/integrations/overview.mdand the focused guides underdocs/integrations/how-to/.
When changing provider behavior, avoid breaking third-party providers and test the exact provider/model path changed when possible.
Provider pull requests must explicitly identify affected providers, limitations, and follow-up work.
Run the narrowest meaningful validation command for the touched area, and ensure relevant CI checks pass before merging.
Usebun installto install dependencies and the repository's Bun scripts for building, testing, smoke testing, and development.
Dependency changes require a concrete project benefit such as a bug fix, security issue, or approved feature; preference alone is insufficient.
Do not change the project's language, core runtime, or dependency stack, or introduce a new runtime, without prior maintainer agreement.
Keep each pull request focused on one issue or clearly scoped improvement and avoid bundling unrelated fixes, features, or refactors.
Files:
src/utils/messageQueueManager.test.tssrc/tasks/LocalMainSessionTask.test.tssrc/screens/replMaxTurnsProp.test.tssrc/main.tsxsrc/screens/REPL.tsxsrc/utils/replMaxTurns.tssrc/query/requestOnlyMessages.test.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/utils/messageQueueManager.test.tssrc/tasks/LocalMainSessionTask.test.tssrc/screens/replMaxTurnsProp.test.tssrc/main.tsxsrc/screens/REPL.tsxsrc/utils/replMaxTurns.tssrc/query/requestOnlyMessages.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/messageQueueManager.test.tssrc/tasks/LocalMainSessionTask.test.tssrc/screens/replMaxTurnsProp.test.tssrc/query/requestOnlyMessages.test.ts
{bin/**,scripts/**,package.json,src/setup.ts,src/main.tsx,src/entrypoints/**}
⚙️ CodeRabbit configuration file
{bin/**,scripts/**,package.json,src/setup.ts,src/main.tsx,src/entrypoints/**}: Review install, launcher, build, packaging, startup, and entrypoint changes for cross-platform compatibility, tracked-source rewrites, env/config precedence, and release safety. Block on changes that can break Windows/macOS/Linux startup or publish unexpected artifacts.
Files:
src/main.tsx
🔇 Additional comments (14)
src/query/requestOnlyMessages.test.ts (1)
314-318: LGTM!Also applies to: 331-331
src/screens/replMaxTurnsProp.test.ts (1)
28-28: LGTM!Also applies to: 86-86
src/tasks/LocalMainSessionTask.test.ts (1)
1-31: LGTM!Also applies to: 50-94
src/utils/messageQueueManager.test.ts (1)
1-36: LGTM!src/utils/replMaxTurns.ts (1)
1-17: LGTM!Also applies to: 19-36, 38-51, 53-79, 81-98, 99-130
src/main.tsx (3)
126-126: LGTM!
3055-3056: LGTM!
949-951: 🎯 Functional CorrectnessDefine print-mode compatibility for non-integer
--max-turnsvalues.
runHeadlessreceivesoptions.maxTurnsunchanged, and no print-mode test covers this option. The previousNumberparser passed through values such as2.5,-3,Infinity, blank input, and unsafe integers; the shared parser now rejects them. Keep the shared parser only if these inputs are intentionally invalid forsrc/screens/REPL.tsx (6)
11-11: LGTM!Also applies to: 42-42, 146-147, 213-213, 577-580, 663-674
2849-2906: LGTM!
2919-2968: LGTM!
3043-3043: LGTM!Also applies to: 3180-3223, 3240-3241
3284-3288: LGTM!Also applies to: 3344-3344, 3358-3362, 3477-3479, 3523-3523
3280-3283: 🎯 Functional CorrectnessRemove this review comment.
resolveReplMaxTurns(0)returnsundefined, so the handoff does not receive0for--max-turns 0./confignormalizes0to the default, and existing tests cover both paths.> Likely an incorrect or invalid review comment.
Wire --max-turns into interactive sessionConfig and honor OPENCLAUDE_MAX_TURNS / CLAUDE_CODE_MAX_TURNS so long autonomous REPL sessions can raise the default 50-turn per-prompt cap (fixes Twigpine#2079).
sessionConfig covered the normal interactive paths; connect, SSH, assistant, and --remote built REPL props without spreading it, so the CLI override was dropped despite help advertising interactive support.
Remote-backed sessions bypass local query(), so forwarding --max-turns into those REPL props over-claimed enforcement. Clarify help/docs and match OPENCLAUDE_MAX_RETRIES precedence when OPENCLAUDE_MAX_TURNS is set but invalid.
Expose replMaxTurns in the Config panel (50/100/200/500) and resolve it after CLI/env so local interactive sessions can raise the per-prompt cap without restarting. Resolve at query time so mid-session /config changes apply on the next prompt.
Match the OPENCLAUDE_MAX_RETRIES contract: a set-but-invalid primary env var uses the default and does not fall through to legacy or /config.
Share the --max-turns Commander description via an imported constant so help and tests stay in sync without breaking the CLI bundle, replace source-only help assertions with Commander behavior coverage, and add the published 0.27.0 entry so web verify-dist passes.
Avoid TS2339 on untyped Commander opts, log invalid OPENCLAUDE_MAX_TURNS like MAX_RETRIES, and clarify that /config shows the persisted preference.
Add regression tests for notification restore after provider dispatch commits ownership, and assert headless --max-turns 0 reaches query() without interactive resolution stripping the value.
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/cli/printMaxTurns.test.ts`:
- Around line 76-94: Extend the boundary regression test in
printMaxTurns.test.ts to invoke the headless --print flow through runHeadless
with --max-turns 0, and spy on ask() to assert it receives maxTurns: 0. Preserve
the existing parsing assertion while replacing or supplementing the direct
query() coverage with the runHeadless-to-ask path.
🪄 Autofix
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: 38034a72-373c-435b-8307-859525c1a993
📒 Files selected for processing (2)
src/cli/printMaxTurns.test.tssrc/tasks/LocalMainSessionTask.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: smoke-and-tests (24.11.x)
- GitHub Check: typecheck
- GitHub Check: smoke-and-tests (22)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
TypeScript code in this repository must use strict mode and ESM imports.
**/*.{ts,tsx}: Add or update tests when a TypeScript or TSX change affects behavior.
Run the relevant TypeScript validation checks for changed code, includingbun run typecheckand, when applicable,bun run typecheck:type-tests.
Files:
src/cli/printMaxTurns.test.tssrc/tasks/LocalMainSessionTask.test.ts
**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*: Preserve existing repository patterns unless intentionally refactoring them.
Keep changes small, readable, and focused; avoid broad rewrites or unrelated cleanup.
Do not reformat unrelated files, and keep comments useful and concise.
Update documentation when setup, commands, or user-facing behavior changes.
Review AI-generated changes for correctness, style consistency, unnecessary noise, and adherence to project architecture before submitting them.
Provider changes must follow the documented integration patterns indocs/integrations/overview.mdand the focused guides underdocs/integrations/how-to/.
When changing provider behavior, avoid breaking third-party providers and test the exact provider/model path changed when possible.
Provider pull requests must explicitly identify affected providers, limitations, and follow-up work.
Run the narrowest meaningful validation command for the touched area, and ensure relevant CI checks pass before merging.
Usebun installto install dependencies and the repository's Bun scripts for building, testing, smoke testing, and development.
Dependency changes require a concrete project benefit such as a bug fix, security issue, or approved feature; preference alone is insufficient.
Do not change the project's language, core runtime, or dependency stack, or introduce a new runtime, without prior maintainer agreement.
Keep each pull request focused on one issue or clearly scoped improvement and avoid bundling unrelated fixes, features, or refactors.
Files:
src/cli/printMaxTurns.test.tssrc/tasks/LocalMainSessionTask.test.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/cli/printMaxTurns.test.tssrc/tasks/LocalMainSessionTask.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/cli/printMaxTurns.test.tssrc/tasks/LocalMainSessionTask.test.ts
🔇 Additional comments (1)
src/tasks/LocalMainSessionTask.test.ts (1)
183-229: LGTM!
Track the deduped successor subset when restoring claimed main-thread task notifications so items already in the settled foreground transcript are not re-queued. Clarify that agent-scoped notifications intentionally stay on their owner drain path (issue Twigpine#2079 scope is interactive turn caps only).
Commit notification ownership when background sessions complete without provider dispatch, forward all task notifications on Ctrl+B again, and restore deferred max-turn cap attachments when continuation is cancelled.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/tasks/LocalMainSessionTask.ts (1)
395-427: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRestore the deferred cap for a registered cancelled continuation.
At Line 395,
finishAbortedTaskrestores queued notifications but does not callonContinuationCancelled.src/screens/REPL.tsxuses that callback to restore the deferredmax_turns_reachedattachment.If a Ctrl+B continuation registers and is stopped before provider dispatch, the foreground cap remains suppressed. Call
onContinuationCancelledfrom this registered cancellation path exactly once. Add a regression test for a registered pre-dispatch abort.Proposed fix
const finishAbortedTask = (): void => { restorePreDispatchNotifications() + onContinuationCancelled?.() // chat:killAgents already marks the task notified and emits this event.As per coding guidelines, TypeScript behavior changes must add or update tests. As per path instructions, add behavioral tests for cancellation flows.
🤖 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/tasks/LocalMainSessionTask.ts` around lines 395 - 427, Update finishAbortedTask to invoke the registered continuation’s onContinuationCancelled callback exactly once when a Ctrl+B continuation is aborted before provider dispatch, alongside restoring queued notifications. Preserve the existing SDK termination deduplication and task-state behavior, and add a behavioral regression test covering registered pre-dispatch cancellation and restoration of the deferred max_turns_reached attachment.Sources: Coding guidelines, Path instructions
🤖 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.
Outside diff comments:
In `@src/tasks/LocalMainSessionTask.ts`:
- Around line 395-427: Update finishAbortedTask to invoke the registered
continuation’s onContinuationCancelled callback exactly once when a Ctrl+B
continuation is aborted before provider dispatch, alongside restoring queued
notifications. Preserve the existing SDK termination deduplication and
task-state behavior, and add a behavioral regression test covering registered
pre-dispatch cancellation and restoration of the deferred max_turns_reached
attachment.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8fd329b4-aecc-46d1-8b37-38968e0cd5a0
📒 Files selected for processing (7)
src/screens/REPL.tsxsrc/screens/replMaxTurns.tssrc/screens/replMaxTurnsProp.test.tssrc/tasks/LocalMainSessionTask.test.tssrc/tasks/LocalMainSessionTask.tssrc/utils/taskNotificationIdentity.test.tssrc/utils/taskNotificationIdentity.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: smoke-and-tests (22)
- GitHub Check: typecheck
- GitHub Check: smoke-and-tests (24.11.x)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
TypeScript code in this repository must use strict mode and ESM imports.
**/*.{ts,tsx}: Add or update tests when a TypeScript or TSX change affects behavior.
Run the relevant TypeScript validation checks for changed code, includingbun run typecheckand, when applicable,bun run typecheck:type-tests.
Files:
src/utils/taskNotificationIdentity.test.tssrc/tasks/LocalMainSessionTask.test.tssrc/utils/taskNotificationIdentity.tssrc/tasks/LocalMainSessionTask.tssrc/screens/replMaxTurnsProp.test.tssrc/screens/REPL.tsxsrc/screens/replMaxTurns.ts
**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*: Preserve existing repository patterns unless intentionally refactoring them.
Keep changes small, readable, and focused; avoid broad rewrites or unrelated cleanup.
Do not reformat unrelated files, and keep comments useful and concise.
Update documentation when setup, commands, or user-facing behavior changes.
Review AI-generated changes for correctness, style consistency, unnecessary noise, and adherence to project architecture before submitting them.
Provider changes must follow the documented integration patterns indocs/integrations/overview.mdand the focused guides underdocs/integrations/how-to/.
When changing provider behavior, avoid breaking third-party providers and test the exact provider/model path changed when possible.
Provider pull requests must explicitly identify affected providers, limitations, and follow-up work.
Run the narrowest meaningful validation command for the touched area, and ensure relevant CI checks pass before merging.
Usebun installto install dependencies and the repository's Bun scripts for building, testing, smoke testing, and development.
Dependency changes require a concrete project benefit such as a bug fix, security issue, or approved feature; preference alone is insufficient.
Do not change the project's language, core runtime, or dependency stack, or introduce a new runtime, without prior maintainer agreement.
Keep each pull request focused on one issue or clearly scoped improvement and avoid bundling unrelated fixes, features, or refactors.
Files:
src/utils/taskNotificationIdentity.test.tssrc/tasks/LocalMainSessionTask.test.tssrc/utils/taskNotificationIdentity.tssrc/tasks/LocalMainSessionTask.tssrc/screens/replMaxTurnsProp.test.tssrc/screens/REPL.tsxsrc/screens/replMaxTurns.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/utils/taskNotificationIdentity.test.tssrc/tasks/LocalMainSessionTask.test.tssrc/utils/taskNotificationIdentity.tssrc/tasks/LocalMainSessionTask.tssrc/screens/replMaxTurnsProp.test.tssrc/screens/REPL.tsxsrc/screens/replMaxTurns.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/taskNotificationIdentity.test.tssrc/tasks/LocalMainSessionTask.test.tssrc/screens/replMaxTurnsProp.test.ts
🔇 Additional comments (1)
src/tasks/LocalMainSessionTask.test.ts (1)
232-273: 📐 Maintainability & Code QualityReport required validation results.
No CI or local command output is included. Before merge, run the focused tests,
bun run typecheck,bun run typecheck:type-testswhen defined, and the repository build, smoke/check, and privacy validation commands. Report the exact commands and results.As per coding guidelines, run relevant TypeScript validation checks. As per path instructions, run targeted tests plus typecheck, build, smoke/check, and privacy validation.
Sources: Coding guidelines, Path instructions
Anchor deferred max-turn restoration to the handed-off transcript tail so a cancelled Ctrl+B handoff cannot attach the prior prompt's cap to a newer turn. Apply the interactive turn cap only in local sessions and align remote-session docs/help wording.
persistentMessages is block-scoped inside onQuery try; read the settled tail from messagesRef in finally so typecheck passes.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/SessionBackgroundHint.tsx (1)
50-74: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winEnable session backgrounding for
task:background.When
CLAUDE_CODE_DISABLE_BACKGROUND_TASKSis unset, use that flag as the availability condition instead ofisEnvTruthy("false"). The current gates prevent handler registration and blockonBackgroundSessionduring local queries. Add a regression test for this path, then runbun run typecheckandbun run typecheck:type-testswhen defined.🤖 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/components/SessionBackgroundHint.tsx` around lines 50 - 74, Update SessionBackgroundHint’s handleBackground registration guard and sessionBgEnabled calculation to use CLAUDE_CODE_DISABLE_BACKGROUND_TASKS, enabling the path when the flag is unset while preserving disabled behavior when it is truthy. Add a regression test covering task:background and onBackgroundSession during local queries. Run bun run typecheck and bun run typecheck:type-tests if those scripts are defined.Sources: Coding guidelines, Path instructions
🤖 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.
Outside diff comments:
In `@src/components/SessionBackgroundHint.tsx`:
- Around line 50-74: Update SessionBackgroundHint’s handleBackground
registration guard and sessionBgEnabled calculation to use
CLAUDE_CODE_DISABLE_BACKGROUND_TASKS, enabling the path when the flag is unset
while preserving disabled behavior when it is truthy. Add a regression test
covering task:background and onBackgroundSession during local queries. Run bun
run typecheck and bun run typecheck:type-tests if those scripts are defined.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: deae8740-3908-4851-a618-5ac37fa7606c
📒 Files selected for processing (6)
docs/advanced-setup.mdsrc/components/SessionBackgroundHint.tsxsrc/screens/REPL.tsxsrc/screens/replMaxTurns.tssrc/screens/replMaxTurnsProp.test.tssrc/utils/replMaxTurns.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: smoke-and-tests (22)
- GitHub Check: smoke-and-tests (24.11.x)
🧰 Additional context used
📓 Path-based instructions (5)
**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*: Preserve existing repository patterns unless intentionally refactoring them.
Keep changes small, readable, and focused; avoid broad rewrites or unrelated cleanup.
Do not reformat unrelated files, and keep comments useful and concise.
Update documentation when setup, commands, or user-facing behavior changes.
Review AI-generated changes for correctness, style consistency, unnecessary noise, and adherence to project architecture before submitting them.
Provider changes must follow the documented integration patterns indocs/integrations/overview.mdand the focused guides underdocs/integrations/how-to/.
When changing provider behavior, avoid breaking third-party providers and test the exact provider/model path changed when possible.
Provider pull requests must explicitly identify affected providers, limitations, and follow-up work.
Run the narrowest meaningful validation command for the touched area, and ensure relevant CI checks pass before merging.
Usebun installto install dependencies and the repository's Bun scripts for building, testing, smoke testing, and development.
Dependency changes require a concrete project benefit such as a bug fix, security issue, or approved feature; preference alone is insufficient.
Do not change the project's language, core runtime, or dependency stack, or introduce a new runtime, without prior maintainer agreement.
Keep each pull request focused on one issue or clearly scoped improvement and avoid bundling unrelated fixes, features, or refactors.
Files:
docs/advanced-setup.mdsrc/components/SessionBackgroundHint.tsxsrc/screens/replMaxTurnsProp.test.tssrc/utils/replMaxTurns.tssrc/screens/REPL.tsxsrc/screens/replMaxTurns.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:
docs/advanced-setup.mdsrc/components/SessionBackgroundHint.tsxsrc/screens/replMaxTurnsProp.test.tssrc/utils/replMaxTurns.tssrc/screens/REPL.tsxsrc/screens/replMaxTurns.ts
{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
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
TypeScript code in this repository must use strict mode and ESM imports.
**/*.{ts,tsx}: Add or update tests when a TypeScript or TSX change affects behavior.
Run the relevant TypeScript validation checks for changed code, includingbun run typecheckand, when applicable,bun run typecheck:type-tests.
Files:
src/components/SessionBackgroundHint.tsxsrc/screens/replMaxTurnsProp.test.tssrc/utils/replMaxTurns.tssrc/screens/REPL.tsxsrc/screens/replMaxTurns.ts
src/components/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Use React + Ink for terminal UI components under
src/components/.
Files:
src/components/SessionBackgroundHint.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/screens/replMaxTurnsProp.test.ts
🔇 Additional comments (5)
src/screens/replMaxTurnsProp.test.ts (1)
8-14: 📐 Maintainability & Code QualityReport the required validation results.
The supplied context has no validation output. Before merge, provide the focused test command and results,
bun run typecheck, andbun run typecheck:type-testsif that script exists. Also report the required build, smoke, and security/privacy validation commands.As per coding guidelines, “Run the relevant TypeScript validation checks for changed code, including
bun run typecheckand, when applicable,bun run typecheck:type-tests.” As per path instructions, “Run focused tests plus typecheck, build, smoke, security/privacy validation, and report exact commands.”Also applies to: 329-378
Sources: Coding guidelines, Path instructions
src/utils/replMaxTurns.ts (1)
17-17: LGTM!docs/advanced-setup.md (1)
481-481: LGTM!src/screens/REPL.tsx (1)
42-42: LGTM!Also applies to: 664-683, 2858-3000, 3075-3075, 3212-3255, 3272-3273, 3312-3326, 3382-3382, 3396-3414, 3529-3531, 3575-3575
src/screens/replMaxTurns.ts (1)
7-7: LGTM!Also applies to: 69-105
There was a problem hiding this comment.
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)
3254-3256: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winDo not let generator cleanup replace the original error.
queryGenerator.return()runs insidefinally. If the generator's own cleanup rejects, that rejection replaces the in-flight error (abort, provider failure) and the caller sees the wrong terminal reason. The turn budget release inonQuerystill runs, butdidThrowreporting and lifecycle logs then carry the cleanup error instead of the real cause.Swallow the cleanup rejection and log it.
♻️ Proposed change
} finally { - if (!generatorDone) await queryGenerator.return(undefined as never); + if (!generatorDone) { + try { + await queryGenerator.return(undefined as never) + } catch (cleanupError) { + logForDebugging( + `query.generator.return_failed ${errorMessage(cleanupError)}`, + { level: 'warn' }, + ) + } + } }🤖 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 3254 - 3256, Update the finally cleanup around queryGenerator.return in the enclosing REPL flow to catch and swallow cleanup rejections while logging the cleanup error, preserving any original in-flight error for the caller, didThrow reporting, and lifecycle logs.
🤖 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.
Outside diff comments:
In `@src/screens/REPL.tsx`:
- Around line 3254-3256: Update the finally cleanup around queryGenerator.return
in the enclosing REPL flow to catch and swallow cleanup rejections while logging
the cleanup error, preserving any original in-flight error for the caller,
didThrow reporting, and lifecycle logs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: ad44b387-874f-482b-95c4-56683df63146
📒 Files selected for processing (1)
src/screens/REPL.tsx
📜 Review details
⚠️ CI failures not shown inline (4)
GitHub Actions: PR Checks / smoke-and-tests (22): fix(repl): make local interactive max-turns configurable
Conclusion: failure
##[group]src/utils/context.test.ts:
(pass) calculateContextPercentages preserves tiny nonzero usage [1.00ms]
(pass) deepseek-v4-flash uses the gateway-safe output cap by default [7.00ms]
(pass) deepseek-v4-flash uses DeepSeek direct API max output cap on api.deepseek.com [1.00ms]
(pass) deepseek-v4-pro uses the gateway-safe output cap by default [7.00ms]
(pass) Ollama deepseek-v4-pro cloud variant uses DeepSeek V4 Pro runtime limits [2.00ms]
(pass) Ollama deepseek-v4-pro cloud variant is modeled as route catalog metadata [1.00ms]
(pass) Ollama deepseek-v4-pro cloud variant clamps oversized output token overrides
(pass) Ollama deepseek-v4-pro cloud variant does not inherit base-model env override prefixes [1.00ms]
(pass) Ollama deepseek-v4-pro cloud variant still honors exact env overrides [1.00ms]
(pass) OpenAI-compatible env override prefixes still match colon-tagged local models [5.00ms]
(pass) Ollama deepseek-v4-pro cloud variant keeps the local max_tokens transport field [1.00ms]
(pass) deepseek-v4-pro uses DeepSeek direct API max output cap on api.deepseek.com
(pass) deepseek-v4-pro keeps gateway routes on the lower output cap [4.00ms]
(pass) deepseek legacy aliases keep their documented provider caps [6.00ms]
(pass) deepseek-v4-pro clamps oversized max output overrides to the provider limit [2.00ms]
(pass) deepseek-v4-flash clamps oversized max output overrides to the provider limit [1.00ms]
(pass) gpt-4o uses provider-specific context and output caps [5.00ms]
(pass) gpt-4o clamps oversized max output overrides to the provider limit [2.00ms]
(pass) gpt-5.5 uses conservative Codex-route context window (issue `#1118`)
(pass) gpt-5.6 family pins the Codex effective input limit on the Codex route [13.00ms]
(pass) gpt-5.6 family keeps the full window on the direct-OpenAI route
(pass) gpt-5.4 family uses provider-specific context and output caps [7.00ms]
(pass) gpt-5.4 family keeps large max output overrides within provider limits [3.00ms]
(...
GitHub Actions: PR Checks / smoke-and-tests (24.11.x): fix(repl): make local interactive max-turns configurable
Conclusion: failure
##[group]src/components/agents/new-agent-creation/wizard-steps/wizardSteps.test.tsx:
(pass) ColorStep does not create a final agent when required wizard data is missing [33.00ms]
(pass) MemoryStep falls back to an empty system prompt when wizard data is incomplete [31.00ms]
(pass) ConfirmStep renders a fallback instead of a blank screen for incomplete wizard data [5.00ms]
##[endgroup]
2 tests skipped:
(skip) getContextEfficiencyAttachment > does not nudge GLM-5.2 below the model-aware pressure threshold
(skip) getContextEfficiencyAttachment > nudges small effective windows once above pressure threshold and repeat interval
7889 pass
2 skip
0 fail
22103 expect() calls
Ran 7891 tests across 660 files. [92.44s]
##[error]The operation was canceled.
GitHub Actions: PR Checks / 0_smoke-and-tests (22).txt: fix(repl): make local interactive max-turns configurable
Conclusion: failure
##[group]src/utils/context.test.ts:
(pass) calculateContextPercentages preserves tiny nonzero usage [1.00ms]
(pass) deepseek-v4-flash uses the gateway-safe output cap by default [7.00ms]
(pass) deepseek-v4-flash uses DeepSeek direct API max output cap on api.deepseek.com [1.00ms]
(pass) deepseek-v4-pro uses the gateway-safe output cap by default [7.00ms]
(pass) Ollama deepseek-v4-pro cloud variant uses DeepSeek V4 Pro runtime limits [2.00ms]
(pass) Ollama deepseek-v4-pro cloud variant is modeled as route catalog metadata [1.00ms]
(pass) Ollama deepseek-v4-pro cloud variant clamps oversized output token overrides
(pass) Ollama deepseek-v4-pro cloud variant does not inherit base-model env override prefixes [1.00ms]
(pass) Ollama deepseek-v4-pro cloud variant still honors exact env overrides [1.00ms]
(pass) OpenAI-compatible env override prefixes still match colon-tagged local models [5.00ms]
(pass) Ollama deepseek-v4-pro cloud variant keeps the local max_tokens transport field [1.00ms]
(pass) deepseek-v4-pro uses DeepSeek direct API max output cap on api.deepseek.com
(pass) deepseek-v4-pro keeps gateway routes on the lower output cap [4.00ms]
(pass) deepseek legacy aliases keep their documented provider caps [6.00ms]
(pass) deepseek-v4-pro clamps oversized max output overrides to the provider limit [2.00ms]
(pass) deepseek-v4-flash clamps oversized max output overrides to the provider limit [1.00ms]
(pass) gpt-4o uses provider-specific context and output caps [5.00ms]
(pass) gpt-4o clamps oversized max output overrides to the provider limit [2.00ms]
(pass) gpt-5.5 uses conservative Codex-route context window (issue `#1118`)
(pass) gpt-5.6 family pins the Codex effective input limit on the Codex route [13.00ms]
(pass) gpt-5.6 family keeps the full window on the direct-OpenAI route
(pass) gpt-5.4 family uses provider-specific context and output caps [7.00ms]
(pass) gpt-5.4 family keeps large max output overrides within provider limits [3.00ms]
(...
GitHub Actions: PR Checks / 3_smoke-and-tests (24.11.x).txt: fix(repl): make local interactive max-turns configurable
Conclusion: failure
##[group]src/components/agents/new-agent-creation/wizard-steps/wizardSteps.test.tsx:
(pass) ColorStep does not create a final agent when required wizard data is missing [33.00ms]
(pass) MemoryStep falls back to an empty system prompt when wizard data is incomplete [31.00ms]
(pass) ConfirmStep renders a fallback instead of a blank screen for incomplete wizard data [5.00ms]
##[endgroup]
2 tests skipped:
(skip) getContextEfficiencyAttachment > does not nudge GLM-5.2 below the model-aware pressure threshold
(skip) getContextEfficiencyAttachment > nudges small effective windows once above pressure threshold and repeat interval
7889 pass
2 skip
0 fail
22103 expect() calls
Ran 7891 tests across 660 files. [92.44s]
##[error]The operation was canceled.
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
TypeScript code in this repository must use strict mode and ESM imports.
**/*.{ts,tsx}: Add or update tests when a TypeScript or TSX change affects behavior.
Run the relevant TypeScript validation checks for changed code, includingbun run typecheckand, when applicable,bun run typecheck:type-tests.
Files:
src/screens/REPL.tsx
**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*: Preserve existing repository patterns unless intentionally refactoring them.
Keep changes small, readable, and focused; avoid broad rewrites or unrelated cleanup.
Do not reformat unrelated files, and keep comments useful and concise.
Update documentation when setup, commands, or user-facing behavior changes.
Review AI-generated changes for correctness, style consistency, unnecessary noise, and adherence to project architecture before submitting them.
Provider changes must follow the documented integration patterns indocs/integrations/overview.mdand the focused guides underdocs/integrations/how-to/.
When changing provider behavior, avoid breaking third-party providers and test the exact provider/model path changed when possible.
Provider pull requests must explicitly identify affected providers, limitations, and follow-up work.
Run the narrowest meaningful validation command for the touched area, and ensure relevant CI checks pass before merging.
Usebun installto install dependencies and the repository's Bun scripts for building, testing, smoke testing, and development.
Dependency changes require a concrete project benefit such as a bug fix, security issue, or approved feature; preference alone is insufficient.
Do not change the project's language, core runtime, or dependency stack, or introduce a new runtime, without prior maintainer agreement.
Keep each pull request focused on one issue or clearly scoped improvement and avoid bundling unrelated fixes, features, or refactors.
Files:
src/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/screens/REPL.tsx
🔇 Additional comments (3)
src/screens/REPL.tsx (3)
664-683: LGTM!
2856-3000: LGTM!
3312-3321: LGTM!Also applies to: 3396-3414, 3529-3531, 3575-3575
Scope the unknown-model context test to [context] warnings only so unrelated import-time debug logs do not fail CI, and clear turn env vars in both that test and replMaxTurnsProp setup to avoid cross-file pollution.
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/utils/context.test.ts`:
- Around line 540-541: Update the environment snapshot and cleanup used by the
test suite around the existing originalEnv and afterEach logic to include
OPENCLAUDE_MAX_TURNS and CLAUDE_CODE_MAX_TURNS, restoring each variable to its
pre-test value after the test completes while preserving the current deletion
within the test.
🪄 Autofix
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: 2f469147-c648-4220-b99f-bf528dba07ce
📒 Files selected for processing (2)
src/screens/replMaxTurnsProp.test.tssrc/utils/context.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: smoke-and-tests (22)
- GitHub Check: smoke-and-tests (24.11.x)
- GitHub Check: typecheck
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
TypeScript code in this repository must use strict mode and ESM imports.
**/*.{ts,tsx}: Add or update tests when a TypeScript or TSX change affects behavior.
Run the relevant TypeScript validation checks for changed code, includingbun run typecheckand, when applicable,bun run typecheck:type-tests.
Files:
src/utils/context.test.tssrc/screens/replMaxTurnsProp.test.ts
**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*: Preserve existing repository patterns unless intentionally refactoring them.
Keep changes small, readable, and focused; avoid broad rewrites or unrelated cleanup.
Do not reformat unrelated files, and keep comments useful and concise.
Update documentation when setup, commands, or user-facing behavior changes.
Review AI-generated changes for correctness, style consistency, unnecessary noise, and adherence to project architecture before submitting them.
Provider changes must follow the documented integration patterns indocs/integrations/overview.mdand the focused guides underdocs/integrations/how-to/.
When changing provider behavior, avoid breaking third-party providers and test the exact provider/model path changed when possible.
Provider pull requests must explicitly identify affected providers, limitations, and follow-up work.
Run the narrowest meaningful validation command for the touched area, and ensure relevant CI checks pass before merging.
Usebun installto install dependencies and the repository's Bun scripts for building, testing, smoke testing, and development.
Dependency changes require a concrete project benefit such as a bug fix, security issue, or approved feature; preference alone is insufficient.
Do not change the project's language, core runtime, or dependency stack, or introduce a new runtime, without prior maintainer agreement.
Keep each pull request focused on one issue or clearly scoped improvement and avoid bundling unrelated fixes, features, or refactors.
Files:
src/utils/context.test.tssrc/screens/replMaxTurnsProp.test.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/utils/context.test.tssrc/screens/replMaxTurnsProp.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/context.test.tssrc/screens/replMaxTurnsProp.test.ts
🔇 Additional comments (2)
src/screens/replMaxTurnsProp.test.ts (1)
1-1: LGTM!Also applies to: 74-76
src/utils/context.test.ts (1)
564-570: LGTM!
Add a runHeadless-to-ask regression that asserts maxTurns 0 is forwarded through the headless print path, and restore OPENCLAUDE_MAX_TURNS env vars in context.test.ts after the unknown-model fallback test mutates them.
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/cli/printMaxTurns.test.ts`:
- Around line 197-213: Update the test around runHeadless and askSpy so
runHeadless failures are not swallowed: create the promise without catch
suppression, then use a try/finally structure that performs the wait and
assertion in the try block and always awaits the run promise’s settlement before
restoring askSpy in finally. Preserve the maxTurns assertion while ensuring both
test failures and asynchronous cleanup remain visible and the spy is
unconditionally restored.
🪄 Autofix
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: ab3d3199-7ab7-4d53-a994-07d773a8a1c3
📒 Files selected for processing (2)
src/cli/printMaxTurns.test.tssrc/utils/context.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: smoke-and-tests (22)
- GitHub Check: typecheck
- GitHub Check: smoke-and-tests (24.11.x)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
TypeScript code in this repository must use strict mode and ESM imports.
**/*.{ts,tsx}: Add or update tests when a TypeScript or TSX change affects behavior.
Run the relevant TypeScript validation checks for changed code, includingbun run typecheckand, when applicable,bun run typecheck:type-tests.
Files:
src/cli/printMaxTurns.test.tssrc/utils/context.test.ts
**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*: Preserve existing repository patterns unless intentionally refactoring them.
Keep changes small, readable, and focused; avoid broad rewrites or unrelated cleanup.
Do not reformat unrelated files, and keep comments useful and concise.
Update documentation when setup, commands, or user-facing behavior changes.
Review AI-generated changes for correctness, style consistency, unnecessary noise, and adherence to project architecture before submitting them.
Provider changes must follow the documented integration patterns indocs/integrations/overview.mdand the focused guides underdocs/integrations/how-to/.
When changing provider behavior, avoid breaking third-party providers and test the exact provider/model path changed when possible.
Provider pull requests must explicitly identify affected providers, limitations, and follow-up work.
Run the narrowest meaningful validation command for the touched area, and ensure relevant CI checks pass before merging.
Usebun installto install dependencies and the repository's Bun scripts for building, testing, smoke testing, and development.
Dependency changes require a concrete project benefit such as a bug fix, security issue, or approved feature; preference alone is insufficient.
Do not change the project's language, core runtime, or dependency stack, or introduce a new runtime, without prior maintainer agreement.
Keep each pull request focused on one issue or clearly scoped improvement and avoid bundling unrelated fixes, features, or refactors.
Files:
src/cli/printMaxTurns.test.tssrc/utils/context.test.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/cli/printMaxTurns.test.tssrc/utils/context.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/cli/printMaxTurns.test.tssrc/utils/context.test.ts
🔇 Additional comments (1)
src/cli/printMaxTurns.test.ts (1)
1-17: 📐 Maintainability & Code QualityRun the required validation before merge.
Validation results are not included. Run focused tests for both files and the required TypeScript checks.
src/cli/printMaxTurns.test.ts#L1-L17: run the focused headless forwarding test.src/utils/context.test.ts#L1-L12: run the focused environment-isolation test.bun test src/cli/printMaxTurns.test.ts src/utils/context.test.ts bun run typecheck if jq -e '.scripts["typecheck:type-tests"]' package.json >/dev/null; then bun run typecheck:type-tests fiAs per coding guidelines, “Run the relevant TypeScript validation checks for changed code.” As per path instructions, “Run targeted tests plus typecheck, build, smoke/check, and security/privacy diagnostics as applicable; report exact commands.”
Mock headless stdout so runHeadless completes cleanly without leaking output, restore spies in finally, and centralize turn-env cleanup in context.test beforeEach.
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/cli/printMaxTurns.test.ts`:
- Around line 201-219: Update the cleanup in the test’s finally block around
runPromise so it awaits runPromise.catch(() => {}) before restoring askSpy and
stdoutSpy, ensuring the pending headless run settles even when waitForAskCall or
the assertion fails.
🪄 Autofix
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: 65e6a6d4-b61d-457a-9d45-e300913f4d6b
📒 Files selected for processing (2)
src/cli/printMaxTurns.test.tssrc/utils/context.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: smoke-and-tests (22)
- GitHub Check: smoke-and-tests (24.11.x)
- GitHub Check: typecheck
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
TypeScript code in this repository must use strict mode and ESM imports.
**/*.{ts,tsx}: Add or update tests when a TypeScript or TSX change affects behavior.
Run the relevant TypeScript validation checks for changed code, includingbun run typecheckand, when applicable,bun run typecheck:type-tests.
Files:
src/cli/printMaxTurns.test.tssrc/utils/context.test.ts
**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*: Preserve existing repository patterns unless intentionally refactoring them.
Keep changes small, readable, and focused; avoid broad rewrites or unrelated cleanup.
Do not reformat unrelated files, and keep comments useful and concise.
Update documentation when setup, commands, or user-facing behavior changes.
Review AI-generated changes for correctness, style consistency, unnecessary noise, and adherence to project architecture before submitting them.
Provider changes must follow the documented integration patterns indocs/integrations/overview.mdand the focused guides underdocs/integrations/how-to/.
When changing provider behavior, avoid breaking third-party providers and test the exact provider/model path changed when possible.
Provider pull requests must explicitly identify affected providers, limitations, and follow-up work.
Run the narrowest meaningful validation command for the touched area, and ensure relevant CI checks pass before merging.
Usebun installto install dependencies and the repository's Bun scripts for building, testing, smoke testing, and development.
Dependency changes require a concrete project benefit such as a bug fix, security issue, or approved feature; preference alone is insufficient.
Do not change the project's language, core runtime, or dependency stack, or introduce a new runtime, without prior maintainer agreement.
Keep each pull request focused on one issue or clearly scoped improvement and avoid bundling unrelated fixes, features, or refactors.
Files:
src/cli/printMaxTurns.test.tssrc/utils/context.test.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/cli/printMaxTurns.test.tssrc/utils/context.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/cli/printMaxTurns.test.tssrc/utils/context.test.ts
🔇 Additional comments (2)
src/cli/printMaxTurns.test.ts (1)
15-15: LGTM!Also applies to: 186-188
src/utils/context.test.ts (1)
35-36: LGTM!Also applies to: 58-59, 149-158, 576-582
Separate model-request lifecycle from provider dispatch acceptance so interruption correction arms before async prep, notification ownership commits only after dispatch, deferred turn caps restore on every abort path, and foreground work stays blocked while handoff preparation runs.
|
#1993 needs to land before this can move forward. |
2 participants
Summary
/config→ Max turns (interactive), CLI--max-turns, andOPENCLAUDE_MAX_TURNS, while retaining the default of50and keeping remote-backed sessions out of this local cap.--max-turns 0as unlimited local turns and emit a cautionary warning.OPENCLAUDE_MAX_TURNSvalues from diagnostics.Session Ctrl+B background handoff remains intentionally disabled in
SessionBackgroundHintand is not part of the shipped user-facing change in this PR.Root cause
Local interactive REPL sessions hard-capped each prompt at 50 turns with no supported override. Long autonomous local workflows (especially models that take many small tool steps) hit that ceiling and forced manual intervention. The existing
--max-turnsflag was documented/wired as print/headless-only, and there was no env or/configpath for interactive use.Impact
/config,openclaude --max-turns N, orOPENCLAUDE_MAX_TURNS=N.--printmax-turn behavior remains on its existingmaxTurnscontract.--remotesessions remain outside the local REPL query-loop cap.Testing
bun test --feature=UNATTENDED_RETRY --max-concurrency=1 src/query/agentStepLimit.test.ts src/screens/replMaxTurnsProp.test.ts— 39 passed, 0 failedbun run typecheckbun run buildnode dist/cli.mjs --versionbun run verify:privacybun run scripts/pr-intent-scan.ts --base upstream/mainNotes
b1ac0c8ecc51b86bf94512f2ca8aa4ace199c3a8CONTRIBUTING.mdandAGENTS.md.Fixes #2079