fix(deps): ship a zero-warning, minimal install - #1784
Conversation
📝 WalkthroughWalkthroughThis PR adds an optional runtime-import helper, routes provider/auth/image-loading code through it, and updates setup docs, package metadata, externals validation, and tests to match the new split between bundled and optional packages. ChangesOptional runtime packages
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
please do extensive review on this bro @jatmn |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@docs/advanced-setup.md`:
- Around line 101-108: The Vertex auth section in the setup guide is broader
than the actual steps shown: the text in the authentication block mentions both
a service-account file and local ADC, but the documented flow only includes ADC
via gcloud auth application-default login. Update the wording around the auth
instructions in this section to match the real setup path, or add the missing
service-account-file step if that is intended, so the guidance is consistent
with the steps under the relevant heading.
In `@scripts/validate-externals.ts`:
- Around line 32-34: The allowlist check in validate-externals.ts is too broad
because INTENTIONALLY_BUNDLED is being applied globally, which can hide missing
SDK externals. Update the validation logic around the runtimeDeps filter so the
bundled exemption is bundle-specific (for example, scoped by package/build
target) rather than shared across SDK and CLI, and ensure the validation still
fails if SDK-only dependencies like react or `@anthropic-ai/sdk` are missing from
the SDK externals set.
- Around line 63-71: The current INTENTIONALLY_BUNDLED validation in
validate-externals.ts only checks declaredDeps, which includes runtimeDeps, so
bundled packages can still slip through if they remain in dependencies. Add a
separate explicit check in the validation flow around the INTENTIONALLY_BUNDLED
and declaredDeps logic to detect any overlap with dependencies (not just
devDependencies) and fail with a clear error message. Keep the existing
stale-entry check, but extend the guard so bundled packages must be moved out of
dependencies to preserve the minimal install contract.
In `@src/utils/optionalRuntimeModule.ts`:
- Around line 44-48: The missing-module detection in optionalRuntimeModule
should not use a raw substring match on message.includes(specifier), since that
can misidentify similar package names. Update the ERR_MODULE_NOT_FOUND check in
the optionalRuntimeModule logic to match the requested specifier as a quoted
token in the error message, using the existing feature/specifier variables, so
only the exact missing package triggers the install hint.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: d937a6c2-2c60-4d9e-8fef-031365fb7f8a
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (14)
docs/advanced-setup.mdknip.jsonpackage.jsonscripts/externals.tsscripts/validate-externals.tssrc/services/api/client.tssrc/services/tokenEstimation.tssrc/tools/FileReadTool/imageProcessor.tssrc/utils/auth.tssrc/utils/aws.tssrc/utils/geminiAuth.tssrc/utils/model/bedrock.tssrc/utils/optionalRuntimeModule.test.tssrc/utils/optionalRuntimeModule.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (18)
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript with strict mode and ESM imports
Files:
src/tools/FileReadTool/imageProcessor.tssrc/utils/optionalRuntimeModule.tssrc/utils/optionalRuntimeModule.test.tssrc/utils/aws.tssrc/services/tokenEstimation.tssrc/utils/geminiAuth.tssrc/utils/auth.tssrc/utils/model/bedrock.tssrc/services/api/client.ts
**/*.{ts,tsx,js,jsx,py,json,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow the existing code style in the touched files
Files:
src/tools/FileReadTool/imageProcessor.tsknip.jsonsrc/utils/optionalRuntimeModule.tssrc/utils/optionalRuntimeModule.test.tssrc/utils/aws.tssrc/services/tokenEstimation.tssrc/utils/geminiAuth.tssrc/utils/auth.tsscripts/validate-externals.tsdocs/advanced-setup.mdscripts/externals.tssrc/utils/model/bedrock.tssrc/services/api/client.tspackage.json
**/*.{ts,tsx,js,jsx,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Keep comments useful and concise
Files:
src/tools/FileReadTool/imageProcessor.tssrc/utils/optionalRuntimeModule.tssrc/utils/optionalRuntimeModule.test.tssrc/utils/aws.tssrc/services/tokenEstimation.tssrc/utils/geminiAuth.tssrc/utils/auth.tsscripts/validate-externals.tsscripts/externals.tssrc/utils/model/bedrock.tssrc/services/api/client.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow TypeScript strict mode and type safety practices by running typecheck before submitting
Files:
src/tools/FileReadTool/imageProcessor.tssrc/utils/optionalRuntimeModule.tssrc/utils/optionalRuntimeModule.test.tssrc/utils/aws.tssrc/services/tokenEstimation.tssrc/utils/geminiAuth.tssrc/utils/auth.tsscripts/validate-externals.tsscripts/externals.tssrc/utils/model/bedrock.tssrc/services/api/client.ts
**
⚙️ CodeRabbit configuration file
**: # AGENTS.md - AI Agent Coding GuideThis guide is for AI coding agents working in the OpenClaude repository. Read it before changing code, and also follow CONTRIBUTING.md for contributor policy, PR expectations, review follow-up, and project scope.
Project Snapshot
OpenClaude is a coding-agent CLI for cloud and local model providers. It supports OpenAI-compatible APIs, Anthropic, Gemini, DeepSeek, Ollama, MCP, local backends, slash commands, tools, agents, and a React/Ink terminal UI.
The installed CLI runs on Node.js
>=22.0.0. Bun is used for source builds, scripts, dependency management, and tests.Work Style
- Keep changes focused on one problem.
- Prefer existing patterns in the file or nearby module.
- Avoid unrelated formatting, renames, dependency changes, or broad rewrites.
- Add or update tests when behavior changes.
- Update docs when setup, commands, provider behavior, or user-facing behavior changes.
- For new features, larger refactors, dependencies, or runtime changes, follow the issue-first guidance in CONTRIBUTING.md.
Stack And Conventions
- TypeScript with strict mode and ESM imports.
- React + Ink for terminal UI.
- Bun lockfile and Bun scripts for development workflows.
- Node runtime for the built CLI.
- Python exists for legacy/local-provider helper code. Do not add new Python code or expand Python-based features unless a maintainer explicitly approves that direction.
Common libraries and patterns:
chalkfor terminal color.commanderfor CLI argument parsing.execafor child processes.- Existing service, provider, settings, permission, and UI patterns over new abstractions.
Repository Map
src/commands/- slash and CLI command implementations.src/components/- React/Ink UI components.src/services/- API, MCP, OAuth, wiki, voice, and other service integrations.src/tools/- tool implementations.src/utils/- shared utilities.- `src/integration...
Files:
src/tools/FileReadTool/imageProcessor.tsknip.jsonsrc/utils/optionalRuntimeModule.tssrc/utils/optionalRuntimeModule.test.tssrc/utils/aws.tssrc/services/tokenEstimation.tssrc/utils/geminiAuth.tssrc/utils/auth.tsscripts/validate-externals.tsdocs/advanced-setup.mdscripts/externals.tssrc/utils/model/bedrock.tssrc/services/api/client.tspackage.json
**/*
⚙️ 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/tools/FileReadTool/imageProcessor.tsknip.jsonsrc/utils/optionalRuntimeModule.tssrc/utils/optionalRuntimeModule.test.tssrc/utils/aws.tssrc/services/tokenEstimation.tssrc/utils/geminiAuth.tssrc/utils/auth.tsscripts/validate-externals.tsdocs/advanced-setup.mdscripts/externals.tssrc/utils/model/bedrock.tssrc/services/api/client.tspackage.json
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/FileReadTool/imageProcessor.ts
{src/services/**/*.ts,src/utils/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
execafor child processes
Files:
src/utils/optionalRuntimeModule.tssrc/utils/optionalRuntimeModule.test.tssrc/utils/aws.tssrc/services/tokenEstimation.tssrc/utils/geminiAuth.tssrc/utils/auth.tssrc/utils/model/bedrock.tssrc/services/api/client.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add or update tests when the change affects behavior
Files:
src/utils/optionalRuntimeModule.test.ts
**/*.test.{ts,tsx,js}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Test the exact provider/model path you changed when possible
Files:
src/utils/optionalRuntimeModule.test.ts
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
src/utils/optionalRuntimeModule.test.ts
{src/commands/**/*.ts,src/services/**/*.ts,src/entrypoints/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
chalkfor terminal color in CLI code
Files:
src/services/tokenEstimation.tssrc/services/api/client.ts
{src/integrations/**/*.ts,src/services/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Test the exact provider/model path you changed when possible for provider modifications
Files:
src/services/tokenEstimation.tssrc/services/api/client.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:
scripts/validate-externals.tsscripts/externals.tspackage.json
docs/**/*.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Update docs when setup, commands, or user-facing behavior changes
Files:
docs/advanced-setup.md
{README.md,CONTRIBUTING.md,docs/**,.github/pull_request_template.md}
⚙️ CodeRabbit configuration file
{README.md,CONTRIBUTING.md,docs/**,.github/pull_request_template.md}: Review docs for accuracy against current code behavior. Flag security or provider claims that overpromise, stale install commands, missing setup caveats, and instructions that could push users toward unsafe credential handling. Keep purely wording-level suggestions non-blocking.
Files:
docs/advanced-setup.md
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}
⚙️ CodeRabbit configuration file
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}: Review provider routing, model selection, env precedence, auth/token handling, OpenAI-compatible shims, retries, proxy behavior, and outbound HTTP behavior with high scrutiny. Block on silent default changes, hidden fallback expansion, credential reuse mistakes, hardcoded provider assumptions, or new network reach that is not intentional and documented.
Files:
src/utils/model/bedrock.tssrc/services/api/client.ts
{bun.lock,bunfig.toml,package.json}
📄 CodeRabbit inference engine (AGENTS.md)
Use Bun lockfile and Bun scripts for development workflows and dependency management
Files:
package.json
🔇 Additional comments (14)
src/tools/FileReadTool/imageProcessor.ts (1)
43-45: LGTM!docs/advanced-setup.md (1)
329-346: LGTM!knip.json (1)
32-36: LGTM!package.json (1)
78-82: LGTM!Also applies to: 92-94, 119-119, 138-158
scripts/externals.ts (1)
9-14: LGTM!Also applies to: 43-69, 80-85
scripts/validate-externals.ts (1)
12-26: LGTM!Also applies to: 48-50
src/utils/optionalRuntimeModule.test.ts (1)
7-45: LGTM!src/services/api/client.ts (1)
488-491: LGTM!Also applies to: 535-538, 551-554, 577-580
src/utils/auth.ts (1)
43-43: LGTM!Also applies to: 871-876
src/utils/geminiAuth.ts (1)
6-6: LGTM!Also applies to: 137-140
src/utils/aws.ts (1)
2-2: LGTM!Also applies to: 65-68
src/utils/model/bedrock.ts (1)
13-16: LGTM!Note: the
@aws-sdk/client-bedrockvs@aws-sdk/client-bedrock-runtimespecifiers used here are the concrete trigger for the substring-guard concern raised at the root cause insrc/utils/optionalRuntimeModule.ts; no change needed in this file.Also applies to: 55-57, 103-105, 153-156
src/services/tokenEstimation.ts (1)
2-2: LGTM!Also applies to: 698-700
src/utils/optionalRuntimeModule.ts (1)
35-51: 🩺 Stability & AvailabilityCheck Bun’s missing-module error shape.
importOptionalRuntimeModuleonly rewrites the error whencode === 'ERR_MODULE_NOT_FOUND'; the current test only checks the friendly message, so add a Bun-specific assertion for the missing-package case or relax the guard if Bun omitscode.
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
I found a few issues that need to be addressed before this is ready.
Findings
-
[P2] Complete CodeRabbit's request to harden the intentionally-bundled validation
scripts/validate-externals.ts:32
INTENTIONALLY_BUNDLEDis currently exempt from both the CLI and SDK externals checks, even though that list includes packages that are only supposed to be bundled in the CLI and external in the SDK, such asreactand@anthropic-ai/sdk. I verified that if either package is removed fromSDK_EXTERNALS, the current validator still reports no missing dependency, so the new build guard would not catch a broken SDK publish surface. The companion validation gap is still present too:validateIntentionallyBundled()only checks that bundled packages exist somewhere inpackage.json, which allows a bundled package to accidentally remain independenciesorpeerDependenciesand still pass while violating the minimal-install contract. Please complete CodeRabbit's validator requests by scoping the bundled exemption per bundle target and adding an explicit check that everyINTENTIONALLY_BUNDLEDpackage is indevDependencies, failing clearly if it is in runtime dependencies or peer dependencies. -
[P2] Keep the image fallback on the optional sharp loader
src/tools/FileReadTool/FileReadTool.ts:1260
The PR movessharpout of runtime dependencies and updatesgetImageProcessor()to emit the newnpm i -g sharpguidance, but this oversized-image fallback still performs a directimport('sharp')aftercompressImageBufferWithTokenLimit()fails. Whensharpis absent in the new minimal install, that first compression failure can be the intendedImageProcessorUnavailableError, but this catch block swallows it, attempts a raw import of the same optional package, logs the module-resolution failure, and returns the original oversized image anyway. That bypasses the documented install hint and can send an image that already exceeded the token budget. Please route this fallback through the same optional image loader or let the missing-image-processor error surface instead of returning the uncompressed image. -
[P2] Complete CodeRabbit's request to match the missing specifier exactly
src/utils/optionalRuntimeModule.ts:44
The guardmessage.includes(specifier)can misidentify a missing transitive package as the requested optional package when the requested specifier is a substring of the real missing module, such assharpversussharp-libvips, or@aws-sdk/client-bedrockversus@aws-sdk/client-bedrock-runtime. That produces a misleading install hint for the wrong package instead of surfacing the real resolver failure. Please complete CodeRabbit's request by matching the requested specifier as an exact quoted token, for example inside single or double quotes, so the friendly error only fires when the requested package itself is missing. -
[P2] Complete CodeRabbit's request to fix the Vertex auth docs drift
docs/advanced-setup.md:108
The updated Vertex section says users can authenticate with either a service-account file or local ADC, but the only concrete command shown isgcloud auth application-default login. A reader expecting the service-account-file path will not find it. Please either add the missing service-account setup step, such as settingGOOGLE_APPLICATION_CREDENTIALS, or remove the "either...or" wording so the docs match the single documented flow.
362feea to
03a242e
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/services/api/client.ts (1)
620-684: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDefer the Vertex auth import until after the skip-auth gate.
Line 620 still imports
google-auth-librarybefore theCLAUDE_CODE_SKIP_VERTEX_AUTHbranch is checked, so a minimal install now throws even though the mock path at Lines 658-668 never usesGoogleAuth.💡 Minimal fix
- const { GoogleAuth } = await importOptionalRuntimeModule( - 'google-auth-library', - 'Vertex AI (GCP) authentication', - ) + const skipVertexAuth = isEnvTruthy(process.env.CLAUDE_CODE_SKIP_VERTEX_AUTH) + const GoogleAuth = skipVertexAuth + ? undefined + : ( + await importOptionalRuntimeModule<typeof import('google-auth-library')>( + 'google-auth-library', + 'Vertex AI (GCP) authentication', + ) + ).GoogleAuth @@ - const googleAuth = isEnvTruthy(process.env.CLAUDE_CODE_SKIP_VERTEX_AUTH) + const googleAuth = skipVertexAuth @@ - : new GoogleAuth({ + : new GoogleAuth!({🤖 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/services/api/client.ts` around lines 620 - 684, Move the optional `google-auth-library` import in `getAnthropicClient()` so it happens only after the `CLAUDE_CODE_SKIP_VERTEX_AUTH` check, since the mock branch does not need `GoogleAuth` and should not fail on minimal installs. Keep the skip-auth path using the existing inline mock, and instantiate `GoogleAuth` only in the non-skipped branch where the Vertex auth setup is actually required.src/tools/FileReadTool/FileReadTool.ts (1)
1262-1282: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAvoid logging the handled missing-processor path twice.
This branch now rethrows
ImageProcessorUnavailableError, but the outer catch still logs the original compression failure first. IfcompressImageBufferWithTokenLimit()fails only because no processor is installed, users get both an error log and the install hint for the same expected condition. Short-circuit that error beforelogError(e).Suggested fix
} catch (e) { + if (e instanceof ImageProcessorUnavailableError) throw e logError(e) // Fallback: heavily compressed version from the SAME buffer, loaded via // the shared optional image processor (NOT a raw import('sharp')). This // keeps the missing-image-processor contract: when no processor is🤖 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/tools/FileReadTool/FileReadTool.ts` around lines 1262 - 1282, The missing-image-processor path in compressImageBufferWithTokenLimit is being handled twice, causing both an error log and the install hint for the same expected condition. Update the outer error handling around the fallback compression flow in FileReadTool to detect ImageProcessorUnavailableError before calling logError(e), and short-circuit by rethrowing or returning immediately so the handled missing-processor case is not logged again. Use the compressImageBufferWithTokenLimit and getImageProcessor/ImageProcessorUnavailableError branches to locate the fix.
🤖 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 `@scripts/externalsValidation.ts`:
- Around line 10-14: The optional-peer install contract is not being checked
because PkgDeps only models dependencies, peerDependencies, and devDependencies,
and validateIntentionallyBundled() never verifies peerDependenciesMeta. Update
the validator to read peerDependenciesMeta and assert the SDK-only peer set is
present there with optional set to true, so a missing or non-optional peer
causes the script to fail. Use the existing PkgDeps type and
validateIntentionallyBundled() logic to locate and enforce this check.
In `@scripts/validate-externals.ts`:
- Around line 19-22: The validation in validate-externals.ts is too weak because
runtimeDeps only checks dependencies and peerDependencies, so
OPTIONAL_RUNTIME_EXTERNALS can be removed from CLI_EXTERNALS or SDK_EXTERNALS
without failing. Update the validation logic around the runtimeDeps and extra
externals checks to explicitly assert that every symbol in
OPTIONAL_RUNTIME_EXTERNALS (for example, the always-external entries defined in
scripts/externals.ts) is present in both bundle allowlists, and fail the script
if any are missing.
In `@src/utils/optionalRuntimeModule.test.ts`:
- Around line 14-95: The current tests only cover the shared module-loading
helpers and miss the provider-specific Bedrock, Foundry, Vertex, and Gemini
branches. Add focused regression tests for the actual provider/runtime
entrypoints that call importOptionalRuntimeModule and importRuntimeModule,
asserting the correct routing and auth/skip-auth behavior for each provider
path. Keep the existing helper tests, but extend coverage to the user-visible
provider/model flows so branch-specific regressions are caught.
---
Outside diff comments:
In `@src/services/api/client.ts`:
- Around line 620-684: Move the optional `google-auth-library` import in
`getAnthropicClient()` so it happens only after the
`CLAUDE_CODE_SKIP_VERTEX_AUTH` check, since the mock branch does not need
`GoogleAuth` and should not fail on minimal installs. Keep the skip-auth path
using the existing inline mock, and instantiate `GoogleAuth` only in the
non-skipped branch where the Vertex auth setup is actually required.
In `@src/tools/FileReadTool/FileReadTool.ts`:
- Around line 1262-1282: The missing-image-processor path in
compressImageBufferWithTokenLimit is being handled twice, causing both an error
log and the install hint for the same expected condition. Update the outer error
handling around the fallback compression flow in FileReadTool to detect
ImageProcessorUnavailableError before calling logError(e), and short-circuit by
rethrowing or returning immediately so the handled missing-processor case is not
logged again. Use the compressImageBufferWithTokenLimit and
getImageProcessor/ImageProcessorUnavailableError branches to locate the fix.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5fa55f8a-0e36-4bd8-9b98-1c533b2c0070
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (17)
docs/advanced-setup.mdknip.jsonpackage.jsonscripts/externals.tsscripts/externalsValidation.test.tsscripts/externalsValidation.tsscripts/validate-externals.tssrc/services/api/client.tssrc/services/tokenEstimation.tssrc/tools/FileReadTool/FileReadTool.tssrc/tools/FileReadTool/imageProcessor.tssrc/utils/auth.tssrc/utils/aws.tssrc/utils/geminiAuth.tssrc/utils/model/bedrock.tssrc/utils/optionalRuntimeModule.test.tssrc/utils/optionalRuntimeModule.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: smoke-and-tests
- GitHub Check: typecheck
🧰 Additional context used
📓 Path-based instructions (18)
**/*.{ts,tsx,js,jsx,py,json,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow the existing code style in the touched files
Files:
knip.jsonsrc/tools/FileReadTool/imageProcessor.tssrc/utils/geminiAuth.tsscripts/externalsValidation.test.tssrc/utils/optionalRuntimeModule.tssrc/utils/aws.tssrc/services/api/client.tssrc/tools/FileReadTool/FileReadTool.tssrc/utils/model/bedrock.tsdocs/advanced-setup.mdsrc/utils/auth.tsscripts/externalsValidation.tsscripts/externals.tssrc/utils/optionalRuntimeModule.test.tssrc/services/tokenEstimation.tsscripts/validate-externals.tspackage.json
**
⚙️ CodeRabbit configuration file
**: # AGENTS.md - AI Agent Coding GuideThis guide is for AI coding agents working in the OpenClaude repository. Read it before changing code, and also follow CONTRIBUTING.md for contributor policy, PR expectations, review follow-up, and project scope.
Project Snapshot
OpenClaude is a coding-agent CLI for cloud and local model providers. It supports OpenAI-compatible APIs, Anthropic, Gemini, DeepSeek, Ollama, MCP, local backends, slash commands, tools, agents, and a React/Ink terminal UI.
The installed CLI runs on Node.js
>=22.0.0. Bun is used for source builds, scripts, dependency management, and tests.Work Style
- Keep changes focused on one problem.
- Prefer existing patterns in the file or nearby module.
- Avoid unrelated formatting, renames, dependency changes, or broad rewrites.
- Add or update tests when behavior changes.
- Update docs when setup, commands, provider behavior, or user-facing behavior changes.
- For new features, larger refactors, dependencies, or runtime changes, follow the issue-first guidance in CONTRIBUTING.md.
Stack And Conventions
- TypeScript with strict mode and ESM imports.
- React + Ink for terminal UI.
- Bun lockfile and Bun scripts for development workflows.
- Node runtime for the built CLI.
- Python exists for legacy/local-provider helper code. Do not add new Python code or expand Python-based features unless a maintainer explicitly approves that direction.
Common libraries and patterns:
chalkfor terminal color.commanderfor CLI argument parsing.execafor child processes.- Existing service, provider, settings, permission, and UI patterns over new abstractions.
Repository Map
src/commands/- slash and CLI command implementations.src/components/- React/Ink UI components.src/services/- API, MCP, OAuth, wiki, voice, and other service integrations.src/tools/- tool implementations.src/utils/- shared utilities.- `src/integration...
Files:
knip.jsonsrc/tools/FileReadTool/imageProcessor.tssrc/utils/geminiAuth.tsscripts/externalsValidation.test.tssrc/utils/optionalRuntimeModule.tssrc/utils/aws.tssrc/services/api/client.tssrc/tools/FileReadTool/FileReadTool.tssrc/utils/model/bedrock.tsdocs/advanced-setup.mdsrc/utils/auth.tsscripts/externalsValidation.tsscripts/externals.tssrc/utils/optionalRuntimeModule.test.tssrc/services/tokenEstimation.tsscripts/validate-externals.tspackage.json
**/*
⚙️ 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:
knip.jsonsrc/tools/FileReadTool/imageProcessor.tssrc/utils/geminiAuth.tsscripts/externalsValidation.test.tssrc/utils/optionalRuntimeModule.tssrc/utils/aws.tssrc/services/api/client.tssrc/tools/FileReadTool/FileReadTool.tssrc/utils/model/bedrock.tsdocs/advanced-setup.mdsrc/utils/auth.tsscripts/externalsValidation.tsscripts/externals.tssrc/utils/optionalRuntimeModule.test.tssrc/services/tokenEstimation.tsscripts/validate-externals.tspackage.json
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript with strict mode and ESM imports
Files:
src/tools/FileReadTool/imageProcessor.tssrc/utils/geminiAuth.tssrc/utils/optionalRuntimeModule.tssrc/utils/aws.tssrc/services/api/client.tssrc/tools/FileReadTool/FileReadTool.tssrc/utils/model/bedrock.tssrc/utils/auth.tssrc/utils/optionalRuntimeModule.test.tssrc/services/tokenEstimation.ts
**/*.{ts,tsx,js,jsx,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Keep comments useful and concise
Files:
src/tools/FileReadTool/imageProcessor.tssrc/utils/geminiAuth.tsscripts/externalsValidation.test.tssrc/utils/optionalRuntimeModule.tssrc/utils/aws.tssrc/services/api/client.tssrc/tools/FileReadTool/FileReadTool.tssrc/utils/model/bedrock.tssrc/utils/auth.tsscripts/externalsValidation.tsscripts/externals.tssrc/utils/optionalRuntimeModule.test.tssrc/services/tokenEstimation.tsscripts/validate-externals.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow TypeScript strict mode and type safety practices by running typecheck before submitting
Files:
src/tools/FileReadTool/imageProcessor.tssrc/utils/geminiAuth.tsscripts/externalsValidation.test.tssrc/utils/optionalRuntimeModule.tssrc/utils/aws.tssrc/services/api/client.tssrc/tools/FileReadTool/FileReadTool.tssrc/utils/model/bedrock.tssrc/utils/auth.tsscripts/externalsValidation.tsscripts/externals.tssrc/utils/optionalRuntimeModule.test.tssrc/services/tokenEstimation.tsscripts/validate-externals.ts
src/{components/permissions,utils/permissions,hooks/toolPermission,tools,entrypoints/sdk}/**
⚙️ CodeRabbit configuration file
src/{components/permissions,utils/permissions,hooks/toolPermission,tools,entrypoints/sdk}/**: Review permission prompts, auto-allow logic, sandbox behavior, SDK permission schemas, shell/PowerShell execution, and background execution paths as security-sensitive. Block on bypasses, unclear trust boundaries, unsafe path handling, missing user visibility, or changes that broaden allowed behavior without an explicit maintainer decision.
Files:
src/tools/FileReadTool/imageProcessor.tssrc/tools/FileReadTool/FileReadTool.ts
{src/services/**/*.ts,src/utils/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
execafor child processes
Files:
src/utils/geminiAuth.tssrc/utils/optionalRuntimeModule.tssrc/utils/aws.tssrc/services/api/client.tssrc/utils/model/bedrock.tssrc/utils/auth.tssrc/utils/optionalRuntimeModule.test.tssrc/services/tokenEstimation.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add or update tests when the change affects behavior
Files:
scripts/externalsValidation.test.tssrc/utils/optionalRuntimeModule.test.ts
**/*.test.{ts,tsx,js}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Test the exact provider/model path you changed when possible
Files:
scripts/externalsValidation.test.tssrc/utils/optionalRuntimeModule.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:
scripts/externalsValidation.test.tsscripts/externalsValidation.tsscripts/externals.tsscripts/validate-externals.tspackage.json
{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:
scripts/externalsValidation.test.tssrc/utils/optionalRuntimeModule.test.ts
{src/commands/**/*.ts,src/services/**/*.ts,src/entrypoints/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
chalkfor terminal color in CLI code
Files:
src/services/api/client.tssrc/services/tokenEstimation.ts
{src/integrations/**/*.ts,src/services/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Test the exact provider/model path you changed when possible for provider modifications
Files:
src/services/api/client.tssrc/services/tokenEstimation.ts
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}
⚙️ CodeRabbit configuration file
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}: Review provider routing, model selection, env precedence, auth/token handling, OpenAI-compatible shims, retries, proxy behavior, and outbound HTTP behavior with high scrutiny. Block on silent default changes, hidden fallback expansion, credential reuse mistakes, hardcoded provider assumptions, or new network reach that is not intentional and documented.
Files:
src/services/api/client.tssrc/utils/model/bedrock.ts
docs/**/*.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Update docs when setup, commands, or user-facing behavior changes
Files:
docs/advanced-setup.md
{README.md,CONTRIBUTING.md,docs/**,.github/pull_request_template.md}
⚙️ CodeRabbit configuration file
{README.md,CONTRIBUTING.md,docs/**,.github/pull_request_template.md}: Review docs for accuracy against current code behavior. Flag security or provider claims that overpromise, stale install commands, missing setup caveats, and instructions that could push users toward unsafe credential handling. Keep purely wording-level suggestions non-blocking.
Files:
docs/advanced-setup.md
{bun.lock,bunfig.toml,package.json}
📄 CodeRabbit inference engine (AGENTS.md)
Use Bun lockfile and Bun scripts for development workflows and dependency management
Files:
package.json
🔇 Additional comments (2)
docs/advanced-setup.md (2)
101-116: LGTM!
334-351: LGTM!
03a242e to
a880a25
Compare
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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 `@docs/advanced-setup.md`:
- Around line 342-347: The AWS Bedrock row in the install table is incomplete
for the authenticated flow, since the runtime now loads AWS credential-provider
modules lazily and users can still miss required packages after following only
this entry. Update the Bedrock row referenced by the AWS Bedrock feature entry
to either include the full auth setup package set needed for normal Bedrock
usage or clearly scope the row so it does not imply a complete install command.
In `@scripts/externalsValidation.ts`:
- Around line 64-99: The validateIntentionallyBundled function currently only
flags unexpected peerDependencies, so SDK-only externals can disappear from
package.json without failing validation. Update validateIntentionallyBundled to
explicitly require every sdkOnlyExternals entry to be present in
pkg.peerDependencies, using the existing symbols sdkOnlyExternals, peerDeps, and
errors, and keep the current checks for devDependencies and direct dependencies.
Add a regression test covering a missing SDK-only external such as
react-reconciler or `@modelcontextprotocol/sdk` so the validator fails when it
drops out of peerDependencies.
- Around line 131-175: The validateOptionalRuntimeExternals helper only verifies
the externals sets and can miss regressions where optional runtime modules are
accidentally shipped by default. Update validateOptionalRuntimeExternals to also
compare OPTIONAL_RUNTIME_EXTERNALS against the packaged runtime dependency lists
(dependencies and peerDependencies) and add an error when any optional runtime
module like sharp, google-auth-library, or AWS optional auth packages appears in
shipped deps. Keep the existing cli/sdk/indirection checks in place and extend
the ValidationResult failure path so overlap with shipped deps is reported
alongside the current externals validation.
In `@scripts/optionalRuntimeSpecifiers.test.ts`:
- Around line 52-55: The test currently only checks a minimum count in the `the
scan finds the known provider load sites` case, so it can miss regressions in
the Bedrock/Foundry/Vertex/Azure optional runtime paths. Update this assertion
to verify the exact expected specifier set produced by the scanner, using the
`sites` collection from `optionalRuntimeSpecifiers.test.ts` and the routing
behavior around `importOptionalRuntimeModule()`. Keep the guard against vacuous
matches, but pin the concrete runtime-import specifiers so any removed or
renamed optional import fails the test.
In `@src/services/api/client.ts`:
- Around line 620-623: The Vertex auth module is being imported before the
CLAUDE_CODE_SKIP_VERTEX_AUTH branch, which causes proxy/test runs to fail when
google-auth-library is missing. Move the importOptionalRuntimeModule call for
GoogleAuth inside the non-skip-auth path in the api client logic, so the mock
googleAuth branch can be used without requiring the optional package; use the
existing CLAUDE_CODE_SKIP_VERTEX_AUTH check and the surrounding Vertex auth
setup in client.ts to place the import behind the guard.
In `@src/tools/FileReadTool/FileReadTool.ts`:
- Around line 1279-1282: The new ImageProcessorUnavailableError contract is
still being swallowed by the attachment read path, so edited-image attachments
won’t surface the install hint. Update the caller in readImageWithTokenBudget
handling within attachments.ts (or keep the current graceful fallback in
FileReadTool.ts until the caller can propagate it) so the chosen behavior is
consistent and intentional. Add a focused regression test around the
readImageWithTokenBudget / attachments flow to verify either the install hint is
surfaced or the failure still degrades to null, depending on the final contract.
In `@src/utils/optionalRuntimeModule.ts`:
- Around line 65-67: The missing-package error in optionalRuntimeModule should
not hard-code a global npm install command because this helper is used by both
the CLI and the optional sdk path. Update the thrown message in the helper that
builds the error for feature/specifier resolution so it gives a neutral install
hint or context-appropriate guidance instead of telling every caller to run npm
i -g, allowing project-local installs to be handled correctly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 088854b3-acee-49be-821a-ab68e1db6719
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (18)
docs/advanced-setup.mdknip.jsonpackage.jsonscripts/externals.tsscripts/externalsValidation.test.tsscripts/externalsValidation.tsscripts/optionalRuntimeSpecifiers.test.tsscripts/validate-externals.tssrc/services/api/client.tssrc/services/tokenEstimation.tssrc/tools/FileReadTool/FileReadTool.tssrc/tools/FileReadTool/imageProcessor.tssrc/utils/auth.tssrc/utils/aws.tssrc/utils/geminiAuth.tssrc/utils/model/bedrock.tssrc/utils/optionalRuntimeModule.test.tssrc/utils/optionalRuntimeModule.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (18)
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript with strict mode and ESM imports
Files:
src/tools/FileReadTool/imageProcessor.tssrc/utils/optionalRuntimeModule.test.tssrc/utils/geminiAuth.tssrc/utils/aws.tssrc/tools/FileReadTool/FileReadTool.tssrc/utils/optionalRuntimeModule.tssrc/services/api/client.tssrc/services/tokenEstimation.tssrc/utils/model/bedrock.tssrc/utils/auth.ts
**/*.{ts,tsx,js,jsx,py,json,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow the existing code style in the touched files
Files:
src/tools/FileReadTool/imageProcessor.tsknip.jsonscripts/optionalRuntimeSpecifiers.test.tssrc/utils/optionalRuntimeModule.test.tssrc/utils/geminiAuth.tssrc/utils/aws.tssrc/tools/FileReadTool/FileReadTool.tsscripts/externalsValidation.test.tsscripts/externalsValidation.tssrc/utils/optionalRuntimeModule.tssrc/services/api/client.tssrc/services/tokenEstimation.tsscripts/validate-externals.tssrc/utils/model/bedrock.tssrc/utils/auth.tsscripts/externals.tsdocs/advanced-setup.mdpackage.json
**/*.{ts,tsx,js,jsx,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Keep comments useful and concise
Files:
src/tools/FileReadTool/imageProcessor.tsscripts/optionalRuntimeSpecifiers.test.tssrc/utils/optionalRuntimeModule.test.tssrc/utils/geminiAuth.tssrc/utils/aws.tssrc/tools/FileReadTool/FileReadTool.tsscripts/externalsValidation.test.tsscripts/externalsValidation.tssrc/utils/optionalRuntimeModule.tssrc/services/api/client.tssrc/services/tokenEstimation.tsscripts/validate-externals.tssrc/utils/model/bedrock.tssrc/utils/auth.tsscripts/externals.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow TypeScript strict mode and type safety practices by running typecheck before submitting
Files:
src/tools/FileReadTool/imageProcessor.tsscripts/optionalRuntimeSpecifiers.test.tssrc/utils/optionalRuntimeModule.test.tssrc/utils/geminiAuth.tssrc/utils/aws.tssrc/tools/FileReadTool/FileReadTool.tsscripts/externalsValidation.test.tsscripts/externalsValidation.tssrc/utils/optionalRuntimeModule.tssrc/services/api/client.tssrc/services/tokenEstimation.tsscripts/validate-externals.tssrc/utils/model/bedrock.tssrc/utils/auth.tsscripts/externals.ts
**
⚙️ CodeRabbit configuration file
**: # AGENTS.md - AI Agent Coding GuideThis guide is for AI coding agents working in the OpenClaude repository. Read it before changing code, and also follow CONTRIBUTING.md for contributor policy, PR expectations, review follow-up, and project scope.
Project Snapshot
OpenClaude is a coding-agent CLI for cloud and local model providers. It supports OpenAI-compatible APIs, Anthropic, Gemini, DeepSeek, Ollama, MCP, local backends, slash commands, tools, agents, and a React/Ink terminal UI.
The installed CLI runs on Node.js
>=22.0.0. Bun is used for source builds, scripts, dependency management, and tests.Work Style
- Keep changes focused on one problem.
- Prefer existing patterns in the file or nearby module.
- Avoid unrelated formatting, renames, dependency changes, or broad rewrites.
- Add or update tests when behavior changes.
- Update docs when setup, commands, provider behavior, or user-facing behavior changes.
- For new features, larger refactors, dependencies, or runtime changes, follow the issue-first guidance in CONTRIBUTING.md.
Stack And Conventions
- TypeScript with strict mode and ESM imports.
- React + Ink for terminal UI.
- Bun lockfile and Bun scripts for development workflows.
- Node runtime for the built CLI.
- Python exists for legacy/local-provider helper code. Do not add new Python code or expand Python-based features unless a maintainer explicitly approves that direction.
Common libraries and patterns:
chalkfor terminal color.commanderfor CLI argument parsing.execafor child processes.- Existing service, provider, settings, permission, and UI patterns over new abstractions.
Repository Map
src/commands/- slash and CLI command implementations.src/components/- React/Ink UI components.src/services/- API, MCP, OAuth, wiki, voice, and other service integrations.src/tools/- tool implementations.src/utils/- shared utilities.- `src/integration...
Files:
src/tools/FileReadTool/imageProcessor.tsknip.jsonscripts/optionalRuntimeSpecifiers.test.tssrc/utils/optionalRuntimeModule.test.tssrc/utils/geminiAuth.tssrc/utils/aws.tssrc/tools/FileReadTool/FileReadTool.tsscripts/externalsValidation.test.tsscripts/externalsValidation.tssrc/utils/optionalRuntimeModule.tssrc/services/api/client.tssrc/services/tokenEstimation.tsscripts/validate-externals.tssrc/utils/model/bedrock.tssrc/utils/auth.tsscripts/externals.tsdocs/advanced-setup.mdpackage.json
**/*
⚙️ 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/tools/FileReadTool/imageProcessor.tsknip.jsonscripts/optionalRuntimeSpecifiers.test.tssrc/utils/optionalRuntimeModule.test.tssrc/utils/geminiAuth.tssrc/utils/aws.tssrc/tools/FileReadTool/FileReadTool.tsscripts/externalsValidation.test.tsscripts/externalsValidation.tssrc/utils/optionalRuntimeModule.tssrc/services/api/client.tssrc/services/tokenEstimation.tsscripts/validate-externals.tssrc/utils/model/bedrock.tssrc/utils/auth.tsscripts/externals.tsdocs/advanced-setup.mdpackage.json
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/FileReadTool/imageProcessor.tssrc/tools/FileReadTool/FileReadTool.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add or update tests when the change affects behavior
Files:
scripts/optionalRuntimeSpecifiers.test.tssrc/utils/optionalRuntimeModule.test.tsscripts/externalsValidation.test.ts
**/*.test.{ts,tsx,js}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Test the exact provider/model path you changed when possible
Files:
scripts/optionalRuntimeSpecifiers.test.tssrc/utils/optionalRuntimeModule.test.tsscripts/externalsValidation.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:
scripts/optionalRuntimeSpecifiers.test.tsscripts/externalsValidation.test.tsscripts/externalsValidation.tsscripts/validate-externals.tsscripts/externals.tspackage.json
{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:
scripts/optionalRuntimeSpecifiers.test.tssrc/utils/optionalRuntimeModule.test.tsscripts/externalsValidation.test.ts
{src/services/**/*.ts,src/utils/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
execafor child processes
Files:
src/utils/optionalRuntimeModule.test.tssrc/utils/geminiAuth.tssrc/utils/aws.tssrc/utils/optionalRuntimeModule.tssrc/services/api/client.tssrc/services/tokenEstimation.tssrc/utils/model/bedrock.tssrc/utils/auth.ts
{src/commands/**/*.ts,src/services/**/*.ts,src/entrypoints/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
chalkfor terminal color in CLI code
Files:
src/services/api/client.tssrc/services/tokenEstimation.ts
{src/integrations/**/*.ts,src/services/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Test the exact provider/model path you changed when possible for provider modifications
Files:
src/services/api/client.tssrc/services/tokenEstimation.ts
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}
⚙️ CodeRabbit configuration file
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}: Review provider routing, model selection, env precedence, auth/token handling, OpenAI-compatible shims, retries, proxy behavior, and outbound HTTP behavior with high scrutiny. Block on silent default changes, hidden fallback expansion, credential reuse mistakes, hardcoded provider assumptions, or new network reach that is not intentional and documented.
Files:
src/services/api/client.tssrc/utils/model/bedrock.ts
docs/**/*.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Update docs when setup, commands, or user-facing behavior changes
Files:
docs/advanced-setup.md
{README.md,CONTRIBUTING.md,docs/**,.github/pull_request_template.md}
⚙️ CodeRabbit configuration file
{README.md,CONTRIBUTING.md,docs/**,.github/pull_request_template.md}: Review docs for accuracy against current code behavior. Flag security or provider claims that overpromise, stale install commands, missing setup caveats, and instructions that could push users toward unsafe credential handling. Keep purely wording-level suggestions non-blocking.
Files:
docs/advanced-setup.md
{bun.lock,bunfig.toml,package.json}
📄 CodeRabbit inference engine (AGENTS.md)
Use Bun lockfile and Bun scripts for development workflows and dependency management
Files:
package.json
a880a25 to
a3bccde
Compare
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 `@scripts/externals.ts`:
- Around line 43-49: The header comment above OPTIONAL_RUNTIME_EXTERNALS is now
describing two different behaviors as one, which is misleading. Update the
comment near OPTIONAL_RUNTIME_EXTERNALS so it clearly separates the general
optional runtime externals from the indirection-only exceptions for
`@anthropic-ai/bedrock-sdk` and `@anthropic-ai/foundry-sdk`, and remove the outdated
claim that all entries “remain in COMMON_EXTERNALS.” Keep the wording concise
and aligned with the intent of the surrounding externals definitions.
In `@scripts/externalsValidation.ts`:
- Around line 142-177: validateOptionalRuntimeExternals only checks that
optional runtime externals are not shipped in dependencies/peerDependencies, but
it does not verify the source-install contract via devDependencies. Update
validateOptionalRuntimeExternals to also assert that every documented optional
package in the source-install optional set is present in pkg.devDependencies,
using the existing optionalRuntimeExternals/indirection set logic as a guide.
Add a regression test around validateOptionalRuntimeExternals that fails when a
required optional package is missing from devDependencies while the current
shipped checks still pass.
In `@src/utils/attachments.ts`:
- Around line 2107-2109: The analytics event in logEvent for
tengu_watched_file_compression_failed is still sending normalizedPath through
the bypass cast, which can leak local filesystem details. Update this call so it
no longer forwards the raw path; use a safe non-identifying value or omit the
file field entirely before passing metadata to logEvent. Keep the fix localized
to the watched-file compression failure path in src/utils/attachments.ts.
In `@src/utils/optionalRuntimeModule.ts`:
- Around line 57-75: The helper importOptionalRuntimeModule currently defaults
its generic type to any, which lets untyped consumers silently bypass strict
checks. Change the default generic on importOptionalRuntimeModule to unknown,
then update the Bedrock, Foundry, Vertex, and Gemini call sites to provide
explicit module types or narrow the imported value before use. If needed, keep
the helper strongly typed by requiring an explicit type argument instead of
falling back to any.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 70296357-741d-4eed-bd1c-aaa2735ac6a4
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (20)
docs/advanced-setup.mdknip.jsonpackage.jsonscripts/externals.tsscripts/externalsValidation.test.tsscripts/externalsValidation.tsscripts/optionalRuntimeSpecifiers.test.tsscripts/validate-externals.tssrc/services/api/client.tssrc/services/tokenEstimation.tssrc/tools/FileReadTool/FileReadTool.tssrc/tools/FileReadTool/imageProcessor.tssrc/utils/attachments.editedImage.test.tssrc/utils/attachments.tssrc/utils/auth.tssrc/utils/aws.tssrc/utils/geminiAuth.tssrc/utils/model/bedrock.tssrc/utils/optionalRuntimeModule.test.tssrc/utils/optionalRuntimeModule.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (18)
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript with strict mode and ESM imports
Files:
src/utils/attachments.editedImage.test.tssrc/services/tokenEstimation.tssrc/tools/FileReadTool/imageProcessor.tssrc/utils/optionalRuntimeModule.test.tssrc/utils/aws.tssrc/utils/auth.tssrc/utils/geminiAuth.tssrc/utils/optionalRuntimeModule.tssrc/utils/model/bedrock.tssrc/utils/attachments.tssrc/services/api/client.tssrc/tools/FileReadTool/FileReadTool.ts
{src/services/**/*.ts,src/utils/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
execafor child processes
Files:
src/utils/attachments.editedImage.test.tssrc/services/tokenEstimation.tssrc/utils/optionalRuntimeModule.test.tssrc/utils/aws.tssrc/utils/auth.tssrc/utils/geminiAuth.tssrc/utils/optionalRuntimeModule.tssrc/utils/model/bedrock.tssrc/utils/attachments.tssrc/services/api/client.ts
**/*.{ts,tsx,js,jsx,py,json,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow the existing code style in the touched files
Files:
src/utils/attachments.editedImage.test.tssrc/services/tokenEstimation.tsknip.jsonsrc/tools/FileReadTool/imageProcessor.tssrc/utils/optionalRuntimeModule.test.tssrc/utils/aws.tsscripts/externals.tsdocs/advanced-setup.mdsrc/utils/auth.tssrc/utils/geminiAuth.tssrc/utils/optionalRuntimeModule.tssrc/utils/model/bedrock.tssrc/utils/attachments.tsscripts/externalsValidation.test.tsscripts/optionalRuntimeSpecifiers.test.tssrc/services/api/client.tsscripts/externalsValidation.tspackage.jsonscripts/validate-externals.tssrc/tools/FileReadTool/FileReadTool.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add or update tests when the change affects behavior
Files:
src/utils/attachments.editedImage.test.tssrc/utils/optionalRuntimeModule.test.tsscripts/externalsValidation.test.tsscripts/optionalRuntimeSpecifiers.test.ts
**/*.test.{ts,tsx,js}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Test the exact provider/model path you changed when possible
Files:
src/utils/attachments.editedImage.test.tssrc/utils/optionalRuntimeModule.test.tsscripts/externalsValidation.test.tsscripts/optionalRuntimeSpecifiers.test.ts
**/*.{ts,tsx,js,jsx,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Keep comments useful and concise
Files:
src/utils/attachments.editedImage.test.tssrc/services/tokenEstimation.tssrc/tools/FileReadTool/imageProcessor.tssrc/utils/optionalRuntimeModule.test.tssrc/utils/aws.tsscripts/externals.tssrc/utils/auth.tssrc/utils/geminiAuth.tssrc/utils/optionalRuntimeModule.tssrc/utils/model/bedrock.tssrc/utils/attachments.tsscripts/externalsValidation.test.tsscripts/optionalRuntimeSpecifiers.test.tssrc/services/api/client.tsscripts/externalsValidation.tsscripts/validate-externals.tssrc/tools/FileReadTool/FileReadTool.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow TypeScript strict mode and type safety practices by running typecheck before submitting
Files:
src/utils/attachments.editedImage.test.tssrc/services/tokenEstimation.tssrc/tools/FileReadTool/imageProcessor.tssrc/utils/optionalRuntimeModule.test.tssrc/utils/aws.tsscripts/externals.tssrc/utils/auth.tssrc/utils/geminiAuth.tssrc/utils/optionalRuntimeModule.tssrc/utils/model/bedrock.tssrc/utils/attachments.tsscripts/externalsValidation.test.tsscripts/optionalRuntimeSpecifiers.test.tssrc/services/api/client.tsscripts/externalsValidation.tsscripts/validate-externals.tssrc/tools/FileReadTool/FileReadTool.ts
**
⚙️ CodeRabbit configuration file
**: # AGENTS.md - AI Agent Coding GuideThis guide is for AI coding agents working in the OpenClaude repository. Read it before changing code, and also follow CONTRIBUTING.md for contributor policy, PR expectations, review follow-up, and project scope.
Project Snapshot
OpenClaude is a coding-agent CLI for cloud and local model providers. It supports OpenAI-compatible APIs, Anthropic, Gemini, DeepSeek, Ollama, MCP, local backends, slash commands, tools, agents, and a React/Ink terminal UI.
The installed CLI runs on Node.js
>=22.0.0. Bun is used for source builds, scripts, dependency management, and tests.Work Style
- Keep changes focused on one problem.
- Prefer existing patterns in the file or nearby module.
- Avoid unrelated formatting, renames, dependency changes, or broad rewrites.
- Add or update tests when behavior changes.
- Update docs when setup, commands, provider behavior, or user-facing behavior changes.
- For new features, larger refactors, dependencies, or runtime changes, follow the issue-first guidance in CONTRIBUTING.md.
Stack And Conventions
- TypeScript with strict mode and ESM imports.
- React + Ink for terminal UI.
- Bun lockfile and Bun scripts for development workflows.
- Node runtime for the built CLI.
- Python exists for legacy/local-provider helper code. Do not add new Python code or expand Python-based features unless a maintainer explicitly approves that direction.
Common libraries and patterns:
chalkfor terminal color.commanderfor CLI argument parsing.execafor child processes.- Existing service, provider, settings, permission, and UI patterns over new abstractions.
Repository Map
src/commands/- slash and CLI command implementations.src/components/- React/Ink UI components.src/services/- API, MCP, OAuth, wiki, voice, and other service integrations.src/tools/- tool implementations.src/utils/- shared utilities.- `src/integration...
Files:
src/utils/attachments.editedImage.test.tssrc/services/tokenEstimation.tsknip.jsonsrc/tools/FileReadTool/imageProcessor.tssrc/utils/optionalRuntimeModule.test.tssrc/utils/aws.tsscripts/externals.tsdocs/advanced-setup.mdsrc/utils/auth.tssrc/utils/geminiAuth.tssrc/utils/optionalRuntimeModule.tssrc/utils/model/bedrock.tssrc/utils/attachments.tsscripts/externalsValidation.test.tsscripts/optionalRuntimeSpecifiers.test.tssrc/services/api/client.tsscripts/externalsValidation.tspackage.jsonscripts/validate-externals.tssrc/tools/FileReadTool/FileReadTool.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/attachments.editedImage.test.tssrc/services/tokenEstimation.tsknip.jsonsrc/tools/FileReadTool/imageProcessor.tssrc/utils/optionalRuntimeModule.test.tssrc/utils/aws.tsscripts/externals.tsdocs/advanced-setup.mdsrc/utils/auth.tssrc/utils/geminiAuth.tssrc/utils/optionalRuntimeModule.tssrc/utils/model/bedrock.tssrc/utils/attachments.tsscripts/externalsValidation.test.tsscripts/optionalRuntimeSpecifiers.test.tssrc/services/api/client.tsscripts/externalsValidation.tspackage.jsonscripts/validate-externals.tssrc/tools/FileReadTool/FileReadTool.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/attachments.editedImage.test.tssrc/utils/optionalRuntimeModule.test.tsscripts/externalsValidation.test.tsscripts/optionalRuntimeSpecifiers.test.ts
{src/commands/**/*.ts,src/services/**/*.ts,src/entrypoints/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
chalkfor terminal color in CLI code
Files:
src/services/tokenEstimation.tssrc/services/api/client.ts
{src/integrations/**/*.ts,src/services/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Test the exact provider/model path you changed when possible for provider modifications
Files:
src/services/tokenEstimation.tssrc/services/api/client.ts
src/{components/permissions,utils/permissions,hooks/toolPermission,tools,entrypoints/sdk}/**
⚙️ CodeRabbit configuration file
src/{components/permissions,utils/permissions,hooks/toolPermission,tools,entrypoints/sdk}/**: Review permission prompts, auto-allow logic, sandbox behavior, SDK permission schemas, shell/PowerShell execution, and background execution paths as security-sensitive. Block on bypasses, unclear trust boundaries, unsafe path handling, missing user visibility, or changes that broaden allowed behavior without an explicit maintainer decision.
Files:
src/tools/FileReadTool/imageProcessor.tssrc/tools/FileReadTool/FileReadTool.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:
scripts/externals.tsscripts/externalsValidation.test.tsscripts/optionalRuntimeSpecifiers.test.tsscripts/externalsValidation.tspackage.jsonscripts/validate-externals.ts
docs/**/*.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Update docs when setup, commands, or user-facing behavior changes
Files:
docs/advanced-setup.md
{README.md,CONTRIBUTING.md,docs/**,.github/pull_request_template.md}
⚙️ CodeRabbit configuration file
{README.md,CONTRIBUTING.md,docs/**,.github/pull_request_template.md}: Review docs for accuracy against current code behavior. Flag security or provider claims that overpromise, stale install commands, missing setup caveats, and instructions that could push users toward unsafe credential handling. Keep purely wording-level suggestions non-blocking.
Files:
docs/advanced-setup.md
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}
⚙️ CodeRabbit configuration file
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}: Review provider routing, model selection, env precedence, auth/token handling, OpenAI-compatible shims, retries, proxy behavior, and outbound HTTP behavior with high scrutiny. Block on silent default changes, hidden fallback expansion, credential reuse mistakes, hardcoded provider assumptions, or new network reach that is not intentional and documented.
Files:
src/utils/model/bedrock.tssrc/services/api/client.ts
{bun.lock,bunfig.toml,package.json}
📄 CodeRabbit inference engine (AGENTS.md)
Use Bun lockfile and Bun scripts for development workflows and dependency management
Files:
package.json
🔇 Additional comments (2)
src/utils/attachments.editedImage.test.ts (1)
5-15: This regression test still misses the optional-image-processor failure path.Using a nonexistent file only exercises ENOENT, so it never proves that
ImageProcessorUnavailableErrorfromreadImageWithTokenBudget()is downgraded tonullfor background edited-image attachments. Please add a focused case that reaches the image-processing branch and asserts the helper degrades instead of surfacing the install hint. As per coding guidelines, "Add or update tests when the change affects behavior," and as per path instructions, "Block when risky runtime changes lack focused regression coverage."Sources: Coding guidelines, Path instructions
src/utils/optionalRuntimeModule.test.ts (1)
14-95: Still missing focused provider-path regression coverage.This suite only validates the shared helper. The risky behavior change is in the Bedrock, Foundry, Vertex skip-auth, and Gemini entrypoints, so these tests can stay green while the actual provider/auth paths regress.
As per coding guidelines, "Add or update tests when the change affects behavior" and "Test the exact provider/model path you changed when possible"; as per path instructions, "Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior."
Sources: Coding guidelines, Path instructions
a3bccde to
c1b1723
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/utils/attachments.editedImage.test.ts`:
- Around line 11-15: The current test in tryReadEditedImageAttachment only
covers a missing file/ENOENT path and does not exercise the fallback/rethrow
behavior in readImageWithTokenBudget or getImageProcessor. Update the test to
mock readImageWithTokenBudget() or getImageProcessor() so it throws
ImageProcessorUnavailableError, then assert tryReadEditedImageAttachment() still
resolves to null. Keep the coverage focused on the editedImage helper path so it
protects the changed behavior from regression.
In `@src/utils/attachments.ts`:
- Around line 2106-2114: The background image failure path in attachments.ts
still sends raw exceptions through logError() inside the catch around
readImageWithTokenBudget, which can leak file paths from thrown messages and
stacks. Update this catch to avoid forwarding the original compressionError
directly: either log a generic sanitized error object/message or skip error
telemetry entirely for this degrade-to-null flow, while keeping the analytics
event payload limited to the sanitized extension.
In `@src/utils/optionalRuntimeModule.ts`:
- Around line 57-62: The optional runtime import helper is defaulting to any,
which leaks implicit any into untyped callers and disables strict checks on
imported module shapes. Update importOptionalRuntimeModule to default its
generic in the helper signature to unknown (or require explicit module types),
then adjust the remaining call sites in api/client, auth, aws, and geminiAuth to
provide concrete types or narrow from unknown after import. Keep the fix
centered on importOptionalRuntimeModule and the affected callers so the module
contract stays type-safe.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 448a545b-a89c-4895-a817-4f78ce8b6b77
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (20)
docs/advanced-setup.mdknip.jsonpackage.jsonscripts/externals.tsscripts/externalsValidation.test.tsscripts/externalsValidation.tsscripts/optionalRuntimeSpecifiers.test.tsscripts/validate-externals.tssrc/services/api/client.tssrc/services/tokenEstimation.tssrc/tools/FileReadTool/FileReadTool.tssrc/tools/FileReadTool/imageProcessor.tssrc/utils/attachments.editedImage.test.tssrc/utils/attachments.tssrc/utils/auth.tssrc/utils/aws.tssrc/utils/geminiAuth.tssrc/utils/model/bedrock.tssrc/utils/optionalRuntimeModule.test.tssrc/utils/optionalRuntimeModule.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (18)
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript with strict mode and ESM imports
Files:
src/utils/attachments.editedImage.test.tssrc/tools/FileReadTool/imageProcessor.tssrc/utils/aws.tssrc/utils/optionalRuntimeModule.test.tssrc/utils/geminiAuth.tssrc/utils/model/bedrock.tssrc/utils/optionalRuntimeModule.tssrc/utils/attachments.tssrc/tools/FileReadTool/FileReadTool.tssrc/services/tokenEstimation.tssrc/utils/auth.tssrc/services/api/client.ts
{src/services/**/*.ts,src/utils/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
execafor child processes
Files:
src/utils/attachments.editedImage.test.tssrc/utils/aws.tssrc/utils/optionalRuntimeModule.test.tssrc/utils/geminiAuth.tssrc/utils/model/bedrock.tssrc/utils/optionalRuntimeModule.tssrc/utils/attachments.tssrc/services/tokenEstimation.tssrc/utils/auth.tssrc/services/api/client.ts
**/*.{ts,tsx,js,jsx,py,json,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow the existing code style in the touched files
Files:
src/utils/attachments.editedImage.test.tssrc/tools/FileReadTool/imageProcessor.tssrc/utils/aws.tssrc/utils/optionalRuntimeModule.test.tssrc/utils/geminiAuth.tsknip.jsonscripts/optionalRuntimeSpecifiers.test.tsdocs/advanced-setup.mdsrc/utils/model/bedrock.tssrc/utils/optionalRuntimeModule.tsscripts/externalsValidation.test.tssrc/utils/attachments.tssrc/tools/FileReadTool/FileReadTool.tssrc/services/tokenEstimation.tspackage.jsonscripts/validate-externals.tsscripts/externalsValidation.tsscripts/externals.tssrc/utils/auth.tssrc/services/api/client.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add or update tests when the change affects behavior
Files:
src/utils/attachments.editedImage.test.tssrc/utils/optionalRuntimeModule.test.tsscripts/optionalRuntimeSpecifiers.test.tsscripts/externalsValidation.test.ts
**/*.test.{ts,tsx,js}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Test the exact provider/model path you changed when possible
Files:
src/utils/attachments.editedImage.test.tssrc/utils/optionalRuntimeModule.test.tsscripts/optionalRuntimeSpecifiers.test.tsscripts/externalsValidation.test.ts
**/*.{ts,tsx,js,jsx,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Keep comments useful and concise
Files:
src/utils/attachments.editedImage.test.tssrc/tools/FileReadTool/imageProcessor.tssrc/utils/aws.tssrc/utils/optionalRuntimeModule.test.tssrc/utils/geminiAuth.tsscripts/optionalRuntimeSpecifiers.test.tssrc/utils/model/bedrock.tssrc/utils/optionalRuntimeModule.tsscripts/externalsValidation.test.tssrc/utils/attachments.tssrc/tools/FileReadTool/FileReadTool.tssrc/services/tokenEstimation.tsscripts/validate-externals.tsscripts/externalsValidation.tsscripts/externals.tssrc/utils/auth.tssrc/services/api/client.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow TypeScript strict mode and type safety practices by running typecheck before submitting
Files:
src/utils/attachments.editedImage.test.tssrc/tools/FileReadTool/imageProcessor.tssrc/utils/aws.tssrc/utils/optionalRuntimeModule.test.tssrc/utils/geminiAuth.tsscripts/optionalRuntimeSpecifiers.test.tssrc/utils/model/bedrock.tssrc/utils/optionalRuntimeModule.tsscripts/externalsValidation.test.tssrc/utils/attachments.tssrc/tools/FileReadTool/FileReadTool.tssrc/services/tokenEstimation.tsscripts/validate-externals.tsscripts/externalsValidation.tsscripts/externals.tssrc/utils/auth.tssrc/services/api/client.ts
**
⚙️ CodeRabbit configuration file
**: # AGENTS.md - AI Agent Coding GuideThis guide is for AI coding agents working in the OpenClaude repository. Read it before changing code, and also follow CONTRIBUTING.md for contributor policy, PR expectations, review follow-up, and project scope.
Project Snapshot
OpenClaude is a coding-agent CLI for cloud and local model providers. It supports OpenAI-compatible APIs, Anthropic, Gemini, DeepSeek, Ollama, MCP, local backends, slash commands, tools, agents, and a React/Ink terminal UI.
The installed CLI runs on Node.js
>=22.0.0. Bun is used for source builds, scripts, dependency management, and tests.Work Style
- Keep changes focused on one problem.
- Prefer existing patterns in the file or nearby module.
- Avoid unrelated formatting, renames, dependency changes, or broad rewrites.
- Add or update tests when behavior changes.
- Update docs when setup, commands, provider behavior, or user-facing behavior changes.
- For new features, larger refactors, dependencies, or runtime changes, follow the issue-first guidance in CONTRIBUTING.md.
Stack And Conventions
- TypeScript with strict mode and ESM imports.
- React + Ink for terminal UI.
- Bun lockfile and Bun scripts for development workflows.
- Node runtime for the built CLI.
- Python exists for legacy/local-provider helper code. Do not add new Python code or expand Python-based features unless a maintainer explicitly approves that direction.
Common libraries and patterns:
chalkfor terminal color.commanderfor CLI argument parsing.execafor child processes.- Existing service, provider, settings, permission, and UI patterns over new abstractions.
Repository Map
src/commands/- slash and CLI command implementations.src/components/- React/Ink UI components.src/services/- API, MCP, OAuth, wiki, voice, and other service integrations.src/tools/- tool implementations.src/utils/- shared utilities.- `src/integration...
Files:
src/utils/attachments.editedImage.test.tssrc/tools/FileReadTool/imageProcessor.tssrc/utils/aws.tssrc/utils/optionalRuntimeModule.test.tssrc/utils/geminiAuth.tsknip.jsonscripts/optionalRuntimeSpecifiers.test.tsdocs/advanced-setup.mdsrc/utils/model/bedrock.tssrc/utils/optionalRuntimeModule.tsscripts/externalsValidation.test.tssrc/utils/attachments.tssrc/tools/FileReadTool/FileReadTool.tssrc/services/tokenEstimation.tspackage.jsonscripts/validate-externals.tsscripts/externalsValidation.tsscripts/externals.tssrc/utils/auth.tssrc/services/api/client.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/attachments.editedImage.test.tssrc/tools/FileReadTool/imageProcessor.tssrc/utils/aws.tssrc/utils/optionalRuntimeModule.test.tssrc/utils/geminiAuth.tsknip.jsonscripts/optionalRuntimeSpecifiers.test.tsdocs/advanced-setup.mdsrc/utils/model/bedrock.tssrc/utils/optionalRuntimeModule.tsscripts/externalsValidation.test.tssrc/utils/attachments.tssrc/tools/FileReadTool/FileReadTool.tssrc/services/tokenEstimation.tspackage.jsonscripts/validate-externals.tsscripts/externalsValidation.tsscripts/externals.tssrc/utils/auth.tssrc/services/api/client.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/attachments.editedImage.test.tssrc/utils/optionalRuntimeModule.test.tsscripts/optionalRuntimeSpecifiers.test.tsscripts/externalsValidation.test.ts
src/{components/permissions,utils/permissions,hooks/toolPermission,tools,entrypoints/sdk}/**
⚙️ CodeRabbit configuration file
src/{components/permissions,utils/permissions,hooks/toolPermission,tools,entrypoints/sdk}/**: Review permission prompts, auto-allow logic, sandbox behavior, SDK permission schemas, shell/PowerShell execution, and background execution paths as security-sensitive. Block on bypasses, unclear trust boundaries, unsafe path handling, missing user visibility, or changes that broaden allowed behavior without an explicit maintainer decision.
Files:
src/tools/FileReadTool/imageProcessor.tssrc/tools/FileReadTool/FileReadTool.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:
scripts/optionalRuntimeSpecifiers.test.tsscripts/externalsValidation.test.tspackage.jsonscripts/validate-externals.tsscripts/externalsValidation.tsscripts/externals.ts
docs/**/*.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Update docs when setup, commands, or user-facing behavior changes
Files:
docs/advanced-setup.md
{README.md,CONTRIBUTING.md,docs/**,.github/pull_request_template.md}
⚙️ CodeRabbit configuration file
{README.md,CONTRIBUTING.md,docs/**,.github/pull_request_template.md}: Review docs for accuracy against current code behavior. Flag security or provider claims that overpromise, stale install commands, missing setup caveats, and instructions that could push users toward unsafe credential handling. Keep purely wording-level suggestions non-blocking.
Files:
docs/advanced-setup.md
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}
⚙️ CodeRabbit configuration file
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}: Review provider routing, model selection, env precedence, auth/token handling, OpenAI-compatible shims, retries, proxy behavior, and outbound HTTP behavior with high scrutiny. Block on silent default changes, hidden fallback expansion, credential reuse mistakes, hardcoded provider assumptions, or new network reach that is not intentional and documented.
Files:
src/utils/model/bedrock.tssrc/services/api/client.ts
{src/commands/**/*.ts,src/services/**/*.ts,src/entrypoints/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
chalkfor terminal color in CLI code
Files:
src/services/tokenEstimation.tssrc/services/api/client.ts
{src/integrations/**/*.ts,src/services/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Test the exact provider/model path you changed when possible for provider modifications
Files:
src/services/tokenEstimation.tssrc/services/api/client.ts
{bun.lock,bunfig.toml,package.json}
📄 CodeRabbit inference engine (AGENTS.md)
Use Bun lockfile and Bun scripts for development workflows and dependency management
Files:
package.json
🔇 Additional comments (3)
src/tools/FileReadTool/imageProcessor.ts (1)
41-45: LGTM!src/tools/FileReadTool/FileReadTool.ts (1)
51-54: LGTM!Also applies to: 1262-1282
scripts/externalsValidation.ts (1)
152-154: Still missing the source-install guard for optional runtime packages.This helper only checks
dependencies/peerDependenciesand bundle externals. Ifsharp,google-auth-library, or one of the AWS/Azure optionals drops out ofdevDependencies,bun installsource builds regress while this validator still passes and the docs claim atdocs/advanced-setup.mdLines 349-350 becomes false. Please add thedevDependenciessubset check here and cover it inscripts/externalsValidation.test.ts.As per path instructions, "Review install, launcher, build, packaging, startup, and entrypoint changes for cross-platform compatibility, tracked-source rewrites, env/config precedence, and release safety."
Source: Path instructions
c1b1723 to
9855c19
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
src/utils/optionalRuntimeModule.ts (2)
57-62: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winDefault this helper to
unknown, notany.This still leaks
anyinto the new optional-module call sites, so the Bedrock/Foundry/Vertex/Gemini paths sidestep strict-mode checking. Change the default tounknown(or require an explicit type argument) and keep the callers typed.Minimal fix
-export async function importOptionalRuntimeModule<T = any>( +export async function importOptionalRuntimeModule<T = unknown>(As per coding guidelines, "Follow TypeScript strict mode and type safety practices by running typecheck before submitting".
🤖 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/optionalRuntimeModule.ts` around lines 57 - 62, The helper importOptionalRuntimeModule currently defaults its generic to any, which leaks unsafe typing into the Bedrock/Foundry/Vertex/Gemini call sites. Change the default type parameter on importOptionalRuntimeModule to unknown (or require callers to specify T explicitly), then update the affected callers so they keep their concrete types and still satisfy strict-mode type checking.Source: Coding guidelines
68-70: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winMake the install hint package-manager neutral.
This helper now backs both the CLI and
./sdk. Hard-codingnpm installcan still send Bun/pnpm/yarn users to the wrong environment, and global non-npm installs may keep failing after they follow the message.Suggested fix
- `${feature} requires the "${specifier}" package, which is not installed. ` + - `Install it with \`npm install ${specifier}\` (add \`-g\` if you installed the CLI globally) to enable it.`, + `${feature} requires the "${specifier}" package, which is not installed. ` + + `Install "${specifier}" in the same environment as OpenClaude to enable it.`,🤖 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/optionalRuntimeModule.ts` around lines 68 - 70, The install hint in optionalRuntimeModule’s error message is npm-specific, which is misleading for Bun/pnpm/yarn users. Update the thrown message in optionalRuntimeModule to use package-manager neutral wording, referencing the missing package via specifier and a generic install instruction instead of hard-coding npm install or -g. Keep the existing feature/specifier context so callers of the helper still get a clear actionable error.src/utils/optionalRuntimeModule.test.ts (1)
14-95: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftAdd regression tests for the actual provider paths changed here.
These tests only pin the shared loader. The PR also rewires the Bedrock, Foundry, Vertex, and Gemini auth/runtime branches, so skip-auth and missing-package regressions can still ship green. Add focused coverage on those entrypoints, not just the helper.
As per coding guidelines, "Add or update tests when the change affects behavior" and "Test the exact provider/model path you changed when possible"; as per path instructions, "Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior."
🤖 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/optionalRuntimeModule.test.ts` around lines 14 - 95, The current tests only cover importOptionalRuntimeModule and importRuntimeModule, so they miss regressions in the provider-specific branches changed by this PR. Add focused regression tests for the actual entrypoints that were rewired in the Bedrock, Foundry, Vertex, and Gemini paths, covering both the skip-auth behavior and the missing-package failure path. Use the relevant provider/model functions or auth/runtime helpers from the diff as anchors so the tests exercise the real user-visible behavior rather than only the shared loader.Sources: Coding guidelines, Path instructions
🤖 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 `@scripts/externalsValidation.ts`:
- Around line 186-197: The OPTIONAL_RUNTIME_EXTERNALS validation in
externalsValidation.ts is incorrectly exempting direct runtime imports from
devDependencies; update the missingFromDev check so only truly private
transitives are skipped. Make sure packages loaded directly by source code, such
as the ones referenced from src/utils/aws.ts and src/services/api/client.ts,
remain required in devDependencies, and keep the logic centered around the
existing transitive set and devDeps lookup.
In `@src/utils/attachments.editedImage.test.ts`:
- Around line 13-37: The tests for tryReadEditedImageAttachment only verify null
վերադարձ and miss the sanitized telemetry behavior, so add focused assertions in
attachments.editedImage.test.ts by mocking logError and logEvent around the
helper calls. Verify the error passed to logError does not contain the edited
image path or other raw path details, and verify logEvent only receives the ext
field in its analytics payload (not normalizedPath or compressionError), using
the existing tryReadEditedImageAttachment and ImageProcessorUnavailableError
cases as the regression coverage.
---
Duplicate comments:
In `@src/utils/optionalRuntimeModule.test.ts`:
- Around line 14-95: The current tests only cover importOptionalRuntimeModule
and importRuntimeModule, so they miss regressions in the provider-specific
branches changed by this PR. Add focused regression tests for the actual
entrypoints that were rewired in the Bedrock, Foundry, Vertex, and Gemini paths,
covering both the skip-auth behavior and the missing-package failure path. Use
the relevant provider/model functions or auth/runtime helpers from the diff as
anchors so the tests exercise the real user-visible behavior rather than only
the shared loader.
In `@src/utils/optionalRuntimeModule.ts`:
- Around line 57-62: The helper importOptionalRuntimeModule currently defaults
its generic to any, which leaks unsafe typing into the
Bedrock/Foundry/Vertex/Gemini call sites. Change the default type parameter on
importOptionalRuntimeModule to unknown (or require callers to specify T
explicitly), then update the affected callers so they keep their concrete types
and still satisfy strict-mode type checking.
- Around line 68-70: The install hint in optionalRuntimeModule’s error message
is npm-specific, which is misleading for Bun/pnpm/yarn users. Update the thrown
message in optionalRuntimeModule to use package-manager neutral wording,
referencing the missing package via specifier and a generic install instruction
instead of hard-coding npm install or -g. Keep the existing feature/specifier
context so callers of the helper still get a clear actionable error.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: d3cadbae-218f-458f-adc8-58b3f94bf949
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (20)
docs/advanced-setup.mdknip.jsonpackage.jsonscripts/externals.tsscripts/externalsValidation.test.tsscripts/externalsValidation.tsscripts/optionalRuntimeSpecifiers.test.tsscripts/validate-externals.tssrc/services/api/client.tssrc/services/tokenEstimation.tssrc/tools/FileReadTool/FileReadTool.tssrc/tools/FileReadTool/imageProcessor.tssrc/utils/attachments.editedImage.test.tssrc/utils/attachments.tssrc/utils/auth.tssrc/utils/aws.tssrc/utils/geminiAuth.tssrc/utils/model/bedrock.tssrc/utils/optionalRuntimeModule.test.tssrc/utils/optionalRuntimeModule.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: typecheck
🧰 Additional context used
📓 Path-based instructions (18)
**/*.{ts,tsx,js,jsx,py,json,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow the existing code style in the touched files
Files:
knip.jsonsrc/utils/attachments.editedImage.test.tssrc/utils/auth.tssrc/utils/aws.tssrc/utils/optionalRuntimeModule.test.tsscripts/optionalRuntimeSpecifiers.test.tssrc/services/tokenEstimation.tssrc/tools/FileReadTool/imageProcessor.tssrc/utils/geminiAuth.tsdocs/advanced-setup.mdsrc/utils/attachments.tssrc/utils/optionalRuntimeModule.tssrc/tools/FileReadTool/FileReadTool.tsscripts/externals.tsscripts/validate-externals.tsscripts/externalsValidation.test.tsscripts/externalsValidation.tssrc/services/api/client.tspackage.jsonsrc/utils/model/bedrock.ts
**
⚙️ CodeRabbit configuration file
**: # AGENTS.md - AI Agent Coding GuideThis guide is for AI coding agents working in the OpenClaude repository. Read it before changing code, and also follow CONTRIBUTING.md for contributor policy, PR expectations, review follow-up, and project scope.
Project Snapshot
OpenClaude is a coding-agent CLI for cloud and local model providers. It supports OpenAI-compatible APIs, Anthropic, Gemini, DeepSeek, Ollama, MCP, local backends, slash commands, tools, agents, and a React/Ink terminal UI.
The installed CLI runs on Node.js
>=22.0.0. Bun is used for source builds, scripts, dependency management, and tests.Work Style
- Keep changes focused on one problem.
- Prefer existing patterns in the file or nearby module.
- Avoid unrelated formatting, renames, dependency changes, or broad rewrites.
- Add or update tests when behavior changes.
- Update docs when setup, commands, provider behavior, or user-facing behavior changes.
- For new features, larger refactors, dependencies, or runtime changes, follow the issue-first guidance in CONTRIBUTING.md.
Stack And Conventions
- TypeScript with strict mode and ESM imports.
- React + Ink for terminal UI.
- Bun lockfile and Bun scripts for development workflows.
- Node runtime for the built CLI.
- Python exists for legacy/local-provider helper code. Do not add new Python code or expand Python-based features unless a maintainer explicitly approves that direction.
Common libraries and patterns:
chalkfor terminal color.commanderfor CLI argument parsing.execafor child processes.- Existing service, provider, settings, permission, and UI patterns over new abstractions.
Repository Map
src/commands/- slash and CLI command implementations.src/components/- React/Ink UI components.src/services/- API, MCP, OAuth, wiki, voice, and other service integrations.src/tools/- tool implementations.src/utils/- shared utilities.- `src/integration...
Files:
knip.jsonsrc/utils/attachments.editedImage.test.tssrc/utils/auth.tssrc/utils/aws.tssrc/utils/optionalRuntimeModule.test.tsscripts/optionalRuntimeSpecifiers.test.tssrc/services/tokenEstimation.tssrc/tools/FileReadTool/imageProcessor.tssrc/utils/geminiAuth.tsdocs/advanced-setup.mdsrc/utils/attachments.tssrc/utils/optionalRuntimeModule.tssrc/tools/FileReadTool/FileReadTool.tsscripts/externals.tsscripts/validate-externals.tsscripts/externalsValidation.test.tsscripts/externalsValidation.tssrc/services/api/client.tspackage.jsonsrc/utils/model/bedrock.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:
knip.jsonsrc/utils/attachments.editedImage.test.tssrc/utils/auth.tssrc/utils/aws.tssrc/utils/optionalRuntimeModule.test.tsscripts/optionalRuntimeSpecifiers.test.tssrc/services/tokenEstimation.tssrc/tools/FileReadTool/imageProcessor.tssrc/utils/geminiAuth.tsdocs/advanced-setup.mdsrc/utils/attachments.tssrc/utils/optionalRuntimeModule.tssrc/tools/FileReadTool/FileReadTool.tsscripts/externals.tsscripts/validate-externals.tsscripts/externalsValidation.test.tsscripts/externalsValidation.tssrc/services/api/client.tspackage.jsonsrc/utils/model/bedrock.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript with strict mode and ESM imports
Files:
src/utils/attachments.editedImage.test.tssrc/utils/auth.tssrc/utils/aws.tssrc/utils/optionalRuntimeModule.test.tssrc/services/tokenEstimation.tssrc/tools/FileReadTool/imageProcessor.tssrc/utils/geminiAuth.tssrc/utils/attachments.tssrc/utils/optionalRuntimeModule.tssrc/tools/FileReadTool/FileReadTool.tssrc/services/api/client.tssrc/utils/model/bedrock.ts
{src/services/**/*.ts,src/utils/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
execafor child processes
Files:
src/utils/attachments.editedImage.test.tssrc/utils/auth.tssrc/utils/aws.tssrc/utils/optionalRuntimeModule.test.tssrc/services/tokenEstimation.tssrc/utils/geminiAuth.tssrc/utils/attachments.tssrc/utils/optionalRuntimeModule.tssrc/services/api/client.tssrc/utils/model/bedrock.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add or update tests when the change affects behavior
Files:
src/utils/attachments.editedImage.test.tssrc/utils/optionalRuntimeModule.test.tsscripts/optionalRuntimeSpecifiers.test.tsscripts/externalsValidation.test.ts
**/*.test.{ts,tsx,js}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Test the exact provider/model path you changed when possible
Files:
src/utils/attachments.editedImage.test.tssrc/utils/optionalRuntimeModule.test.tsscripts/optionalRuntimeSpecifiers.test.tsscripts/externalsValidation.test.ts
**/*.{ts,tsx,js,jsx,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Keep comments useful and concise
Files:
src/utils/attachments.editedImage.test.tssrc/utils/auth.tssrc/utils/aws.tssrc/utils/optionalRuntimeModule.test.tsscripts/optionalRuntimeSpecifiers.test.tssrc/services/tokenEstimation.tssrc/tools/FileReadTool/imageProcessor.tssrc/utils/geminiAuth.tssrc/utils/attachments.tssrc/utils/optionalRuntimeModule.tssrc/tools/FileReadTool/FileReadTool.tsscripts/externals.tsscripts/validate-externals.tsscripts/externalsValidation.test.tsscripts/externalsValidation.tssrc/services/api/client.tssrc/utils/model/bedrock.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow TypeScript strict mode and type safety practices by running typecheck before submitting
Files:
src/utils/attachments.editedImage.test.tssrc/utils/auth.tssrc/utils/aws.tssrc/utils/optionalRuntimeModule.test.tsscripts/optionalRuntimeSpecifiers.test.tssrc/services/tokenEstimation.tssrc/tools/FileReadTool/imageProcessor.tssrc/utils/geminiAuth.tssrc/utils/attachments.tssrc/utils/optionalRuntimeModule.tssrc/tools/FileReadTool/FileReadTool.tsscripts/externals.tsscripts/validate-externals.tsscripts/externalsValidation.test.tsscripts/externalsValidation.tssrc/services/api/client.tssrc/utils/model/bedrock.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/attachments.editedImage.test.tssrc/utils/optionalRuntimeModule.test.tsscripts/optionalRuntimeSpecifiers.test.tsscripts/externalsValidation.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:
scripts/optionalRuntimeSpecifiers.test.tsscripts/externals.tsscripts/validate-externals.tsscripts/externalsValidation.test.tsscripts/externalsValidation.tspackage.json
{src/commands/**/*.ts,src/services/**/*.ts,src/entrypoints/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
chalkfor terminal color in CLI code
Files:
src/services/tokenEstimation.tssrc/services/api/client.ts
{src/integrations/**/*.ts,src/services/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Test the exact provider/model path you changed when possible for provider modifications
Files:
src/services/tokenEstimation.tssrc/services/api/client.ts
src/{components/permissions,utils/permissions,hooks/toolPermission,tools,entrypoints/sdk}/**
⚙️ CodeRabbit configuration file
src/{components/permissions,utils/permissions,hooks/toolPermission,tools,entrypoints/sdk}/**: Review permission prompts, auto-allow logic, sandbox behavior, SDK permission schemas, shell/PowerShell execution, and background execution paths as security-sensitive. Block on bypasses, unclear trust boundaries, unsafe path handling, missing user visibility, or changes that broaden allowed behavior without an explicit maintainer decision.
Files:
src/tools/FileReadTool/imageProcessor.tssrc/tools/FileReadTool/FileReadTool.ts
docs/**/*.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Update docs when setup, commands, or user-facing behavior changes
Files:
docs/advanced-setup.md
{README.md,CONTRIBUTING.md,docs/**,.github/pull_request_template.md}
⚙️ CodeRabbit configuration file
{README.md,CONTRIBUTING.md,docs/**,.github/pull_request_template.md}: Review docs for accuracy against current code behavior. Flag security or provider claims that overpromise, stale install commands, missing setup caveats, and instructions that could push users toward unsafe credential handling. Keep purely wording-level suggestions non-blocking.
Files:
docs/advanced-setup.md
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}
⚙️ CodeRabbit configuration file
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}: Review provider routing, model selection, env precedence, auth/token handling, OpenAI-compatible shims, retries, proxy behavior, and outbound HTTP behavior with high scrutiny. Block on silent default changes, hidden fallback expansion, credential reuse mistakes, hardcoded provider assumptions, or new network reach that is not intentional and documented.
Files:
src/services/api/client.tssrc/utils/model/bedrock.ts
{bun.lock,bunfig.toml,package.json}
📄 CodeRabbit inference engine (AGENTS.md)
Use Bun lockfile and Bun scripts for development workflows and dependency management
Files:
package.json
🔇 Additional comments (2)
docs/advanced-setup.md (1)
101-116: LGTM!Also applies to: 334-351
src/utils/attachments.ts (1)
2119-2121: 🗄️ Data Integrity & IntegrationNo change needed:
extis already using the analytics marker type, so thislogEvent()payload matches the existing metadata pattern.> Likely an incorrect or invalid review comment.
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
Findings
1. optionalRuntimeSpecifiers.test.ts regex misses generic-annotated call sites and under-counts expected specifiers
File: scripts/optionalRuntimeSpecifiers.test.ts
Severity: Medium — the regression test gives a false sense of security.
The test uses this regex to find every importOptionalRuntimeModule call site:
const CALL_RE = /importOptionalRuntimeModule\(\s*['"]([^'"]+)['"]/gThis pattern expects the opening parenthesis immediately after the function name. It does not match call sites that pass an explicit type argument before the parenthesis, such as those in src/utils/model/bedrock.ts and src/services/tokenEstimation.ts:
// src/utils/model/bedrock.ts
const { BedrockClient } = await importOptionalRuntimeModule<
typeof import('@aws-sdk/client-bedrock')
>('@aws-sdk/client-bedrock', 'AWS Bedrock')
// src/services/tokenEstimation.ts
const { CountTokensCommand } = await importOptionalRuntimeModule<
typeof import('@aws-sdk/client-bedrock-runtime')
>('@aws-sdk/client-bedrock-runtime', 'AWS Bedrock')A quick scan with a regex that allows the generic (/importOptionalRuntimeModule(?:<[^>]+>)?\(\s*['"]([^'"]+)['"]/g) finds two additional specifiers in src/utils/model/bedrock.ts (@aws-sdk/client-bedrock, @aws-sdk/client-bedrock-runtime) and one in src/services/tokenEstimation.ts (@aws-sdk/client-bedrock-runtime). The current EXPECTED_SPECIFIERS list omits them:
const EXPECTED_SPECIFIERS = [
'@anthropic-ai/bedrock-sdk',
'@anthropic-ai/foundry-sdk',
'@azure/identity',
'@aws-sdk/credential-providers',
'google-auth-library',
].sort()Impact: A future contributor could add a new generic-annotated importOptionalRuntimeModule<...>(...) call for a package that is not in OPTIONAL_RUNTIME_EXTERNALS, and this test would not catch it. The current code is safe only because those two AWS packages happen to already be declared, not because the test is complete.
Suggested fix:
- Update the regex to allow an optional generic type argument before the opening paren:
const CALL_RE = /importOptionalRuntimeModule(?:<[^>]+>)?\(\s*['"]([^'"]+)['"]/g- Update
EXPECTED_SPECIFIERSto include the missing AWS SDK packages:
const EXPECTED_SPECIFIERS = [
'@anthropic-ai/bedrock-sdk',
'@anthropic-ai/foundry-sdk',
'@anthropic-ai/bedrock-sdk', // already present
'@aws-sdk/client-bedrock', // missing
'@aws-sdk/client-bedrock-runtime', // missing
'@aws-sdk/credential-providers',
'@azure/identity',
'google-auth-library',
].sort()(After deduplication, of course.)
2. importOptionalRuntimeModule defaults generic to any, disabling strict-mode checks at most call sites
File: src/utils/optionalRuntimeModule.ts
Severity: Medium — maintainability/type-safety regression.
export async function importOptionalRuntimeModule<T = any>(
specifier: string,
feature: string,
): Promise<T>Most production call sites do not pass a type argument:
src/services/api/client.ts: Bedrock, Foundry, Azure, Vertexsrc/utils/auth.ts: GCP credentials checksrc/utils/aws.ts: AWS credentials providersrc/utils/geminiAuth.ts: Gemini ADC
Because the default is any, destructured imports such as const { AnthropicBedrock } = await importOptionalRuntimeModule(...) are typed as any, so the compiler cannot verify property names, constructor signatures, or return shapes. This undermines the project's strict-mode conventions.
Suggested fix: Change the default to unknown:
export async function importOptionalRuntimeModule<T = unknown>(
specifier: string,
feature: string,
): Promise<T>Then either:
- Require each caller to supply the concrete module type (
importOptionalRuntimeModule<typeof import('...')>(...)); - Or cast/declare the imported shape at each call site.
This keeps the helper generic while forcing callers to be explicit about the module contract.
3. Missing focused regression tests for the rewired provider/auth paths
Files: src/services/api/client.ts, src/utils/model/bedrock.ts, src/utils/auth.ts, src/utils/geminiAuth.ts, src/utils/aws.ts
Severity: Medium — the risky runtime behavior is not exercised end-to-end.
The PR adds two new test files:
src/utils/optionalRuntimeModule.test.ts— tests the shared loader in isolation.scripts/optionalRuntimeSpecifiers.test.ts— static scan of call sites (currently incomplete; see Finding #1).
Neither test verifies that:
getAnthropicClientwithCLAUDE_CODE_USE_BEDROCK=1still works when@anthropic-ai/bedrock-sdkis installed and fails with the new actionable message when it is not.getAnthropicClientwithCLAUDE_CODE_USE_FOUNDRY=1still resolves@azure/identityand@anthropic-ai/foundry-sdk.checkGcpCredentialsValid/ Vertex auth paths degrade or surface the install hint for missinggoogle-auth-library.clearAwsIniCacheandcreateBedrockRuntimeClientbehave correctly with/without@aws-sdk/credential-providers.
Suggested fix: Add at least one focused test per rewired subsystem. These can be unit tests with the optional module mocked or, better, a test that exercises the real module resolution path in an isolated subprocess so regressions in the importOptionalRuntimeModule wiring are caught.
4. Install-hint messages are npm-specific
Files: src/utils/optionalRuntimeModule.ts, src/tools/FileReadTool/imageProcessor.ts
Severity: Low — UX polish.
The new actionable error messages hard-code npm install:
throw new Error(
`${feature} requires the "${specifier}" package, which is not installed. ` +
`Install it with \`npm install ${specifier}\` (add \`-g\` if you installed the CLI globally) to enable it.`,
)Similarly, ImageProcessorUnavailableError says:
'Image support is not installed. Run `npm i -g sharp` to enable reading and processing images.'OpenClaude supports Bun/pnpm/yarn users and the SDK path, where -g is not always appropriate. A package-manager-neutral message ("Install <specifier> in the same environment as OpenClaude") would avoid sending non-npm users to the wrong tool.
The published package declared 62 runtime `dependencies`, but `dist/cli.mjs`
is a fully-bundled esbuild output that inlines almost all of them. End users
therefore installed ~476 transitive packages — including three subtrees the
bundle never needs at install time, each emitting an install warning:
- node-domexception (deprecated) via google-auth-library
- protobufjs (allow-scripts) via @grpc/* (already bundled into dist)
- sharp (allow-scripts) native image module
The repo's `overrides`/`allowScripts` silence these locally, but those are
root-only npm settings and are ignored when the package is installed as a
dependency — so end users saw the warnings.
Core changes:
- package.json: runtime dependencies trimmed 62 -> 3 (@orama/orama,
@orama/plugin-data-persistence, @vscode/ripgrep). Bundled packages, plus
the optional sharp/google-auth-library, move to devDependencies so they
are built/tested but not shipped.
- package.json: @anthropic-ai/sdk, @modelcontextprotocol/sdk, react and
react-reconciler declared as OPTIONAL peerDependencies — externalized by
the ./sdk bundle but bundled into the CLI. Optional peers keep the CLI
install minimal and warning-free while still resolving for ./sdk consumers.
- externals.ts: sharp, google-auth-library and @anthropic-ai/bedrock-sdk
marked OPTIONAL_RUNTIME_EXTERNALS (loaded on demand, not shipped).
- validate-externals.ts: runtime deps validate against externals; bundled
deps validate against dependencies + devDependencies.
- client.ts: load @anthropic-ai/bedrock-sdk via the runtime importer so
esbuild no longer inlines it and hoists its static @aws-sdk import into
the CLI bundle (that was a startup crash for default installs).
Optional-dependency UX (consistent, actionable errors):
- New src/utils/optionalRuntimeModule.ts exports importRuntimeModule and
importOptionalRuntimeModule. The optional variant translates a missing
package (code === 'ERR_MODULE_NOT_FOUND', specifier present in message)
into "<feature> requires "<pkg>" ... Run `npm i -g <pkg>`". Generic so
typed call sites keep their module types.
- Routed ALL optional-package load sites through it (previously only one
did): google-auth-library (client.ts, auth.ts, geminiAuth.ts),
@anthropic-ai/foundry-sdk + @azure/identity (client.ts), and the
@aws-sdk/* Bedrock paths (model/bedrock.ts, tokenEstimation.ts, aws.ts).
- imageProcessor.ts: sharp-missing error now says `npm i -g sharp`.
- docs/advanced-setup.md: new "Optional provider packages" table and a
Vertex note documenting the on-demand installs.
- Unit test for the helper (friendly error, success path, specifier match,
raw passthrough).
- knip.json: ignore google-auth-library (now loaded via runtime string).
Verified on the current tree:
- tsc, build/validate-externals, knip, and tests all pass.
- npm pack + install --omit=dev adds 8 packages, zero deprecation/
allow-scripts/funding warnings; --version/--help/mcp list run.
- With packages absent, CLAUDE_CODE_USE_BEDROCK and CLAUDE_CODE_USE_VERTEX
print the friendly `npm i -g <pkg>` error (verified end-to-end).
- ./sdk imports once its optional peers are present (24 exports, no warns).
- Bundled ajv + ajv-formats validate with no ajv installed; no unguarded
native runtime requires (fsevents absent in chokidar 4; bun:sqlite Bun-only).
Trade-off: image reads, AWS Bedrock, Azure Foundry and GCP/Vertex now prompt
a one-time `npm i -g <pkg>` instead of being shipped to every user.
Co-Authored-By: OpenClaude <openclaude@gitlawb.com>
Review fixes (CodeRabbit + jatmn):
- validate-externals: the INTENTIONALLY_BUNDLED exemption is now scoped per
bundle. The CLI exempts every bundled package; the SDK does NOT exempt
packages declared as peerDependencies (keyed on package.json, an independent
source of truth) so dropping react/@anthropic-ai/sdk from SDK_EXTERNALS now
fails validation instead of silently passing. Added an explicit minimal-
install contract check: bundled packages must be devDependencies-only — never
in `dependencies`, and only the SDK-external subset may be optional peers.
Validation logic extracted to scripts/externalsValidation.ts + tests.
- FileReadTool oversized-image fallback now loads via the shared
getImageProcessor() (not a raw import('sharp')) and re-throws
ImageProcessorUnavailableError, so a missing processor surfaces the
`npm i -g sharp` install hint instead of returning an over-budget image.
- optionalRuntimeModule: match the missing specifier as a QUOTED token, not a
raw substring, so a missing transitive package whose name contains the
requested one (sharp vs sharp-libvips, @aws-sdk/client-bedrock vs
@aws-sdk/client-bedrock-runtime) no longer triggers the wrong install hint.
Predicate extracted to isMissingSpecifierError() with regression tests.
- docs/advanced-setup.md: the Vertex auth section now shows both documented
paths (gcloud ADC and a GOOGLE_APPLICATION_CREDENTIALS service-account file).
Review fixes (round 2, CodeRabbit):
- validate-externals: assert the optional-peer install contract — every
peerDependency must be { optional: true } in peerDependenciesMeta
(validateOptionalPeers), so losing that flag fails the build instead of
silently reintroducing install warnings.
- validate-externals: hard-check OPTIONAL_RUNTIME_EXTERNALS placement
(validateOptionalRuntimeexternals). Anything esbuild can see statically must
stay external in BOTH bundles (dropping sharp/google-auth-library now fails);
the runtime-indirection-only subset (new RUNTIME_INDIRECTION_ONLY_EXTERNALS)
must stay OUT of externals so esbuild never re-exposes their static imports.
- Deeper-dig fix: @anthropic-ai/foundry-sdk was misclassified as
INTENTIONALLY_BUNDLED, but it is loaded only through the Function indirection
(esbuild never sees it, so it was never actually bundled) — its sole presence
in dist is the specifier string. Per the PR's own "Azure Foundry now prompts"
trade-off it is on-demand, so it now lives in OPTIONAL_RUNTIME_EXTERNALS +
RUNTIME_INDIRECTION_ONLY_EXTERNALS (mirroring bedrock-sdk). sandbox-runtime is
genuinely statically imported, so it stays bundled.
- Provider-routing coverage (scripts/optionalRuntimeSpecifiers.test.ts): a
static scan asserts every importOptionalRuntimeModule specifier is a declared
OPTIONAL_RUNTIME_EXTERNAL and never also INTENTIONALLY_BUNDLED — the
invariant that keeps a provider's optional package loadable on demand.
- All new validators extracted to scripts/externalsValidation.ts with tests.
Review fixes (round 3, CodeRabbit):
- client.ts: gate the Vertex google-auth-library import behind the non-skip
branch. CLAUDE_CODE_SKIP_VERTEX_AUTH (proxy/test) uses a mock GoogleAuth and
must not require the optional package; it was loaded unconditionally before.
- optionalRuntimeModule: drop the hard-coded `npm i -g`. The helper backs both
the global CLI and project-local ./sdk consumers, so the hint is now
context-neutral ("npm install <pkg>" / add -g for the global CLI).
- validate-externals: every SDK_ONLY_EXTERNALS entry must STAY a
peerDependency (a dropped peer leaves runtimeDeps while the SDK still
externalizes it); and OPTIONAL_RUNTIME_EXTERNALS must never be shipped (fail
on overlap with dependencies/peerDependencies). Both with tests + live-verified.
- optionalRuntimeSpecifiers.test: pin the EXACT set of optionally-loaded
specifiers instead of a >=5 count (a count passes even if a provider path
regresses).
- attachments: extract tryReadEditedImageAttachment() — background watched-file
image attachments DEGRADE to null on any failure (incl.
ImageProcessorUnavailableError) so a missing optional package never aborts a
turn, while the explicit FileReadTool path still surfaces the install hint.
Deterministic regression test (bad path -> null).
- docs: Bedrock row notes profile-based auth also needs
@aws-sdk/credential-providers; install-hint wording matches the new message.
Review fixes (round 4, CodeRabbit):
- attachments: stop sending the raw file path through the analytics
bypass-cast (tengu_watched_file_compression_failed). Send only the safe
file extension via getFileExtensionForAnalytics, matching the existing
tengu_file_read_dedup pattern, so no usernames/project paths can leak.
- externals.ts: corrected the OPTIONAL_RUNTIME_EXTERNALS header comment,
which still claimed all entries "remain in COMMON_EXTERNALS" — no longer
true since the indirection-only subset (bedrock/foundry) must stay OUT of
the externals lists.
(Other CodeRabbit comments on this push re-surface items already addressed in
prior commits: the peerDependenciesMeta-optional check (validateOptionalPeers),
the SDK-peers-present and optional-not-shipped validator rules, the
exact-specifier-set test, the attachments degrade contract + test, and the
context-neutral install hint are all present. The "assert every optional
external is a devDependency" suggestion is intentionally NOT applied: @aws-sdk/*
and @azure/identity are transitive devDeps via bedrock-sdk/foundry-sdk, so a
blanket assertion would be incorrect; source resolution is covered by the
build + tests that import these packages.)
Review fixes (round 5, CodeRabbit):
- attachments: stop leaking file paths via logError in the background-image
degrade path. readImageWithTokenBudget can throw path-bearing messages
(e.g. "Image file is empty: <path>") and logError persists message/stack, so
log only the error TYPE name now. (Analytics payload was already sanitized.)
- attachments: tryReadEditedImageAttachment takes an injectable reader so the
degrade contract is tested for the EXACT error types — ImageProcessorUnavailableError
and a path-bearing read error both degrade to null (not just ENOENT) — plus a
success case. No mocking.
- validate-externals: enforce the source-install half of the optional contract.
Non-transitive OPTIONAL_RUNTIME_EXTERNALS must be devDependencies so `bun
install` source builds resolve them. The new TRANSITIVE_OPTIONAL_EXTERNALS
documents the exemption (@aws-sdk/* via @anthropic-ai/bedrock-sdk, @azure/identity
via @anthropic-ai/foundry-sdk — provided transitively, not direct devDeps). A
blanket "all optionals are devDeps" check would have wrongly failed on those.
Tests + live-verified (dropping sharp from devDependencies now fails).
Review fixes (round 6, CodeRabbit + jatmn):
- optionalRuntimeSpecifiers.test: the call-site scan regex missed
generic-annotated calls (importOptionalRuntimeModule<...>(...)) in
model/bedrock.ts and tokenEstimation.ts, so the exact-set assertion was
incomplete. Regex now allows an optional generic; EXPECTED_SPECIFIERS adds
@aws-sdk/client-bedrock and @aws-sdk/client-bedrock-runtime (7 total).
- importOptionalRuntimeModule default generic is now <T = unknown> (was any),
so destructured imports are no longer silently any. Every call site now
supplies its module type — typeof import('<pkg>') where the package is
type-resolvable (bedrock-sdk, foundry-sdk, @aws-sdk/credential-providers,
google-auth-library), and a named minimal-shape alias for @azure/identity
(not a direct devDep, so typeof import can't resolve it). This gives
compile-time verification of each provider's module contract (export names,
shapes) — the structural answer to the "cover the provider branches" ask.
- attachments: tryReadEditedImageAttachment takes injectable {read,log,track};
a new test asserts the sanitized-telemetry contract directly — the logError
payload is path-free and the analytics payload carries only `ext`, never the
edited-image path.
9855c19 to
a71a73f
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
scripts/externals.ts (1)
99-104: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
TRANSITIVE_OPTIONAL_EXTERNALSmay exempt direct runtime imports that aren't actually hoisted.
@aws-sdk/credential-providersand@azure/identityare listed here as "transitive" (resolved via@anthropic-ai/bedrock-sdk/@anthropic-ai/foundry-sdk), which exempts them from thedevDependenciescheck invalidateOptionalRuntimeExternals. But perscripts/optionalRuntimeSpecifiers.test.ts(EXPECTED_SPECIFIERS) both are loaded directly viaimportOptionalRuntimeModule.@anthropic-ai/bedrock-sdkstatically imports@aws-sdk/client-bedrock-runtime, not@aws-sdk/credential-providers, so the latter isn't guaranteed to be present in the dep tree — source/dev builds would then rely on undeclared hoisting and can silently break. Direct runtime optionals should stay requireddevDependencies; keep only truly private transitives exempt.This mirrors the previously raised, still-unresolved concern.
As per path instructions, "Review install, launcher, build, packaging, startup, and entrypoint changes for ... release safety."
#!/bin/bash # Confirm whether credential-providers / azure-identity are direct devDeps, # and whether bedrock-sdk actually declares credential-providers. python - <<'PY' import json, pathlib pkg = json.loads(pathlib.Path("package.json").read_text()) dev = pkg.get("devDependencies", {}) for d in ["`@aws-sdk/credential-providers`","`@azure/identity`", "`@anthropic-ai/bedrock-sdk`","`@anthropic-ai/foundry-sdk`"]: print(f"{d}: direct devDependency = {d in dev}") PY # Inspect bedrock-sdk's declared deps if present in lockfile/node_modules metadata. fd -t f 'package.json' node_modules/@anthropic-ai/bedrock-sdk 2>/dev/null \ | xargs -r -I{} sh -c 'echo "== {} =="; jq ".dependencies" {}' fd -t f 'package.json' node_modules/@anthropic-ai/foundry-sdk 2>/dev/null \ | xargs -r -I{} sh -c 'echo "== {} =="; jq ".dependencies" {}'Source: Path instructions
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: be2a8617-a9ea-45ae-9620-751e3faa71e3
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (20)
docs/advanced-setup.mdknip.jsonpackage.jsonscripts/externals.tsscripts/externalsValidation.test.tsscripts/externalsValidation.tsscripts/optionalRuntimeSpecifiers.test.tsscripts/validate-externals.tssrc/services/api/client.tssrc/services/tokenEstimation.tssrc/tools/FileReadTool/FileReadTool.tssrc/tools/FileReadTool/imageProcessor.tssrc/utils/attachments.editedImage.test.tssrc/utils/attachments.tssrc/utils/auth.tssrc/utils/aws.tssrc/utils/geminiAuth.tssrc/utils/model/bedrock.tssrc/utils/optionalRuntimeModule.test.tssrc/utils/optionalRuntimeModule.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (18)
**/*.{ts,tsx,js,jsx,py,json,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow the existing code style in the touched files
Files:
knip.jsonsrc/utils/geminiAuth.tssrc/utils/attachments.editedImage.test.tssrc/tools/FileReadTool/imageProcessor.tssrc/tools/FileReadTool/FileReadTool.tssrc/services/tokenEstimation.tsdocs/advanced-setup.mdsrc/utils/auth.tsscripts/optionalRuntimeSpecifiers.test.tssrc/utils/optionalRuntimeModule.tsscripts/validate-externals.tssrc/utils/model/bedrock.tsscripts/externalsValidation.test.tssrc/utils/aws.tssrc/utils/attachments.tspackage.jsonsrc/utils/optionalRuntimeModule.test.tsscripts/externalsValidation.tsscripts/externals.tssrc/services/api/client.ts
**
⚙️ CodeRabbit configuration file
**: # AGENTS.md - AI Agent Coding GuideThis guide is for AI coding agents working in the OpenClaude repository. Read it before changing code, and also follow CONTRIBUTING.md for contributor policy, PR expectations, review follow-up, and project scope.
Project Snapshot
OpenClaude is a coding-agent CLI for cloud and local model providers. It supports OpenAI-compatible APIs, Anthropic, Gemini, DeepSeek, Ollama, MCP, local backends, slash commands, tools, agents, and a React/Ink terminal UI.
The installed CLI runs on Node.js
>=22.0.0. Bun is used for source builds, scripts, dependency management, and tests.Work Style
- Keep changes focused on one problem.
- Prefer existing patterns in the file or nearby module.
- Avoid unrelated formatting, renames, dependency changes, or broad rewrites.
- Add or update tests when behavior changes.
- Update docs when setup, commands, provider behavior, or user-facing behavior changes.
- For new features, larger refactors, dependencies, or runtime changes, follow the issue-first guidance in CONTRIBUTING.md.
Stack And Conventions
- TypeScript with strict mode and ESM imports.
- React + Ink for terminal UI.
- Bun lockfile and Bun scripts for development workflows.
- Node runtime for the built CLI.
- Python exists for legacy/local-provider helper code. Do not add new Python code or expand Python-based features unless a maintainer explicitly approves that direction.
Common libraries and patterns:
chalkfor terminal color.commanderfor CLI argument parsing.execafor child processes.- Existing service, provider, settings, permission, and UI patterns over new abstractions.
Repository Map
src/commands/- slash and CLI command implementations.src/components/- React/Ink UI components.src/services/- API, MCP, OAuth, wiki, voice, and other service integrations.src/tools/- tool implementations.src/utils/- shared utilities.- `src/integration...
Files:
knip.jsonsrc/utils/geminiAuth.tssrc/utils/attachments.editedImage.test.tssrc/tools/FileReadTool/imageProcessor.tssrc/tools/FileReadTool/FileReadTool.tssrc/services/tokenEstimation.tsdocs/advanced-setup.mdsrc/utils/auth.tsscripts/optionalRuntimeSpecifiers.test.tssrc/utils/optionalRuntimeModule.tsscripts/validate-externals.tssrc/utils/model/bedrock.tsscripts/externalsValidation.test.tssrc/utils/aws.tssrc/utils/attachments.tspackage.jsonsrc/utils/optionalRuntimeModule.test.tsscripts/externalsValidation.tsscripts/externals.tssrc/services/api/client.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:
knip.jsonsrc/utils/geminiAuth.tssrc/utils/attachments.editedImage.test.tssrc/tools/FileReadTool/imageProcessor.tssrc/tools/FileReadTool/FileReadTool.tssrc/services/tokenEstimation.tsdocs/advanced-setup.mdsrc/utils/auth.tsscripts/optionalRuntimeSpecifiers.test.tssrc/utils/optionalRuntimeModule.tsscripts/validate-externals.tssrc/utils/model/bedrock.tsscripts/externalsValidation.test.tssrc/utils/aws.tssrc/utils/attachments.tspackage.jsonsrc/utils/optionalRuntimeModule.test.tsscripts/externalsValidation.tsscripts/externals.tssrc/services/api/client.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript with strict mode and ESM imports
Files:
src/utils/geminiAuth.tssrc/utils/attachments.editedImage.test.tssrc/tools/FileReadTool/imageProcessor.tssrc/tools/FileReadTool/FileReadTool.tssrc/services/tokenEstimation.tssrc/utils/auth.tssrc/utils/optionalRuntimeModule.tssrc/utils/model/bedrock.tssrc/utils/aws.tssrc/utils/attachments.tssrc/utils/optionalRuntimeModule.test.tssrc/services/api/client.ts
{src/services/**/*.ts,src/utils/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
execafor child processes
Files:
src/utils/geminiAuth.tssrc/utils/attachments.editedImage.test.tssrc/services/tokenEstimation.tssrc/utils/auth.tssrc/utils/optionalRuntimeModule.tssrc/utils/model/bedrock.tssrc/utils/aws.tssrc/utils/attachments.tssrc/utils/optionalRuntimeModule.test.tssrc/services/api/client.ts
**/*.{ts,tsx,js,jsx,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Keep comments useful and concise
Files:
src/utils/geminiAuth.tssrc/utils/attachments.editedImage.test.tssrc/tools/FileReadTool/imageProcessor.tssrc/tools/FileReadTool/FileReadTool.tssrc/services/tokenEstimation.tssrc/utils/auth.tsscripts/optionalRuntimeSpecifiers.test.tssrc/utils/optionalRuntimeModule.tsscripts/validate-externals.tssrc/utils/model/bedrock.tsscripts/externalsValidation.test.tssrc/utils/aws.tssrc/utils/attachments.tssrc/utils/optionalRuntimeModule.test.tsscripts/externalsValidation.tsscripts/externals.tssrc/services/api/client.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow TypeScript strict mode and type safety practices by running typecheck before submitting
Files:
src/utils/geminiAuth.tssrc/utils/attachments.editedImage.test.tssrc/tools/FileReadTool/imageProcessor.tssrc/tools/FileReadTool/FileReadTool.tssrc/services/tokenEstimation.tssrc/utils/auth.tsscripts/optionalRuntimeSpecifiers.test.tssrc/utils/optionalRuntimeModule.tsscripts/validate-externals.tssrc/utils/model/bedrock.tsscripts/externalsValidation.test.tssrc/utils/aws.tssrc/utils/attachments.tssrc/utils/optionalRuntimeModule.test.tsscripts/externalsValidation.tsscripts/externals.tssrc/services/api/client.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add or update tests when the change affects behavior
Files:
src/utils/attachments.editedImage.test.tsscripts/optionalRuntimeSpecifiers.test.tsscripts/externalsValidation.test.tssrc/utils/optionalRuntimeModule.test.ts
**/*.test.{ts,tsx,js}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Test the exact provider/model path you changed when possible
Files:
src/utils/attachments.editedImage.test.tsscripts/optionalRuntimeSpecifiers.test.tsscripts/externalsValidation.test.tssrc/utils/optionalRuntimeModule.test.ts
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
src/utils/attachments.editedImage.test.tsscripts/optionalRuntimeSpecifiers.test.tsscripts/externalsValidation.test.tssrc/utils/optionalRuntimeModule.test.ts
src/{components/permissions,utils/permissions,hooks/toolPermission,tools,entrypoints/sdk}/**
⚙️ CodeRabbit configuration file
src/{components/permissions,utils/permissions,hooks/toolPermission,tools,entrypoints/sdk}/**: Review permission prompts, auto-allow logic, sandbox behavior, SDK permission schemas, shell/PowerShell execution, and background execution paths as security-sensitive. Block on bypasses, unclear trust boundaries, unsafe path handling, missing user visibility, or changes that broaden allowed behavior without an explicit maintainer decision.
Files:
src/tools/FileReadTool/imageProcessor.tssrc/tools/FileReadTool/FileReadTool.ts
{src/commands/**/*.ts,src/services/**/*.ts,src/entrypoints/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
chalkfor terminal color in CLI code
Files:
src/services/tokenEstimation.tssrc/services/api/client.ts
{src/integrations/**/*.ts,src/services/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Test the exact provider/model path you changed when possible for provider modifications
Files:
src/services/tokenEstimation.tssrc/services/api/client.ts
docs/**/*.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Update docs when setup, commands, or user-facing behavior changes
Files:
docs/advanced-setup.md
{README.md,CONTRIBUTING.md,docs/**,.github/pull_request_template.md}
⚙️ CodeRabbit configuration file
{README.md,CONTRIBUTING.md,docs/**,.github/pull_request_template.md}: Review docs for accuracy against current code behavior. Flag security or provider claims that overpromise, stale install commands, missing setup caveats, and instructions that could push users toward unsafe credential handling. Keep purely wording-level suggestions non-blocking.
Files:
docs/advanced-setup.md
{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:
scripts/optionalRuntimeSpecifiers.test.tsscripts/validate-externals.tsscripts/externalsValidation.test.tspackage.jsonscripts/externalsValidation.tsscripts/externals.ts
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}
⚙️ CodeRabbit configuration file
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}: Review provider routing, model selection, env precedence, auth/token handling, OpenAI-compatible shims, retries, proxy behavior, and outbound HTTP behavior with high scrutiny. Block on silent default changes, hidden fallback expansion, credential reuse mistakes, hardcoded provider assumptions, or new network reach that is not intentional and documented.
Files:
src/utils/model/bedrock.tssrc/services/api/client.ts
{bun.lock,bunfig.toml,package.json}
📄 CodeRabbit inference engine (AGENTS.md)
Use Bun lockfile and Bun scripts for development workflows and dependency management
Files:
package.json
🔇 Additional comments (23)
src/utils/optionalRuntimeModule.test.ts (1)
14-99: 🩺 Stability & Availability | 🏗️ Heavy liftProvider-branch coverage is still open.
These tests pin only the shared helper. The Bedrock/Foundry/Vertex/Gemini runtime + auth paths that consume
importOptionalRuntimeModuleremain untested, so routing/skip-auth regressions can ship green. (Those provider files sit in a separate layer of this stack — fine to land the focused regression tests alongside that layer rather than here.)As per coding guidelines, "Test the exact provider/model path you changed when possible."
Sources: Coding guidelines, Path instructions
docs/advanced-setup.md (2)
99-116: LGTM!
334-351: LGTM!package.json (2)
78-82: LGTM!
148-158: 🩺 Stability & AvailabilityNo change needed — these packages are bundled into the CLI build already; only the separate SDK bundle keeps them external, so the global install path is unaffected.
> Likely an incorrect or invalid review comment.src/utils/optionalRuntimeModule.ts (1)
25-75: LGTM!knip.json (1)
34-35: 📐 Maintainability & Code QualityNo change needed for
google-auth-library— it’s already declared indevDependencies, so thisignoreDependenciesentry will suppress Knip’s unused-listed-dependency warning as intended.> Likely an incorrect or invalid review comment.src/tools/FileReadTool/imageProcessor.ts (1)
43-45: LGTM!src/tools/FileReadTool/FileReadTool.ts (1)
51-54: LGTM!Also applies to: 1262-1285
src/utils/attachments.ts (2)
2084-2130: LGTM!Also applies to: 2193-2196
6-6: 🩺 Stability & AvailabilityNo change needed
src/-rooted imports are supported by the repo’sbaseUrl/pathsconfig, and this file already uses the same import style elsewhere.> Likely an incorrect or invalid review comment.src/utils/attachments.editedImage.test.ts (1)
14-82: LGTM!src/services/api/client.ts (2)
666-701: LGTM!
536-544: 🩺 Stability & AvailabilityNo externals-list change needed
@anthropic-ai/{bedrock,foundry}-sdkare intentionally excluded from the externals lists because they’re loaded only through the runtime importer; the other optional packages here are already declared inscripts/externals.tsand covered by validation/tests.> Likely an incorrect or invalid review comment.src/services/tokenEstimation.ts (1)
698-700: LGTM!src/utils/auth.ts (1)
871-875: LGTM!src/utils/aws.ts (1)
65-67: LGTM!src/utils/geminiAuth.ts (1)
137-139: LGTM!src/utils/model/bedrock.ts (1)
13-16: LGTM!Also applies to: 55-57, 103-105, 153-156
scripts/externalsValidation.ts (1)
146-221: LGTM!scripts/externalsValidation.test.ts (1)
1-251: LGTM!scripts/validate-externals.ts (1)
8-80: LGTM!scripts/optionalRuntimeSpecifiers.test.ts (1)
56-93: LGTM!
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
I found issues that need to be addressed before this is ready.
Findings
-
[P2] Move directly-loaded optional packages out of
TRANSITIVE_OPTIONAL_EXTERNALS
scripts/externals.ts:99-105andpackage.json
@aws-sdk/client-bedrock,@aws-sdk/client-sts, and@azure/identityare loaded directly viaimportOptionalRuntimeModule(src/utils/model/bedrock.ts:13-16,src/utils/model/bedrock.ts:55-57,src/utils/model/bedrock.ts:153-156,src/utils/aws.ts:52-54,src/services/api/client.ts:603-609), yet they are listed inTRANSITIVE_OPTIONAL_EXTERNALS. That listing exempts them from thedevDependenciescheck invalidateOptionalRuntimeExternals, but none of them are actually transitive dependencies of the declared parents:@anthropic-ai/bedrock-sdkonly depends on@aws-sdk/client-bedrock-runtimeand@aws-sdk/credential-providers; it does not pull in@aws-sdk/client-bedrockor@aws-sdk/client-sts.@anthropic-ai/foundry-sdkonly depends on@anthropic-ai/sdk; it does not pull in@azure/identity.
Because they are not inpackage.jsonorbun.lock, source/bun installbuilds do not resolve them, so the Bedrock/Foundry/STS code paths will hit the friendly install hint even for contributors who have installed all devDependencies. This also makesdocs/advanced-setup.md:349-350inaccurate when it claims all optional packages are already present as dev dependencies for source builds.
Please remove these three packages fromTRANSITIVE_OPTIONAL_EXTERNALSand add them todevDependenciesso the source-install contract matches the runtime contract.
-
[P3]
ImageProcessorUnavailableErrorstill hardcodes an npm-specific global install hint
src/tools/FileReadTool/imageProcessor.ts:44
The error message still readsRun \npm i -g sharp`, which is inconsistent with the context-neutral wording you adopted insrc/utils/optionalRuntimeModule.ts:70after the review feedback. For SDK consumers or non-npm users the global npm hint is misleading. Please update this message to the same neutral pattern, e.g. "Install the \sharp` package (add `-g` if you installed the CLI globally) to enable reading and processing images." -
[P3]
checkStsCallerIdentityloads an optional package through a raw dynamic import
src/utils/aws.ts:52-54
This is the remaining optional-package load site that bypassesimportOptionalRuntimeModule. It is inconsistent with the PR's stated intent of routing all optional loads through the shared helper, and a missing@aws-sdk/client-stswill surface a rawERR_MODULE_NOT_FOUNDinstead of the actionable install hint. The error is swallowed here, so the user never sees the real cause. Please route this throughimportOptionalRuntimeModulefor consistency.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
package.json (1)
82-97: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAdd the missing build-time externals to
package.json.@aws-sdk/client-bedrock-runtimeand@aws-sdk/credential-providersare listed inCOMMON_EXTERNALSand need to stay installable forvalidate-externals/build;google-auth-libraryis already declared.🤖 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 `@package.json` around lines 82 - 97, `package.json` is missing build-time externals that are referenced by `COMMON_EXTERNALS`, so add `@aws-sdk/client-bedrock-runtime` and `@aws-sdk/credential-providers` to the dependencies that must stay installable for `validate-externals` and the build. Keep `google-auth-library` declared as-is, and verify the package list still matches the symbols used by the external validation path.
🤖 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/aws.ts`:
- Around line 51-58: The AWS missing-package label in checkStsCallerIdentity()
is too specific and points to the wrong feature. Update the
importOptionalRuntimeModule call in checkStsCallerIdentity() to use a generic
AWS auth/credentials label that also fits clearAwsIniCache() and the auth
refresh/export flows, so the error message reflects the shared AWS
authentication context.
---
Outside diff comments:
In `@package.json`:
- Around line 82-97: `package.json` is missing build-time externals that are
referenced by `COMMON_EXTERNALS`, so add `@aws-sdk/client-bedrock-runtime` and
`@aws-sdk/credential-providers` to the dependencies that must stay installable
for `validate-externals` and the build. Keep `google-auth-library` declared
as-is, and verify the package list still matches the symbols used by the
external validation path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 89c0d2b9-01df-4935-a176-adaad16331b2
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
package.jsonscripts/externals.tsscripts/externalsValidation.test.tsscripts/optionalRuntimeSpecifiers.test.tssrc/services/api/client.optionalRuntime.test.tssrc/services/api/client.tssrc/tools/FileReadTool/imageProcessor.tssrc/utils/aws.tssrc/utils/geminiAuth.optionalRuntime.test.ts
📜 Review details
⚠️ CI failures not shown inline (3)
GitHub Actions: PR Checks / smoke-and-tests (24.11.x): fix(deps): ship a zero-warning, minimal install
Conclusion: failure
##[group]src/services/api/client.test.ts:
(pass) first-party Anthropic requests execute the configured fetch wrapper without runtime symbol errors [6.00ms]
(pass) routes Gemini provider requests through the OpenAI-compatible shim [1.00ms]
(pass) routes env-only MiniMax requests through the Anthropic-compatible API [1.00ms]
(pass) env-only MiniMax fallback preserves legacy OPENAI_MODEL as Anthropic model [1.00ms]
(pass) env-only MiniMax fallback drops stale OpenAI shim options [1.00ms]
(pass) env-only MiniMax fallback replaces stale non-MiniMax model env
(pass) env-only MiniMax fallback does not override explicit OpenAI credentials
(pass) env-only MiniMax fallback ignores non-MiniMax base overrides
(pass) routes env-only AI/ML API requests through the OpenAI-compatible shim despite an ambient OpenAI key [2.00ms]
(pass) routes env-only xAI requests through the OpenAI-compatible shim [1.00ms]
(pass) env-only xAI fallback replaces stale OpenAI credentials and model env [1.00ms]
(pass) env-only xAI fallback preserves xAI OPENAI_API_BASE host overrides [1.00ms]
(pass) env-only xAI fallback drops unsupported OpenAI shim options [8.00ms]
(pass) env-only xAI fallback ignores non-xAI base overrides
(pass) env-only xAI wins when MiniMax key is also present [1.00ms]
134 | test('Vertex skip-auth branch does not load google-auth-library', async () => {
135 | process.env.CLAUDE_CODE_USE_VERTEX = '1'
136 | process.env.CLAUDE_CODE_SKIP_VERTEX_***REDACTED***
137 |
138 | const importOptionalRuntimeModule = mock(async (specifier: string, feature: string) => {
139 | throw friendlyMissing(specifier, feature)
^
error: AWS Bedrock requires the "`@anthropic-ai/bedrock-sdk`" package, which is not installed. Install it with `npm install `@anthropic-ai/bedrock-sdk`` (add `-g` if you installed the CLI globally) to enable it.
at <anonymous> (/home/runner/work/openclaude/openclaude/src/services/api/client.optionalRuntime.test.ts:139:11)
...
GitHub Actions: PR Checks / 3_smoke-and-tests (24.11.x).txt: fix(deps): ship a zero-warning, minimal install
Conclusion: failure
##[group]src/services/api/client.test.ts:
(pass) first-party Anthropic requests execute the configured fetch wrapper without runtime symbol errors [6.00ms]
(pass) routes Gemini provider requests through the OpenAI-compatible shim [1.00ms]
(pass) routes env-only MiniMax requests through the Anthropic-compatible API [1.00ms]
(pass) env-only MiniMax fallback preserves legacy OPENAI_MODEL as Anthropic model [1.00ms]
(pass) env-only MiniMax fallback drops stale OpenAI shim options [1.00ms]
(pass) env-only MiniMax fallback replaces stale non-MiniMax model env
(pass) env-only MiniMax fallback does not override explicit OpenAI credentials
(pass) env-only MiniMax fallback ignores non-MiniMax base overrides
(pass) routes env-only AI/ML API requests through the OpenAI-compatible shim despite an ambient OpenAI key [2.00ms]
(pass) routes env-only xAI requests through the OpenAI-compatible shim [1.00ms]
(pass) env-only xAI fallback replaces stale OpenAI credentials and model env [1.00ms]
(pass) env-only xAI fallback preserves xAI OPENAI_API_BASE host overrides [1.00ms]
(pass) env-only xAI fallback drops unsupported OpenAI shim options [8.00ms]
(pass) env-only xAI fallback ignores non-xAI base overrides
(pass) env-only xAI wins when MiniMax key is also present [1.00ms]
134 | test('Vertex skip-auth branch does not load google-auth-library', async () => {
135 | process.env.CLAUDE_CODE_USE_VERTEX = '1'
136 | process.env.CLAUDE_CODE_SKIP_VERTEX_***REDACTED***
137 |
138 | const importOptionalRuntimeModule = mock(async (specifier: string, feature: string) => {
139 | throw friendlyMissing(specifier, feature)
^
error: AWS Bedrock requires the "`@anthropic-ai/bedrock-sdk`" package, which is not installed. Install it with `npm install `@anthropic-ai/bedrock-sdk`` (add `-g` if you installed the CLI globally) to enable it.
at <anonymous> (/home/runner/work/openclaude/openclaude/src/services/api/client.optionalRuntime.test.ts:139:11)
...
GitHub Actions: PR Checks / smoke-and-tests (22): fix(deps): ship a zero-warning, minimal install
Conclusion: failure
##[group]src/services/api/client.test.ts:
(pass) first-party Anthropic requests execute the configured fetch wrapper without runtime symbol errors [2.00ms]
(pass) routes Gemini provider requests through the OpenAI-compatible shim [5.00ms]
(pass) routes env-only MiniMax requests through the Anthropic-compatible API [2.00ms]
(pass) env-only MiniMax fallback preserves legacy OPENAI_MODEL as Anthropic model [1.00ms]
(pass) env-only MiniMax fallback drops stale OpenAI shim options
(pass) env-only MiniMax fallback replaces stale non-MiniMax model env [1.00ms]
(pass) env-only MiniMax fallback does not override explicit OpenAI credentials
(pass) env-only MiniMax fallback ignores non-MiniMax base overrides [1.00ms]
(pass) routes env-only AI/ML API requests through the OpenAI-compatible shim despite an ambient OpenAI key [1.00ms]
(pass) routes env-only xAI requests through the OpenAI-compatible shim [1.00ms]
(pass) env-only xAI fallback replaces stale OpenAI credentials and model env [1.00ms]
(pass) env-only xAI fallback preserves xAI OPENAI_API_BASE host overrides [1.00ms]
(pass) env-only xAI fallback drops unsupported OpenAI shim options [2.00ms]
(pass) env-only xAI fallback ignores non-xAI base overrides
(pass) env-only xAI wins when MiniMax key is also present [7.00ms]
134 | test('Vertex skip-auth branch does not load google-auth-library', async () => {
135 | process.env.CLAUDE_CODE_USE_VERTEX = '1'
136 | process.env.CLAUDE_CODE_SKIP_VERTEX_***REDACTED***
137 |
138 | const importOptionalRuntimeModule = mock(async (specifier: string, feature: string) => {
139 | throw friendlyMissing(specifier, feature)
^
error: AWS Bedrock requires the "`@anthropic-ai/bedrock-sdk`" package, which is not installed. Install it with `npm install `@anthropic-ai/bedrock-sdk`` (add `-g` if you installed the CLI globally) to enable it.
at <anonymous> (/home/runner/work/openclaude/openclaude/src/services/api/client.optionalRuntime.test.ts:139:...
🧰 Additional context used
📓 Path-based instructions (7)
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,tsx}: Use TypeScript with strict mode
Use ESM imports in TypeScript source files
Files:
src/tools/FileReadTool/imageProcessor.tssrc/services/api/client.optionalRuntime.test.tssrc/utils/aws.tssrc/utils/geminiAuth.optionalRuntime.test.tssrc/services/api/client.ts
**
⚙️ CodeRabbit configuration file
**: # AGENTS.md - AI Agent Coding GuideThis guide is for AI coding agents working in the OpenClaude repository. Read it before changing code, and also follow CONTRIBUTING.md for contributor policy, PR expectations, review follow-up, and project scope.
Project Snapshot
OpenClaude is a coding-agent CLI for cloud and local model providers. It supports OpenAI-compatible APIs, Anthropic, Gemini, DeepSeek, Ollama, MCP, local backends, slash commands, tools, agents, and a React/Ink terminal UI.
The installed CLI runs on Node.js
>=22.0.0. Bun is used for source builds, scripts, dependency management, and tests.Work Style
- Keep changes focused on one problem.
- Prefer existing patterns in the file or nearby module.
- Avoid unrelated formatting, renames, dependency changes, or broad rewrites.
- Add or update tests when behavior changes.
- Update docs when setup, commands, provider behavior, or user-facing behavior changes.
- For new features, larger refactors, dependencies, or runtime changes, follow the issue-first guidance in CONTRIBUTING.md.
Stack And Conventions
- TypeScript with strict mode and ESM imports.
- React + Ink for terminal UI.
- Bun lockfile and Bun scripts for development workflows.
- Node runtime for the built CLI.
- Python exists for legacy/local-provider helper code. Do not add new Python code or expand Python-based features unless a maintainer explicitly approves that direction.
Common libraries and patterns:
chalkfor terminal color.commanderfor CLI argument parsing.execafor child processes.- Existing service, provider, settings, permission, and UI patterns over new abstractions.
Repository Map
src/commands/- slash and CLI command implementations.src/components/- React/Ink UI components.src/services/- API, MCP, OAuth, wiki, voice, and other service integrations.src/tools/- tool implementations.src/utils/- shared utilities.- `src/integration...
Files:
src/tools/FileReadTool/imageProcessor.tssrc/services/api/client.optionalRuntime.test.tsscripts/optionalRuntimeSpecifiers.test.tssrc/utils/aws.tssrc/utils/geminiAuth.optionalRuntime.test.tsscripts/externals.tsscripts/externalsValidation.test.tspackage.jsonsrc/services/api/client.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/tools/FileReadTool/imageProcessor.tssrc/services/api/client.optionalRuntime.test.tsscripts/optionalRuntimeSpecifiers.test.tssrc/utils/aws.tssrc/utils/geminiAuth.optionalRuntime.test.tsscripts/externals.tsscripts/externalsValidation.test.tspackage.jsonsrc/services/api/client.ts
src/{components/permissions,utils/permissions,hooks/toolPermission,tools,entrypoints/sdk}/**
⚙️ CodeRabbit configuration file
src/{components/permissions,utils/permissions,hooks/toolPermission,tools,entrypoints/sdk}/**: Review permission prompts, auto-allow logic, sandbox behavior, SDK permission schemas, shell/PowerShell execution, and background execution paths as security-sensitive. Block on bypasses, unclear trust boundaries, unsafe path handling, missing user visibility, or changes that broaden allowed behavior without an explicit maintainer decision.
Files:
src/tools/FileReadTool/imageProcessor.ts
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}
⚙️ CodeRabbit configuration file
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}: Review provider routing, model selection, env precedence, auth/token handling, OpenAI-compatible shims, retries, proxy behavior, and outbound HTTP behavior with high scrutiny. Block on silent default changes, hidden fallback expansion, credential reuse mistakes, hardcoded provider assumptions, or new network reach that is not intentional and documented.
Files:
src/services/api/client.optionalRuntime.test.tssrc/services/api/client.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/services/api/client.optionalRuntime.test.tsscripts/optionalRuntimeSpecifiers.test.tssrc/utils/geminiAuth.optionalRuntime.test.tsscripts/externalsValidation.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:
scripts/optionalRuntimeSpecifiers.test.tsscripts/externals.tsscripts/externalsValidation.test.tspackage.json
🪛 GitHub Actions: PR Checks / 0_smoke-and-tests (22).txt
src/services/api/client.optionalRuntime.test.ts
[error] 139-139: Test failed due to missing runtime dependency: AWS Bedrock requires the "@anthropic-ai/bedrock-sdk" package, which is not installed. Install it with npm install @anthropic-ai/bedrock-sdk`` (add -g if you installed the CLI globally) to enable it.
src/services/api/client.ts
[error] 555-555: getAnthropicClient threw because the optional AWS Bedrock runtime dependency "@anthropic-ai/bedrock-sdk" is not installed.
🪛 GitHub Actions: PR Checks / 3_smoke-and-tests (24.11.x).txt
src/services/api/client.optionalRuntime.test.ts
[error] 139-139: AWS Bedrock requires the "@anthropic-ai/bedrock-sdk" package, which is not installed. Install it with npm install @anthropic-ai/bedrock-sdk`` (add -g if you installed the CLI globally) to enable it.
src/services/api/client.ts
[error] 555-555: getAnthropicClient failed because "@anthropic-ai/bedrock-sdk" is required for AWS Bedrock but is not installed.
🪛 GitHub Actions: PR Checks / smoke-and-tests (22)
src/services/api/client.optionalRuntime.test.ts
[error] 139-139: Test failed because AWS Bedrock requires the "@anthropic-ai/bedrock-sdk" package, which is not installed. Install it with npm install @anthropic-ai/bedrock-sdk`` (use -g if you installed the CLI globally).
src/services/api/client.ts
[error] 555-555: Runtime/client initialization failed while creating an Anthropic client: missing dependency "@anthropic-ai/bedrock-sdk" (triggered via optional runtime import).
🪛 GitHub Actions: PR Checks / smoke-and-tests (24.11.x)
src/services/api/client.optionalRuntime.test.ts
[error] 139-139: AWS Bedrock requires the "@anthropic-ai/bedrock-sdk" package, which is not installed. Install it with npm install @anthropic-ai/bedrock-sdk`` (add -g if you installed the CLI globally) to enable it.
src/services/api/client.ts
[error] 555-555: Failure originates from getAnthropicClient calling optional runtime behavior that throws due to missing "@anthropic-ai/bedrock-sdk".
🪛 GitHub Check: smoke-and-tests (22)
src/services/api/client.optionalRuntime.test.ts
[failure] 139-139: error: AWS Bedrock requires the "@anthropic-ai/bedrock-sdk" package
at <anonymous> (/home/runner/work/openclaude/openclaude/src/services/api/client.optionalRuntime.test.ts:139:11)
at getAnthropicClient (/home/runner/work/openclaude/openclaude/src/services/api/client.ts:555:40)
at async <anonymous> (/home/runner/work/openclaude/openclaude/src/services/api/client.test.ts:1277:9)
[failure] 139-139: error: AWS Bedrock requires the "@anthropic-ai/bedrock-sdk" package
at <anonymous> (/home/runner/work/openclaude/openclaude/src/services/api/client.optionalRuntime.test.ts:139:11)
at getAnthropicClient (/home/runner/work/openclaude/openclaude/src/services/api/client.ts:555:40)
at async <anonymous> (/home/runner/work/openclaude/openclaude/src/services/api/client.test.ts:987:9)
[failure] 139-139: error: AWS Bedrock requires the "@anthropic-ai/bedrock-sdk" package
at <anonymous> (/home/runner/work/openclaude/openclaude/src/services/api/client.optionalRuntime.test.ts:139:11)
at getAnthropicClient (/home/runner/work/openclaude/openclaude/src/services/api/client.ts:555:40)
at async <anonymous> (/home/runner/work/openclaude/openclaude/src/services/api/client.test.ts:962:9)
🪛 GitHub Check: smoke-and-tests (24.11.x)
src/services/api/client.optionalRuntime.test.ts
[failure] 139-139: error: AWS Bedrock requires the "@anthropic-ai/bedrock-sdk" package
at <anonymous> (/home/runner/work/openclaude/openclaude/src/services/api/client.optionalRuntime.test.ts:139:11)
at getAnthropicClient (/home/runner/work/openclaude/openclaude/src/services/api/client.ts:555:40)
at async <anonymous> (/home/runner/work/openclaude/openclaude/src/services/api/client.test.ts:1277:9)
[failure] 139-139: error: AWS Bedrock requires the "@anthropic-ai/bedrock-sdk" package
at <anonymous> (/home/runner/work/openclaude/openclaude/src/services/api/client.optionalRuntime.test.ts:139:11)
at getAnthropicClient (/home/runner/work/openclaude/openclaude/src/services/api/client.ts:555:40)
at async <anonymous> (/home/runner/work/openclaude/openclaude/src/services/api/client.test.ts:987:9)
[failure] 139-139: error: AWS Bedrock requires the "@anthropic-ai/bedrock-sdk" package
at <anonymous> (/home/runner/work/openclaude/openclaude/src/services/api/client.optionalRuntime.test.ts:139:11)
at getAnthropicClient (/home/runner/work/openclaude/openclaude/src/services/api/client.ts:555:40)
at async <anonymous> (/home/runner/work/openclaude/openclaude/src/services/api/client.test.ts:962:9)
🔇 Additional comments (10)
src/services/api/client.ts (1)
524-533: LGTM!Confirmed the previously-flagged Vertex auth gating issue (loading
google-auth-librarybefore theCLAUDE_CODE_SKIP_VERTEX_AUTHcheck) is now correctly fixed — the import only happens in the non-skip branch.Also applies to: 577-579, 592-595, 617-618, 652-687
src/utils/geminiAuth.optionalRuntime.test.ts (1)
1-55: LGTM! Samemock.moduleisolation pattern flagged inclient.optionalRuntime.test.tsapplies here too, but no CI failure evidence for this file specifically — see the comment on that file for the root-cause concern.src/tools/FileReadTool/imageProcessor.ts (1)
41-48: LGTM!scripts/externals.ts (1)
95-97: LGTM! Previous feedback about the misleading header comment appears addressed.Also applies to: 110-116
scripts/externalsValidation.test.ts (1)
215-251: LGTM! Switching the transitive-exemption fixture to a synthetic package name decouples the test from a real dependency's status.scripts/optionalRuntimeSpecifiers.test.ts (1)
56-65: LGTM! Keeps the pinned specifier list in sync with the new@aws-sdk/client-stsruntime import.src/services/api/client.optionalRuntime.test.ts (1)
19-26: 🩺 Stability & AvailabilityNo change needed here:
client.test.tsalready uses the same shared mutation lock, so thismock.moduleoverride is serialized and should not bleed into that suite.> Likely an incorrect or invalid review comment.package.json (3)
78-81: Runtime dependency placement matches the externals contract.Keeping
@orama/orama,@orama/plugin-data-persistence, and@vscode/ripgrepas regulardependenciesaligns withscripts/externals.ts, which keeps these external from the esbuild bundle: export const COMMON_EXTERNALS: string[] = [ // Native image processing 'sharp', // Cloud provider SDKs '@aws-sdk/client-bedrock', '@aws-sdk/client-bedrock-runtime', '@aws-sdk/client-sts', '@aws-sdk/credential-providers', '@azure/identity', 'google-auth-library', //@vscode/ripgrepships a platform-specific binary alongside its Since these three (unlike the AWS/Azure/Google packages) don't have an optional-runtime-loader fallback described in the PR, they correctly remain required at install time.
122-122: LGTM!Also applies to: 141-141
151-161: 🩺 Stability & AvailabilityNo action needed. These peers are already optional, and the CLI bundle keeps them bundled rather than externalized.
> Likely an incorrect or invalid review 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 `@src/utils/geminiAuth.ts`:
- Around line 137-143: The dependency on google-auth-library in
createDefaultGoogleAuth is currently hidden behind the optionalRuntimeImporter
parameter, so the regression scanner cannot detect it. Update
createDefaultGoogleAuth in geminiAuth.ts to call the default
importOptionalRuntimeModule helper directly, or otherwise ensure the specifier
is exposed in a form that scripts/optionalRuntimeSpecifiers.test.ts matches.
Keep the unique google-auth-library import path visible without relying on
aliased indirection.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: de90620d-c17e-4553-91bd-8aff55f2fda9
📒 Files selected for processing (6)
scripts/optionalRuntimeSpecifiers.test.tssrc/services/api/client.optionalRuntime.test.tssrc/services/api/client.tssrc/utils/aws.tssrc/utils/geminiAuth.optionalRuntime.test.tssrc/utils/geminiAuth.ts
📜 Review details
⚠️ CI failures not shown inline (4)
GitHub Actions: PR Checks / smoke-and-tests (22): fix(deps): ship a zero-warning, minimal install
Conclusion: failure
cutable input
(pass) replay tool lifecycle records > normalizes denied file-tool replay inputs to match allowed retry inputs
(pass) replay tool lifecycle records > records one error terminal status when post-call result processing fails [1.00ms]
(pass) query lifecycle tool-use cleanup > successful tool execution leaves no active lifecycle tool use [1.00ms]
(pass) query lifecycle tool-use cleanup > custom validation failure leaves no active lifecycle tool use
(pass) query lifecycle tool-use cleanup > schema validation failure does not end a lifecycle entry that never started [1.00ms]
(pass) query lifecycle tool-use cleanup > unknown tool does not end a lifecycle entry that never started
(pass) query lifecycle tool-use cleanup > permission denial leaves no active lifecycle tool use
(pass) query lifecycle tool-use cleanup > already-aborted tool execution uses reason-aware result for query-timeout
(pass) query lifecycle tool-use cleanup > already-aborted tool execution uses reason-aware result for hard_max [1.00ms]
(pass) query lifecycle tool-use cleanup > already-aborted tool execution uses reason-aware result for background
(pass) query lifecycle tool-use cleanup > already-aborted tool execution uses reason-aware result for interrupt
(pass) query lifecycle tool-use cleanup > already-aborted tool execution uses reason-aware result for user-abort
(pass) query lifecycle tool-use cleanup > thrown tool error leaves no active lifecycle tool use
(pass) query lifecycle tool-use cleanup > aborted tool execution leaves no active lifecycle tool use
(pass) query lifecycle tool-use cleanup > permission-updated input can retrack lifecycle metadata and still ends once [1.00ms]
(pass) query lifecycle tool-use cleanup > permission-updated input does not resurrect externally ended lifecycle tracking
(pass) normalizeToolInputForValidation > treats blank Read.pages as omitted [1.00ms]
(pass) normalizeToolInputForValidation > treats null Read.pages as omitted
(pass) ...
GitHub Actions: PR Checks / smoke-and-tests (24.11.x): fix(deps): ship a zero-warning, minimal install
Conclusion: failure
essful tool execution leaves no active lifecycle tool use [2.00ms]
(pass) query lifecycle tool-use cleanup > custom validation failure leaves no active lifecycle tool use
(pass) query lifecycle tool-use cleanup > schema validation failure does not end a lifecycle entry that never started [1.00ms]
(pass) query lifecycle tool-use cleanup > unknown tool does not end a lifecycle entry that never started
(pass) query lifecycle tool-use cleanup > permission denial leaves no active lifecycle tool use [1.00ms]
(pass) query lifecycle tool-use cleanup > already-aborted tool execution uses reason-aware result for query-timeout
(pass) query lifecycle tool-use cleanup > already-aborted tool execution uses reason-aware result for hard_max
(pass) query lifecycle tool-use cleanup > already-aborted tool execution uses reason-aware result for background
(pass) query lifecycle tool-use cleanup > already-aborted tool execution uses reason-aware result for interrupt
(pass) query lifecycle tool-use cleanup > already-aborted tool execution uses reason-aware result for user-abort [1.00ms]
(pass) query lifecycle tool-use cleanup > thrown tool error leaves no active lifecycle tool use
(pass) query lifecycle tool-use cleanup > aborted tool execution leaves no active lifecycle tool use [1.00ms]
(pass) query lifecycle tool-use cleanup > permission-updated input can retrack lifecycle metadata and still ends once [1.00ms]
(pass) query lifecycle tool-use cleanup > permission-updated input does not resurrect externally ended lifecycle tracking
(pass) normalizeToolInputForValidation > treats blank Read.pages as omitted
(pass) normalizeToolInputForValidation > treats null Read.pages as omitted
(pass) normalizeToolInputForValidation > wraps Gemini-style single AskUserQuestion payloads [2.00ms]
(pass) normalizeToolInputForValidation > leaves already valid AskUserQuestion payloads unchanged
(pass) normalizeToolInputForValidation > does not normalize unrelated tool inputs
##[endgrou...
GitHub Actions: PR Checks / 0_smoke-and-tests (24.11.x).txt: fix(deps): ship a zero-warning, minimal install
Conclusion: failure
reaming: strips <think> tag block from assistant content deltas [2.00ms]
(pass) streaming: strips <think> tag split across multiple content chunks [1.00ms]
(pass) streaming: preserves prose without tags (no phrase-based false positive) [2.00ms]
(pass) strips credentials and query params from URL in fetch network error message [1.00ms]
(pass) classifies localhost transport failures with actionable category marker [2.00ms]
(pass) transport failures are not labeled with HTTP status 503 [1.00ms]
(pass) propagates AbortError without wrapping it as transport failure [1.00ms]
(pass) classifies chat-completions endpoint 404 failures with endpoint_not_found marker [2.00ms]
(pass) self-heals localhost resolution failures by retrying local loopback base URL [1.00ms]
(pass) uses native Ollama chat endpoint when local base URL omits /v1 [1.00ms]
(pass) keeps remote Ollama-named gateways on chat completions [2.00ms]
(pass) keeps HTTPS localhost Ollama-port proxies on chat completions [1.00ms]
(pass) self-heals tool-call incompatibility by retrying local Ollama requests without tools [2.00ms]
(pass) preserves valid tool_result and drops orphan tool_result [1.00ms]
(pass) drops empty assistant message when only thinking block was present and stripped [1.00ms]
(pass) drops empty assistant message when only redacted_thinking block was present and stripped [1.00ms]
(pass) injects semantic assistant message when tool result is followed by user message [1.00ms]
(pass) Moonshot: uses max_tokens (not max_completion_tokens) and strips store [1.00ms]
(pass) Cerebras: strips unsupported store on chat_completions (`#1023`) [1.00ms]
(pass) Local provider (vLLM/Ollama/etc.): strips unsupported store on chat_completions (`#672`) [1.00ms]
(pass) Mistral: strips unsupported store on chat_completions (`#739`) [1.00ms]
(pass) Mistral host fallback: strips store on an unresolved Mistral-host route (`#739`) [1.00ms]
(pass) hasMistralApiHost matches the Mistral host and its subdomains ...
GitHub Actions: PR Checks / 3_smoke-and-tests (22).txt: fix(deps): ship a zero-warning, minimal install
Conclusion: failure
s consecutive assistant messages preserving tool_calls (issue `#202`) [1.00ms]
(pass) non-streaming: reasoning_content emitted as thinking block only when content is null
(pass) non-streaming: empty string content does not fall through to reasoning_content as text [1.00ms]
(pass) non-streaming: real content takes precedence over reasoning_content
(pass) non-streaming: preserves response body when usage parsing fails [1.00ms]
(pass) non-streaming: preserves response.url routing metadata after body read [1.00ms]
(pass) non-streaming: strips <think> tag block from assistant content [1.00ms]
(pass) streaming: thinking block closed before tool call [1.00ms]
(pass) streaming: strips <think> tag block from assistant content deltas [1.00ms]
(pass) streaming: strips <think> tag split across multiple content chunks [1.00ms]
(pass) streaming: preserves prose without tags (no phrase-based false positive) [1.00ms]
(pass) strips credentials and query params from URL in fetch network error message [1.00ms]
(pass) classifies localhost transport failures with actionable category marker [1.00ms]
(pass) transport failures are not labeled with HTTP status 503 [1.00ms]
(pass) propagates AbortError without wrapping it as transport failure [1.00ms]
(pass) classifies chat-completions endpoint 404 failures with endpoint_not_found marker [1.00ms]
(pass) self-heals localhost resolution failures by retrying local loopback base URL [1.00ms]
(pass) uses native Ollama chat endpoint when local base URL omits /v1 [1.00ms]
(pass) keeps remote Ollama-named gateways on chat completions
(pass) keeps HTTPS localhost Ollama-port proxies on chat completions [1.00ms]
(pass) self-heals tool-call incompatibility by retrying local Ollama requests without tools [1.00ms]
(pass) preserves valid tool_result and drops orphan tool_result [1.00ms]
(pass) drops empty assistant message when only thinking block was present and stripped
(pass) drops empty assistant message when only redacted_thin...
🧰 Additional context used
📓 Path-based instructions (6)
**
⚙️ CodeRabbit configuration file
**: # AGENTS.md - AI Agent Coding GuideThis guide is for AI coding agents working in the OpenClaude repository. Read it before changing code, and also follow CONTRIBUTING.md for contributor policy, PR expectations, review follow-up, and project scope.
Project Snapshot
OpenClaude is a coding-agent CLI for cloud and local model providers. It supports OpenAI-compatible APIs, Anthropic, Gemini, DeepSeek, Ollama, MCP, local backends, slash commands, tools, agents, and a React/Ink terminal UI.
The installed CLI runs on Node.js
>=22.0.0. Bun is used for source builds, scripts, dependency management, and tests.Work Style
- Keep changes focused on one problem.
- Prefer existing patterns in the file or nearby module.
- Avoid unrelated formatting, renames, dependency changes, or broad rewrites.
- Add or update tests when behavior changes.
- Update docs when setup, commands, provider behavior, or user-facing behavior changes.
- For new features, larger refactors, dependencies, or runtime changes, follow the issue-first guidance in CONTRIBUTING.md.
Stack And Conventions
- TypeScript with strict mode and ESM imports.
- React + Ink for terminal UI.
- Bun lockfile and Bun scripts for development workflows.
- Node runtime for the built CLI.
- Python exists for legacy/local-provider helper code. Do not add new Python code or expand Python-based features unless a maintainer explicitly approves that direction.
Common libraries and patterns:
chalkfor terminal color.commanderfor CLI argument parsing.execafor child processes.- Existing service, provider, settings, permission, and UI patterns over new abstractions.
Repository Map
src/commands/- slash and CLI command implementations.src/components/- React/Ink UI components.src/services/- API, MCP, OAuth, wiki, voice, and other service integrations.src/tools/- tool implementations.src/utils/- shared utilities.- `src/integration...
Files:
scripts/optionalRuntimeSpecifiers.test.tssrc/utils/geminiAuth.optionalRuntime.test.tssrc/utils/aws.tssrc/services/api/client.optionalRuntime.test.tssrc/utils/geminiAuth.tssrc/services/api/client.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:
scripts/optionalRuntimeSpecifiers.test.tssrc/utils/geminiAuth.optionalRuntime.test.tssrc/utils/aws.tssrc/services/api/client.optionalRuntime.test.tssrc/utils/geminiAuth.tssrc/services/api/client.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:
scripts/optionalRuntimeSpecifiers.test.ts
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
scripts/optionalRuntimeSpecifiers.test.tssrc/utils/geminiAuth.optionalRuntime.test.tssrc/services/api/client.optionalRuntime.test.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,tsx}: Use TypeScript with strict mode
Use ESM imports in TypeScript source files
Files:
src/utils/geminiAuth.optionalRuntime.test.tssrc/utils/aws.tssrc/services/api/client.optionalRuntime.test.tssrc/utils/geminiAuth.tssrc/services/api/client.ts
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}
⚙️ CodeRabbit configuration file
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}: Review provider routing, model selection, env precedence, auth/token handling, OpenAI-compatible shims, retries, proxy behavior, and outbound HTTP behavior with high scrutiny. Block on silent default changes, hidden fallback expansion, credential reuse mistakes, hardcoded provider assumptions, or new network reach that is not intentional and documented.
Files:
src/services/api/client.optionalRuntime.test.tssrc/services/api/client.ts
🔇 Additional comments (6)
src/services/api/client.ts (1)
58-67: LGTM!Also applies to: 536-544, 588-621, 663-698
src/utils/aws.ts (1)
51-58: LGTM!Also applies to: 64-79
src/utils/geminiAuth.ts (1)
6-6: LGTM!Also applies to: 44-46, 149-183, 206-250
src/services/api/client.optionalRuntime.test.ts (1)
19-148: LGTM!src/utils/geminiAuth.optionalRuntime.test.ts (1)
1-47: LGTM!scripts/optionalRuntimeSpecifiers.test.ts (1)
34-96: LGTM!Pinned
EXPECTED_SPECIFIERSset correctly addresses the prior "count-only" gap. Note the separate comment onsrc/utils/geminiAuth.tsregarding an indirect call site this regex can't see.
* fix(deps): ship a zero-warning, minimal install
The published package declared 62 runtime `dependencies`, but `dist/cli.mjs`
is a fully-bundled esbuild output that inlines almost all of them. End users
therefore installed ~476 transitive packages — including three subtrees the
bundle never needs at install time, each emitting an install warning:
- node-domexception (deprecated) via google-auth-library
- protobufjs (allow-scripts) via @grpc/* (already bundled into dist)
- sharp (allow-scripts) native image module
The repo's `overrides`/`allowScripts` silence these locally, but those are
root-only npm settings and are ignored when the package is installed as a
dependency — so end users saw the warnings.
Core changes:
- package.json: runtime dependencies trimmed 62 -> 3 (@orama/orama,
@orama/plugin-data-persistence, @vscode/ripgrep). Bundled packages, plus
the optional sharp/google-auth-library, move to devDependencies so they
are built/tested but not shipped.
- package.json: @anthropic-ai/sdk, @modelcontextprotocol/sdk, react and
react-reconciler declared as OPTIONAL peerDependencies — externalized by
the ./sdk bundle but bundled into the CLI. Optional peers keep the CLI
install minimal and warning-free while still resolving for ./sdk consumers.
- externals.ts: sharp, google-auth-library and @anthropic-ai/bedrock-sdk
marked OPTIONAL_RUNTIME_EXTERNALS (loaded on demand, not shipped).
- validate-externals.ts: runtime deps validate against externals; bundled
deps validate against dependencies + devDependencies.
- client.ts: load @anthropic-ai/bedrock-sdk via the runtime importer so
esbuild no longer inlines it and hoists its static @aws-sdk import into
the CLI bundle (that was a startup crash for default installs).
Optional-dependency UX (consistent, actionable errors):
- New src/utils/optionalRuntimeModule.ts exports importRuntimeModule and
importOptionalRuntimeModule. The optional variant translates a missing
package (code === 'ERR_MODULE_NOT_FOUND', specifier present in message)
into "<feature> requires "<pkg>" ... Run `npm i -g <pkg>`". Generic so
typed call sites keep their module types.
- Routed ALL optional-package load sites through it (previously only one
did): google-auth-library (client.ts, auth.ts, geminiAuth.ts),
@anthropic-ai/foundry-sdk + @azure/identity (client.ts), and the
@aws-sdk/* Bedrock paths (model/bedrock.ts, tokenEstimation.ts, aws.ts).
- imageProcessor.ts: sharp-missing error now says `npm i -g sharp`.
- docs/advanced-setup.md: new "Optional provider packages" table and a
Vertex note documenting the on-demand installs.
- Unit test for the helper (friendly error, success path, specifier match,
raw passthrough).
- knip.json: ignore google-auth-library (now loaded via runtime string).
Verified on the current tree:
- tsc, build/validate-externals, knip, and tests all pass.
- npm pack + install --omit=dev adds 8 packages, zero deprecation/
allow-scripts/funding warnings; --version/--help/mcp list run.
- With packages absent, CLAUDE_CODE_USE_BEDROCK and CLAUDE_CODE_USE_VERTEX
print the friendly `npm i -g <pkg>` error (verified end-to-end).
- ./sdk imports once its optional peers are present (24 exports, no warns).
- Bundled ajv + ajv-formats validate with no ajv installed; no unguarded
native runtime requires (fsevents absent in chokidar 4; bun:sqlite Bun-only).
Trade-off: image reads, AWS Bedrock, Azure Foundry and GCP/Vertex now prompt
a one-time `npm i -g <pkg>` instead of being shipped to every user.
Co-Authored-By: OpenClaude <openclaude@gitlawb.com>
Review fixes (CodeRabbit + jatmn):
- validate-externals: the INTENTIONALLY_BUNDLED exemption is now scoped per
bundle. The CLI exempts every bundled package; the SDK does NOT exempt
packages declared as peerDependencies (keyed on package.json, an independent
source of truth) so dropping react/@anthropic-ai/sdk from SDK_EXTERNALS now
fails validation instead of silently passing. Added an explicit minimal-
install contract check: bundled packages must be devDependencies-only — never
in `dependencies`, and only the SDK-external subset may be optional peers.
Validation logic extracted to scripts/externalsValidation.ts + tests.
- FileReadTool oversized-image fallback now loads via the shared
getImageProcessor() (not a raw import('sharp')) and re-throws
ImageProcessorUnavailableError, so a missing processor surfaces the
`npm i -g sharp` install hint instead of returning an over-budget image.
- optionalRuntimeModule: match the missing specifier as a QUOTED token, not a
raw substring, so a missing transitive package whose name contains the
requested one (sharp vs sharp-libvips, @aws-sdk/client-bedrock vs
@aws-sdk/client-bedrock-runtime) no longer triggers the wrong install hint.
Predicate extracted to isMissingSpecifierError() with regression tests.
- docs/advanced-setup.md: the Vertex auth section now shows both documented
paths (gcloud ADC and a GOOGLE_APPLICATION_CREDENTIALS service-account file).
Review fixes (round 2, CodeRabbit):
- validate-externals: assert the optional-peer install contract — every
peerDependency must be { optional: true } in peerDependenciesMeta
(validateOptionalPeers), so losing that flag fails the build instead of
silently reintroducing install warnings.
- validate-externals: hard-check OPTIONAL_RUNTIME_EXTERNALS placement
(validateOptionalRuntimeexternals). Anything esbuild can see statically must
stay external in BOTH bundles (dropping sharp/google-auth-library now fails);
the runtime-indirection-only subset (new RUNTIME_INDIRECTION_ONLY_EXTERNALS)
must stay OUT of externals so esbuild never re-exposes their static imports.
- Deeper-dig fix: @anthropic-ai/foundry-sdk was misclassified as
INTENTIONALLY_BUNDLED, but it is loaded only through the Function indirection
(esbuild never sees it, so it was never actually bundled) — its sole presence
in dist is the specifier string. Per the PR's own "Azure Foundry now prompts"
trade-off it is on-demand, so it now lives in OPTIONAL_RUNTIME_EXTERNALS +
RUNTIME_INDIRECTION_ONLY_EXTERNALS (mirroring bedrock-sdk). sandbox-runtime is
genuinely statically imported, so it stays bundled.
- Provider-routing coverage (scripts/optionalRuntimeSpecifiers.test.ts): a
static scan asserts every importOptionalRuntimeModule specifier is a declared
OPTIONAL_RUNTIME_EXTERNAL and never also INTENTIONALLY_BUNDLED — the
invariant that keeps a provider's optional package loadable on demand.
- All new validators extracted to scripts/externalsValidation.ts with tests.
Review fixes (round 3, CodeRabbit):
- client.ts: gate the Vertex google-auth-library import behind the non-skip
branch. CLAUDE_CODE_SKIP_VERTEX_AUTH (proxy/test) uses a mock GoogleAuth and
must not require the optional package; it was loaded unconditionally before.
- optionalRuntimeModule: drop the hard-coded `npm i -g`. The helper backs both
the global CLI and project-local ./sdk consumers, so the hint is now
context-neutral ("npm install <pkg>" / add -g for the global CLI).
- validate-externals: every SDK_ONLY_EXTERNALS entry must STAY a
peerDependency (a dropped peer leaves runtimeDeps while the SDK still
externalizes it); and OPTIONAL_RUNTIME_EXTERNALS must never be shipped (fail
on overlap with dependencies/peerDependencies). Both with tests + live-verified.
- optionalRuntimeSpecifiers.test: pin the EXACT set of optionally-loaded
specifiers instead of a >=5 count (a count passes even if a provider path
regresses).
- attachments: extract tryReadEditedImageAttachment() — background watched-file
image attachments DEGRADE to null on any failure (incl.
ImageProcessorUnavailableError) so a missing optional package never aborts a
turn, while the explicit FileReadTool path still surfaces the install hint.
Deterministic regression test (bad path -> null).
- docs: Bedrock row notes profile-based auth also needs
@aws-sdk/credential-providers; install-hint wording matches the new message.
Review fixes (round 4, CodeRabbit):
- attachments: stop sending the raw file path through the analytics
bypass-cast (tengu_watched_file_compression_failed). Send only the safe
file extension via getFileExtensionForAnalytics, matching the existing
tengu_file_read_dedup pattern, so no usernames/project paths can leak.
- externals.ts: corrected the OPTIONAL_RUNTIME_EXTERNALS header comment,
which still claimed all entries "remain in COMMON_EXTERNALS" — no longer
true since the indirection-only subset (bedrock/foundry) must stay OUT of
the externals lists.
(Other CodeRabbit comments on this push re-surface items already addressed in
prior commits: the peerDependenciesMeta-optional check (validateOptionalPeers),
the SDK-peers-present and optional-not-shipped validator rules, the
exact-specifier-set test, the attachments degrade contract + test, and the
context-neutral install hint are all present. The "assert every optional
external is a devDependency" suggestion is intentionally NOT applied: @aws-sdk/*
and @azure/identity are transitive devDeps via bedrock-sdk/foundry-sdk, so a
blanket assertion would be incorrect; source resolution is covered by the
build + tests that import these packages.)
Review fixes (round 5, CodeRabbit):
- attachments: stop leaking file paths via logError in the background-image
degrade path. readImageWithTokenBudget can throw path-bearing messages
(e.g. "Image file is empty: <path>") and logError persists message/stack, so
log only the error TYPE name now. (Analytics payload was already sanitized.)
- attachments: tryReadEditedImageAttachment takes an injectable reader so the
degrade contract is tested for the EXACT error types — ImageProcessorUnavailableError
and a path-bearing read error both degrade to null (not just ENOENT) — plus a
success case. No mocking.
- validate-externals: enforce the source-install half of the optional contract.
Non-transitive OPTIONAL_RUNTIME_EXTERNALS must be devDependencies so `bun
install` source builds resolve them. The new TRANSITIVE_OPTIONAL_EXTERNALS
documents the exemption (@aws-sdk/* via @anthropic-ai/bedrock-sdk, @azure/identity
via @anthropic-ai/foundry-sdk — provided transitively, not direct devDeps). A
blanket "all optionals are devDeps" check would have wrongly failed on those.
Tests + live-verified (dropping sharp from devDependencies now fails).
Review fixes (round 6, CodeRabbit + jatmn):
- optionalRuntimeSpecifiers.test: the call-site scan regex missed
generic-annotated calls (importOptionalRuntimeModule<...>(...)) in
model/bedrock.ts and tokenEstimation.ts, so the exact-set assertion was
incomplete. Regex now allows an optional generic; EXPECTED_SPECIFIERS adds
@aws-sdk/client-bedrock and @aws-sdk/client-bedrock-runtime (7 total).
- importOptionalRuntimeModule default generic is now <T = unknown> (was any),
so destructured imports are no longer silently any. Every call site now
supplies its module type — typeof import('<pkg>') where the package is
type-resolvable (bedrock-sdk, foundry-sdk, @aws-sdk/credential-providers,
google-auth-library), and a named minimal-shape alias for @azure/identity
(not a direct devDep, so typeof import can't resolve it). This gives
compile-time verification of each provider's module contract (export names,
shapes) — the structural answer to the "cover the provider branches" ask.
- attachments: tryReadEditedImageAttachment takes injectable {read,log,track};
a new test asserts the sanitized-telemetry contract directly — the logError
payload is path-free and the analytics payload carries only `ext`, never the
edited-image path.
* fix(deps): address optional runtime review findings
* test(deps): isolate optional runtime importer mocks
* fix(deps): clarify AWS optional auth labels
* fix(deps): close optional runtime review gaps
---------
Co-authored-by: jatmn <the@jat.mn>
(cherry picked from commit 2edec9a)
(cherry picked from commit 1663982b79fcd349451079d5bf2ddfea2c9b39ae)
…herry-pick cleanup for 3-provider scope) OpenCC supports only anthropic/ollama/openai-compatible. The Twigpine#1784 cherry-pick left bedrock/vertex branches in tokenEstimation.ts that reference removed modules (../utils/model/bedrock.js, VERTEX_COUNT_TOKENS_ALLOWED_BETAS) and use string values ('bedrock', 'vertex') that don't exist in OpenCC's APIProvider union. - Drop 5 imports (bedrock helpers, VERTEX_COUNT_TOKENS_ALLOWED_BETAS, CountTokensCommandInput, getDefaultSonnetModel, isEnvTruthy, etc.) - Remove bedrock early-return + countTokensWithBedrock helper (-59 lines) - Remove vertex beta filter from countMessagesTokensWithAPI - Remove bedrock/vertex/Haiku-mode branching in countTokensViaHaikuFallback (use getSmallFastModel() directly) - Drop `model:` param from getAnthropicClient calls (only needed for bedrock wiring that no longer exists) - Restore `@ts-ignore` on attachment-content type mismatch that Twigpine#1784 introduced (pre-Twigpine#1784 already had the suppression; restore parity) (cherry picked from commit a3783079bdd7273a11f6bad12a524cbefc47d619)
…cherry-pick Cluster E (Twigpine#1784 zero-warning install) cherry-pick added a `scripts/externalsValidation.ts` post-build check that requires all OPTIONAL_RUNTIME_EXTERNALS to be explicit devDependencies. Several entries that OpenCC's 3-provider scope (anthropic/ollama/openai-compatible) does not need at runtime were still listed: - Removed from `src/utils/proxy.ts`: `getAWSClientProxyConfig()` function + AWS SDK dynamic imports. The function was added by upstream Twigpine#1784 but never called from OpenCC source (verified `grep -rn 'getAWSClientProxyConfig' src/`). The unused `importOptionalRuntimeModule` import was also removed. - Removed from `scripts/externals.ts` (COMMON_EXTERNALS + OPTIONAL_RUNTIME_EXTERNALS): `@aws-sdk/client-bedrock`, `@aws-sdk/client-bedrock-runtime`, `@aws-sdk/client-sts`, `@aws-sdk/credential-provider-node`, `@aws-sdk/credential-providers`, `@smithy/core`, `@smithy/node-http-handler`, `@azure/identity`. These were added by Twigpine#1784 for AWS proxy / Bedrock / Vertex support that OpenCC does not use. - Removed from `INTENTIONALLY_BUNDLED`: `graphology`, `graphology-metrics`, `js-tiktoken`. These are for upstream's Twigpine#1867 repo-map feature (Tier 3, not ported to OpenCC) and were never imported in `src/`. - Added explicit `devDependencies`: `@anthropic-ai/bedrock-sdk`, `@anthropic-ai/foundry-sdk`, `google-auth-library`. These are still referenced from the optional-runtime-module tests + the Vertex auth path in `src/utils/auth.ts` (transitive deps already installed but not declared; required by the new externals validator). After this commit `bun run build` passes the post-build externals validation (55 bundled packages, all devDependencies-only). `bun run dev` reaches the CLI launch step and only fails with "Input must be provided" when run without stdin or --print args, which is expected CLI behavior. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2 participants
The published package declared 62 runtime
dependencies, butdist/cli.mjsis a fully-bundled esbuild output that inlines almost all of them. End users therefore installed ~476 transitive packages — including three subtrees the bundle never needs at install time, each emitting an install warning:The repo's
overrides/allowScriptssilence these locally, but those are root-only npm settings and are ignored when the package is installed as a dependency — so end users saw the warnings.Core changes:
Optional-dependency UX (consistent, actionable errors):
npm i -g <pkg>". Generic so typed call sites keep their module types.npm i -g sharp.Verified on the current tree:
npm i -g <pkg>error (verified end-to-end).Trade-off: image reads, AWS Bedrock, Azure Foundry and GCP/Vertex now prompt a one-time
npm i -g <pkg>instead of being shipped to every user.Summary by CodeRabbit