fix(status): show active provider route instead of legacy provider bucket - #1673
Conversation
The /status command collapsed many concrete providers (OpenRouter, Groq, Ollama, Fireworks AI, etc.) into a single "OpenAI-compatible" label, making multi-provider setups hard to verify and debug. When apiProvider resolves to the generic "openai" bucket, /status now uses route metadata to surface the real active route: Provider route: OpenRouter Transport: OpenAI-compatible API OpenAI base URL: https://openrouter.ai/api/v1 Model: anthropic/claude-sonnet-4.5 Credential: OPENROUTER_API_KEY configured The legacy "OpenAI-compatible" label and fallback are preserved for unknown custom base URLs. Dedicated provider buckets (nvidia-nim, minimax, codex, github, xai, gemini, bedrock, vertex, foundry, firstParty, mistral) already have accurate labels and are left untouched. Credential display uses env-var names only (never values). Transport kind and route label come from the existing descriptor-driven route metadata; no new hardcoded provider maps or network calls are introduced.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📜 Recent review details🧰 Additional context used📓 Path-based instructions (10)src/**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
{src/services/**/*.ts,src/utils/**/*.ts}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{ts,tsx,js,jsx,py,json,md}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*.test.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*.test.{ts,tsx,js}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*.{ts,tsx,js,jsx,py}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*⚙️ CodeRabbit configuration file
Files:
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}⚙️ CodeRabbit configuration file
Files:
**⚙️ CodeRabbit configuration file
Files:
🔇 Additional comments (2)
📝 WalkthroughWalkthroughGeneralizes secret-key detection from a fixed set to pattern-based heuristics ( ChangesRoute-aware provider status display
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/utils/status.tsx (1)
494-510:⚠️ Potential issue | 🟠 Major | ⚡ Quick winInclude route-specific credentials in the redaction source.
The new credential summary knows route credentials via
getRouteCredentialEnvVars, but base URL/model redaction still usessecretSource, which omits route-specific secrets such asOPENROUTER_API_KEY,GROQ_API_KEY, andFIREWORKS_API_KEY. If one of those secret values appears in a displayed env-derived field,/statuswill not redact it.src/integrations/routeMetadata.tsLines 581-604 already provides the route credential env-var contract.🛡️ Proposed fix
const transportLabel = routeId ? getRouteProviderTypeLabel(routeId) : null; + const routeSecretSource: SecretValueSource = routeId + ? Object.fromEntries( + getRouteCredentialEnvVars(routeId).map(name => [ + name, + process.env[name], + ]), + ) + : {}; + const redactionSource: SecretValueSource = { + ...secretSource, + ...routeSecretSource, + }; if (transportLabel) { properties.push({ label: 'Transport', value: transportLabel, }); @@ metadata.baseUrlLabel, getOpenAICompatibleBaseUrlForStatus(routeId), - secretSource, + redactionSource, ); @@ 'Model', modelDisplay, - secretSource, + redactionSource, );🤖 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/status.tsx` around lines 494 - 510, The pushRedactedProperty calls for base URL and model display in the status handler are using only the basic secretSource for redaction, which omits route-specific credentials like OPENROUTER_API_KEY, GROQ_API_KEY, and FIREWORKS_API_KEY. Instead of passing secretSource directly to pushRedactedProperty, obtain the route-specific credentials using getRouteCredentialEnvVars (referenced in src/integrations/routeMetadata.ts lines 581-604) for the current routeId and merge them with the existing secretSource, then pass this combined redaction source to both pushRedactedProperty calls to ensure route-specific secret values are properly redacted in the displayed fields.
🤖 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/status.tsx`:
- Around line 132-162: The resolveDisplayRouteId function passes raw process.env
to resolveActiveRouteIdFromEnv, but getOpenAICompatibleBaseUrlForStatus trims
env var values, creating inconsistent behavior where blank OPENAI_BASE_URL
values with whitespace may cause route resolution to differ from what is
displayed. Normalize the OPENAI_BASE_URL and OPENAI_API_BASE environment
variables by trimming whitespace before passing process.env to
resolveActiveRouteIdFromEnv in the resolveDisplayRouteId function, ensuring that
both the route resolver and display helper operate on consistently normalized
inputs.
---
Outside diff comments:
In `@src/utils/status.tsx`:
- Around line 494-510: The pushRedactedProperty calls for base URL and model
display in the status handler are using only the basic secretSource for
redaction, which omits route-specific credentials like OPENROUTER_API_KEY,
GROQ_API_KEY, and FIREWORKS_API_KEY. Instead of passing secretSource directly to
pushRedactedProperty, obtain the route-specific credentials using
getRouteCredentialEnvVars (referenced in src/integrations/routeMetadata.ts lines
581-604) for the current routeId and merge them with the existing secretSource,
then pass this combined redaction source to both pushRedactedProperty calls to
ensure route-specific secret values are properly redacted in the displayed
fields.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a7a13787-0403-4466-9d76-0c5b879e0a3e
📒 Files selected for processing (2)
src/utils/status.routes.test.tssrc/utils/status.tsx
📜 Review details
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Keep comments useful and concise in code
Files:
src/utils/status.tsxsrc/utils/status.routes.test.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/status.tsxsrc/utils/status.routes.test.ts
**
⚙️ CodeRabbit configuration file
**: # Contributing to OpenClaudeThanks for contributing.
OpenClaude is a fast-moving open-source coding-agent CLI with support for multiple providers, local backends, MCP, and a terminal-first workflow. The best contributions here are focused, well-tested, and easy to review.
Before You Start
- Search existing issues and discussions before opening a new thread.
- Check open pull requests for work that overlaps with your contribution. If a PR already exists that addresses the same change, open an issue or discussion first to align on direction — duplicate PRs may be closed without review.
- Use issues for confirmed bugs and actionable feature work.
- Use discussions for setup help, ideas, and general community conversation.
- For larger changes, open an issue first so the scope is clear before implementation.
- For security reports, follow SECURITY.md.
Pull Requests
Every PR needs a reason. Your PR description must include:
- what changed and why
- the user or developer impact
- the exact checks you ran
- a linked issue when one exists, using
Fixes#123, `Closes `#123, or another clear link- screenshots when the PR touches UI, terminal presentation, or the VS Code extension
- which provider path was tested when the PR changes provider behavior
The PR author is responsible for ensuring their PR is merge-ready. PRs with merge conflicts will not be reviewed or approved until the conflicts are resolved.
Issues are the recommended starting point for anything non-trivial — opening one first helps avoid wasted effort if the change is out of scope or already being worked on. Small fixes, doc corrections, and obvious improvements can stand on their own without a linked issue, as long as the PR description explains the intent.
What Gets Closed Without Review
PRs may be closed without review...
Files:
src/utils/status.tsxsrc/utils/status.routes.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/status.routes.test.ts
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
I found an issue that needs to be addressed before this is ready.
Findings
- [P2] Redact OPENAI_API_BASE before showing it in /status
src/utils/status.tsx:205
This PR now falls back toOPENAI_API_BASEfor the displayed OpenAI-compatible base URL, but that value is still passed through the genericpushRedactedPropertypath, which only masks whole values that exactly match a known secret. A legacy base URL such ashttps://openrouter.ai/api/v1?api_key=...is therefore printed with the query secret intact in/status; before this change, that alias was not displayed at all. Please run the displayed base URL through the existing URL query redaction path, or equivalent query-param redaction, before adding it to status and cover theOPENAI_API_BASEalias case.
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)
src/utils/status.routes.test.ts (1)
12-32:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winClear provider env by prefix, not a partial allow-list.
This list misses other route/provider env-only paths, so a developer shell with e.g. another provider credential can still steer
buildPropertiesWithRealProvider()and make these tests order/env-dependent. Prefer clearing provider-related prefixes or deriving the list from route metadata. As per coding guidelines, tests should be reviewed for “isolation of global/env/config state.”🧪 Proposed fix
-const PROVIDER_ENV_VARS = [ - 'CLAUDE_CODE_USE_OPENAI', - 'CLAUDE_CODE_USE_GEMINI', - 'CLAUDE_CODE_USE_GITHUB', - 'CLAUDE_CODE_USE_MISTRAL', - 'CLAUDE_CODE_USE_BEDROCK', - 'CLAUDE_CODE_USE_VERTEX', - 'CLAUDE_CODE_USE_FOUNDRY', - 'CLAUDE_CODE_PROVIDER_PROFILE_ENV_APPLIED', - 'OPENAI_BASE_URL', - 'OPENAI_API_BASE', - 'OPENAI_API_KEY', - 'OPENAI_MODEL', - 'OPENROUTER_API_KEY', - 'GROQ_API_KEY', - 'FIREWORKS_API_KEY', - 'GEMINI_BASE_URL', - 'GEMINI_MODEL', - 'GEMINI_API_KEY', - 'NVIDIA_NIM', -] +const PROVIDER_ENV_PREFIXES = [ + 'CLAUDE_CODE_USE_', + 'CLAUDE_CODE_PROVIDER_PROFILE_', + 'OPENAI_', + 'OPENROUTER_', + 'GROQ_', + 'FIREWORKS_', + 'GEMINI_', + 'GITHUB_', + 'MISTRAL_', + 'NVIDIA_NIM', + 'XAI_', + 'MINIMAX_', + 'XIAOMI_MIMO_', + 'VENICE_', + 'NEARAI_', +] @@ function clearProviderEnv(): void { - for (const key of PROVIDER_ENV_VARS) { - delete process.env[key] + for (const key of Object.keys(process.env)) { + if (PROVIDER_ENV_PREFIXES.some(prefix => key.startsWith(prefix))) { + delete process.env[key] + } } }Also applies to: 50-54
🤖 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/status.routes.test.ts` around lines 12 - 32, The PROVIDER_ENV_VARS list is a hardcoded allow-list that is incomplete and fragile, allowing untracked provider environment variables to interfere with tests and create order/env-dependent behavior. Instead of maintaining this partial list, clear provider-related environment variables by using prefix matching (e.g., clear all vars starting with known provider prefixes like "CLAUDE_CODE_USE_", "OPENAI_", "GEMINI_", "GROQ_", "FIREWORKS_", and "NVIDIA_NIM") or derive the complete list from route metadata. Apply this refactoring consistently at both occurrences in the test file (the initial PROVIDER_ENV_VARS definition and its usage around line 50-54) to ensure proper test isolation regardless of developer shell environment state.Source: Coding guidelines
🤖 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/status.tsx`:
- Around line 127-142: In the pushRedactedUrlProperty function, swap the order
of redaction calls so that URL redaction happens first, before configured-secret
redaction. Currently the code applies redactSecretValueForDisplay to the value
first, then redactUrlForDisplay; instead, apply redactUrlForDisplay to the
original value first, then apply redactSecretValueForDisplay to the result. This
ensures that configured secrets appearing as substrings within URLs (such as in
query parameters) are properly masked even if they wouldn't be caught by
URL-specific redaction logic.
---
Outside diff comments:
In `@src/utils/status.routes.test.ts`:
- Around line 12-32: The PROVIDER_ENV_VARS list is a hardcoded allow-list that
is incomplete and fragile, allowing untracked provider environment variables to
interfere with tests and create order/env-dependent behavior. Instead of
maintaining this partial list, clear provider-related environment variables by
using prefix matching (e.g., clear all vars starting with known provider
prefixes like "CLAUDE_CODE_USE_", "OPENAI_", "GEMINI_", "GROQ_", "FIREWORKS_",
and "NVIDIA_NIM") or derive the complete list from route metadata. Apply this
refactoring consistently at both occurrences in the test file (the initial
PROVIDER_ENV_VARS definition and its usage around line 50-54) to ensure proper
test isolation regardless of developer shell environment state.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c5eae268-e54d-404b-868d-70bce087aa36
📒 Files selected for processing (2)
src/utils/status.routes.test.tssrc/utils/status.tsx
📜 Review details
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Keep comments useful and concise in code
Files:
src/utils/status.routes.test.tssrc/utils/status.tsx
**/*
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
src/utils/status.routes.test.tssrc/utils/status.tsx
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
src/utils/status.routes.test.ts
**
⚙️ CodeRabbit configuration file
**: # Contributing to OpenClaudeThanks for contributing.
OpenClaude is a fast-moving open-source coding-agent CLI with support for multiple providers, local backends, MCP, and a terminal-first workflow. The best contributions here are focused, well-tested, and easy to review.
Before You Start
- Search existing issues and discussions before opening a new thread.
- Check open pull requests for work that overlaps with your contribution. If a PR already exists that addresses the same change, open an issue or discussion first to align on direction — duplicate PRs may be closed without review.
- Use issues for confirmed bugs and actionable feature work.
- Use discussions for setup help, ideas, and general community conversation.
- For larger changes, open an issue first so the scope is clear before implementation.
- For security reports, follow SECURITY.md.
Pull Requests
Every PR needs a reason. Your PR description must include:
- what changed and why
- the user or developer impact
- the exact checks you ran
- a linked issue when one exists, using
Fixes#123, `Closes `#123, or another clear link- screenshots when the PR touches UI, terminal presentation, or the VS Code extension
- which provider path was tested when the PR changes provider behavior
The PR author is responsible for ensuring their PR is merge-ready. PRs with merge conflicts will not be reviewed or approved until the conflicts are resolved.
Issues are the recommended starting point for anything non-trivial — opening one first helps avoid wasted effort if the change is out of scope or already being worked on. Small fixes, doc corrections, and obvious improvements can stand on their own without a linked issue, as long as the PR description explains the intent.
What Gets Closed Without Review
PRs may be closed without review...
Files:
src/utils/status.routes.test.tssrc/utils/status.tsx
🔇 Additional comments (2)
src/utils/status.tsx (1)
25-25: LGTM!Also applies to: 482-487, 492-497, 511-516, 537-542, 572-577, 602-602, 607-607
src/utils/status.routes.test.ts (1)
158-170: LGTM!
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
Thanks for the update. I rechecked the changed paths and found issues that still need to be addressed.
Findings
-
[P2] Complete CodeRabbit's route-secret redaction request for model substrings
src/utils/status.tsx:611
CodeRabbit's earlier request was to include route-specific credentials in redaction for displayed env-derived fields, including the model row. The current patch adds the route credentials toredactionSource, butpushRedactedProperty()only masks values that exactly equal a configured secret or look like standalone secret tokens. IfOPENROUTER_API_KEY=sk-or-SECRET-VALUE-123andOPENAI_MODEL=prefix-sk-or-SECRET-VALUE-123-suffix,/statusstill printsprefix-sk-or-SECRET-VALUE-123-suffixin the Model row. Please complete that review request by redacting configured route-secret substrings in model/status text too, not only in URLs or exact-value matches. -
[P2] Drop URL fragments before showing
OPENAI_API_BASEin/status
src/utils/status.tsx:157
The new status path now falls back toOPENAI_API_BASE, butpushRedactedUrlPropertyonly callsredactUrlForDisplay(), which redacts userinfo and query parameters while preserving the URL fragment. WithOPENAI_API_BASE=https://openrouter.ai/api/v1?api_key=querysecret#access_token=fragsecret,/statusdisplayshttps://openrouter.ai/api/v1?api_key=redacted#access_token=fragsecret, so a copied status block can still expose fragment-carried tokens. Please strip the fragment, or redact token-like fragment contents, before adding the base URL to the status properties and cover the alias case in the tests.
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/urlRedaction.test.ts`:
- Around line 29-35: The current test only covers the happy path where URLs are
properly parsed by the URL parser. The fallback regex path at line 54 in
src/utils/urlRedaction.ts was also modified to handle fragment removal, but it
lacks regression test coverage. Add a second test case in this test function
that uses a malformed URL format (that would fail normal URL parsing) containing
a fragment with sensitive data to verify the fallback regex path in the
redactUrlForDisplay function also properly removes fragments and prevents secret
leaks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 5f827b0f-faf0-4f39-b675-b4390bd11e3c
📒 Files selected for processing (4)
src/utils/status.routes.test.tssrc/utils/status.tsxsrc/utils/urlRedaction.test.tssrc/utils/urlRedaction.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Keep comments useful and concise in code
Files:
src/utils/urlRedaction.test.tssrc/utils/urlRedaction.tssrc/utils/status.routes.test.tssrc/utils/status.tsx
**/*
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
src/utils/urlRedaction.test.tssrc/utils/urlRedaction.tssrc/utils/status.routes.test.tssrc/utils/status.tsx
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
src/utils/urlRedaction.test.tssrc/utils/status.routes.test.ts
**
⚙️ CodeRabbit configuration file
**: # Contributing to OpenClaudeThanks for contributing.
OpenClaude is a fast-moving open-source coding-agent CLI with support for multiple providers, local backends, MCP, and a terminal-first workflow. The best contributions here are focused, well-tested, and easy to review.
Before You Start
- Search existing issues and discussions before opening a new thread.
- Check open pull requests for work that overlaps with your contribution. If a PR already exists that addresses the same change, open an issue or discussion first to align on direction — duplicate PRs may be closed without review.
- Use issues for confirmed bugs and actionable feature work.
- Use discussions for setup help, ideas, and general community conversation.
- For larger changes, open an issue first so the scope is clear before implementation.
- For security reports, follow SECURITY.md.
Pull Requests
Every PR needs a reason. Your PR description must include:
- what changed and why
- the user or developer impact
- the exact checks you ran
- a linked issue when one exists, using
Fixes#123, `Closes `#123, or another clear link- screenshots when the PR touches UI, terminal presentation, or the VS Code extension
- which provider path was tested when the PR changes provider behavior
The PR author is responsible for ensuring their PR is merge-ready. PRs with merge conflicts will not be reviewed or approved until the conflicts are resolved.
Issues are the recommended starting point for anything non-trivial — opening one first helps avoid wasted effort if the change is out of scope or already being worked on. Small fixes, doc corrections, and obvious improvements can stand on their own without a linked issue, as long as the PR description explains the intent.
What Gets Closed Without Review
PRs may be closed without review...
Files:
src/utils/urlRedaction.test.tssrc/utils/urlRedaction.tssrc/utils/status.routes.test.tssrc/utils/status.tsx
🔇 Additional comments (3)
src/utils/status.routes.test.ts (1)
184-196: LGTM!Also applies to: 254-264
src/utils/urlRedaction.ts (1)
45-55: LGTM!src/utils/status.tsx (1)
121-125: LGTM!
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
I found an issue that needs to be addressed before this is ready.
Findings
- [P2] Isolate the mocked provider tests from the real-provider case
src/utils/status.routes.test.ts:299
The new status route tests are order-dependent becausebuildPropertiesWithProvider()mocks./model/providers.js, but the later "real getAPIProvider" test can still observe the previous mocked provider unless another mocked OpenAI test happens to run in between. For example,bun test src/utils/status.routes.test.ts --test-name-pattern "GitHub route|end-to-end"runsGitHub route remains clearand then the real OpenRouter test; the latter fails withProvider routeasundefinedinstead ofOpenRouter. Please make the real-provider test independent of prior mocked imports, or split/reset the mocked module state so focused/filtered runs exercise the same behavior as the full file.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/utils/status.tsx (1)
162-173:⚠️ Potential issue | 🟠 MajorAdd double-encoded secret variants to redaction coverage.
A configured secret like
abc/defappearing double-encoded in an unrecognized query parameter (e.g.,?debug=abc%252Fdef) will leak in the/statusdisplay.URLSearchParamsdecodes one layer, andredactConfiguredSecretUrlQueryValuesonly redacts exact query values; since the param name is not recognized as sensitive, the URL passes through toredactStatusTextForDisplay. The substring redaction then checks only the raw, once-encoded, and lowercase-once-encoded forms—missing the double-encoded variant that actually appears inURL.toString().Extend
redactConfiguredSecretSubstringsto check double-encoded variants (up to depth 2). Also add a test case for a configured secret in a double-encoded unrecognized query parameter to prevent regression.🛡️ Proposed fix
+function getUrlEncodedSecretVariants(secret: string): string[] { + const variants = new Set([secret]); + let encoded = secret; + + for (let depth = 0; depth < 2; depth += 1) { + encoded = encodeURIComponent(encoded); + variants.add(encoded); + variants.add( + encoded.replace(/%[0-9A-F]{2}/g, match => match.toLowerCase()), + ); + } + + return Array.from(variants); +} + function redactConfiguredSecretSubstrings( value: string, secretSource: SecretValueSource, ): string { @@ for (const secret of secrets) { - const encodedSecret = encodeURIComponent(secret); - const encodedSecretWithLowercaseEscapes = encodedSecret.replace( - /%[0-9A-F]{2}/g, - match => match.toLowerCase(), - ); - for (const variant of new Set([ - secret, - encodedSecret, - encodedSecretWithLowercaseEscapes, - ])) { + for (const variant of getUrlEncodedSecretVariants(secret)) { redacted = redacted.split(variant).join('redacted'); } }🤖 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/status.tsx` around lines 162 - 173, The redactConfiguredSecretSubstrings function currently only checks for raw, once-encoded, and lowercase-once-encoded secret variants, missing double-encoded variants that may appear in URL.toString() output (e.g., abc%252Fdef). Extend the variant generation loop to include double-encoded versions by applying URL encoding twice to the original secret and also create a lowercase variant of the double-encoded form. Additionally, add a test case that verifies a configured secret appearing as a double-encoded value in an unrecognized query parameter is properly redacted in the status display output.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/utils/status.tsx`:
- Around line 162-173: The redactConfiguredSecretSubstrings function currently
only checks for raw, once-encoded, and lowercase-once-encoded secret variants,
missing double-encoded variants that may appear in URL.toString() output (e.g.,
abc%252Fdef). Extend the variant generation loop to include double-encoded
versions by applying URL encoding twice to the original secret and also create a
lowercase variant of the double-encoded form. Additionally, add a test case that
verifies a configured secret appearing as a double-encoded value in an
unrecognized query parameter is properly redacted in the status display output.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b4305916-7139-4390-8564-bef705d56641
📒 Files selected for processing (3)
src/utils/status.routes.test.tssrc/utils/status.test.tssrc/utils/status.tsx
📜 Review details
🧰 Additional context used
📓 Path-based instructions (10)
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript with strict mode and ESM imports
Files:
src/utils/status.test.tssrc/utils/status.routes.test.tssrc/utils/status.tsx
{src/services/**/*.ts,src/utils/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
execafor child processes
Files:
src/utils/status.test.tssrc/utils/status.routes.test.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/status.test.tssrc/utils/status.routes.test.tssrc/utils/status.tsx
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add or update tests when the change affects behavior
Files:
src/utils/status.test.tssrc/utils/status.routes.test.ts
**/*.test.{ts,tsx,js}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Test the exact provider/model path you changed when possible
Files:
src/utils/status.test.tssrc/utils/status.routes.test.ts
**/*.{ts,tsx,js,jsx,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Keep comments useful and concise
Files:
src/utils/status.test.tssrc/utils/status.routes.test.tssrc/utils/status.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow TypeScript strict mode and type safety practices by running typecheck before submitting
Files:
src/utils/status.test.tssrc/utils/status.routes.test.tssrc/utils/status.tsx
**/*
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
src/utils/status.test.tssrc/utils/status.routes.test.tssrc/utils/status.tsx
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
src/utils/status.test.tssrc/utils/status.routes.test.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/status.test.tssrc/utils/status.routes.test.tssrc/utils/status.tsx
🔇 Additional comments (3)
src/utils/status.routes.test.ts (1)
229-244: LGTM!src/utils/status.test.ts (1)
156-170: LGTM!src/utils/status.tsx (1)
87-87: LGTM!Also applies to: 130-160, 180-212, 214-249
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
Thanks for the update. I rechecked the changed paths and found one issue that still needs to be addressed.
Findings
- [P2] Complete CodeRabbit's request to cover encoded secret variants
src/utils/status.tsx:163
CodeRabbit's latest review item calls out double-encoded configured secrets, and the current patch still misses that class./statusnow redacts raw, once-encoded, and form-encoded configured credentials, but if a configured route secret appears in a URL representation that survives serialization differently fromencodeURIComponent(secret), it is still printed. For example, withOPENROUTER_API_KEY=abc/defSecret987andOPENAI_API_BASE=https://openrouter.ai/api/v1?debug=abc%252FdefSecret987&timeout=30, the displayedOpenAI base URLremainshttps://openrouter.ai/api/v1?debug=abc%252FdefSecret987&timeout=30; the same happens for other double-encoded values such asBearer%2520abcorlongplussecret%252Bvalue. Please complete that review request by redacting bounded encoded variants or decoded query values, and add regression coverage for these serialized forms.
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
Thanks for the update. I rechecked the changed paths and found one issue that still needs to be addressed.
Findings
- [P2] Complete CodeRabbit's encoded-secret request outside parsed query values
src/utils/status.tsx:163
CodeRabbit's latest review item asked for double-encoded configured secret coverage. The current patch handles that only for values thatredactConfiguredSecretUrlQueryValues()can parse as query parameters, but the shared status substring path still leaks the same class everywhere else. For example, withOPENAI_API_KEY=abc/defSecret987,/statusstill displays the secret whenOPENAI_BASE_URLis eithernot a url?debug=abc%252FdefSecret987&timeout=30orhttps://api.openai.com/abc%252FdefSecret987/v1; likewiseOPENAI_MODEL=gpt-4o abc%252FdefSecret987is printed unchanged in the model row. That happens becauseredactConfiguredSecretSubstrings()only checks the raw and once-encoded variants. Please complete that review request by applying bounded encoded-variant redaction to this shared substring/fallback path, or otherwise decode configured-secret display values before rendering them, and add regression coverage for at least one non-query display path.
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
Thanks for the update. I rechecked the changed paths and found one issue that still needs to be addressed.
Findings
- [P2] Redact encoded configured secrets beyond encodeURIComponent variants
src/utils/status.tsx:160
The shared configured-secret substring path still misses percent-encoded forms thatencodeURIComponent()does not generate. For example, withOPENAI_API_KEY=abc!defSecret987,/statusstill displays the secret whenOPENAI_BASE_URL=https://api.openai.com/abc%21defSecret987/v1orOPENAI_MODEL=gpt-4o abc%21defSecret987, becausegetConfiguredSecretSubstringVariants()adds the rawabc!defSecret987variant but not the valid%21serialization. The same class applies to other characters that can be percent-encoded whileencodeURIComponent()leaves them raw. Please complete the encoded-secret redaction by comparing decoded display segments or by generating the percent-encoded variants for this character class, and add a regression for a non-query path/model value.
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
Thanks for the update. I rechecked the previously discussed paths and do not see any remaining actionable issues from my side.
@kevincodex1 LGTM
…cket (Twigpine#1673) * fix(status): show active provider route instead of legacy bucket The /status command collapsed many concrete providers (OpenRouter, Groq, Ollama, Fireworks AI, etc.) into a single "OpenAI-compatible" label, making multi-provider setups hard to verify and debug. When apiProvider resolves to the generic "openai" bucket, /status now uses route metadata to surface the real active route: Provider route: OpenRouter Transport: OpenAI-compatible API OpenAI base URL: https://openrouter.ai/api/v1 Model: anthropic/claude-sonnet-4.5 Credential: OPENROUTER_API_KEY configured The legacy "OpenAI-compatible" label and fallback are preserved for unknown custom base URLs. Dedicated provider buckets (nvidia-nim, minimax, codex, github, xai, gemini, bedrock, vertex, foundry, firstParty, mistral) already have accurate labels and are left untouched. Credential display uses env-var names only (never values). Transport kind and route label come from the existing descriptor-driven route metadata; no new hardcoded provider maps or network calls are introduced. * fix(status): include route status defaults * fix(status): address route status review findings * fix(status): cover route secret redaction review * fix(status): avoid duplicate route resolution * fix(status): redact base URL query credentials * fix(status): harden status URL secret redaction * fix(status): redact route secrets in status text * test(status): cover fallback URL fragment redaction * test(status): isolate route status provider imports * fix(status): redact encoded route secrets * fix(status): redact encoded query secrets safely * fix(status): redact nested encoded query secrets * fix(status): redact encoded secret substrings * fix(status): redact strict encoded secret variants
3 participants
Summary
/statussurface the active integration route label for OpenAI-compatible route-backed providers such as OpenAI, OpenRouter, Groq, Ollama, and Fireworks AI.Problem
/statusused the legacygetAPIProvider()bucket, so many concrete routes appeared as genericOpenAI-compatible. That made multi-provider setups harder to verify, especially when a route was selected through descriptor metadata or legacy OpenAI-compatible environment variables.Solution
OPENAI_BASE_URL, fallbackOPENAI_API_BASE, or descriptor defaults when appropriate.Provider routes tested
OPENAI_BASE_URLOPENAI_API_BASETests
bun test src/utils/status.routes.test.ts— 13 pass, 0 failbun run test:provider— 753 pass, 0 failbun run typecheck— passedbun run build— passednode bin/openclaude --version—0.18.0 (OpenClaude)Manual verification
Checked status property output with fake env configurations for OpenAI, Ollama, Gemini, GitHub, OpenRouter, OpenRouter through
OPENAI_API_BASE, and Fireworks AI env-only routing.Notes
Open PR searches for route-aware status/status provider route/OpenRouter/Groq/Fireworks/provider label overlap did not find an existing duplicate PR.
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
#fragments.Tests