Files
645d596ea4 fix(code-reviewer): require inline diff input and preserve read-only search in embedded-search builds (#2102)
* 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>
2026-08-19 13:55:48 +08:00

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.