fix(claude): preserve generated subagent effort - #705
Conversation
Add a validated claudeCode.subagentEffort setting and render it into generated ocx agent frontmatter. Claude Code 2.1.220 collapses xhigh and max custom-agent effort to a legacy thinking budget, so carry the exact value through an owned system directive and restore it at the routed inbound boundary. Invalid persisted values degrade without discarding unrelated provider or account configuration. Co-Authored-By: Claude <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds validated ChangesClaude Code subagent effort
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant ClaudeCode
participant GeneratedAgent
participant OpenCodex
participant Provider
ClaudeCode->>GeneratedAgent: load model, effort, and ocx directives
GeneratedAgent->>ClaudeCode: send routed subagent request
ClaudeCode->>OpenCodex: send system ocx-route and ocx-effort directives
OpenCodex->>OpenCodex: set output_config.effort and remove thinking
OpenCodex->>Provider: forward exact reasoning_effort
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/claude-messages-endpoint.test.ts`:
- Around line 785-805: Extend the endpoint regression coverage beside the
existing max-effort test to send an ocx-effort directive of xhigh through the
same request path, then assert the captured upstream request preserves
reasoning_effort as xhigh. Keep the existing max assertion and test setup
pattern unchanged so anthropicToResponsesTranslation is verified for both
distinct effort tiers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 9a89848d-0c0f-46a0-94aa-49d3cb5ba134
📒 Files selected for processing (11)
docs-site/src/content/docs/reference/configuration.mddocs-site/src/content/docs/ru/reference/configuration.mdsrc/claude/agents-inject.tssrc/claude/inbound.tssrc/config.tssrc/server/claude-messages.tssrc/types.tstests/claude-agents-inject.test.tstests/claude-inbound.test.tstests/claude-messages-endpoint.test.tstests/config.test.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5635eda700
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| anthropicBody.output_config = { effort: effortOverride }; | ||
| delete anthropicBody.thinking; |
There was a problem hiding this comment.
When a generated ocx-self agent targets a raw model such as claude-haiku-4-5 or claude-sonnet-4-* and the request carries a native Anthropic credential, wantsNativePassthrough runs after this block. These lines unconditionally delete the valid legacy thinking: { type: "enabled", budget_tokens: ... } control and replace it with the adaptive-only effort field, even though src/adapters/anthropic.ts explicitly retains the legacy budget shape for these older families. Such native subagent turns therefore lose extended thinking or are rejected rather than preserving the configured effort; restrict this restoration to translated routed requests or compile the model-appropriate native thinking shape before passthrough.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 6d5bea1. Added focused regression coverage; 174 targeted tests and the full 5966-pass suite completed with 0 failures. Typecheck, privacy scan, and diff check also passed.
| warnDegradedClaudeSubagentEffort(parsed); | ||
| warnDegradedNativeSubagentConfig(parsed, config); | ||
| return normalizeNativeSubagentSync(config, parsed); | ||
| return normalizeClaudeSubagentEffort(normalizeNativeSubagentSync(config, parsed), parsed); |
There was a problem hiding this comment.
When persisted config.json contains an invalid optional value such as claudeCode.subagentEffort: "ultra", this return removes it only from the loaded object while leaving the raw file unchanged. startServer then arms the Claude subtree baseline from that normalized object, so the first unrelated saveConfigPreservingClaudeCode call treats the raw invalid subtree as a later hand edit and copies it back into the live config; a subsequent management agent sync can consequently render unsupported effort frontmatter despite the promised safe-ignore behavior. Normalize the persisted subtree used by the hand-edit comparison or persist the cleaned value before arming the baseline.
AGENTS.md reference: src/AGENTS.md:L10-L10
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 6d5bea1. Added focused regression coverage; 174 targeted tests and the full 5966-pass suite completed with 0 failures. Typecheck, privacy scan, and diff check also passed.
| function warnDegradedClaudeSubagentEffort(rawParsed: unknown): void { | ||
| const rawEffort = rawClaudeSubagentEffort(rawParsed); | ||
| if (rawEffort !== undefined && !isClaudeSubagentEffort(rawEffort)) { | ||
| console.warn(`⚠️ config.json claudeCode.subagentEffort ${JSON.stringify(rawEffort)} is invalid (expected ${CLAUDE_SUBAGENT_EFFORTS.join(", ")}) — ignoring it. Other settings were preserved.`); |
There was a problem hiding this comment.
When a malformed persisted value is itself a credential—for example, an API key accidentally pasted into claudeCode.subagentEffort—this warning writes the complete secret to service logs via JSON.stringify(rawEffort). The value is unnecessary for diagnosing the allowed-value error and can also be an object containing credentials, so report only its type or omit it entirely rather than serializing untrusted configuration content.
AGENTS.md reference: AGENTS.md:L109-L115
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 6d5bea1. Added focused regression coverage; 174 targeted tests and the full 5966-pass suite completed with 0 failures. Typecheck, privacy scan, and diff check also passed.
Extend the routed endpoint regression to verify that the shared 31,999-token Claude Code budget preserves both xhigh and max as distinct outbound effort tiers. Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
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)
tests/config.test.ts (1)
167-195: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRestore the warning spy in a
finallyblock.If any assertion fails,
warnSpy.mockRestore()is skipped and later tests run withconsole.warnstill mocked, masking diagnostics or causing order-dependent failures.Proposed fix
const invalidEffort = "credential-like-value"; const warnSpy = spyOn(console, "warn").mockImplementation(() => {}); - writeConfig({ + try { + writeConfig({ // ... - }); + }); - const config = loadConfig(); - const diagnostics = readConfigDiagnostics(); - // assertions... - warnSpy.mockRestore(); + const config = loadConfig(); + const diagnostics = readConfigDiagnostics(); + // assertions... + } finally { + warnSpy.mockRestore(); + }🤖 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 `@tests/config.test.ts` around lines 167 - 195, Wrap the assertions and diagnostic checks following the warnSpy setup in a try/finally block, and move warnSpy.mockRestore() into the finally clause. Preserve all existing expectations while ensuring console.warn is restored even when a test assertion fails.
🤖 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 `@tests/config.test.ts`:
- Around line 167-195: Wrap the assertions and diagnostic checks following the
warnSpy setup in a try/finally block, and move warnSpy.mockRestore() into the
finally clause. Preserve all existing expectations while ensuring console.warn
is restored even when a test assertion fails.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2052c03e-911b-4a3a-b81b-ffb6a76580ac
📒 Files selected for processing (5)
src/config.tssrc/server/claude-messages.tstests/claude-messages-endpoint.test.tstests/config-user-edits.test.tstests/config.test.ts
…ion hardening, Windows scheduler fixes Promotes dev to main for the v2.7.43 quick-fix release: - fix(gui): renew loopback dashboard sessions silently instead of prompting - PR #705 + follow-up: Claude effort restoration boundary hardening - PRs #694/#698/#699 and Windows scheduler compare/denial fixes
Summary
claudeCode.subagentEffort(low | medium | high | xhigh | max) and render it into every generatedocx-*Claude Code agent definition.ocx-effortsystem directive. Claude Code 2.1.220 collapses custom-agentxhighandmaxfrontmatter intothinking.budget_tokens=31999; the routed inbound boundary restores the configuredoutput_config.effortbefore translation.Closes #704.
Live verification
A fresh Claude Code 2.1.220 session was launched through the patched source and dispatched the generated
ocx-deepseek-v4-proagent with a deterministic arithmetic prompt.Sanitized observed result:
Before restoring the owned effort directive, the same live path produced
requestedEffort=highandeffectiveEffort=high, confirming that frontmatter generation alone was insufficient on Claude Code 2.1.220.No credentials, account identifiers, request IDs, absolute user paths, or raw payloads are included.
Verification
bun test --cwd <worktree> ./tests/claude-agents-inject.test.ts ./tests/config.test.ts ./tests/claude-inbound.test.ts ./tests/claude-messages-endpoint.test.ts— 160 passed, 0 failedbun run test— 5964 passed, 1 skipped, 0 failed across 429 filesbun run typecheck— passedbun run privacy:scan— passedbun run --cwd gui lint— passedbun run --cwd docs-site build— 146 pages builtgit diff --check— passedopencode-go/deepseek-v4-prolive smoke — HTTP 200 with requested/effectivemaxChecklist
🤖 Generated with Claude Code
Summary by CodeRabbit
claudeCode.subagentEffort(low, medium, high, xhigh, max) to control effort for generated Claude Code subagents.subagentEffortvalues are rejected/ignored with warnings and sanitized in saved configuration.