mirror of
https://github.com/Gitlawb/openclaude.git
synced 2026-08-24 10:14:19 -05:00
* feat: add code reviewer agent * feat(agent): add code-reviewer built-in agent implementation and tests Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(code-reviewer): address review comments - Fix broken step numbering in system prompt (glob/grep as sub-bullets) - Remove unnecessary wrapper in getSystemPrompt - Drop unused beforeEach/afterEach lifecycle in tests - Remove redundant registration test (beforeAll already throws) - Fix CLAUDE_CONFIG_DIR leak in beforeAll (restore in finally) - Replace @ts-ignore with explicit ToolUseContext cast - Use placeholder in README agentRouting example Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * ci: trigger rerun — pre-existing test failures on main * fix(code-reviewer): enforce read-only contract by disallowing Bash Add Bash to disallowedTools so the reviewer cannot run shell commands regardless of parent session's acceptEdits/bypassPermissions mode. resolveAgentTools() treated undefined tools as wildcard — Bash was available and could auto-approve mkdir/rm/mv in acceptEdits mode. Remove Bash guidance from system prompt; diff must now be supplied by the caller inline. Update test to assert Bash is in disallowedTools. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(code-reviewer): deny all shell tools (Bash and PowerShell) Use SHELL_TOOL_NAMES constant instead of just BASH_TOOL_NAME to ensure all shell-capable tools are denied from the code-reviewer agent. This prevents Windows sessions with PowerShell enabled from bypassing the read-only contract. - Import SHELL_TOOL_NAMES from shellToolUtils - Use spread operator to include both Bash and PowerShell in disallowedTools - Update test to verify PowerShell is denied Fixes the finding: [P2] Deny all shell tools for the reviewer agent * fix(code-reviewer): explicit read-only allow-list; drop unrelated artifacts Switch code-reviewer to an explicit `tools` allow-list (Read, Glob, Grep) instead of relying on wildcard access minus a deny-list. resolveAgentTools() resolves only the named tools, so write-capable mcp__* server tools (and any other mutation-capable tool) can never be handed to the read-only reviewer. Keep the mutation deny-list as defense-in-depth. Remove generated/scratch artifacts unrelated to the reviewer agent: AGENTS.md, ARCHITECTURE.md, the .openlore/ .gitignore rule, temp_reference/, and the .tmp/sdk-consumer-* scratch files. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix: restore temp_reference/ gitignore entry from main Entry was present on main from #1350 and was unintentionally removed during PR cleanup. Restoring it so temp_reference/ scratch directories remain untracked after merge. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(code-reviewer): require inline diff input and preserve read-only search in embedded-search builds Summary: - Update the `code-reviewer` built-in agent to require the caller to provide the diff or changed hunks inline in the prompt. - Preserve read-only search behavior in embedded-search builds by omitting `Glob`/`Grep` from the explicit tool allow-list when embedded search tools are enabled. - Keep a strict read-only policy by disallowing shell and mutation tools. Usage: - The `code-reviewer` agent is now explicitly guided to only review changes when the diff is provided inline. - In embedded-search builds, the agent only receives `Read` and cannot access `Glob`/`Grep`. - This prevents the agent from attempting shell-based diff discovery and enforces caller-provided diff input. Test plan: - `bun test src/tools/AgentTool/built-in/codeReviewerAgent.test.ts` — 12 pass, 0 fail - `bun run build` — success - `bun run smoke` — success - `bun run security:pr-scan` — success - `git diff --check` — success Credit: prior work from #1381/#1420. * fix(code-reviewer): clear cached agent definitions and markdown loader cache after test cleanup * fix(code-reviewer): address all P2/P3 review findings from jatmn - Always list [Read, Glob, Grep] in the tool allow-list; in embedded-search builds resolveAgentTools() silently drops unavailable Glob/Grep at runtime. The system prompt explicitly documents the narrowed search contract for embedded builds instead of silently degrading. - Update the code-reviewer invocation example in prompt.ts to include the diff inline, satisfying the reviewer's contract and preventing an avoidable extra turn. - Rewrite the test suite with the same isolation protocol as loadAgentsDir.test.ts: shared mutation lock, OPENCLAUDE_CONFIG_DIR, setClaudeConfigHomeDirForTesting, setAllowedSettingSources, and full env/cache restore in finally blocks. - Exercise both embedded-search branches: non-embedded tests verify Glob/Grep guidance, embedded tests verify the limited-search documentation and Read-only path. - Fix settings file path in README.md and docs/agent-routing.md from ~/.openclaude.json to ~/.openclaude/settings.json (the path the runtime actually loads). - Add blank line after fenced code block (MD031), add code-reviewer to the routable built-in agent list in both README and agent-routing docs. * fix(code-reviewer): restore prior setting sources in test cleanup, add credential security warning - Capture getAllowedSettingSources() before overwriting and restore it in the finally block instead of resetting to the default list, preventing state leakage to concurrent suites. - Add plaintext-credential security warning before the agentModels JSON example in README. - Update 'All settings-driven' to 'Configured via settings, agent frontmatter, and environment variables' for accuracy. * fix(code-reviewer): address remaining PR feedback (P1/P3) * docs: document feature gate for Explore and Plan agents * docs: document inline-diff requirement for code-reviewer agent * fix(code-reviewer): address P1/P2 review findings — teammate boundary, resume safety, lock-aware tests [P1] Reject built-in agent types from teammate spawn path to preserve read-only boundary. The teammate branch bypasses resolveAgentTools(), so built-ins like code-reviewer would receive Bash/Edit/Write tools. Guard added in AgentTool.tsx before spawnTeammate is called. [P1] Fail closed when resuming an unavailable agent type instead of silently falling back to GENERAL_PURPOSE_AGENT. A resumed code-reviewer must never gain edit-capable tools through a compatibility fallback. [P2] Snapshot environment and config state only after acquiring the shared mutation lock in codeReviewerAgent.test.ts. Moved from module- scope const to post-lock capture in beforeAll, with cleanup and lock release in afterAll's finally block. Regression tests added for all three findings. * fix(code-reviewer): guard lock release against failed acquisition Only call releaseSharedMutationLock() in afterAll when the lock was successfully acquired. Prevents releasing another suite's lock if acquireSharedMutationLock() throws on timeout. * fix(code-reviewer): remove trailing whitespace * fix(code-reviewer): reliably block built-in teammate spawns Reject built-in agent types from the teammate spawn path by looking them up in allAgents rather than activeAgents. This ensures the restriction remains intact even when built-in agents are disabled (e.g. via CLAUDE_AGENT_SDK_DISABLE_BUILTIN_AGENTS) and omitted from activeAgents. Regression test added. * fix(code-reviewer): preserve original agent identity when resuming a read-only reviewer Background-agent metadata persisted only agentType, so resuming selected whichever active definition currently had that name. A built-in code-reviewer could be started, followed by a project/SDK definition named code-reviewer taking precedence; resuming the original agent would then use the replacement's ordinary wildcard tool set and grant that reviewer transcript Bash/Edit/Write tools. Persist the agent definition source and verify it matches the resolved definition on resume, rejecting the resumption if the original was spoofed. * fix(code-reviewer): address remaining CodeRabbit feedback on test cleanup and metadata source * fix(code-reviewer): address P1/P2 issues for teammate spawns and resume safety * docs: make OpenLore prerequisite explicitly optional in AGENTS.md * docs: fix pinned OpenLore version in AGENTS.md * Fix review issues * Revert AGENTS.md changes * Restore AGENTS.md to match upstream/main * fix(agent): address maintainer feedback on teammate spawns and resume persistence * test(agent): add regression coverage for legacy source-less agent resume * fix(agent): propagation pass — batch fork regression, TeamCreate policy, trailing whitespace, verification gate docs * fix(batch): allow specific custom agent types while requiring subagent_type --------- Co-authored-by: Laurent FRANCOISE <lfrancoise@gmail.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
119 lines
4.5 KiB
Markdown
119 lines
4.5 KiB
Markdown
# Agent Routing and Step Limits
|
|
|
|
OpenClaude can route different agents to different models, and custom agents
|
|
can cap how many tool-use steps they may execute. Both features live in
|
|
settings and agent frontmatter — no code changes required.
|
|
|
|
## Agent step limits
|
|
|
|
Custom agents can define `maxSteps` as a positive integer to cap how many
|
|
tool-use steps a sub-agent may execute. When the limit is reached, OpenClaude
|
|
stops additional tool calls and asks the sub-agent for a concise final summary
|
|
covering completed work, findings, remaining tasks, and whether another run is
|
|
needed. Omitting `maxSteps`, or setting it to an invalid value such as `0` or
|
|
malformed input, preserves the default unlimited behavior.
|
|
|
|
```markdown
|
|
---
|
|
name: bounded-researcher
|
|
description: Use for focused research with bounded tool use
|
|
maxSteps: 8
|
|
---
|
|
|
|
You are a focused research agent.
|
|
```
|
|
|
|
## Agent routing
|
|
|
|
OpenClaude can route different agents to different models through
|
|
settings-based routing. This is useful for cost optimization or splitting work
|
|
by model strength.
|
|
|
|
Add to `~/.openclaude/settings.json`:
|
|
|
|
> **Note:** `api_key` values in `settings.json` are stored in plaintext. Keep this file private and do not commit it to version control.
|
|
|
|
```json
|
|
{
|
|
"agentModels": {
|
|
"deepseek-v4-flash": {
|
|
"base_url": "https://api.deepseek.com/v1",
|
|
"api_key": "sk-your-key"
|
|
},
|
|
"zai-default": {
|
|
"model": "glm-5.2",
|
|
"base_url": "https://api.z.ai/api/coding/paas/v4",
|
|
"api_key": "sk-your-key"
|
|
},
|
|
"gpt-4o": {
|
|
"base_url": "https://api.openai.com/v1",
|
|
"api_key": "sk-your-key"
|
|
}
|
|
},
|
|
"agentRouting": {
|
|
"Explore": "deepseek-v4-flash",
|
|
"Plan": "gpt-4o",
|
|
"general-purpose": "gpt-4o",
|
|
"frontend-dev": "zai-default",
|
|
"default": "gpt-4o"
|
|
}
|
|
}
|
|
```
|
|
|
|
When no routing match is found, the global provider remains the fallback.
|
|
|
|
`agentRouting` values and explicit Agent tool `model` overrides match keys in
|
|
`agentModels`. By default, that key is also the model string sent to the
|
|
provider. Set `agentModels.<key>.model` when you want a local route key such
|
|
as `zai-default` to call a different provider model name such as `glm-5.2`.
|
|
|
|
> **Note:** `/provider` changes the global/parent provider for your current
|
|
> session. `agentModels` and `agentRouting` are specifically for configuring
|
|
> per-agent provider overrides while keeping the parent session unchanged.
|
|
|
|
> **Note:** `api_key` values in `settings.json` are stored in plaintext. Keep
|
|
> this file private and do not commit it to version control.
|
|
|
|
**Model-only routes (same provider):** Omit `base_url` and `api_key` to run an
|
|
agent on a different model using your *current* provider's endpoint and key —
|
|
no credential duplication:
|
|
|
|
```json
|
|
{
|
|
"agentModels": {
|
|
"mini": { "model": "gpt-5-mini" }
|
|
},
|
|
"agentRouting": {
|
|
"verification": "mini"
|
|
}
|
|
}
|
|
```
|
|
|
|
**Built-in agents are routable by their type name.** Useful keys:
|
|
`verification` (the read-only auditor that runs before completion; **feature-gated**: requires `VERIFICATION_AGENT` and `tengu_hive_evidence` flag), `Explore`
|
|
and `Plan` (if feature-gated on), and `code-reviewer` (requires diff inline). For example, `"agentRouting": { "verification": "mini" }` runs the
|
|
verifier on `gpt-5-mini` while your main session stays on its model, but only when the verification gate is active. Absent
|
|
any entry, the verifier inherits the main-loop model.
|
|
|
|
## GitHub Copilot sub-agent optimization
|
|
|
|
When `CLAUDE_CODE_USE_GITHUB=1`, OpenClaude serializes sub-agent execution to
|
|
reduce GitHub Copilot Premium Request consumption. Default behavior is
|
|
`GITHUB_COPILOT_MAX_SUBAGENTS=1` (synchronous, one sub-agent at a time).
|
|
Tuning vars (all optional):
|
|
|
|
| Var | Effect |
|
|
|---|---|
|
|
| `GITHUB_COPILOT_MAX_SUBAGENTS=0` | Suppress sub-agents entirely (sub-agents throw an error). |
|
|
| `GITHUB_COPILOT_MAX_SUBAGENTS=1` | Force synchronous execution. **Default.** |
|
|
| `GITHUB_COPILOT_MAX_SUBAGENTS=2..10` | Parsed/clamped but not enforced differently from `=1` (any positive cap = synchronous). |
|
|
| `GITHUB_COPILOT_ALLOW_SUBAGENTS=1` | Re-enable parallel/background sub-agents, overriding the cap. |
|
|
| `GITHUB_COPILOT_FORCE_SYNC_SUBAGENTS=1` | Force synchronous execution regardless of cap. |
|
|
| `GITHUB_COPILOT_OPTIMIZATION_DISABLED=1` | Disable all of the above; sub-agents run as before this feature. |
|
|
|
|
The `is_async` field reported in the `tengu_agent_tool_selected` event and the
|
|
agent metadata reflects the final execution mode (i.e., `false` when
|
|
synchronous is forced). See `.env.example` for the full descriptions.
|
|
|
|
For best results, use models with strong tool/function calling support.
|