Repository navigation
feat(agents): add per-agent step limits - #1815
Conversation
Add maxSteps agent configuration for markdown, JSON, plugin, and SDK agent definitions. Enforce the limit in subagent query execution by blocking over-limit tool calls, preserving a no-tool summary turn, and recording an agent_step_limit terminal reason. Add focused coverage for default behavior, invalid values, multi-turn accumulation, plugin parsing, failure-loop interaction, and summary-tool blocking.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
💤 Files with no reviewable changes (1)
📜 Recent review details
|
| Layer / File(s) | Summary |
|---|---|
Contracts and markers src/query/agentStepLimit.ts, src/query/transitions.ts, src/types/message.ts, src/query/toolFailureLoopGuard.ts, src/utils/messages.ts, src/services/api/claude.ts, src/query/toolFailureLoopGuard.test.ts |
Adds the step-limit terminal reason, message metadata, synthetic tool-result marker, API normalization override, and failure-guard handling for marked results. |
SDK types and agent loading src/utils/frontmatterParser.ts, src/entrypoints/sdk/coreSchemas.ts, src/entrypoints/sdk.d.ts, README.md, src/tools/AgentTool/loadAgentsDir.ts, src/utils/plugins/loadPluginAgents.ts, src/tools/AgentTool/loadAgentsDir.test.ts, src/utils/plugins/loadPluginAgents.test.ts, tests/sdk/package-consumer-types.test.ts |
Adds maxSteps to SDK types, schemas, frontmatter parsing, JSON parsing, plugin agent loading, README guidance, and related tests. |
SDK agent injection src/entrypoints/sdk/agentDefinitions.ts, src/entrypoints/sdk/query.ts, src/entrypoints/sdk/v2.ts, src/entrypoints/sdk/agentDefinitions.test.ts, tests/sdk/query-happy-path.test.ts, tests/sdk/sdk-v2-lifecycle.test.ts |
Adds SDK agent definition builders and merges, injects SDK agents during query/session startup, and drains injection failures before prompts. |
Query loop enforcement src/query.ts, src/query/agentStepLimit.test.ts |
Tracks agentStepLimit through the query loop, blocks excess tool calls, emits synthetic tool results, requests summary turns, and returns reason: 'agent_step_limit'. |
Run-agent wiring and counting src/tools/AgentTool/agentToolUtils.ts, src/tools/AgentTool/runAgent.ts |
Adds maxSteps to runAgent, forwards it into query(), captures the terminal result, and excludes marked step-limit tool uses from counting. |
SDK source UI src/components/ContextVisualization.tsx, src/components/agents/AgentsList.tsx, src/components/agents/types.ts, src/components/agents/utils.ts, src/components/agents/AgentsMenu.tsx, src/components/agents/agentFileUtils.ts, src/utils/analyzeContext.ts, src/utils/settings/constants.ts, src/tools/AgentTool/agentDisplay.ts, src/components/agents/AgentsMenu.test.tsx |
Adds the sdk source across agent grouping, display helpers, file-path handling, and agent list/menu visibility rules. |
Estimated code review effort
🎯 5 (Critical) | ⏱️ ~120 minutes
Suggested reviewers
- jatmn
- kevincodex1
🚥 Pre-merge checks | ✅ 6 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 13.79% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (6 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title is concise and accurately describes the main change: per-agent step limits. |
| Description check | ✅ Passed | The description covers summary, implementation, tests, and risk, though it omits the template's Impact and Notes sections. |
| Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Risk Surface Disclosed | ✅ Passed | PR touches SDK/plugin/MCP paths, and its Risk section says defaults are unchanged and no blocker is introduced. |
| No Hidden Policy Change | ✅ Passed | Policy-adjacent changes (maxSteps enforcement, SDK agent precedence, MCP/tool-pool handling) are directly part of the feature and documented; no hidden unrelated policy toggle found. |
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/entrypoints/sdk/query.ts (1)
545-564: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winValidate SDK
agents[*].maxStepsbefore injecting it.This path copies
def.maxStepsstraight intoinjectAgents()with only a truthiness check, so SDK callers can still pass invalid values like-1,1.5, or'2'at runtime. That breaks the positive-integer contract you added for markdown/JSON/plugin agents and makes SDK-defined agents behave differently from every other agent source.Suggested fix
+import { AgentDefinitionSchema } from './coreSchemas.js' + if (self.userAgents && Object.keys(self.userAgents).length > 0) { - const userAgents: Array<{ - agentType: string - whenToUse: string - getSystemPrompt: () => string - tools?: string[] - disallowedTools?: string[] - model?: string - maxTurns?: number - maxSteps?: number - }> = Object.entries(self.userAgents).map(([name, def]) => ({ - agentType: name, - whenToUse: def.description ?? name, - getSystemPrompt: () => def.prompt ?? '', - ...(def.tools ? { tools: def.tools } : {}), - ...(def.disallowedTools ? { disallowedTools: def.disallowedTools } : {}), - ...(def.model ? { model: def.model } : {}), - ...(def.maxTurns ? { maxTurns: def.maxTurns } : {}), - ...(def.maxSteps ? { maxSteps: def.maxSteps } : {}), - })) + const userAgents = Object.entries(self.userAgents).flatMap(([name, def]) => { + const parsed = AgentDefinitionSchema().safeParse(def) + if (!parsed.success) { + self.pushAgentFailure({ + type: 'agent_load_failure', + stage: 'injection', + error_message: `Invalid SDK agent '${name}': ${parsed.error.message}`, + }) + return [] + } + + return [{ + agentType: name, + whenToUse: parsed.data.description, + getSystemPrompt: () => parsed.data.prompt, + ...(parsed.data.tools ? { tools: parsed.data.tools } : {}), + ...(parsed.data.disallowedTools ? { disallowedTools: parsed.data.disallowedTools } : {}), + ...(parsed.data.model ? { model: parsed.data.model } : {}), + ...(parsed.data.maxTurns !== undefined ? { maxTurns: parsed.data.maxTurns } : {}), + ...(parsed.data.maxSteps !== undefined ? { maxSteps: parsed.data.maxSteps } : {}), + }] + })🤖 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/entrypoints/sdk/query.ts` around lines 545 - 564, The SDK agent mapping in query.ts is passing def.maxSteps directly into injectAgents() without validating it, so invalid values can slip through. Update the userAgents-to-agent conversion in the self.userAgents branch to validate maxSteps the same way as other agent sources, ensuring it is a positive integer before including it. Keep the fix localized to the Object.entries(self.userAgents).map(...) logic and the agent shape used by injectAgents().
🤖 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 `@README.md`:
- Around line 280-293: Clarify the `maxSteps` docs in the agent configuration
section: `maxSteps` must be a positive integer, and invalid values like `0` or
malformed input are ignored and fall back to the default behavior. Update the
explanation near the `maxSteps` field in the `bounded-researcher` example to
reflect the actual validation behavior so users know only positive integers cap
tool-use steps.
In `@src/query.ts`:
- Around line 2029-2035: The forced-summary path in query.ts is returning too
early from the agent-step termination branch, which prevents the final summary
from ever being produced. Update the logic around
shouldTerminateAgentStepSummary and nextAgentStepLimit so that when
summaryRequested is already true and tool_use is still emitted, the loop stays
alive for one more summary-only turn or synthesizes the final summary instead of
exiting immediately. Keep the fix localized to the summary/termination flow in
the query loop and preserve the existing agent step limit handling for
non-summary cases.
In `@src/query/agentStepLimit.test.ts`:
- Around line 266-270: The agent step limit tests currently only assert the
returned reason, so regressions in the terminal payload fields can slip through.
Update the affected cases in agentStepLimit.test.ts to also assert the full
`agent_step_limit` payload with `toMatchObject(...)`, covering fields like
`turnCount`, `stepsUsed`, and `maxSteps` on the returned result. Use the
existing terminal result assertions around `returned` to locate the right spots
and add at least one focused payload check in each risky scenario.
In `@src/tools/AgentTool/loadAgentsDir.test.ts`:
- Around line 206-225: The duplicate-name merge in
getAgentDefinitionsWithOverrides is still prioritizing the user-scoped agent
over the project-scoped one, so update the precedence logic in loadAgentsDir to
ensure projectSettings entries win when agentType matches. Check the code path
that combines loaded agent definitions and resolves conflicts, and make sure the
project agent’s maxSteps and source are preserved instead of being overwritten
by the user agent.
---
Outside diff comments:
In `@src/entrypoints/sdk/query.ts`:
- Around line 545-564: The SDK agent mapping in query.ts is passing def.maxSteps
directly into injectAgents() without validating it, so invalid values can slip
through. Update the userAgents-to-agent conversion in the self.userAgents branch
to validate maxSteps the same way as other agent sources, ensuring it is a
positive integer before including it. Keep the fix localized to the
Object.entries(self.userAgents).map(...) logic and the agent shape used by
injectAgents().
🪄 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: 06066b1d-e33e-4274-b341-6ddf10dd80be
⛔ Files ignored due to path filters (1)
src/entrypoints/sdk/coreTypes.generated.tsis excluded by!src/entrypoints/sdk/coreTypes.generated.ts
📒 Files selected for processing (20)
README.mdsrc/entrypoints/sdk.d.tssrc/entrypoints/sdk/coreSchemas.tssrc/entrypoints/sdk/query.tssrc/query.tssrc/query/agentStepLimit.test.tssrc/query/agentStepLimit.tssrc/query/toolFailureLoopGuard.test.tssrc/query/toolFailureLoopGuard.tssrc/query/transitions.tssrc/services/api/claude.tssrc/tools/AgentTool/agentToolUtils.tssrc/tools/AgentTool/loadAgentsDir.test.tssrc/tools/AgentTool/loadAgentsDir.tssrc/tools/AgentTool/runAgent.tssrc/types/message.tssrc/utils/frontmatterParser.tssrc/utils/messages.tssrc/utils/plugins/loadPluginAgents.test.tssrc/utils/plugins/loadPluginAgents.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (18)
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript with strict mode and ESM imports
Files:
src/query/agentStepLimit.tssrc/types/message.tssrc/utils/plugins/loadPluginAgents.tssrc/utils/plugins/loadPluginAgents.test.tssrc/tools/AgentTool/loadAgentsDir.test.tssrc/entrypoints/sdk.d.tssrc/tools/AgentTool/agentToolUtils.tssrc/utils/frontmatterParser.tssrc/utils/messages.tssrc/services/api/claude.tssrc/entrypoints/sdk/query.tssrc/query/transitions.tssrc/query/toolFailureLoopGuard.test.tssrc/tools/AgentTool/loadAgentsDir.tssrc/query/toolFailureLoopGuard.tssrc/query/agentStepLimit.test.tssrc/tools/AgentTool/runAgent.tssrc/entrypoints/sdk/coreSchemas.tssrc/query.ts
**/*.{ts,tsx,js,jsx,py,json,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow the existing code style in the touched files
Files:
src/query/agentStepLimit.tssrc/types/message.tsREADME.mdsrc/utils/plugins/loadPluginAgents.tssrc/utils/plugins/loadPluginAgents.test.tssrc/tools/AgentTool/loadAgentsDir.test.tssrc/entrypoints/sdk.d.tssrc/tools/AgentTool/agentToolUtils.tssrc/utils/frontmatterParser.tssrc/utils/messages.tssrc/services/api/claude.tssrc/entrypoints/sdk/query.tssrc/query/transitions.tssrc/query/toolFailureLoopGuard.test.tssrc/tools/AgentTool/loadAgentsDir.tssrc/query/toolFailureLoopGuard.tssrc/query/agentStepLimit.test.tssrc/tools/AgentTool/runAgent.tssrc/entrypoints/sdk/coreSchemas.tssrc/query.ts
**/*.{ts,tsx,js,jsx,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Keep comments useful and concise
Files:
src/query/agentStepLimit.tssrc/types/message.tssrc/utils/plugins/loadPluginAgents.tssrc/utils/plugins/loadPluginAgents.test.tssrc/tools/AgentTool/loadAgentsDir.test.tssrc/entrypoints/sdk.d.tssrc/tools/AgentTool/agentToolUtils.tssrc/utils/frontmatterParser.tssrc/utils/messages.tssrc/services/api/claude.tssrc/entrypoints/sdk/query.tssrc/query/transitions.tssrc/query/toolFailureLoopGuard.test.tssrc/tools/AgentTool/loadAgentsDir.tssrc/query/toolFailureLoopGuard.tssrc/query/agentStepLimit.test.tssrc/tools/AgentTool/runAgent.tssrc/entrypoints/sdk/coreSchemas.tssrc/query.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow TypeScript strict mode and type safety practices by running typecheck before submitting
Files:
src/query/agentStepLimit.tssrc/types/message.tssrc/utils/plugins/loadPluginAgents.tssrc/utils/plugins/loadPluginAgents.test.tssrc/tools/AgentTool/loadAgentsDir.test.tssrc/entrypoints/sdk.d.tssrc/tools/AgentTool/agentToolUtils.tssrc/utils/frontmatterParser.tssrc/utils/messages.tssrc/services/api/claude.tssrc/entrypoints/sdk/query.tssrc/query/transitions.tssrc/query/toolFailureLoopGuard.test.tssrc/tools/AgentTool/loadAgentsDir.tssrc/query/toolFailureLoopGuard.tssrc/query/agentStepLimit.test.tssrc/tools/AgentTool/runAgent.tssrc/entrypoints/sdk/coreSchemas.tssrc/query.ts
**
⚙️ CodeRabbit configuration file
**: # AGENTS.md - AI Agent Coding GuideThis guide is for AI coding agents working in the OpenClaude repository. Read it before changing code, and also follow CONTRIBUTING.md for contributor policy, PR expectations, review follow-up, and project scope.
Project Snapshot
OpenClaude is a coding-agent CLI for cloud and local model providers. It supports OpenAI-compatible APIs, Anthropic, Gemini, DeepSeek, Ollama, MCP, local backends, slash commands, tools, agents, and a React/Ink terminal UI.
The installed CLI runs on Node.js
>=22.0.0. Bun is used for source builds, scripts, dependency management, and tests.Work Style
- Keep changes focused on one problem.
- Prefer existing patterns in the file or nearby module.
- Avoid unrelated formatting, renames, dependency changes, or broad rewrites.
- Add or update tests when behavior changes.
- Update docs when setup, commands, provider behavior, or user-facing behavior changes.
- For new features, larger refactors, dependencies, or runtime changes, follow the issue-first guidance in CONTRIBUTING.md.
Stack And Conventions
- TypeScript with strict mode and ESM imports.
- React + Ink for terminal UI.
- Bun lockfile and Bun scripts for development workflows.
- Node runtime for the built CLI.
- Python exists for legacy/local-provider helper code. Do not add new Python code or expand Python-based features unless a maintainer explicitly approves that direction.
Common libraries and patterns:
chalkfor terminal color.commanderfor CLI argument parsing.execafor child processes.- Existing service, provider, settings, permission, and UI patterns over new abstractions.
Repository Map
src/commands/- slash and CLI command implementations.src/components/- React/Ink UI components.src/services/- API, MCP, OAuth, wiki, voice, and other service integrations.src/tools/- tool implementations.src/utils/- shared utilities.- `src/integration...
Files:
src/query/agentStepLimit.tssrc/types/message.tsREADME.mdsrc/utils/plugins/loadPluginAgents.tssrc/utils/plugins/loadPluginAgents.test.tssrc/tools/AgentTool/loadAgentsDir.test.tssrc/entrypoints/sdk.d.tssrc/tools/AgentTool/agentToolUtils.tssrc/utils/frontmatterParser.tssrc/utils/messages.tssrc/services/api/claude.tssrc/entrypoints/sdk/query.tssrc/query/transitions.tssrc/query/toolFailureLoopGuard.test.tssrc/tools/AgentTool/loadAgentsDir.tssrc/query/toolFailureLoopGuard.tssrc/query/agentStepLimit.test.tssrc/tools/AgentTool/runAgent.tssrc/entrypoints/sdk/coreSchemas.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/query/agentStepLimit.tssrc/types/message.tsREADME.mdsrc/utils/plugins/loadPluginAgents.tssrc/utils/plugins/loadPluginAgents.test.tssrc/tools/AgentTool/loadAgentsDir.test.tssrc/entrypoints/sdk.d.tssrc/tools/AgentTool/agentToolUtils.tssrc/utils/frontmatterParser.tssrc/utils/messages.tssrc/services/api/claude.tssrc/entrypoints/sdk/query.tssrc/query/transitions.tssrc/query/toolFailureLoopGuard.test.tssrc/tools/AgentTool/loadAgentsDir.tssrc/query/toolFailureLoopGuard.tssrc/query/agentStepLimit.test.tssrc/tools/AgentTool/runAgent.tssrc/entrypoints/sdk/coreSchemas.tssrc/query.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:
README.md
{src/services/**/*.ts,src/utils/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
execafor child processes
Files:
src/utils/plugins/loadPluginAgents.tssrc/utils/plugins/loadPluginAgents.test.tssrc/utils/frontmatterParser.tssrc/utils/messages.tssrc/services/api/claude.ts
src/{skills,utils/plugins,services/mcp}/**
⚙️ CodeRabbit configuration file
src/{skills,utils/plugins,services/mcp}/**: Review skill/plugin/MCP behavior as a trust boundary. Check registry fetches, local and remote installs, path normalization, hash verification, revocation/trust metadata, tools_required handling, config-home behavior, and startup-time loading. Block on path traversal risk, unverified downloads, silent trust promotion, or unexpected code/tool activation.
Files:
src/utils/plugins/loadPluginAgents.tssrc/utils/plugins/loadPluginAgents.test.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add or update tests when the change affects behavior
Files:
src/utils/plugins/loadPluginAgents.test.tssrc/tools/AgentTool/loadAgentsDir.test.tssrc/query/toolFailureLoopGuard.test.tssrc/query/agentStepLimit.test.ts
**/*.test.{ts,tsx,js}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Test the exact provider/model path you changed when possible
Files:
src/utils/plugins/loadPluginAgents.test.tssrc/tools/AgentTool/loadAgentsDir.test.tssrc/query/toolFailureLoopGuard.test.tssrc/query/agentStepLimit.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/plugins/loadPluginAgents.test.tssrc/tools/AgentTool/loadAgentsDir.test.tssrc/query/toolFailureLoopGuard.test.tssrc/query/agentStepLimit.test.ts
src/{components/permissions,utils/permissions,hooks/toolPermission,tools,entrypoints/sdk}/**
⚙️ CodeRabbit configuration file
src/{components/permissions,utils/permissions,hooks/toolPermission,tools,entrypoints/sdk}/**: Review permission prompts, auto-allow logic, sandbox behavior, SDK permission schemas, shell/PowerShell execution, and background execution paths as security-sensitive. Block on bypasses, unclear trust boundaries, unsafe path handling, missing user visibility, or changes that broaden allowed behavior without an explicit maintainer decision.
Files:
src/tools/AgentTool/loadAgentsDir.test.tssrc/tools/AgentTool/agentToolUtils.tssrc/entrypoints/sdk/query.tssrc/tools/AgentTool/loadAgentsDir.tssrc/tools/AgentTool/runAgent.tssrc/entrypoints/sdk/coreSchemas.ts
{src/commands/**/*.ts,src/services/**/*.ts,src/entrypoints/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
chalkfor terminal color in CLI code
Files:
src/entrypoints/sdk.d.tssrc/services/api/claude.tssrc/entrypoints/sdk/query.tssrc/entrypoints/sdk/coreSchemas.ts
{src/commands/**/*.ts,src/entrypoints/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
commanderfor CLI argument parsing
Files:
src/entrypoints/sdk.d.tssrc/entrypoints/sdk/query.tssrc/entrypoints/sdk/coreSchemas.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/entrypoints/sdk.d.tssrc/entrypoints/sdk/query.tssrc/entrypoints/sdk/coreSchemas.ts
{src/integrations/**/*.ts,src/services/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Test the exact provider/model path you changed when possible for provider modifications
Files:
src/services/api/claude.ts
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}
⚙️ CodeRabbit configuration file
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}: Review provider routing, model selection, env precedence, auth/token handling, OpenAI-compatible shims, retries, proxy behavior, and outbound HTTP behavior with high scrutiny. Block on silent default changes, hidden fallback expansion, credential reuse mistakes, hardcoded provider assumptions, or new network reach that is not intentional and documented.
Files:
src/services/api/claude.ts
🪛 GitHub Actions: PR Checks / 1_smoke-and-tests.txt
src/tools/AgentTool/loadAgentsDir.test.ts
[error] 225-225: Test failed in 'agent definition loading' > loads maxSteps from markdown agent frontmatter. expect(received).toBe(expected) failed: Expected maxSteps to be 5 but received 1.
🪛 GitHub Actions: PR Checks / smoke-and-tests
src/tools/AgentTool/loadAgentsDir.test.ts
[error] 225-225: Test failed in 'loads maxSteps from markdown agent frontmatter'. expect(received).toBe(expected) failed: Expected 5, Received 1.
🪛 GitHub Check: smoke-and-tests
src/tools/AgentTool/loadAgentsDir.test.ts
[failure] 225-225: error: expect(received).toBe(expected)
Expected: 5
Received: 1
at <anonymous> (/home/runner/work/openclaude/openclaude/src/tools/AgentTool/loadAgentsDir.test.ts:225:29)
🔇 Additional comments (5)
src/utils/messages.ts (1)
471-490: LGTM!Also applies to: 528-531
src/services/api/claude.ts (1)
721-721: LGTM!Also applies to: 1298-1301
src/query/toolFailureLoopGuard.test.ts (1)
25-49: LGTM!Also applies to: 283-310
src/tools/AgentTool/loadAgentsDir.test.ts (1)
162-204: LGTM!Also applies to: 228-242
src/utils/plugins/loadPluginAgents.test.ts (1)
16-142: LGTM!
3ed4b00 to
011f081
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/entrypoints/sdk/query.ts`:
- Around line 546-557: Avoid mutating the AppState-held agent list before
injection succeeds: in `injectAgents()` the `agentDefs` object already stored on
`self` is being modified when `agentDefs.activeAgents.push(...userAgents)` runs,
so failed injection leaks partial state. Build a separate merged agent list (or
clone `agentDefs`/`activeAgents`) for the injection attempt, and only write back
to the AppState-held object after the injection path completes successfully;
keep the existing `buildSdkUserAgents()` and `pushAgentFailure()` behavior
unchanged.
In `@src/query.ts`:
- Around line 283-299: The hasAssistantSummaryText helper is too strict because
it checks each text block independently, so a valid summary split across
multiple text parts can be missed. Update hasAssistantSummaryText in
src/query.ts to aggregate all text content from assistantMessage.message.content
first, then perform the marker check against the combined string instead of
per-part, so split replies are still recognized as having an assistant summary.
🪄 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: dd818015-a0d2-4f1d-938e-aa209712c005
📒 Files selected for processing (7)
README.mdsrc/entrypoints/sdk/agentDefinitions.test.tssrc/entrypoints/sdk/agentDefinitions.tssrc/entrypoints/sdk/query.tssrc/query.tssrc/query/agentStepLimit.test.tstests/sdk/query-happy-path.test.ts
📜 Review details
⚠️ CI failures not shown inline (2)
GitHub Actions: PR Checks / smoke-and-tests: feat(agents): add per-agent step limits
Conclusion: failure
behavior > increments while push in flight and resets after cap via onDebounceFire [2.00ms]
(pass) rescheduleCount behavior > clears pushInProgress when executePush completes [1.00ms]
(pass) rescheduleCount behavior > skips the capped follow-up push when suppression is set mid-flight [1.00ms]
(pass) rescheduleCount behavior > capped reschedule chains follow-up push after in-flight push completes [2.00ms]
(pass) rescheduleCount behavior > does not queue multiple duplicate follow-up pushes when one is already queued [1.00ms]
(pass) executePush identity safety > clears currentPushPromise when it was set to the executing promise [1.00ms]
(pass) executePush identity safety > preserves currentPushPromise when replaced during yield point [4.00ms]
##[endgroup]
##[group]src/services/tips/tipLink.test.ts:
(pass) renderSponsorLink > hyperlinks supported: name is an OSC 8 link, no trailing raw url [1.00ms]
(pass) renderSponsorLink > hyperlinks NOT supported: plain name + dimmed url trailing
(pass) renderSponsorLink > no url → plain name, nothing trailing (either branch)
(pass) renderSponsorLink > strips control chars from the advertiser name (no escape injection)
(pass) renderSponsorLink > rejects non-http(s) URLs (javascript:/file:) → no link
(pass) renderSponsorLink > strips control chars from the url before validating it
(pass) renderSponsorLink > a malicious url cannot inject extra escape sequences into the output
##[endgroup]
##[group]src/services/tips/gitlawbEarn.test.ts:
(pass) gitlawb earning tips > disabled by default (no ads config)
(pass) gitlawb earning tips > enabled once /ads on set enabled + earnCode
(pass) gitlawb earning tips > cadence: every 2nd eligible slot by default
(pass) gitlawb earning tips > OPENCLAUDE_ADS_TIP_EVERY=1 shows every turn
(pass) gitlawb earning tips > disabled → never shows and never increments the counter
(pass) gitlawb earning tips > content falls back to a static line when the ads service is unreachable [1.00ms...
GitHub Actions: PR Checks / 0_smoke-and-tests.txt: feat(agents): add per-agent step limits
Conclusion: failure
trims OPENGATEWAY_API_KEY before bearer auth [5.00ms]
(pass) gitlawb opengateway provider flag ignores blank OPENGATEWAY_API_KEY and uses OPENAI_API_KEY fallback [6.00ms]
(pass) gitlawb opengateway provider flag sends OPENGATEWAY_API_KEY to OPENGATEWAY_BASE_URL override [1.00ms]
(pass) gitlawb opengateway provider flag sends OPENGATEWAY_API_KEY to custom OPENAI_BASE_URL fallback [2.00ms]
(pass) gitlawb opengateway provider flag prefers OPENGATEWAY_API_KEY over generic OPENAI_API_KEY for custom base URL [1.00ms]
(pass) gitlawb opengateway provider flag prefers OPENGATEWAY_API_KEY over generic OPENAI_API_KEYS pool [2.00ms]
(pass) gitlawb opengateway provider flag uses generic OPENAI_API_KEYS pool before generic OPENAI_API_KEY fallback [1.00ms]
(pass) gitlawb opengateway stored provider profile key becomes bearer auth [2.00ms]
(pass) openai route still sends OPENAI_API_KEY as bearer auth [5.00ms]
(pass) OPENAI_API_KEYS rejects placeholder values before sending requests [7.00ms]
(pass) OPENAI_API_KEYS rotates to the next key on rate-limit failure [7.00ms]
(pass) comma-separated OPENAI_API_KEY rotates to the next key on rate-limit failure [3.00ms]
(pass) OPENAI_API_KEYS does not rotate through pool on provider 5xx outage [4.00ms]
(pass) OPENAI_API_KEYS preserves cooldown state across client requests [6.00ms]
(pass) OPENAI_API_KEYS rotates Azure api-key auth on auth failure [4.00ms]
(pass) OPENAI_API_KEYS does not reuse auth-disabled credentials across client requests [6.00ms]
(pass) OPENAI_API_KEYS permanently evicts 403 auth failures [7.00ms]
(pass) does not use BNKR_API_KEY for non-Bankr OpenAI-compatible routes [2.00ms]
(pass) preserves Gemini tool call extra_content from streaming chunks [1.00ms]
(pass) preserves Gemini thought signature from streaming delta extra_content [1.00ms]
(pass) preserves Gemini thought signature from non-streaming message extra_content [1.00ms]
(pass) converts Gemini raw tool-call text into streaming tool_use blocks ...
🧰 Additional context used
📓 Path-based instructions (14)
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript with strict mode and ESM imports
Files:
src/entrypoints/sdk/agentDefinitions.test.tssrc/entrypoints/sdk/agentDefinitions.tssrc/entrypoints/sdk/query.tssrc/query/agentStepLimit.test.tssrc/query.ts
{src/commands/**/*.ts,src/services/**/*.ts,src/entrypoints/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
chalkfor terminal color in CLI code
Files:
src/entrypoints/sdk/agentDefinitions.test.tssrc/entrypoints/sdk/agentDefinitions.tssrc/entrypoints/sdk/query.ts
{src/commands/**/*.ts,src/entrypoints/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
commanderfor CLI argument parsing
Files:
src/entrypoints/sdk/agentDefinitions.test.tssrc/entrypoints/sdk/agentDefinitions.tssrc/entrypoints/sdk/query.ts
**/*.{ts,tsx,js,jsx,py,json,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow the existing code style in the touched files
Files:
src/entrypoints/sdk/agentDefinitions.test.tstests/sdk/query-happy-path.test.tsREADME.mdsrc/entrypoints/sdk/agentDefinitions.tssrc/entrypoints/sdk/query.tssrc/query/agentStepLimit.test.tssrc/query.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add or update tests when the change affects behavior
Files:
src/entrypoints/sdk/agentDefinitions.test.tstests/sdk/query-happy-path.test.tssrc/query/agentStepLimit.test.ts
**/*.test.{ts,tsx,js}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Test the exact provider/model path you changed when possible
Files:
src/entrypoints/sdk/agentDefinitions.test.tstests/sdk/query-happy-path.test.tssrc/query/agentStepLimit.test.ts
**/*.{ts,tsx,js,jsx,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Keep comments useful and concise
Files:
src/entrypoints/sdk/agentDefinitions.test.tstests/sdk/query-happy-path.test.tssrc/entrypoints/sdk/agentDefinitions.tssrc/entrypoints/sdk/query.tssrc/query/agentStepLimit.test.tssrc/query.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow TypeScript strict mode and type safety practices by running typecheck before submitting
Files:
src/entrypoints/sdk/agentDefinitions.test.tstests/sdk/query-happy-path.test.tssrc/entrypoints/sdk/agentDefinitions.tssrc/entrypoints/sdk/query.tssrc/query/agentStepLimit.test.tssrc/query.ts
**
⚙️ CodeRabbit configuration file
**: # AGENTS.md - AI Agent Coding GuideThis guide is for AI coding agents working in the OpenClaude repository. Read it before changing code, and also follow CONTRIBUTING.md for contributor policy, PR expectations, review follow-up, and project scope.
Project Snapshot
OpenClaude is a coding-agent CLI for cloud and local model providers. It supports OpenAI-compatible APIs, Anthropic, Gemini, DeepSeek, Ollama, MCP, local backends, slash commands, tools, agents, and a React/Ink terminal UI.
The installed CLI runs on Node.js
>=22.0.0. Bun is used for source builds, scripts, dependency management, and tests.Work Style
- Keep changes focused on one problem.
- Prefer existing patterns in the file or nearby module.
- Avoid unrelated formatting, renames, dependency changes, or broad rewrites.
- Add or update tests when behavior changes.
- Update docs when setup, commands, provider behavior, or user-facing behavior changes.
- For new features, larger refactors, dependencies, or runtime changes, follow the issue-first guidance in CONTRIBUTING.md.
Stack And Conventions
- TypeScript with strict mode and ESM imports.
- React + Ink for terminal UI.
- Bun lockfile and Bun scripts for development workflows.
- Node runtime for the built CLI.
- Python exists for legacy/local-provider helper code. Do not add new Python code or expand Python-based features unless a maintainer explicitly approves that direction.
Common libraries and patterns:
chalkfor terminal color.commanderfor CLI argument parsing.execafor child processes.- Existing service, provider, settings, permission, and UI patterns over new abstractions.
Repository Map
src/commands/- slash and CLI command implementations.src/components/- React/Ink UI components.src/services/- API, MCP, OAuth, wiki, voice, and other service integrations.src/tools/- tool implementations.src/utils/- shared utilities.- `src/integration...
Files:
src/entrypoints/sdk/agentDefinitions.test.tstests/sdk/query-happy-path.test.tsREADME.mdsrc/entrypoints/sdk/agentDefinitions.tssrc/entrypoints/sdk/query.tssrc/query/agentStepLimit.test.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/entrypoints/sdk/agentDefinitions.test.tstests/sdk/query-happy-path.test.tsREADME.mdsrc/entrypoints/sdk/agentDefinitions.tssrc/entrypoints/sdk/query.tssrc/query/agentStepLimit.test.tssrc/query.ts
src/{components/permissions,utils/permissions,hooks/toolPermission,tools,entrypoints/sdk}/**
⚙️ CodeRabbit configuration file
src/{components/permissions,utils/permissions,hooks/toolPermission,tools,entrypoints/sdk}/**: Review permission prompts, auto-allow logic, sandbox behavior, SDK permission schemas, shell/PowerShell execution, and background execution paths as security-sensitive. Block on bypasses, unclear trust boundaries, unsafe path handling, missing user visibility, or changes that broaden allowed behavior without an explicit maintainer decision.
Files:
src/entrypoints/sdk/agentDefinitions.test.tssrc/entrypoints/sdk/agentDefinitions.tssrc/entrypoints/sdk/query.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/entrypoints/sdk/agentDefinitions.test.tssrc/entrypoints/sdk/agentDefinitions.tssrc/entrypoints/sdk/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/entrypoints/sdk/agentDefinitions.test.tstests/sdk/query-happy-path.test.tssrc/query/agentStepLimit.test.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:
README.md
🔇 Additional comments (1)
src/query/agentStepLimit.test.ts (1)
266-271: LGTM!Also applies to: 408-413, 484-489, 519-519, 536-541, 559-648
011f081 to
2840943
Compare
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/entrypoints/sdk/agentDefinitions.ts`:
- Around line 45-49: The normalization in agentDefinitions should not fill a
missing SDK agent prompt with an empty string, because that converts an invalid
definition into a valid one. Update the logic around safeParse() and
normalizedDef so prompt remains required and omitted prompts cause validation
failure instead of defaulting to ''. Keep the existing description fallback, but
remove the prompt coercion so the agent_load_failure path is triggered for
malformed definitions.
In `@src/entrypoints/sdk/query.ts`:
- Around line 564-569: Mirror the injected SDK agents into
agentDefinitions.allAgents as well as activeAgents in query.ts after injection
succeeds, since the current state update only patches activeAgents and leaves
consumers like AgentsMenu, MCPSettings, and Doctor without the injected agents.
Update the self.appStateStore.setState path in the injection flow to merge the
injected agent set into both agentDefinitions.activeAgents and
agentDefinitions.allAgents so the SDK-provided agents are visible everywhere
that reads from the full agent list.
In `@tests/sdk/query-happy-path.test.ts`:
- Around line 196-230: The test in query-happy-path.test.ts should also verify
ordering, not just presence, for the failed SDK agent injection path. Update the
existing `failed SDK agent injection does not expose uninjected user agents`
case to assert that the `agent_load_failure` event from
`query()`/`MockQueryEngine` is emitted before any assistant output or other
engine messages, since `drainAgentFailureQueue()` is meant to preserve
pre-output ordering. Keep the existing checks for `stage === 'injection'`, the
injected error message, and `supportedAgents()` not containing `leaky`, but add
an ordering assertion using the collected `messages` array to ensure the failure
event appears first among relevant stream items.
🪄 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: e3fb0ec2-2cbe-4e7b-86ef-783f12056f94
📒 Files selected for processing (7)
README.mdsrc/entrypoints/sdk/agentDefinitions.test.tssrc/entrypoints/sdk/agentDefinitions.tssrc/entrypoints/sdk/query.tssrc/query.tssrc/query/agentStepLimit.test.tstests/sdk/query-happy-path.test.ts
📜 Review details
⚠️ CI failures not shown inline (2)
GitHub Actions: PR Checks / smoke-and-tests: feat(agents): add per-agent step limits
Conclusion: failure
diagnostics [3.00ms]
##[endgroup]
##[group]src/services/teamMemorySync/watcher.test.ts:
(pass) rescheduleCount behavior > increments while push in flight and resets after cap via onDebounceFire [2.00ms]
(pass) rescheduleCount behavior > clears pushInProgress when executePush completes [1.00ms]
(pass) rescheduleCount behavior > skips the capped follow-up push when suppression is set mid-flight [4.00ms]
(pass) rescheduleCount behavior > capped reschedule chains follow-up push after in-flight push completes [2.00ms]
(pass) rescheduleCount behavior > does not queue multiple duplicate follow-up pushes when one is already queued [1.00ms]
(pass) executePush identity safety > clears currentPushPromise when it was set to the executing promise [1.00ms]
(pass) executePush identity safety > preserves currentPushPromise when replaced during yield point [1.00ms]
##[endgroup]
##[group]src/services/tips/tipLink.test.ts:
(pass) renderSponsorLink > hyperlinks supported: name is an OSC 8 link, no trailing raw url
(pass) renderSponsorLink > hyperlinks NOT supported: plain name + dimmed url trailing
(pass) renderSponsorLink > no url → plain name, nothing trailing (either branch)
(pass) renderSponsorLink > strips control chars from the advertiser name (no escape injection)
(pass) renderSponsorLink > rejects non-http(s) URLs (javascript:/file:) → no link
(pass) renderSponsorLink > strips control chars from the url before validating it
(pass) renderSponsorLink > a malicious url cannot inject extra escape sequences into the output
##[endgroup]
##[group]src/services/tips/gitlawbEarn.test.ts:
(pass) gitlawb earning tips > disabled by default (no ads config)
(pass) gitlawb earning tips > enabled once /ads on set enabled + earnCode
(pass) gitlawb earning tips > cadence: every 2nd eligible slot by default
(pass) gitlawb earning tips > OPENCLAUDE_ADS_TIP_EVERY=1 shows every turn [1.00ms]
(pass) gitlawb earning tips > disabled → never shows and never increments the coun...
GitHub Actions: PR Checks / 1_smoke-and-tests.txt: feat(agents): add per-agent step limits
Conclusion: failure
way provider flag trims OPENGATEWAY_API_KEY before bearer auth [5.00ms]
(pass) gitlawb opengateway provider flag ignores blank OPENGATEWAY_API_KEY and uses OPENAI_API_KEY fallback [5.00ms]
(pass) gitlawb opengateway provider flag sends OPENGATEWAY_API_KEY to OPENGATEWAY_BASE_URL override [1.00ms]
(pass) gitlawb opengateway provider flag sends OPENGATEWAY_API_KEY to custom OPENAI_BASE_URL fallback [2.00ms]
(pass) gitlawb opengateway provider flag prefers OPENGATEWAY_API_KEY over generic OPENAI_API_KEY for custom base URL [1.00ms]
(pass) gitlawb opengateway provider flag prefers OPENGATEWAY_API_KEY over generic OPENAI_API_KEYS pool [2.00ms]
(pass) gitlawb opengateway provider flag uses generic OPENAI_API_KEYS pool before generic OPENAI_API_KEY fallback [1.00ms]
(pass) gitlawb opengateway stored provider profile key becomes bearer auth [2.00ms]
(pass) openai route still sends OPENAI_API_KEY as bearer auth [5.00ms]
(pass) OPENAI_API_KEYS rejects placeholder values before sending requests [4.00ms]
(pass) OPENAI_API_KEYS rotates to the next key on rate-limit failure [9.00ms]
(pass) comma-separated OPENAI_API_KEY rotates to the next key on rate-limit failure [3.00ms]
(pass) OPENAI_API_KEYS does not rotate through pool on provider 5xx outage [3.00ms]
(pass) OPENAI_API_KEYS preserves cooldown state across client requests [6.00ms]
(pass) OPENAI_API_KEYS rotates Azure api-key auth on auth failure [4.00ms]
(pass) OPENAI_API_KEYS does not reuse auth-disabled credentials across client requests [6.00ms]
(pass) OPENAI_API_KEYS permanently evicts 403 auth failures [8.00ms]
(pass) does not use BNKR_API_KEY for non-Bankr OpenAI-compatible routes [2.00ms]
(pass) preserves Gemini tool call extra_content from streaming chunks [1.00ms]
(pass) preserves Gemini thought signature from streaming delta extra_content [2.00ms]
(pass) preserves Gemini thought signature from non-streaming message extra_content [1.00ms]
(pass) converts Gemini raw tool-call text into streamin...
🧰 Additional context used
📓 Path-based instructions (14)
**/*.{ts,tsx,js,jsx,py,json,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow the existing code style in the touched files
Files:
README.mdsrc/entrypoints/sdk/agentDefinitions.test.tssrc/entrypoints/sdk/agentDefinitions.tstests/sdk/query-happy-path.test.tssrc/entrypoints/sdk/query.tssrc/query/agentStepLimit.test.tssrc/query.ts
**
⚙️ CodeRabbit configuration file
**: # AGENTS.md - AI Agent Coding GuideThis guide is for AI coding agents working in the OpenClaude repository. Read it before changing code, and also follow CONTRIBUTING.md for contributor policy, PR expectations, review follow-up, and project scope.
Project Snapshot
OpenClaude is a coding-agent CLI for cloud and local model providers. It supports OpenAI-compatible APIs, Anthropic, Gemini, DeepSeek, Ollama, MCP, local backends, slash commands, tools, agents, and a React/Ink terminal UI.
The installed CLI runs on Node.js
>=22.0.0. Bun is used for source builds, scripts, dependency management, and tests.Work Style
- Keep changes focused on one problem.
- Prefer existing patterns in the file or nearby module.
- Avoid unrelated formatting, renames, dependency changes, or broad rewrites.
- Add or update tests when behavior changes.
- Update docs when setup, commands, provider behavior, or user-facing behavior changes.
- For new features, larger refactors, dependencies, or runtime changes, follow the issue-first guidance in CONTRIBUTING.md.
Stack And Conventions
- TypeScript with strict mode and ESM imports.
- React + Ink for terminal UI.
- Bun lockfile and Bun scripts for development workflows.
- Node runtime for the built CLI.
- Python exists for legacy/local-provider helper code. Do not add new Python code or expand Python-based features unless a maintainer explicitly approves that direction.
Common libraries and patterns:
chalkfor terminal color.commanderfor CLI argument parsing.execafor child processes.- Existing service, provider, settings, permission, and UI patterns over new abstractions.
Repository Map
src/commands/- slash and CLI command implementations.src/components/- React/Ink UI components.src/services/- API, MCP, OAuth, wiki, voice, and other service integrations.src/tools/- tool implementations.src/utils/- shared utilities.- `src/integration...
Files:
README.mdsrc/entrypoints/sdk/agentDefinitions.test.tssrc/entrypoints/sdk/agentDefinitions.tstests/sdk/query-happy-path.test.tssrc/entrypoints/sdk/query.tssrc/query/agentStepLimit.test.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:
README.mdsrc/entrypoints/sdk/agentDefinitions.test.tssrc/entrypoints/sdk/agentDefinitions.tstests/sdk/query-happy-path.test.tssrc/entrypoints/sdk/query.tssrc/query/agentStepLimit.test.tssrc/query.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:
README.md
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript with strict mode and ESM imports
Files:
src/entrypoints/sdk/agentDefinitions.test.tssrc/entrypoints/sdk/agentDefinitions.tssrc/entrypoints/sdk/query.tssrc/query/agentStepLimit.test.tssrc/query.ts
{src/commands/**/*.ts,src/services/**/*.ts,src/entrypoints/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
chalkfor terminal color in CLI code
Files:
src/entrypoints/sdk/agentDefinitions.test.tssrc/entrypoints/sdk/agentDefinitions.tssrc/entrypoints/sdk/query.ts
{src/commands/**/*.ts,src/entrypoints/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
commanderfor CLI argument parsing
Files:
src/entrypoints/sdk/agentDefinitions.test.tssrc/entrypoints/sdk/agentDefinitions.tssrc/entrypoints/sdk/query.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add or update tests when the change affects behavior
Files:
src/entrypoints/sdk/agentDefinitions.test.tstests/sdk/query-happy-path.test.tssrc/query/agentStepLimit.test.ts
**/*.test.{ts,tsx,js}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Test the exact provider/model path you changed when possible
Files:
src/entrypoints/sdk/agentDefinitions.test.tstests/sdk/query-happy-path.test.tssrc/query/agentStepLimit.test.ts
**/*.{ts,tsx,js,jsx,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Keep comments useful and concise
Files:
src/entrypoints/sdk/agentDefinitions.test.tssrc/entrypoints/sdk/agentDefinitions.tstests/sdk/query-happy-path.test.tssrc/entrypoints/sdk/query.tssrc/query/agentStepLimit.test.tssrc/query.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow TypeScript strict mode and type safety practices by running typecheck before submitting
Files:
src/entrypoints/sdk/agentDefinitions.test.tssrc/entrypoints/sdk/agentDefinitions.tstests/sdk/query-happy-path.test.tssrc/entrypoints/sdk/query.tssrc/query/agentStepLimit.test.tssrc/query.ts
src/{components/permissions,utils/permissions,hooks/toolPermission,tools,entrypoints/sdk}/**
⚙️ CodeRabbit configuration file
src/{components/permissions,utils/permissions,hooks/toolPermission,tools,entrypoints/sdk}/**: Review permission prompts, auto-allow logic, sandbox behavior, SDK permission schemas, shell/PowerShell execution, and background execution paths as security-sensitive. Block on bypasses, unclear trust boundaries, unsafe path handling, missing user visibility, or changes that broaden allowed behavior without an explicit maintainer decision.
Files:
src/entrypoints/sdk/agentDefinitions.test.tssrc/entrypoints/sdk/agentDefinitions.tssrc/entrypoints/sdk/query.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/entrypoints/sdk/agentDefinitions.test.tssrc/entrypoints/sdk/agentDefinitions.tssrc/entrypoints/sdk/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/entrypoints/sdk/agentDefinitions.test.tstests/sdk/query-happy-path.test.tssrc/query/agentStepLimit.test.ts
🔇 Additional comments (2)
src/query.ts (1)
52-52: LGTM!Also applies to: 264-300, 356-386, 2069-2074
src/query/agentStepLimit.test.ts (1)
266-271: LGTM!Also applies to: 408-413, 484-489, 519-519, 536-573, 576-662
2840943 to
824a85f
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/entrypoints/sdk/query.ts (1)
154-161: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winMake
descriptionoptional in the SDKagentstype.
buildSdkUserAgents()accepts a missing description and falls back to the agent name, andsrc/entrypoints/sdk/agentDefinitions.test.tsnow locks that behavior in. Keepingoptions.agents[*].descriptionrequired makes a valid runtime shape fail for strict TypeScript callers.Suggested fix
agents?: Record<string, { - description: string + description?: string prompt: string tools?: string[] disallowedTools?: string[] model?: string maxTurns?: number🤖 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/entrypoints/sdk/query.ts` around lines 154 - 161, Make the SDK agents type match the runtime behavior by making description optional in the agents object used in query-related types. Update the Record definition in the query SDK types so buildSdkUserAgents() can accept agents without a description and fall back to the agent name, and keep the shape aligned with the agentDefinitions test coverage.
🤖 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 `@tests/sdk/query-happy-path.test.ts`:
- Around line 223-239: The current regression test only verifies
supportedAgents(), so it misses the leaked leaky entry in
appStateStore.agentDefinitions.allAgents after a failed injection. Update the
existing query-happy-path test to assert the user-visible state that
menus/diagnostics use, by checking q.appStateStore.agentDefinitions.allAgents
(or the equivalent all-agents source used by Query) no longer contains leaky
after the agent_load_failure path, while still keeping the failure ordering
assertion and supportedAgents() check in place.
---
Outside diff comments:
In `@src/entrypoints/sdk/query.ts`:
- Around line 154-161: Make the SDK agents type match the runtime behavior by
making description optional in the agents object used in query-related types.
Update the Record definition in the query SDK types so buildSdkUserAgents() can
accept agents without a description and fall back to the agent name, and keep
the shape aligned with the agentDefinitions test coverage.
🪄 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: 770b06e3-3a3d-44d9-8993-524826ad0ea2
📒 Files selected for processing (7)
README.mdsrc/entrypoints/sdk/agentDefinitions.test.tssrc/entrypoints/sdk/agentDefinitions.tssrc/entrypoints/sdk/query.tssrc/query.tssrc/query/agentStepLimit.test.tstests/sdk/query-happy-path.test.ts
📜 Review details
⚠️ CI failures not shown inline (2)
GitHub Actions: PR Checks / smoke-and-tests: feat(agents): add per-agent step limits
Conclusion: failure
havior > increments while push in flight and resets after cap via onDebounceFire [1.00ms]
(pass) rescheduleCount behavior > clears pushInProgress when executePush completes [1.00ms]
(pass) rescheduleCount behavior > skips the capped follow-up push when suppression is set mid-flight [1.00ms]
(pass) rescheduleCount behavior > capped reschedule chains follow-up push after in-flight push completes [2.00ms]
(pass) rescheduleCount behavior > does not queue multiple duplicate follow-up pushes when one is already queued [4.00ms]
(pass) executePush identity safety > clears currentPushPromise when it was set to the executing promise [2.00ms]
(pass) executePush identity safety > preserves currentPushPromise when replaced during yield point
##[endgroup]
##[group]src/services/tips/tipLink.test.ts:
(pass) renderSponsorLink > hyperlinks supported: name is an OSC 8 link, no trailing raw url
(pass) renderSponsorLink > hyperlinks NOT supported: plain name + dimmed url trailing
(pass) renderSponsorLink > no url → plain name, nothing trailing (either branch)
(pass) renderSponsorLink > strips control chars from the advertiser name (no escape injection)
(pass) renderSponsorLink > rejects non-http(s) URLs (javascript:/file:) → no link
(pass) renderSponsorLink > strips control chars from the url before validating it
(pass) renderSponsorLink > a malicious url cannot inject extra escape sequences into the output
##[endgroup]
##[group]src/services/tips/gitlawbEarn.test.ts:
(pass) gitlawb earning tips > disabled by default (no ads config) [1.00ms]
(pass) gitlawb earning tips > enabled once /ads on set enabled + earnCode
(pass) gitlawb earning tips > cadence: every 2nd eligible slot by default
(pass) gitlawb earning tips > OPENCLAUDE_ADS_TIP_EVERY=1 shows every turn
(pass) gitlawb earning tips > disabled → never shows and never increments the counter
(pass) gitlawb earning tips > content falls back to a static line when the ads service is unreachable [1.00ms]
(pass) g...
GitHub Actions: PR Checks / 2_smoke-and-tests.txt: feat(agents): add per-agent step limits
Conclusion: failure
.00ms]
(pass) gitlawb opengateway provider flag ignores blank OPENGATEWAY_API_KEY and uses OPENAI_API_KEY fallback [6.00ms]
(pass) gitlawb opengateway provider flag sends OPENGATEWAY_API_KEY to OPENGATEWAY_BASE_URL override [1.00ms]
(pass) gitlawb opengateway provider flag sends OPENGATEWAY_API_KEY to custom OPENAI_BASE_URL fallback [1.00ms]
(pass) gitlawb opengateway provider flag prefers OPENGATEWAY_API_KEY over generic OPENAI_API_KEY for custom base URL [2.00ms]
(pass) gitlawb opengateway provider flag prefers OPENGATEWAY_API_KEY over generic OPENAI_API_KEYS pool [1.00ms]
(pass) gitlawb opengateway provider flag uses generic OPENAI_API_KEYS pool before generic OPENAI_API_KEY fallback [2.00ms]
(pass) gitlawb opengateway stored provider profile key becomes bearer auth [3.00ms]
(pass) openai route still sends OPENAI_API_KEY as bearer auth [4.00ms]
(pass) OPENAI_API_KEYS rejects placeholder values before sending requests [6.00ms]
(pass) OPENAI_API_KEYS rotates to the next key on rate-limit failure [8.00ms]
(pass) comma-separated OPENAI_API_KEY rotates to the next key on rate-limit failure [3.00ms]
(pass) OPENAI_API_KEYS does not rotate through pool on provider 5xx outage [4.00ms]
(pass) OPENAI_API_KEYS preserves cooldown state across client requests [7.00ms]
(pass) OPENAI_API_KEYS rotates Azure api-key auth on auth failure [4.00ms]
(pass) OPENAI_API_KEYS does not reuse auth-disabled credentials across client requests [6.00ms]
(pass) OPENAI_API_KEYS permanently evicts 403 auth failures [7.00ms]
(pass) does not use BNKR_API_KEY for non-Bankr OpenAI-compatible routes [2.00ms]
(pass) preserves Gemini tool call extra_content from streaming chunks [2.00ms]
(pass) preserves Gemini thought signature from streaming delta extra_content [1.00ms]
(pass) preserves Gemini thought signature from non-streaming message extra_content [1.00ms]
(pass) converts Gemini raw tool-call text into streaming tool_use blocks [3.00ms]
(pass) converts Gemini raw tool-call ...
🧰 Additional context used
📓 Path-based instructions (14)
**/*.{ts,tsx,js,jsx,py,json,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow the existing code style in the touched files
Files:
README.mdsrc/entrypoints/sdk/agentDefinitions.test.tssrc/entrypoints/sdk/agentDefinitions.tssrc/entrypoints/sdk/query.tstests/sdk/query-happy-path.test.tssrc/query/agentStepLimit.test.tssrc/query.ts
**
⚙️ CodeRabbit configuration file
**: # AGENTS.md - AI Agent Coding GuideThis guide is for AI coding agents working in the OpenClaude repository. Read it before changing code, and also follow CONTRIBUTING.md for contributor policy, PR expectations, review follow-up, and project scope.
Project Snapshot
OpenClaude is a coding-agent CLI for cloud and local model providers. It supports OpenAI-compatible APIs, Anthropic, Gemini, DeepSeek, Ollama, MCP, local backends, slash commands, tools, agents, and a React/Ink terminal UI.
The installed CLI runs on Node.js
>=22.0.0. Bun is used for source builds, scripts, dependency management, and tests.Work Style
- Keep changes focused on one problem.
- Prefer existing patterns in the file or nearby module.
- Avoid unrelated formatting, renames, dependency changes, or broad rewrites.
- Add or update tests when behavior changes.
- Update docs when setup, commands, provider behavior, or user-facing behavior changes.
- For new features, larger refactors, dependencies, or runtime changes, follow the issue-first guidance in CONTRIBUTING.md.
Stack And Conventions
- TypeScript with strict mode and ESM imports.
- React + Ink for terminal UI.
- Bun lockfile and Bun scripts for development workflows.
- Node runtime for the built CLI.
- Python exists for legacy/local-provider helper code. Do not add new Python code or expand Python-based features unless a maintainer explicitly approves that direction.
Common libraries and patterns:
chalkfor terminal color.commanderfor CLI argument parsing.execafor child processes.- Existing service, provider, settings, permission, and UI patterns over new abstractions.
Repository Map
src/commands/- slash and CLI command implementations.src/components/- React/Ink UI components.src/services/- API, MCP, OAuth, wiki, voice, and other service integrations.src/tools/- tool implementations.src/utils/- shared utilities.- `src/integration...
Files:
README.mdsrc/entrypoints/sdk/agentDefinitions.test.tssrc/entrypoints/sdk/agentDefinitions.tssrc/entrypoints/sdk/query.tstests/sdk/query-happy-path.test.tssrc/query/agentStepLimit.test.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:
README.mdsrc/entrypoints/sdk/agentDefinitions.test.tssrc/entrypoints/sdk/agentDefinitions.tssrc/entrypoints/sdk/query.tstests/sdk/query-happy-path.test.tssrc/query/agentStepLimit.test.tssrc/query.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:
README.md
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript with strict mode and ESM imports
Files:
src/entrypoints/sdk/agentDefinitions.test.tssrc/entrypoints/sdk/agentDefinitions.tssrc/entrypoints/sdk/query.tssrc/query/agentStepLimit.test.tssrc/query.ts
{src/commands/**/*.ts,src/services/**/*.ts,src/entrypoints/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
chalkfor terminal color in CLI code
Files:
src/entrypoints/sdk/agentDefinitions.test.tssrc/entrypoints/sdk/agentDefinitions.tssrc/entrypoints/sdk/query.ts
{src/commands/**/*.ts,src/entrypoints/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
commanderfor CLI argument parsing
Files:
src/entrypoints/sdk/agentDefinitions.test.tssrc/entrypoints/sdk/agentDefinitions.tssrc/entrypoints/sdk/query.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add or update tests when the change affects behavior
Files:
src/entrypoints/sdk/agentDefinitions.test.tstests/sdk/query-happy-path.test.tssrc/query/agentStepLimit.test.ts
**/*.test.{ts,tsx,js}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Test the exact provider/model path you changed when possible
Files:
src/entrypoints/sdk/agentDefinitions.test.tstests/sdk/query-happy-path.test.tssrc/query/agentStepLimit.test.ts
**/*.{ts,tsx,js,jsx,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Keep comments useful and concise
Files:
src/entrypoints/sdk/agentDefinitions.test.tssrc/entrypoints/sdk/agentDefinitions.tssrc/entrypoints/sdk/query.tstests/sdk/query-happy-path.test.tssrc/query/agentStepLimit.test.tssrc/query.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow TypeScript strict mode and type safety practices by running typecheck before submitting
Files:
src/entrypoints/sdk/agentDefinitions.test.tssrc/entrypoints/sdk/agentDefinitions.tssrc/entrypoints/sdk/query.tstests/sdk/query-happy-path.test.tssrc/query/agentStepLimit.test.tssrc/query.ts
src/{components/permissions,utils/permissions,hooks/toolPermission,tools,entrypoints/sdk}/**
⚙️ CodeRabbit configuration file
src/{components/permissions,utils/permissions,hooks/toolPermission,tools,entrypoints/sdk}/**: Review permission prompts, auto-allow logic, sandbox behavior, SDK permission schemas, shell/PowerShell execution, and background execution paths as security-sensitive. Block on bypasses, unclear trust boundaries, unsafe path handling, missing user visibility, or changes that broaden allowed behavior without an explicit maintainer decision.
Files:
src/entrypoints/sdk/agentDefinitions.test.tssrc/entrypoints/sdk/agentDefinitions.tssrc/entrypoints/sdk/query.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/entrypoints/sdk/agentDefinitions.test.tssrc/entrypoints/sdk/agentDefinitions.tssrc/entrypoints/sdk/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/entrypoints/sdk/agentDefinitions.test.tstests/sdk/query-happy-path.test.tssrc/query/agentStepLimit.test.ts
🔇 Additional comments (2)
src/query.ts (1)
52-52: LGTM!Also applies to: 264-300, 2069-2074
src/query/agentStepLimit.test.ts (1)
266-271: LGTM!Also applies to: 408-413, 484-489, 519-519, 536-573, 576-662
824a85f to
0c343cc
Compare
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
I found a couple of issues that need to be addressed before this is ready.
Findings
-
[P2] Complete CodeRabbit's request to make SDK agent descriptions optional
src/entrypoints/sdk/query.ts:154
buildSdkUserAgents()accepts an SDK agent withoutdescriptionand falls back to the agent name, and the newmissingDescriptionregression test locks that runtime behavior in. The public SDK query option type still requiresagents[*].descriptionhere and in the exportedsrc/entrypoints/sdk.d.ts, so strict TypeScript callers cannot use this valid runtime shape without casts. Please complete the CodeRabbit request by making the SDKagentsdescription field optional everywhere this query option type is exposed/generated. -
[P2] Thread SDK agent injection through persistent SDK sessions
src/entrypoints/sdk/v2.ts:270
The one-shot SDK query path now builds SDK-provided agents withbuildSdkUserAgents(), merges them into bothactiveAgentsandallAgents, and preservesmaxStepsbefore callinginjectAgents(). The persistent SDK session path still only loads filesystem agents fromgetAgentDefinitionsWithOverrides()and injectsagentDefs.activeAgents, so SDK-provided agent definitions do not participate inunstable_v2_createSession(...).sendMessage()and therefore cannot use the newmaxStepssupport there. Please apply the same SDK-agent merge/injection path to persistent SDK sessions or clearly keep this feature out of that API surface.
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/entrypoints/sdk/agentDefinitions.ts (1)
64-89: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReject invalid
maxSteps/maxTurnsinstead of dropping them.Deleting these fields turns a malformed cap into an uncapped agent, so values like
0,1.5, or JS-typed garbage quietly bypass the new step/turn limit instead of surfacing anagent_load_failure. Let schema validation reject the definition, or report the invalid field here, but don't strip it.Proposed fix
const candidate = def as Partial<SdkAgentDefinitionInput> const normalizedDef = { ...candidate, description: candidate.description ?? name, } const maxTurns = normalizePositiveInteger(candidate.maxTurns) const maxSteps = normalizePositiveInteger(candidate.maxSteps) if (candidate.maxTurns !== undefined) { - if (maxTurns === undefined) { - delete normalizedDef.maxTurns - } else { - normalizedDef.maxTurns = maxTurns - } + if (maxTurns === undefined) { + reportInvalidAgent(name, 'maxTurns must be a positive integer') + return [] + } + normalizedDef.maxTurns = maxTurns } if (candidate.maxSteps !== undefined) { - if (maxSteps === undefined) { - delete normalizedDef.maxSteps - } else { - normalizedDef.maxSteps = maxSteps - } + if (maxSteps === undefined) { + reportInvalidAgent(name, 'maxSteps must be a positive integer') + return [] + } + normalizedDef.maxSteps = maxSteps }🤖 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/entrypoints/sdk/agentDefinitions.ts` around lines 64 - 89, The normalization in agentDefinitions should not delete invalid maxTurns/maxSteps values before validation, because that converts malformed caps into uncapped agents. In the candidate/normalizedDef flow, keep the original values on the object and let AgentDefinitionSchema().safeParse reject bad inputs, or explicitly call reportInvalidAgent with the offending field when normalizePositiveInteger returns undefined for a provided value. Ensure the fix is applied in the parsing path around normalizePositiveInteger, normalizedDef, and reportInvalidAgent so invalid limits surface as agent_load_failure instead of being silently dropped.
🤖 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/entrypoints/sdk/agentDefinitions.ts`:
- Around line 64-89: The normalization in agentDefinitions should not delete
invalid maxTurns/maxSteps values before validation, because that converts
malformed caps into uncapped agents. In the candidate/normalizedDef flow, keep
the original values on the object and let AgentDefinitionSchema().safeParse
reject bad inputs, or explicitly call reportInvalidAgent with the offending
field when normalizePositiveInteger returns undefined for a provided value.
Ensure the fix is applied in the parsing path around normalizePositiveInteger,
normalizedDef, and reportInvalidAgent so invalid limits surface as
agent_load_failure instead of being silently dropped.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 69d3e608-37d7-4003-8e80-a53720aac72c
📒 Files selected for processing (20)
src/components/ContextVisualization.tsxsrc/components/agents/AgentsList.tsxsrc/components/agents/AgentsMenu.test.tsxsrc/components/agents/AgentsMenu.tsxsrc/components/agents/agentFileUtils.tssrc/components/agents/types.tssrc/components/agents/utils.tssrc/entrypoints/sdk.d.tssrc/entrypoints/sdk/agentDefinitions.test.tssrc/entrypoints/sdk/agentDefinitions.tssrc/entrypoints/sdk/coreSchemas.tssrc/entrypoints/sdk/query.tssrc/entrypoints/sdk/v2.tssrc/tools/AgentTool/agentDisplay.tssrc/tools/AgentTool/loadAgentsDir.tssrc/utils/analyzeContext.tssrc/utils/settings/constants.tstests/sdk/package-consumer-types.test.tstests/sdk/query-happy-path.test.tstests/sdk/sdk-v2-lifecycle.test.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (15)
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript with strict mode and ESM imports
Files:
src/components/agents/AgentsList.tsxsrc/utils/settings/constants.tssrc/components/agents/agentFileUtils.tssrc/components/agents/types.tssrc/utils/analyzeContext.tssrc/components/agents/utils.tssrc/components/agents/AgentsMenu.test.tsxsrc/entrypoints/sdk/coreSchemas.tssrc/entrypoints/sdk.d.tssrc/tools/AgentTool/agentDisplay.tssrc/entrypoints/sdk/agentDefinitions.test.tssrc/components/agents/AgentsMenu.tsxsrc/entrypoints/sdk/v2.tssrc/components/ContextVisualization.tsxsrc/entrypoints/sdk/query.tssrc/tools/AgentTool/loadAgentsDir.tssrc/entrypoints/sdk/agentDefinitions.ts
src/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use React + Ink for terminal UI implementations
Files:
src/components/agents/AgentsList.tsxsrc/components/agents/agentFileUtils.tssrc/components/agents/types.tssrc/components/agents/utils.tssrc/components/agents/AgentsMenu.test.tsxsrc/components/agents/AgentsMenu.tsxsrc/components/ContextVisualization.tsx
**/*.{ts,tsx,js,jsx,py,json,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow the existing code style in the touched files
Files:
src/components/agents/AgentsList.tsxsrc/utils/settings/constants.tssrc/components/agents/agentFileUtils.tssrc/components/agents/types.tssrc/utils/analyzeContext.tssrc/components/agents/utils.tstests/sdk/package-consumer-types.test.tssrc/components/agents/AgentsMenu.test.tsxsrc/entrypoints/sdk/coreSchemas.tssrc/entrypoints/sdk.d.tssrc/tools/AgentTool/agentDisplay.tssrc/entrypoints/sdk/agentDefinitions.test.tssrc/components/agents/AgentsMenu.tsxtests/sdk/sdk-v2-lifecycle.test.tssrc/entrypoints/sdk/v2.tssrc/components/ContextVisualization.tsxsrc/entrypoints/sdk/query.tssrc/tools/AgentTool/loadAgentsDir.tstests/sdk/query-happy-path.test.tssrc/entrypoints/sdk/agentDefinitions.ts
**/*.{ts,tsx,js,jsx,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Keep comments useful and concise
Files:
src/components/agents/AgentsList.tsxsrc/utils/settings/constants.tssrc/components/agents/agentFileUtils.tssrc/components/agents/types.tssrc/utils/analyzeContext.tssrc/components/agents/utils.tstests/sdk/package-consumer-types.test.tssrc/components/agents/AgentsMenu.test.tsxsrc/entrypoints/sdk/coreSchemas.tssrc/entrypoints/sdk.d.tssrc/tools/AgentTool/agentDisplay.tssrc/entrypoints/sdk/agentDefinitions.test.tssrc/components/agents/AgentsMenu.tsxtests/sdk/sdk-v2-lifecycle.test.tssrc/entrypoints/sdk/v2.tssrc/components/ContextVisualization.tsxsrc/entrypoints/sdk/query.tssrc/tools/AgentTool/loadAgentsDir.tstests/sdk/query-happy-path.test.tssrc/entrypoints/sdk/agentDefinitions.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow TypeScript strict mode and type safety practices by running typecheck before submitting
Files:
src/components/agents/AgentsList.tsxsrc/utils/settings/constants.tssrc/components/agents/agentFileUtils.tssrc/components/agents/types.tssrc/utils/analyzeContext.tssrc/components/agents/utils.tstests/sdk/package-consumer-types.test.tssrc/components/agents/AgentsMenu.test.tsxsrc/entrypoints/sdk/coreSchemas.tssrc/entrypoints/sdk.d.tssrc/tools/AgentTool/agentDisplay.tssrc/entrypoints/sdk/agentDefinitions.test.tssrc/components/agents/AgentsMenu.tsxtests/sdk/sdk-v2-lifecycle.test.tssrc/entrypoints/sdk/v2.tssrc/components/ContextVisualization.tsxsrc/entrypoints/sdk/query.tssrc/tools/AgentTool/loadAgentsDir.tstests/sdk/query-happy-path.test.tssrc/entrypoints/sdk/agentDefinitions.ts
**
⚙️ CodeRabbit configuration file
**: # AGENTS.md - AI Agent Coding GuideThis guide is for AI coding agents working in the OpenClaude repository. Read it before changing code, and also follow CONTRIBUTING.md for contributor policy, PR expectations, review follow-up, and project scope.
Project Snapshot
OpenClaude is a coding-agent CLI for cloud and local model providers. It supports OpenAI-compatible APIs, Anthropic, Gemini, DeepSeek, Ollama, MCP, local backends, slash commands, tools, agents, and a React/Ink terminal UI.
The installed CLI runs on Node.js
>=22.0.0. Bun is used for source builds, scripts, dependency management, and tests.Work Style
- Keep changes focused on one problem.
- Prefer existing patterns in the file or nearby module.
- Avoid unrelated formatting, renames, dependency changes, or broad rewrites.
- Add or update tests when behavior changes.
- Update docs when setup, commands, provider behavior, or user-facing behavior changes.
- For new features, larger refactors, dependencies, or runtime changes, follow the issue-first guidance in CONTRIBUTING.md.
Stack And Conventions
- TypeScript with strict mode and ESM imports.
- React + Ink for terminal UI.
- Bun lockfile and Bun scripts for development workflows.
- Node runtime for the built CLI.
- Python exists for legacy/local-provider helper code. Do not add new Python code or expand Python-based features unless a maintainer explicitly approves that direction.
Common libraries and patterns:
chalkfor terminal color.commanderfor CLI argument parsing.execafor child processes.- Existing service, provider, settings, permission, and UI patterns over new abstractions.
Repository Map
src/commands/- slash and CLI command implementations.src/components/- React/Ink UI components.src/services/- API, MCP, OAuth, wiki, voice, and other service integrations.src/tools/- tool implementations.src/utils/- shared utilities.- `src/integration...
Files:
src/components/agents/AgentsList.tsxsrc/utils/settings/constants.tssrc/components/agents/agentFileUtils.tssrc/components/agents/types.tssrc/utils/analyzeContext.tssrc/components/agents/utils.tstests/sdk/package-consumer-types.test.tssrc/components/agents/AgentsMenu.test.tsxsrc/entrypoints/sdk/coreSchemas.tssrc/entrypoints/sdk.d.tssrc/tools/AgentTool/agentDisplay.tssrc/entrypoints/sdk/agentDefinitions.test.tssrc/components/agents/AgentsMenu.tsxtests/sdk/sdk-v2-lifecycle.test.tssrc/entrypoints/sdk/v2.tssrc/components/ContextVisualization.tsxsrc/entrypoints/sdk/query.tssrc/tools/AgentTool/loadAgentsDir.tstests/sdk/query-happy-path.test.tssrc/entrypoints/sdk/agentDefinitions.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/components/agents/AgentsList.tsxsrc/utils/settings/constants.tssrc/components/agents/agentFileUtils.tssrc/components/agents/types.tssrc/utils/analyzeContext.tssrc/components/agents/utils.tstests/sdk/package-consumer-types.test.tssrc/components/agents/AgentsMenu.test.tsxsrc/entrypoints/sdk/coreSchemas.tssrc/entrypoints/sdk.d.tssrc/tools/AgentTool/agentDisplay.tssrc/entrypoints/sdk/agentDefinitions.test.tssrc/components/agents/AgentsMenu.tsxtests/sdk/sdk-v2-lifecycle.test.tssrc/entrypoints/sdk/v2.tssrc/components/ContextVisualization.tsxsrc/entrypoints/sdk/query.tssrc/tools/AgentTool/loadAgentsDir.tstests/sdk/query-happy-path.test.tssrc/entrypoints/sdk/agentDefinitions.ts
{src/services/**/*.ts,src/utils/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
execafor child processes
Files:
src/utils/settings/constants.tssrc/utils/analyzeContext.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add or update tests when the change affects behavior
Files:
tests/sdk/package-consumer-types.test.tssrc/components/agents/AgentsMenu.test.tsxsrc/entrypoints/sdk/agentDefinitions.test.tstests/sdk/sdk-v2-lifecycle.test.tstests/sdk/query-happy-path.test.ts
**/*.test.{ts,tsx,js}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Test the exact provider/model path you changed when possible
Files:
tests/sdk/package-consumer-types.test.tssrc/components/agents/AgentsMenu.test.tsxsrc/entrypoints/sdk/agentDefinitions.test.tstests/sdk/sdk-v2-lifecycle.test.tstests/sdk/query-happy-path.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:
tests/sdk/package-consumer-types.test.tssrc/components/agents/AgentsMenu.test.tsxsrc/entrypoints/sdk/agentDefinitions.test.tstests/sdk/sdk-v2-lifecycle.test.tstests/sdk/query-happy-path.test.ts
{src/commands/**/*.ts,src/services/**/*.ts,src/entrypoints/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
chalkfor terminal color in CLI code
Files:
src/entrypoints/sdk/coreSchemas.tssrc/entrypoints/sdk.d.tssrc/entrypoints/sdk/agentDefinitions.test.tssrc/entrypoints/sdk/v2.tssrc/entrypoints/sdk/query.tssrc/entrypoints/sdk/agentDefinitions.ts
{src/commands/**/*.ts,src/entrypoints/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
commanderfor CLI argument parsing
Files:
src/entrypoints/sdk/coreSchemas.tssrc/entrypoints/sdk.d.tssrc/entrypoints/sdk/agentDefinitions.test.tssrc/entrypoints/sdk/v2.tssrc/entrypoints/sdk/query.tssrc/entrypoints/sdk/agentDefinitions.ts
src/{components/permissions,utils/permissions,hooks/toolPermission,tools,entrypoints/sdk}/**
⚙️ CodeRabbit configuration file
src/{components/permissions,utils/permissions,hooks/toolPermission,tools,entrypoints/sdk}/**: Review permission prompts, auto-allow logic, sandbox behavior, SDK permission schemas, shell/PowerShell execution, and background execution paths as security-sensitive. Block on bypasses, unclear trust boundaries, unsafe path handling, missing user visibility, or changes that broaden allowed behavior without an explicit maintainer decision.
Files:
src/entrypoints/sdk/coreSchemas.tssrc/tools/AgentTool/agentDisplay.tssrc/entrypoints/sdk/agentDefinitions.test.tssrc/entrypoints/sdk/v2.tssrc/entrypoints/sdk/query.tssrc/tools/AgentTool/loadAgentsDir.tssrc/entrypoints/sdk/agentDefinitions.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/entrypoints/sdk/coreSchemas.tssrc/entrypoints/sdk.d.tssrc/entrypoints/sdk/agentDefinitions.test.tssrc/entrypoints/sdk/v2.tssrc/entrypoints/sdk/query.tssrc/entrypoints/sdk/agentDefinitions.ts
🔇 Additional comments (7)
src/components/agents/agentFileUtils.ts (1)
113-115: LGTM!Also applies to: 150-152
src/components/agents/types.ts (1)
16-16: LGTM!src/components/agents/AgentsMenu.test.tsx (1)
383-455: LGTM!Also applies to: 457-496
src/components/agents/AgentsMenu.tsx (1)
138-138: LGTM!Also applies to: 206-206, 324-324
src/entrypoints/sdk/v2.ts (1)
115-116: LGTM!Also applies to: 279-338, 351-351, 365-365
src/tools/AgentTool/loadAgentsDir.ts (1)
151-155: LGTM!Also applies to: 168-168, 181-185, 210-218
tests/sdk/sdk-v2-lifecycle.test.ts (1)
351-353: 🎯 Functional CorrectnessNo issue here The cast is only line-wrapped; there isn’t a duplicated expression.
> Likely an incorrect or invalid review comment.
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
Summary
CodeRabbit has a still-valid requested change on this PR, and the contribution guide asks authors to address automated review findings before waiting for maintainer review. Please complete that item as part of the next update.
Findings
- [P2] Complete CodeRabbit's request to report invalid SDK step limits
src/entrypoints/sdk/agentDefinitions.ts:78
CodeRabbit's SDK validation request is still valid:buildSdkUserAgentsnormalizesmaxSteps/maxTurnsby deleting invalid provided values beforeAgentDefinitionSchema().safeParse()runs, so SDK agents with values likemaxSteps: 0,-1,1.5, or'2'are still injected as uncapped agents instead of emitting theagent_load_failurepath wired inquery.tsandv2.ts. That leaves SDK callers with no signal that their configured cap was ignored, while the PR claims SDK definitions get positive-integer validation. Please keep the offending values visible to schema validation or explicitly report them viareportInvalidAgentbefore injecting the agent.
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
Thanks for the update. I rechecked the previously discussed paths and do not see any remaining actionable issues from my side.
@kevincodex1 LGTM
… limits Port the upstream commit 1827d84 (Twigpine#1815) to OpenCC's architecture by functional sync instead of direct cherry-pick (the cherry-pick conflicts 4 modify/delete on OpenCC-deleted SDK entrypoints + 4 content conflicts). This commit covers parsing, types, helper wiring, and the constants file out of upstream's ~2700-line patch. The core enforcement loop in src/tools/AgentTool/runAgent.ts (caller-side agentStepLimit injection) and the regression test src/query/agentStepLimit.test.ts remain — they need a follow-up commit to land. Ported: - src/query/agentStepLimit.ts: AGENT_STEP_LIMIT_TOOL_RESULT_PREFIX constant - src/query.ts: agentStepLimit QueryParams field, State field, normalize, counter increment, transition carry-through, toolsForModel gating when summaryRequested - src/query/toolFailureLoopGuard.ts: interaction with step-limit nudges - src/services/api/claude.ts: agent_step_limit terminal reason recording - src/tools/AgentTool/agentToolUtils.ts: countToolUses helper - src/tools/AgentTool/loadAgentsDir.ts: markdown agent maxSteps parse - src/types/message.ts: isAgentStepLimitToolResult + sourceToolAssistantUUID - src/utils/frontmatterParser.ts: maxSteps frontmatter extraction - src/utils/messages.ts: createAssistantMessage helper - src/utils/plugins/loadPluginAgents.ts: plugin agent maxSteps parse Deliberately skipped (OpenCC has different SDK architecture): - src/entrypoints/sdk/{query.ts, v2.ts, agentDefinitions.ts}* - tests/sdk/*.{test.ts} Co-Authored-By: Claude <noreply@anthropic.com>
…et transitions Carry the agentStepLimit state through the stop_hook_blocking and token_budget_continuation transitions in queryLoop, matching upstream's pattern of forwarding step-limit state on every State-restart path so the counter survives. Continues the partial sync of Twigpine#1815. Co-Authored-By: Claude <noreply@anthropic.com>
Closes the remaining 30% of the upstream 1827d84 functional sync: - src/tools/AgentTool/runAgent.ts: caller-side agentStepLimit injection into QueryParams at subagent invocation. Resolve configuredMaxSteps from the agent definition, pass `{ maxSteps, agentType }` only when the agent has an explicit positive maxSteps. Logs agent_step_limit terminal-reason outcomes. - src/query/agentStepLimit.test.ts (NEW, 693 lines): full port of upstream's 8 regression cases — default behavior, invalid values, multi-turn accumulation, plugin parsing, failure-loop interaction, summary-tool blocking. Adapted to OpenCC's bun:test runner and user-message shape via @ts-nocheck + targeted any-casts (mirroring toolFailureLoopGuard.test.ts convention). - src/query/transitions.ts (NEW): Terminal + Continue union types with the agent_step_limit variant. Replaces ad-hoc inline return shapes. - src/query.ts: increments agentStepLimit state-stamp on remaining state-restart paths (image_error, prompt_too_long, hard_max_query). Skipped (no-op for OpenCC, phantom SDK variants): - src/utils/settings/constants.ts: upstream adds 'sdk' to SettingSource; OpenCC has no SDK entrypoint, so the variant is never produced. - src/utils/analyzeContext.ts: same reason — upstream's 'sdk' source variant never appears in OpenCC analyzeContext output. Closes the functional-sync port of Twigpine#1815. Without runAgent.ts injection the agent_step_limit machinery parsed in 3ed0b83 was inert; this commit wires it end-to-end. Co-Authored-By: Claude <noreply@anthropic.com>
…wigpine#1815 + chore(commands): disable /login /logout)
3 participants
Summary
maxStepsconfiguration so subagents can be capped by tool-use steps.Implementation
maxStepsfrom markdown, JSON, plugin, and SDK agent definitions with positive-integer validation.agent_step_limitterminal reason through subagent execution and debug logging.Tests
bun test src/query/agentStepLimit.test.ts src/query/toolFailureLoopGuard.test.ts src/tools/AgentTool/**/*.test.ts src/utils/plugins/loadPluginAgents.test.tsbun run typecheckbun run typecheck:type-testsbun run buildbun run smokebun run security:pr-scangit diff --checkbun run checkwas also run locally; it failed in full-suite mode with order-dependent failures, and the failed groups passed when rerun in isolation with--feature=UNATTENDED_RETRY --max-concurrency=1.Risk
maxSteps.Summary by CodeRabbit
maxSteps, limiting tool-use steps and triggering forced concise summaries when reached.maxSteps, with UI display/grouping for SDK agents.messageNormalizationToolsoverride for message/tool normalization control.maxStepsare ignored (default unlimited behavior preserved).agent_load_failureevents.maxSteps, forced-summary behavior, streaming semantics, and injection/MCP filtering cases.