feat: add ultrathink keyword detection and ultracode effort level (#1551)#1630
feat: add ultrathink keyword detection and ultracode effort level (#1551)#1630adityachaudhary99 wants to merge 11 commits into
Conversation
…tlawb#1551) - Remove isUltrathinkEnabled() build-flag gate from getUltrathinkEffortAttachment so ultrathink keyword works for all providers without a feature flag - Add 'ultracode' EffortLevel: maps to xhigh API effort + injects per-turn system-reminder granting standing multi-agent orchestration permission - Add ultracode_mode Attachment type and handler in messages.ts - Update /effort argumentHint to include xhigh and ultracode - getAvailableEffortLevels() includes ultracode for all effort-supporting models - Update coreTypes.generated.ts and coreSchemas.ts supportedEffortLevels to include ultracode Closes Gitlawb#1551 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📜 Recent review details🧰 Additional context used📓 Path-based instructions (10)src/**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
{src/services/**/*.ts,src/utils/**/*.ts}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{ts,tsx,js,jsx,py,json,md}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*.test.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*.test.{ts,tsx,js}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*.{ts,tsx,js,jsx,py}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*⚙️ CodeRabbit configuration file
Files:
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}⚙️ CodeRabbit configuration file
Files:
**⚙️ CodeRabbit configuration file
Files:
🔇 Additional comments (12)
📝 WalkthroughWalkthroughIntroduces Changesultracode Effort Level
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 6 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 (2)
src/utils/effort.ts (2)
212-224:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep
ultracodeout of persisted settings.
toPersistableEffort()is the write allow-list forsettings.json, andgetInitialEffortSetting()on Line 231 reads it back on startup. Returning'ultracode'here turns a session-only mode into a cross-session default, which also re-enables its standing multi-agent permission reminder unexpectedly.🤖 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/utils/effort.ts` around lines 212 - 224, The toPersistableEffort() function currently includes 'ultracode' in its allow-list condition, which causes it to be persisted to settings.json when it should remain session-only. Remove the 'value === ultracode' condition from the if statement in the toPersistableEffort() function so that 'ultracode' is not returned and therefore not persisted, while keeping all other values (low, medium, high, max, xhigh) in the allow-list.
15-22:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't widen
EffortLevelwithout fixingstandardEffortToOpenAI().Adding
'ultracode'here makesstandardEffortToOpenAI()inconsistent with its return type: the fallback cast on Line 179 now passes'ultracode'through as if it were anOpenAIEffortLevel. Any caller that routes a generalEffortLevelthrough that helper can emit an invalid wire value instead of'xhigh'/'high'.🤖 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/utils/effort.ts` around lines 15 - 22, The addition of 'ultracode' to the EFFORT_LEVELS array exposes an inconsistency in the standardEffortToOpenAI() function: its fallback cast (around line 179) allows any EffortLevel value to pass through unchecked as an OpenAIEffortLevel, but 'ultracode' is not a valid OpenAIEffortLevel. Fix standardEffortToOpenAI() by removing the unsafe fallback cast and instead explicitly mapping all EffortLevel values (including 'ultracode') to valid OpenAIEffortLevel values using a complete switch statement or object mapping that covers all cases and maps 'ultracode' to an appropriate OpenAIEffortLevel like 'xhigh'.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/utils/attachments.ts`:
- Around line 1477-1483: The ultracode feature must be consistently gated by the
same provider/model check across both locations to prevent non-Anthropic
providers from accessing this capability. In src/utils/attachments.ts lines
1477-1483, modify the getUltracodePermissionAttachment function to add a
provider/model gate check (same gate used elsewhere for ultracode support)
before returning the ultracode_mode attachment, preventing manually set
effortValue values on other providers from granting the extra orchestration
permission. In src/utils/effort.ts lines 160-162, apply that same provider/model
gate check before adding ultracode to the advertised effort levels, ensuring
non-Anthropic models do not surface an unsupported option.
---
Outside diff comments:
In `@src/utils/effort.ts`:
- Around line 212-224: The toPersistableEffort() function currently includes
'ultracode' in its allow-list condition, which causes it to be persisted to
settings.json when it should remain session-only. Remove the 'value ===
ultracode' condition from the if statement in the toPersistableEffort() function
so that 'ultracode' is not returned and therefore not persisted, while keeping
all other values (low, medium, high, max, xhigh) in the allow-list.
- Around line 15-22: The addition of 'ultracode' to the EFFORT_LEVELS array
exposes an inconsistency in the standardEffortToOpenAI() function: its fallback
cast (around line 179) allows any EffortLevel value to pass through unchecked as
an OpenAIEffortLevel, but 'ultracode' is not a valid OpenAIEffortLevel. Fix
standardEffortToOpenAI() by removing the unsafe fallback cast and instead
explicitly mapping all EffortLevel values (including 'ultracode') to valid
OpenAIEffortLevel values using a complete switch statement or object mapping
that covers all cases and maps 'ultracode' to an appropriate OpenAIEffortLevel
like 'xhigh'.
🪄 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: 558f6596-ce5e-4779-bc6d-6f5c3f7ab0bc
⛔ 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 (7)
src/commands/effort/index.tssrc/components/messages/nullRenderingAttachments.tssrc/entrypoints/sdk/coreSchemas.tssrc/entrypoints/sdk/runtimeTypes.tssrc/utils/attachments.tssrc/utils/effort.tssrc/utils/messages.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx,js,jsx,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.{ts,tsx,js,jsx,py}: Follow the existing code style in the touched files
Keep comments useful and concise
Files:
src/commands/effort/index.tssrc/entrypoints/sdk/runtimeTypes.tssrc/components/messages/nullRenderingAttachments.tssrc/utils/messages.tssrc/entrypoints/sdk/coreSchemas.tssrc/utils/attachments.tssrc/utils/effort.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Typecheck TypeScript code before submitting (use
bun run typecheck)
Files:
src/commands/effort/index.tssrc/entrypoints/sdk/runtimeTypes.tssrc/components/messages/nullRenderingAttachments.tssrc/utils/messages.tssrc/entrypoints/sdk/coreSchemas.tssrc/utils/attachments.tssrc/utils/effort.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/commands/effort/index.tssrc/entrypoints/sdk/runtimeTypes.tssrc/components/messages/nullRenderingAttachments.tssrc/utils/messages.tssrc/entrypoints/sdk/coreSchemas.tssrc/utils/attachments.tssrc/utils/effort.ts
**
⚙️ CodeRabbit configuration file
**: # Contributing to OpenClaudeThanks for contributing.
OpenClaude is a fast-moving open-source coding-agent CLI with support for multiple providers, local backends, MCP, and a terminal-first workflow. The best contributions here are focused, well-tested, and easy to review.
Before You Start
- Search existing issues and discussions before opening a new thread.
- Check open pull requests for work that overlaps with your contribution. If a PR already exists that addresses the same change, open an issue or discussion first to align on direction — duplicate PRs may be closed without review.
- Use issues for confirmed bugs and actionable feature work.
- Use discussions for setup help, ideas, and general community conversation.
- For larger changes, open an issue first so the scope is clear before implementation.
- For security reports, follow SECURITY.md.
Pull Requests
Every PR needs a reason. Your PR description must include:
- what changed and why
- the user or developer impact
- the exact checks you ran
- a linked issue when one exists, using
Fixes#123, `Closes `#123, or another clear link- screenshots when the PR touches UI, terminal presentation, or the VS Code extension
- which provider path was tested when the PR changes provider behavior
The PR author is responsible for ensuring their PR is merge-ready. PRs with merge conflicts will not be reviewed or approved until the conflicts are resolved.
Issues are the recommended starting point for anything non-trivial — opening one first helps avoid wasted effort if the change is out of scope or already being worked on. Small fixes, doc corrections, and obvious improvements can stand on their own without a linked issue, as long as the PR description explains the intent.
What Gets Closed Without Review
PRs may be closed without review...
Files:
src/commands/effort/index.tssrc/entrypoints/sdk/runtimeTypes.tssrc/components/messages/nullRenderingAttachments.tssrc/utils/messages.tssrc/entrypoints/sdk/coreSchemas.tssrc/utils/attachments.tssrc/utils/effort.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/runtimeTypes.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/runtimeTypes.tssrc/entrypoints/sdk/coreSchemas.ts
🔇 Additional comments (4)
src/entrypoints/sdk/runtimeTypes.ts (1)
13-13: LGTM!src/entrypoints/sdk/coreSchemas.ts (1)
1083-1086: LGTM!src/components/messages/nullRenderingAttachments.ts (1)
21-57: LGTM!src/utils/messages.ts (1)
4262-4270: LGTM!
…EffortParams ultracode is not a persistable effort level — it maps to xhigh on the wire via resolveAppliedEffort. Narrowing toPersistableEffort's return type to Exclude<EffortLevel, 'ultracode'> and configureEffortParams' parameter to Exclude<EffortValue, 'ultracode'> satisfies the settings Zod schema and BetaOutputConfig.effort without adding ultracode to either. Fixes typecheck errors in effort.tsx, EffortCallout.tsx, ModelPicker.tsx, and claude.ts introduced when ultracode was added to EffortLevel. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…only - `getAvailableEffortLevels`: gate ultracode on `getAPIProvider() === 'firstParty'` so it never surfaces in the level picker for third-party / OpenCode routes - Change inner guard from `modelSupportsEffort` to `modelSupportsXHighEffort` since ultracode resolves to xhigh and should only appear for opus-4-7/4-8 - `standardEffortToOpenAI`: map ultracode → xhigh alongside max so the OpenAI wire shim never sees an invalid string - `getUltracodePermissionAttachment`: skip attachment when provider is not firstParty, preventing accidental injection on third-party routes - Update test expectation for opus-4-8 on firstParty (now correctly includes ultracode) and remove unreliable per-provider levels assertion from the modelUsesOpenAIEffort test (bun module-mock caching makes it non-deterministic)
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/utils/attachments.ts (1)
1478-1484:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd
modelSupportsXHighEffortguard to match the availability gate.The current gate checks only
firstPartyprovider, butgetAvailableEffortLevels(effort.ts:160) also requiresmodelSupportsXHighEffort(model). Without the model check here, a manually seteffortValue='ultracode'on a non-xhigh-supporting model (e.g., sonnet-4-6) would still inject the ultracode_mode attachment, granting multi-agent orchestration permission inappropriately.🔒 Proposed fix to add model support check
+import { modelSupportsXHighEffort } from './effort.js' + function getUltracodePermissionAttachment(toolUseContext: ToolUseContext): Attachment[] { const effort = toolUseContext.getAppState().effortValue - if (effort !== 'ultracode' || getAPIProvider() !== 'firstParty') { + if ( + effort !== 'ultracode' || + getAPIProvider() !== 'firstParty' || + !modelSupportsXHighEffort(toolUseContext.options.mainLoopModel) + ) { return [] } return [{ type: 'ultracode_mode' }] }Source: Past review comments
🤖 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/utils/attachments.ts` around lines 1478 - 1484, The getUltracodePermissionAttachment function currently only guards against non-firstParty providers but is missing a model support check. To match the availability gate used in getAvailableEffortLevels, add a check for modelSupportsXHighEffort(model) to the guard condition. Extract the model from toolUseContext (similar to how you extract effortValue) and include it in the if condition that returns an empty array, ensuring the ultracode_mode attachment is only injected when both the provider is firstParty AND the model actually supports the xhigh effort level.
🤖 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.
Duplicate comments:
In `@src/utils/attachments.ts`:
- Around line 1478-1484: The getUltracodePermissionAttachment function currently
only guards against non-firstParty providers but is missing a model support
check. To match the availability gate used in getAvailableEffortLevels, add a
check for modelSupportsXHighEffort(model) to the guard condition. Extract the
model from toolUseContext (similar to how you extract effortValue) and include
it in the if condition that returns an empty array, ensuring the ultracode_mode
attachment is only injected when both the provider is firstParty AND the model
actually supports the xhigh effort level.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 21702efb-8bae-44b4-b328-d05da3278ef9
📒 Files selected for processing (3)
src/utils/attachments.tssrc/utils/effort.codex.test.tssrc/utils/effort.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx,js,jsx,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.{ts,tsx,js,jsx,py}: Follow the existing code style in the touched files
Keep comments useful and concise
Files:
src/utils/effort.codex.test.tssrc/utils/attachments.tssrc/utils/effort.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Typecheck TypeScript code before submitting (use
bun run typecheck)
Files:
src/utils/effort.codex.test.tssrc/utils/attachments.tssrc/utils/effort.ts
**/*
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
src/utils/effort.codex.test.tssrc/utils/attachments.tssrc/utils/effort.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/effort.codex.test.ts
**
⚙️ CodeRabbit configuration file
**: # Contributing to OpenClaudeThanks for contributing.
OpenClaude is a fast-moving open-source coding-agent CLI with support for multiple providers, local backends, MCP, and a terminal-first workflow. The best contributions here are focused, well-tested, and easy to review.
Before You Start
- Search existing issues and discussions before opening a new thread.
- Check open pull requests for work that overlaps with your contribution. If a PR already exists that addresses the same change, open an issue or discussion first to align on direction — duplicate PRs may be closed without review.
- Use issues for confirmed bugs and actionable feature work.
- Use discussions for setup help, ideas, and general community conversation.
- For larger changes, open an issue first so the scope is clear before implementation.
- For security reports, follow SECURITY.md.
Pull Requests
Every PR needs a reason. Your PR description must include:
- what changed and why
- the user or developer impact
- the exact checks you ran
- a linked issue when one exists, using
Fixes#123, `Closes `#123, or another clear link- screenshots when the PR touches UI, terminal presentation, or the VS Code extension
- which provider path was tested when the PR changes provider behavior
The PR author is responsible for ensuring their PR is merge-ready. PRs with merge conflicts will not be reviewed or approved until the conflicts are resolved.
Issues are the recommended starting point for anything non-trivial — opening one first helps avoid wasted effort if the change is out of scope or already being worked on. Small fixes, doc corrections, and obvious improvements can stand on their own without a linked issue, as long as the PR description explains the intent.
What Gets Closed Without Review
PRs may be closed without review...
Files:
src/utils/effort.codex.test.tssrc/utils/attachments.tssrc/utils/effort.ts
🔇 Additional comments (10)
src/utils/effort.ts (4)
15-22: LGTM!
160-162: LGTM!
166-180: LGTM!
212-225: LGTM!Also applies to: 272-303, 360-375
src/utils/attachments.ts (4)
701-703: LGTM!
852-857: LGTM!
1470-1476: LGTM!
67-67: LGTM!src/utils/effort.codex.test.ts (2)
198-201: LGTM!
250-262: LGTM!
getUltracodePermissionAttachment only checked firstParty provider, not whether the active model supports xhigh. A stale settings.json with effortValue='ultracode' on a non-xhigh model (e.g. sonnet-4-6) would still inject the multi-agent orchestration permission, even though getAvailableEffortLevels would never have offered ultracode for that model. Mirror the same gate used in effort.ts's level picker. Addresses coderabbit review comment on PR Gitlawb#1630.
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
I found a couple of command-path issues that need to be addressed before this is ready.
Findings
-
[P2] Gate manual ultracode selection the same way as the picker
src/commands/effort/effort.tsx:113
The picker only offersultracodethroughgetAvailableEffortLevels(), and the attachment path only grants the multi-agent permission when the provider isfirstPartyand the model supports xhigh. The manual slash command bypasses that gate becauseexecuteEffort()accepts everyEffortLevel, so/effort ultracodesucceeds on unsupported providers or models. In those cases the user seesSet effort level to ultracode... xhigh effort + standing permission..., but the runtime either clamps the API effort tohighor sends onlyxhigh, andgetUltracodePermissionAttachment()returns no permission attachment. Please rejectultracodeon unsupported current provider/model combinations, or route the manual command through the same availability check used by the picker before confirming the mode. -
[P3] Update
/effort helpto include ultracode
src/commands/effort/effort.tsx:178
The command metadata now advertises[low|medium|high|xhigh|max|ultracode|auto], and the parser acceptsultracode, but/effort helpstill printsUsage: /effort [low|medium|high|max|xhigh|auto]and does not describe ultracode. The invalid-argument message at line 121 has the same stale option list. Please update the command help/error text so users can discover the new session-only mode from the command itself.
…update help text Addresses review feedback on Gitlawb#1630: - /effort ultracode now routes through UltracodeCommand which uses getAvailableEffortLevels(model) — the same gate as the picker — so unsupported provider/model combos get a clear error message instead of silently accepting the mode - Update help text and invalid-arg error to include ultracode with a note that it is session-only and Anthropic first-party + xhigh models only Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/commands/effort/effort.tsx`:
- Around line 176-185: The UltracodeCommand function introduces new behavior
with provider/model gating and a dedicated dispatch for the ultracode effort
level, but lacks corresponding command-level tests. Add tests that cover both
outcomes: the scenario where ultracode is not available for the current model
and provider (which should return the unavailable message), and the scenario
where ultracode is available (which should apply the ultracode session state via
setEffortValue). These tests should verify the behavior of
getAvailableEffortLevels, the conditional logic, and the proper dispatch of
setEffortValue when the ultracode path is available.
🪄 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: 71436238-920e-43e1-8832-6cc723938195
📒 Files selected for processing (1)
src/commands/effort/effort.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: typecheck
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx,py,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow the existing code style in the touched files
Files:
src/commands/effort/effort.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add or update tests when the change affects behavior in TypeScript and JavaScript files
Files:
src/commands/effort/effort.tsx
**/*
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
src/commands/effort/effort.tsx
**
⚙️ CodeRabbit configuration file
**: # Contributing to OpenClaudeThanks for contributing.
OpenClaude is a fast-moving open-source coding-agent CLI with support for multiple providers, local backends, MCP, and a terminal-first workflow. The best contributions here are focused, well-tested, and easy to review.
Before You Start
- Search existing issues and discussions before opening a new thread.
- Check open pull requests for work that overlaps with your contribution. If a PR already exists that addresses the same change, open an issue or discussion first to align on direction — duplicate PRs may be closed without review.
- Use issues for confirmed bugs and actionable feature work.
- Use discussions for setup help, ideas, and general community conversation.
- For larger changes, open an issue first so the scope is clear before implementation.
- For security reports, follow SECURITY.md.
Pull Requests
Every PR needs a reason. Your PR description must include:
- what changed and why
- the user or developer impact
- the exact checks you ran
- a linked issue when one exists, using
Fixes#123, `Closes `#123, or another clear link- screenshots when the PR touches UI, terminal presentation, or the VS Code extension
- which provider path was tested when the PR changes provider behavior
The PR author is responsible for ensuring their PR is merge-ready. PRs with merge conflicts will not be reviewed or approved until the conflicts are resolved.
Issues are the recommended starting point for anything non-trivial — opening one first helps avoid wasted effort if the change is out of scope or already being worked on. Small fixes, doc corrections, and obvious improvements can stand on their own without a linked issue, as long as the PR description explains the intent.
What Gets Closed Without Review
PRs may be closed without review...
Files:
src/commands/effort/effort.tsx
🔇 Additional comments (1)
src/commands/effort/effort.tsx (1)
7-7: LGTM!Also applies to: 121-122, 189-190
| function UltracodeCommand({ onDone }: { onDone: LocalJSXCommandOnDone }) { | ||
| const model = useMainLoopModel(); | ||
| const result = React.useMemo(() => { | ||
| if (!getAvailableEffortLevels(model).includes('ultracode')) { | ||
| return { message: 'ultracode is not available for your current model and provider. Use /effort without arguments to see available options.' }; | ||
| } | ||
| return setEffortValue('ultracode' as EffortValue); | ||
| }, [model]); | ||
| return <ApplyEffortAndClose result={result} onDone={onDone} />; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add command-level tests for the new ultracode path.
This introduces new behavior (provider/model gating + dedicated dispatch for /effort ultracode) but there’s no command-path test in this diff to lock both outcomes: unavailable gate message and available path applying ultracode session state.
As per coding guidelines, "Add or update tests when the change affects behavior in TypeScript and JavaScript files."
Also applies to: 198-201
🤖 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/commands/effort/effort.tsx` around lines 176 - 185, The UltracodeCommand
function introduces new behavior with provider/model gating and a dedicated
dispatch for the ultracode effort level, but lacks corresponding command-level
tests. Add tests that cover both outcomes: the scenario where ultracode is not
available for the current model and provider (which should return the
unavailable message), and the scenario where ultracode is available (which
should apply the ultracode session state via setEffortValue). These tests should
verify the behavior of getAvailableEffortLevels, the conditional logic, and the
proper dispatch of setEffortValue when the ultracode path is available.
Source: Coding guidelines
CodeRabbit requested tests for both branches of UltracodeCommand: the unavailable-for-model/provider path (returns the unavailable message) and the available path (dispatches setEffortValue and applies the ultracode session state).
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/commands/effort/effort.test.tsx`:
- Line 128: The Bun.sleep(10) call in the effort.test.tsx test file appears to
be an arbitrary delay. Investigate whether this sleep is actually required by
checking what React effects or async operations need to settle before the test
assertions run. If the delay is necessary, add a comment explaining why. If the
test can complete synchronously or should properly wait for specific async
operations, remove the sleep and either let the test complete naturally or use
appropriate testing utilities like waitFor to explicitly wait for the specific
effects or state changes needed.
- Around line 102-176: The existing test coverage for the UltracodeCommand
effectively tests the core happy and unhappy paths with the
mainLoopModelForSession varying between models with and without ultracode
support. Consider adding optional follow-up tests to cover additional edge
cases: create a test case where a non-firstParty provider blocks ultracode
availability even when the model would otherwise support it, add a test case
that validates the environment variable override behavior and ensures
persistence is prevented when appropriate, and add a test case that covers the
settings update failure path to ensure proper error handling when the state
update fails.
- Line 2: Remove the unused import statement that imports
stripVTControlCharacters as stripAnsi from 'node:util' at the top of the file.
Since this alias is never referenced anywhere in the test file, the entire
import line can be safely deleted.
- Line 109: The type assertion `{} as never` is incorrect for the `_context`
parameter in the `call` function invocation. The function expects `_context:
unknown`, not `never`. Remove the `as never` type assertion and pass an empty
object `{}` directly as the second argument to the `call` function. This
provides the correct type that the function signature expects while maintaining
the intended behavior of passing an empty context object.
🪄 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: 30ce0f0d-3a68-400b-9c4c-59ba6fcaaeaf
📒 Files selected for processing (1)
src/commands/effort/effort.test.tsx
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx,js,jsx,py,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow the existing code style in the touched files
Files:
src/commands/effort/effort.test.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add or update tests when the change affects behavior in TypeScript and JavaScript files
Files:
src/commands/effort/effort.test.tsx
**/*
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
src/commands/effort/effort.test.tsx
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
src/commands/effort/effort.test.tsx
**
⚙️ CodeRabbit configuration file
**: # Contributing to OpenClaudeThanks for contributing.
OpenClaude is a fast-moving open-source coding-agent CLI with support for multiple providers, local backends, MCP, and a terminal-first workflow. The best contributions here are focused, well-tested, and easy to review.
Before You Start
- Search existing issues and discussions before opening a new thread.
- Check open pull requests for work that overlaps with your contribution. If a PR already exists that addresses the same change, open an issue or discussion first to align on direction — duplicate PRs may be closed without review.
- Use issues for confirmed bugs and actionable feature work.
- Use discussions for setup help, ideas, and general community conversation.
- For larger changes, open an issue first so the scope is clear before implementation.
- For security reports, follow SECURITY.md.
Pull Requests
Every PR needs a reason. Your PR description must include:
- what changed and why
- the user or developer impact
- the exact checks you ran
- a linked issue when one exists, using
Fixes#123, `Closes `#123, or another clear link- screenshots when the PR touches UI, terminal presentation, or the VS Code extension
- which provider path was tested when the PR changes provider behavior
The PR author is responsible for ensuring their PR is merge-ready. PRs with merge conflicts will not be reviewed or approved until the conflicts are resolved.
Issues are the recommended starting point for anything non-trivial — opening one first helps avoid wasted effort if the change is out of scope or already being worked on. Small fixes, doc corrections, and obvious improvements can stand on their own without a linked issue, as long as the PR description explains the intent.
What Gets Closed Without Review
PRs may be closed without review...
Files:
src/commands/effort/effort.test.tsx
🔇 Additional comments (4)
src/commands/effort/effort.test.tsx (4)
145-145: ⚡ Quick winSame type assertion concern as line 109.
Verify the expected type for the second argument to
call.
168-168: 💤 Low valueSame arbitrary sleep timing as line 128.
Consider verifying if this delay is necessary or can be removed.
32-66: LGTM!
68-100: LGTM!
| test('/effort ultracode reports unavailable for a model without ultracode support', async () => { | ||
| const { call } = await importFreshEffortCommandModule() | ||
| const messages: (string | undefined)[] = [] | ||
| const onDone = (result?: string) => { | ||
| messages.push(result) | ||
| } | ||
|
|
||
| const element = await call(onDone, {} as never, 'ultracode') | ||
| const { stdout, stdin } = createTestStreams() | ||
|
|
||
| const instance = await render( | ||
| <AppStateProvider | ||
| initialState={{ | ||
| ...getDefaultAppState(), | ||
| mainLoopModelForSession: 'claude-sonnet-4-6', | ||
| }} | ||
| > | ||
| {element} | ||
| </AppStateProvider>, | ||
| { | ||
| stdout: stdout as unknown as NodeJS.WriteStream, | ||
| stdin: stdin as unknown as NodeJS.ReadStream, | ||
| patchConsole: false, | ||
| }, | ||
| ) | ||
|
|
||
| await Bun.sleep(10) | ||
| instance.unmount() | ||
| stdin.end() | ||
| stdout.end() | ||
|
|
||
| expect(messages).toEqual([ | ||
| 'ultracode is not available for your current model and provider. Use /effort without arguments to see available options.', | ||
| ]) | ||
| }) | ||
|
|
||
| test('/effort ultracode applies the ultracode session effort when available', async () => { | ||
| const { call } = await importFreshEffortCommandModule() | ||
| const messages: (string | undefined)[] = [] | ||
| const onDone = (result?: string) => { | ||
| messages.push(result) | ||
| } | ||
|
|
||
| const element = await call(onDone, {} as never, 'ultracode') | ||
| const { stdout, stdin } = createTestStreams() | ||
|
|
||
| let finalEffortValue: string | number | undefined | ||
| const instance = await render( | ||
| <AppStateProvider | ||
| initialState={{ | ||
| ...getDefaultAppState(), | ||
| mainLoopModelForSession: 'claude-opus-4-8', | ||
| }} | ||
| onChangeAppState={({ newState }) => { | ||
| finalEffortValue = newState.effortValue | ||
| }} | ||
| > | ||
| {element} | ||
| </AppStateProvider>, | ||
| { | ||
| stdout: stdout as unknown as NodeJS.WriteStream, | ||
| stdin: stdin as unknown as NodeJS.ReadStream, | ||
| patchConsole: false, | ||
| }, | ||
| ) | ||
|
|
||
| await Bun.sleep(10) | ||
| instance.unmount() | ||
| stdin.end() | ||
| stdout.end() | ||
|
|
||
| expect(messages).toHaveLength(1) | ||
| expect(messages[0]).toMatch(/^Set effort level to ultracode/) | ||
| expect(finalEffortValue).toBe('ultracode') | ||
| }) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚖️ Poor tradeoff
Test coverage is solid for core happy/unhappy paths.
The two tests effectively cover both branches of UltracodeCommand:
- Unavailable: model lacks xhigh support → correct error message
- Available: model supports xhigh → correct state update and success message
Optional edge cases for follow-up:
- Non-firstParty provider blocking ultracode
- Environment variable override preventing persistence
- Settings update failure path
Not blocking, but worth considering for comprehensive coverage.
🤖 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/commands/effort/effort.test.tsx` around lines 102 - 176, The existing
test coverage for the UltracodeCommand effectively tests the core happy and
unhappy paths with the mainLoopModelForSession varying between models with and
without ultracode support. Consider adding optional follow-up tests to cover
additional edge cases: create a test case where a non-firstParty provider blocks
ultracode availability even when the model would otherwise support it, add a
test case that validates the environment variable override behavior and ensures
persistence is prevented when appropriate, and add a test case that covers the
settings update failure path to ensure proper error handling when the state
update fails.
- Remove unused stripAnsi import
- Fix incorrect `{} as never` context arg in call() invocations
- Document why Bun.sleep(10) is needed before asserting onDone messages
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
Thanks for the update. I rechecked the changed paths and found a couple of issues that still need to be addressed.
Findings
-
[P2] Apply ultracode permission when the env override selects it
src/utils/attachments.ts:1480
AddingultracodetoEFFORT_LEVELSmakesCLAUDE_CODE_EFFORT_LEVEL=ultracodeparse as a valid override;resolveAppliedEffort()then maps that override toxhigh, and/effort currentreportsCurrent effort level: ultracode. However, the newultracode_modeattachment only checkstoolUseContext.getAppState().effortValue, so env-selected ultracode sends the API effort without the standing multi-agent permission that defines ultracode. Please either rejectultracodefrom the env override path or have the attachment gate account for the effective env-selected effort as well. -
[P3] Include ultracode in the top-level effort flag
src/main.tsx:944
The new level is usable from/effort, but the top-level--effortparser still allow-lists onlylow, medium, high, xhigh, max, soopenclaude --effort ultracodeis rejected before the session can start. Since this PR adds ultracode as a session-only effort level and the startup state already feedsoptions.effortthroughparseEffortValue(), please update the option help and allow-list so CLI startup supports the same effort level as the in-session command.
getUltracodePermissionAttachment only checked effortValue from app state, so CLAUDE_CODE_EFFORT_LEVEL=ultracode sent the xhigh API effort without the ultracode_mode permission attachment. Fix: also check getEffortEnvOverride() so env-selected ultracode produces the same attachment as the interactive /effort ultracode command. Also add 'ultracode' to the --effort CLI allow-list and update the help text so `claude --effort ultracode` is accepted. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
Thanks for the update. I rechecked the changed paths and found one issue that still needs to be addressed.
Findings
- [P2] Gate startup/env ultracode selection before accepting it
src/main.tsx:944
The interactive/effort ultracodepath now checksgetAvailableEffortLevels()before setting the session state, but the startup flag still accepts--effort ultracodeup front and copies it directly intoAppState.effortValuein both headless and interactive startup.CLAUDE_CODE_EFFORT_LEVEL=ultracodefollows the same ungated path throughgetEffortEnvOverride(). On unsupported providers or models,resolveAppliedEffort()then downgrades the API effort toxhighorhigh, whilegetUltracodePermissionAttachment()returns noultracode_modeattachment because it requires first-party plus an xhigh-capable model. That leaves users who start with--provider openai --effort ultracode, or set the env var on an unsupported model, seeing/selecting ultracode without the standing multi-agent permission that defines the mode. Please apply the same provider/model availability gate to the startup/env-selected ultracode path, or reject/ignore ultracode there when the permission attachment cannot be emitted.
…el availability Add clampUltracodeEffort() helper that falls back to 'max' when the current model/provider cannot emit the ultracode_mode permission attachment. Apply at both AppState init sites (headless + interactive startup) so --effort ultracode and CLAUDE_CODE_EFFORT_LEVEL=ultracode are silently clamped on unsupported configurations, matching the guard already present on the interactive /effort ultracode path. Update showCurrentEffort() to apply the same clamp before displaying the effective level. 3 new unit tests cover the non-firstParty, firstParty non-xhigh, and firstParty+opus-4-8 (preserved) cases. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main.tsx`:
- Line 2941: The clampUltracodeEffort call is using resolvedInitialModel when it
should use the effective session model that will actually execute the session.
Update the clampUltracodeEffort function call to pass effectiveModel instead of
resolvedInitialModel as the second argument to ensure ultracode effort is
clamped against the correct model that the session will run on, preventing
inconsistent behavior between interactive and headless startup.
🪄 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: 2d668531-d5be-4955-aa44-b79c70f1a8c1
📒 Files selected for processing (4)
src/commands/effort/effort.tsxsrc/main.tsxsrc/utils/effort.codex.test.tssrc/utils/effort.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx,js,jsx,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Keep comments useful and concise in code
Files:
src/utils/effort.codex.test.tssrc/main.tsxsrc/commands/effort/effort.tsxsrc/utils/effort.ts
**/*
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
src/utils/effort.codex.test.tssrc/main.tsxsrc/commands/effort/effort.tsxsrc/utils/effort.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/effort.codex.test.ts
**
⚙️ CodeRabbit configuration file
**: # Contributing to OpenClaudeThanks for contributing.
OpenClaude is a fast-moving open-source coding-agent CLI with support for multiple providers, local backends, MCP, and a terminal-first workflow. The best contributions here are focused, well-tested, and easy to review.
Before You Start
- Search existing issues and discussions before opening a new thread.
- Check open pull requests for work that overlaps with your contribution. If a PR already exists that addresses the same change, open an issue or discussion first to align on direction — duplicate PRs may be closed without review.
- Use issues for confirmed bugs and actionable feature work.
- Use discussions for setup help, ideas, and general community conversation.
- For larger changes, open an issue first so the scope is clear before implementation.
- For security reports, follow SECURITY.md.
Pull Requests
Every PR needs a reason. Your PR description must include:
- what changed and why
- the user or developer impact
- the exact checks you ran
- a linked issue when one exists, using
Fixes#123, `Closes `#123, or another clear link- screenshots when the PR touches UI, terminal presentation, or the VS Code extension
- which provider path was tested when the PR changes provider behavior
The PR author is responsible for ensuring their PR is merge-ready. PRs with merge conflicts will not be reviewed or approved until the conflicts are resolved.
Issues are the recommended starting point for anything non-trivial — opening one first helps avoid wasted effort if the change is out of scope or already being worked on. Small fixes, doc corrections, and obvious improvements can stand on their own without a linked issue, as long as the PR description explains the intent.
What Gets Closed Without Review
PRs may be closed without review...
Files:
src/utils/effort.codex.test.tssrc/main.tsxsrc/commands/effort/effort.tsxsrc/utils/effort.ts
{bin/**,scripts/**,package.json,src/setup.ts,src/main.tsx,src/entrypoints/**}
⚙️ CodeRabbit configuration file
{bin/**,scripts/**,package.json,src/setup.ts,src/main.tsx,src/entrypoints/**}: Review install, launcher, build, packaging, startup, and entrypoint changes for cross-platform compatibility, tracked-source rewrites, env/config precedence, and release safety. Block on changes that can break Windows/macOS/Linux startup or publish unexpected artifacts.
Files:
src/main.tsx
🔇 Additional comments (4)
src/utils/effort.ts (1)
256-261: LGTM!src/main.tsx (1)
55-55: LGTM!Also applies to: 2545-2545
src/commands/effort/effort.tsx (1)
7-7: LGTM!Also applies to: 65-66
src/utils/effort.codex.test.ts (1)
250-278: LGTM!
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
Thanks for the update. I rechecked the changed paths and found a couple of issues that still need to be addressed.
Findings
-
[P2] Complete CodeRabbit's request to clamp against the session model
src/main.tsx:2941
CodeRabbit's review item is still valid: the interactive startup path clamps--effort ultracodewithresolvedInitialModel, while the session can actually run on an agent-provided model throughmainLoopModelForSession. For example, if the default/user model supports ultracode but the selected startup agent runs a non-xhigh model, the initial state can keepeffortValue: 'ultracode';useMainLoopModel()then sends turns to the agent model, sogetUltracodePermissionAttachment()drops the permission reminder whileresolveAppliedEffort()only sends xhigh/high. The headless path already useseffectiveModel ?? resolvedInitialModel, so please complete the CodeRabbit request by applying the same effective-session-model clamp to the interactive initial state. -
[P2] Keep command-level ultracode from losing its permission reminder
src/utils/effort.ts:18
Addingultracodeto the globalEFFORT_LEVELSallow-list makeseffort: ultracodevalid for skill/plugin prompt commands, but those command turns do not get the defining ultracode permission attachment.processSlashCommand()builds attachment messages before returningcommand.efforttoonQuery, andgetUltracodePermissionAttachment()only checks AppState/env, so a command witheffort: ultracodesends xhigh API effort without the standing multi-agent permission that this PR says makes ultracode different from xhigh. Please either keepultracodeout of command/skill frontmatter until that surface is supported, or thread the command-level effort into attachment generation so the permission reminder is emitted on that turn.
…racode in frontmatter Addresses jatmn's 2026-06-16 review (two P2s): 1. Interactive startup clamped `--effort ultracode` against `resolvedInitialModel`, but the session can run on an agent-provided model via `mainLoopModelForSession`. If the user model supports ultracode while the startup agent runs a non-xhigh model, the initial state kept `effortValue: 'ultracode'` — useMainLoopModel() then routes turns to the agent model, so getUltracodePermissionAttachment() drops the permission while resolveAppliedEffort() only sends xhigh/high. Now uses `effectiveModel ?? resolvedInitialModel`, matching the headless path. 2. Adding `ultracode` to EFFORT_LEVELS made `effort: ultracode` valid in skill/agent/plugin frontmatter, but those command turns build attachments from AppState/env — not command.effort — so they'd send xhigh without the defining multi-agent permission. Added parseFrontmatterEffortValue() which rejects ultracode (session-only) while passing all other levels/integers through, and wired it into the four frontmatter parse sites (skills, agents, plugin agents, plugin commands) with a dedicated debug message. Build ✅. effort/loader tests ✅ locally. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Thanks — both P2s addressed in 1. Interactive startup clamp against the effective session model ( 2. Command-level ultracode losing its permission reminder ( Build ✅; 18/18 effort + 6/6 loader tests pass locally. |
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 (2)
src/utils/effort.ts (2)
270-278:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep
ultracodeout of model-picker persistence.Line 277 can now return
ultracodebecauseEffortLevelwas widened, but this helper decides what value to persist. That breaks the session-only contract and can make the multi-agent permission sticky if a caller saves the result directly.Proposed fix
export function resolvePickerEffortPersistence( picked: EffortLevel | undefined, modelDefault: EffortLevel, priorPersisted: EffortLevel | undefined, toggledInPicker: boolean, -): EffortLevel | undefined { +): Exclude<EffortLevel, 'ultracode'> | undefined { const hadExplicit = priorPersisted !== undefined || toggledInPicker - return hadExplicit || picked !== modelDefault ? picked : undefined + const persistablePicked = toPersistableEffort(picked) + return hadExplicit || persistablePicked !== modelDefault + ? persistablePicked + : undefined }🤖 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/utils/effort.ts` around lines 270 - 278, The resolvePickerEffortPersistence function can now return ultracode because EffortLevel was widened, but this breaks the session-only contract for ultracode and risks making the permission sticky if saved. Modify the return statement in resolvePickerEffortPersistence to filter out ultracode values before returning. Check if the picked value equals ultracode and return undefined in that case instead of persisting the ultracode value, ensuring ultracode remains session-only and never gets persisted to storage.
280-283:⚠️ Potential issue | 🟠 Major | ⚡ Quick winClamp
ultracodeto the applied effort, not alwaysmax.Line 282 rewrites unsupported
ultracodebefore startup stores it in AppState. On an xhigh-unsupported but max-capable model,--effort ultracodesendsmax, while the env override path reachesresolveAppliedEffort()and resolves the same value tohigh.Proposed fix
export function clampUltracodeEffort(effort: EffortValue | undefined, model: string): EffortValue | undefined { if (effort === 'ultracode' && !getAvailableEffortLevels(model).includes('ultracode')) { - return 'max' + return modelSupportsXHighEffort(model) ? 'xhigh' : 'high' } return effort }🤖 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/utils/effort.ts` around lines 280 - 283, The clampUltracodeEffort function currently returns a hardcoded 'max' when ultracode is unsupported, but this creates inconsistency with the env override path through resolveAppliedEffort() which may resolve to a different effort level like 'high' on models that support xhigh but not ultracode. Instead of returning 'max', apply the same effort resolution logic that resolveAppliedEffort() uses to determine the appropriate clamped effort level, ensuring both code paths (startup and env override) produce consistent results for unsupported ultracode on any given model.
🤖 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/utils/effort.ts`:
- Around line 270-278: The resolvePickerEffortPersistence function can now
return ultracode because EffortLevel was widened, but this breaks the
session-only contract for ultracode and risks making the permission sticky if
saved. Modify the return statement in resolvePickerEffortPersistence to filter
out ultracode values before returning. Check if the picked value equals
ultracode and return undefined in that case instead of persisting the ultracode
value, ensuring ultracode remains session-only and never gets persisted to
storage.
- Around line 280-283: The clampUltracodeEffort function currently returns a
hardcoded 'max' when ultracode is unsupported, but this creates inconsistency
with the env override path through resolveAppliedEffort() which may resolve to a
different effort level like 'high' on models that support xhigh but not
ultracode. Instead of returning 'max', apply the same effort resolution logic
that resolveAppliedEffort() uses to determine the appropriate clamped effort
level, ensuring both code paths (startup and env override) produce consistent
results for unsupported ultracode on any given model.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: aa26d8bf-5ba4-4cc4-90ae-de3d1fa9cc4f
📒 Files selected for processing (7)
src/main.tsxsrc/skills/loadSkillsDir.tssrc/tools/AgentTool/loadAgentsDir.tssrc/utils/effort.codex.test.tssrc/utils/effort.tssrc/utils/plugins/loadPluginAgents.tssrc/utils/plugins/loadPluginCommands.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx,js,jsx,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Keep comments useful and concise in code
Files:
src/utils/plugins/loadPluginAgents.tssrc/utils/plugins/loadPluginCommands.tssrc/utils/effort.codex.test.tssrc/skills/loadSkillsDir.tssrc/main.tsxsrc/tools/AgentTool/loadAgentsDir.tssrc/utils/effort.ts
**/*
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
src/utils/plugins/loadPluginAgents.tssrc/utils/plugins/loadPluginCommands.tssrc/utils/effort.codex.test.tssrc/skills/loadSkillsDir.tssrc/main.tsxsrc/tools/AgentTool/loadAgentsDir.tssrc/utils/effort.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/loadPluginCommands.tssrc/skills/loadSkillsDir.ts
**
⚙️ CodeRabbit configuration file
**: # Contributing to OpenClaudeThanks for contributing.
OpenClaude is a fast-moving open-source coding-agent CLI with support for multiple providers, local backends, MCP, and a terminal-first workflow. The best contributions here are focused, well-tested, and easy to review.
Before You Start
- Search existing issues and discussions before opening a new thread.
- Check open pull requests for work that overlaps with your contribution. If a PR already exists that addresses the same change, open an issue or discussion first to align on direction — duplicate PRs may be closed without review.
- Use issues for confirmed bugs and actionable feature work.
- Use discussions for setup help, ideas, and general community conversation.
- For larger changes, open an issue first so the scope is clear before implementation.
- For security reports, follow SECURITY.md.
Pull Requests
Every PR needs a reason. Your PR description must include:
- what changed and why
- the user or developer impact
- the exact checks you ran
- a linked issue when one exists, using
Fixes#123, `Closes `#123, or another clear link- screenshots when the PR touches UI, terminal presentation, or the VS Code extension
- which provider path was tested when the PR changes provider behavior
The PR author is responsible for ensuring their PR is merge-ready. PRs with merge conflicts will not be reviewed or approved until the conflicts are resolved.
Issues are the recommended starting point for anything non-trivial — opening one first helps avoid wasted effort if the change is out of scope or already being worked on. Small fixes, doc corrections, and obvious improvements can stand on their own without a linked issue, as long as the PR description explains the intent.
What Gets Closed Without Review
PRs may be closed without review...
Files:
src/utils/plugins/loadPluginAgents.tssrc/utils/plugins/loadPluginCommands.tssrc/utils/effort.codex.test.tssrc/skills/loadSkillsDir.tssrc/main.tsxsrc/tools/AgentTool/loadAgentsDir.tssrc/utils/effort.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/effort.codex.test.ts
{bin/**,scripts/**,package.json,src/setup.ts,src/main.tsx,src/entrypoints/**}
⚙️ CodeRabbit configuration file
{bin/**,scripts/**,package.json,src/setup.ts,src/main.tsx,src/entrypoints/**}: Review install, launcher, build, packaging, startup, and entrypoint changes for cross-platform compatibility, tracked-source rewrites, env/config precedence, and release safety. Block on changes that can break Windows/macOS/Linux startup or publish unexpected artifacts.
Files:
src/main.tsx
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.ts
🔇 Additional comments (8)
src/utils/effort.ts (2)
15-22: LGTM!Also applies to: 137-180, 204-249
303-405: LGTM!src/main.tsx (1)
55-55: LGTM!Also applies to: 944-950, 2545-2545, 2941-2941
src/skills/loadSkillsDir.ts (1)
30-31: LGTM!Also applies to: 229-236
src/tools/AgentTool/loadAgentsDir.ts (1)
18-21: LGTM!Also applies to: 609-620
src/utils/plugins/loadPluginAgents.ts (1)
15-15: LGTM!Also applies to: 143-153
src/utils/plugins/loadPluginCommands.ts (1)
11-11: LGTM!Also applies to: 282-290
src/utils/effort.codex.test.ts (1)
280-299: LGTM!
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
Thanks for the update. I rechecked the changed paths and found two issues that still need to be addressed.
Findings
-
[P2] Keep ultrathink behind the rollout gate
src/utils/attachments.ts:1471
This patch removes theisUltrathinkEnabled()check fromgetUltrathinkEffortAttachment(), so the model-facingultrathink_effortattachment is now emitted for any input containing the keyword even when theULTRATHINKbuild flag/GrowthBook gate is disabled. The UI still hides highlighting and the notification behindisUltrathinkEnabled(), but the hidden backend behavior can still silently raise the current turn to high effort. Please restore the feature-gate check in the attachment path so disabled rollouts do not affect API behavior. -
[P2] Complete CodeRabbit's request to clamp ultracode consistently
src/utils/effort.ts:280
CodeRabbit's later review item is still valid:clampUltracodeEffort()rewrites unsupportedultracodeto hardcodedmax, while the normalresolveAppliedEffort()path maps rawultracodetoxhighorhigh. That leaves equivalent user requests taking different API paths. For example, onclaude-opus-4-6(max-capable but not xhigh/ultracode-capable),--effort ultracodeis clamped into app state asmaxand then sendsmax, butCLAUDE_CODE_EFFORT_LEVEL=ultracoderesolves the same requested mode tohigh. Please complete the CodeRabbit request by having the startup/display clamp use the same applied-effort resolution as the env/app-state path, and update the tests that currently assert the hardcodedmaxfallback.
…applied effort Addresses jatmn's review on Gitlawb#1630: - attachments.ts: restore the isUltrathinkEnabled() rollout gate in getUltrathinkEffortAttachment so the hidden ultrathink_effort attachment is no longer emitted (silently raising the turn to high effort) when the ULTRATHINK build flag / GrowthBook rollout is disabled. The UI already gated highlighting and the notification; the backend path now matches. - effort.ts: clampUltracodeEffort no longer hardcodes 'max' for unsupported ultracode. It now mirrors resolveAppliedEffort's ultracode mapping (xhigh when the model is xhigh-capable, else high), so `--effort ultracode` (clamped into app state) and CLAUDE_CODE_EFFORT_LEVEL=ultracode (resolved live) send the SAME effort to the API. They previously diverged on max-capable-but-not-xhigh models like opus-4-6 (max vs high). - effort.codex.test.ts: update the two tests that asserted the hardcoded 'max' fallback to the consistent xhigh/high values, and pin each to resolveAppliedEffort('ultracode') so the two paths can't drift again. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
Thanks for the update. I rechecked the changed paths and found a couple of issues that should be addressed.
Findings
-
[P2] Honor provider overrides before emitting ultracode permission
src/utils/attachments.ts
getUltracodePermissionAttachment()only checks the process-widegetAPIProvider()before adding theultracode_modereminder. Subagents can now run through a per-agentproviderOverride(runAgent.tssetsoptions.providerOverride, andquery.tspasses that togetAnthropicClient(), which routes through the OpenAI shim), while the attachment path still sees the global first-party provider and the inherited sessioneffortValue. In a first-party parent session withultracodeenabled, a routed subagent whose effective model passesmodelSupportsXHighEffort()will therefore receive the standing multi-agent permission reminder even though the actual request is sent to the third-party override. Please gate this attachment on the effective request provider, or suppress it whenevertoolUseContext.options.providerOverrideis set, so the first-party-only ultracode permission does not leak into routed third-party agent calls. -
[P3] Update the PR description to match the restored ultrathink gate
PR description
The current PR description still says ultrathink keyword detection is "always active", that theisUltrathinkEnabled()gate was removed fromgetUltrathinkEffortAttachment(), and that prompts now unconditionally inject the high-effort reminder. The current patch does the opposite after the latest fix:src/utils/attachments.tsnow checksisUltrathinkEnabled()before emittingultrathink_effort, so disabledULTRATHINK/ GrowthBook rollouts do not affect API behavior. Please update the PR summary/files/testing text so reviewers and release-note consumers do not merge a description that promises behavior the code no longer implements.
2 participants
Closes #1551
Summary
Two related CLI feature-parity improvements:
ultrathink keyword detection (always active)
isUltrathinkEnabled()build-flag gate (feature('ULTRATHINK')+ GrowthBooktengu_turtle_carbon) fromgetUltrathinkEffortAttachment().\bultrathink\b/ikeyword in a user prompt now unconditionally injects a per-turn system-reminder requesting high reasoning effort — no build flag or flag rollout required.isUltrathinkEnabled()itself is not removed; it is still used ineffort.tsfor Opus medium defaults.ultracodeeffort levelEffortLevelvalue:'ultracode'.'xhigh'on the wire (or'high'for models that don't support xhigh)./effort ultracodeon the Anthropic provider path.effortValue === 'ultracode', injects a standing system-reminder granting permission to orchestrate multi-agent workflows (spawn subagents, parallelize tasks, coordinate tool calls without confirmation).Files changed (8)
src/entrypoints/sdk/runtimeTypes.ts'ultracode'toEffortLevelunionsrc/entrypoints/sdk/coreTypes.generated.ts'ultracode'tosupportedEffortLevelsunionsrc/entrypoints/sdk/coreSchemas.ts'ultracode'to Zod schema forsupportedEffortLevelssrc/utils/effort.tssrc/utils/attachments.tsultracode_modeattachment + handlersrc/utils/messages.tscase 'ultracode_mode':system-reminder injectionsrc/components/messages/nullRenderingAttachments.ts'ultracode_mode'to null-render listsrc/commands/effort/index.tsultracodeTesting
bun run typecheckin worktree: zero new errors (pre-existing baseline: missing@types/node,react,bun:bundle,zod/v4— not introduced by this PR).isUltrathinkEnabled()call site ineffort.ts:451preserved and still compiles.Summary by CodeRabbit