Skip to content

fix(status): show active provider route instead of legacy provider bucket#1673

Open
chioarub wants to merge 12 commits into
Gitlawb:mainfrom
chioarub:fix/route-aware-status
Open

fix(status): show active provider route instead of legacy provider bucket#1673
chioarub wants to merge 12 commits into
Gitlawb:mainfrom
chioarub:fix/route-aware-status

Conversation

@chioarub

@chioarub chioarub commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Make /status surface the active integration route label for OpenAI-compatible route-backed providers such as OpenAI, OpenRouter, Groq, Ollama, and Fireworks AI.
  • Add route-aware transport, base URL, model, and credential-source summaries without displaying secret values.
  • Preserve existing dedicated provider status labels for Gemini, GitHub, Bedrock, Vertex, Foundry, and other legacy provider buckets.

Problem

/status used the legacy getAPIProvider() bucket, so many concrete routes appeared as generic OpenAI-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

  • Resolve the display route through the integration registry metadata.
  • Use descriptor labels and transport metadata for known concrete routes.
  • Show OPENAI_BASE_URL, fallback OPENAI_API_BASE, or descriptor defaults when appropriate.
  • Show configured credential env var names only, never credential values.
  • Fall back to the existing legacy provider display for unknown/custom routes.

Provider routes tested

  • OpenAI
  • Ollama
  • OpenRouter via OPENAI_BASE_URL
  • OpenRouter via OPENAI_API_BASE
  • Groq
  • Fireworks AI env-only configuration
  • Gemini
  • GitHub
  • Unknown custom OpenAI-compatible route

Tests

  • bun test src/utils/status.routes.test.ts — 13 pass, 0 fail
  • bun run test:provider — 753 pass, 0 fail
  • bun run typecheck — passed
  • bun run build — passed
  • node bin/openclaude --version0.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

    • Route-aware status output for OpenAI-compatible integrations, including route-specific “Transport” labeling and conditional credential summaries when applicable.
  • Bug Fixes

    • More accurate route-specific base URL/model rendering.
    • Improved secret redaction for credential-like values and URL displays, including #fragment removal and substring-aware masking.
  • Tests

    • Added/expanded Bun coverage for deterministic route resolution and credential redaction, plus URL and provider profile redaction scenarios (including GitHub-style tokens).

chioarub added 2 commits June 16, 2026 14:26
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.
@coderabbitai

coderabbitai Bot commented Jun 16, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 6f286d01-4800-4a82-9358-73b2374776d4

📥 Commits

Reviewing files that changed from the base of the PR and between 5c0e256 and d23c595.

📒 Files selected for processing (5)
  • src/utils/providerProfile.test.ts
  • src/utils/providerSecrets.test.ts
  • src/utils/providerSecrets.ts
  • src/utils/status.routes.test.ts
  • src/utils/status.tsx
💤 Files with no reviewable changes (5)
  • src/utils/providerSecrets.test.ts
  • src/utils/providerProfile.test.ts
  • src/utils/providerSecrets.ts
  • src/utils/status.routes.test.ts
  • src/utils/status.tsx

📝 Walkthrough

Walkthrough

Generalizes secret-key detection from a fixed set to pattern-based heuristics (_API_KEY, _TOKEN, _SECRET*, ghu_*). Removes URL fragments during display to prevent credential leakage. Adds route resolution helpers to status.tsx to display provider route labels, transport types, and configured credential env var names with redaction. Updates secret-redaction test coverage and adds comprehensive test coverage for route labeling, credential handling, and secret-value masking across OpenAI, Ollama, OpenRouter, Groq, Fireworks, Gemini, GitHub, and fallback routes.

Changes

Route-aware provider status display

Layer / File(s) Summary
URL fragment redaction
src/utils/urlRedaction.ts, src/utils/urlRedaction.test.ts
redactUrlForDisplay now clears the URL fragment (parsed.hash) during parsing and strips fragments via regex in the fallback path to prevent sensitive data in fragments from leaking into displayed base URLs. Tests verify fragments are dropped while query parameters remain properly redacted.
Generalized secret-key detection mechanism
src/utils/providerSecrets.ts, src/utils/providerSecrets.test.ts
SecretValueSource type widened to arbitrary string keys. New pattern-based secret recognition extends beyond fixed SECRET_ENV_KEYS to include common suffixes (_API_KEY, _TOKEN, _SECRET*) and GitHub prefix (ghu_). collectSecretValues scans source object keys and includes values only when they match known keys or recognized patterns. Tests added for GitHub token redaction and suffix-based pattern matching.
Route-aware status helpers and rendering
src/utils/status.tsx
Adds route-metadata imports and internal helpers to normalize OPENAI_BASE_URL, resolve displayable route id, summarize configured credential env vars, and compute route-aware base URL/model with env overrides and defaults. buildAPIProviderProperties conditionally displays "Provider route" label for legacy openai bucket when route is resolved. OpenAI-compatible rendering gains "Transport" property, route-aware base URL with URL-specific redaction, extended secret redaction sources with route credentials, and conditional "Credential" property. Base URL rendering for Anthropic, Bedrock, Vertex, and Foundry switched to URL-redacting helper.
Route status test infrastructure and comprehensive cases
src/utils/status.routes.test.ts, src/utils/providerProfile.test.ts
New status.routes.test.ts establishes deterministic env management with snapshot/restore utilities and shared mutation lock. Comprehensive test coverage for route label/transport/model display across OpenAI, Ollama, OpenRouter, Groq, Fireworks, Gemini, GitHub; base URL redaction with credential-like query parameters and fragments; secret-redaction guarantees preventing credential leakage; end-to-end real provider resolution; conditional credential inclusion; and route-label omission for local routes without configured secrets. Extended providerProfile.test.ts with GROQ API key masking and non-secret value handling.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • Gitlawb/openclaude#1665: Centralized provider secret env keys and route-aware secretSource construction in status.tsx align directly with this PR's generalized secret-key detection and status rendering redaction wiring.
  • Gitlawb/openclaude#1672: URL/base-URL handling changes including fragment and secret redaction updates in status.tsx overlap with this PR's URL fragment stripping and enhanced status-redaction pipeline.

Suggested labels

bug

Suggested reviewers

  • jatmn
  • kevincodex1
🚥 Pre-merge checks | ✅ 6 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 30.43% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (6 passed)
Check name Status Explanation
Title check ✅ Passed Title accurately describes the main change: showing active provider route instead of legacy provider bucket in status output.
Description check ✅ Passed Description includes summary of changes, problem statement, solution approach, tested providers, test results, and manual verification covering all required template sections.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Risk Surface Disclosed ✅ Passed PR addresses auth/provider routing risk surfaces with explicit safeguards: two-layer secret redaction, credential env var names-only display, URL parameter redaction with fragment stripping, and de...
No Hidden Policy Change ✅ Passed PR changes are isolated to status display (status.tsx), secret redaction (providerSecrets.ts), and URL redaction (urlRedaction.ts)—no routing defaults, credential selection, telemetry, or permissio...

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 win

Include route-specific credentials in the redaction source.

The new credential summary knows route credentials via getRouteCredentialEnvVars, but base URL/model redaction still uses secretSource, which omits route-specific secrets such as OPENROUTER_API_KEY, GROQ_API_KEY, and FIREWORKS_API_KEY. If one of those secret values appears in a displayed env-derived field, /status will not redact it. src/integrations/routeMetadata.ts Lines 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

📥 Commits

Reviewing files that changed from the base of the PR and between 7be9dce and 9c2642e.

📒 Files selected for processing (2)
  • src/utils/status.routes.test.ts
  • src/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.tsx
  • src/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.tsx
  • src/utils/status.routes.test.ts
**

⚙️ CodeRabbit configuration file

**: # Contributing to OpenClaude

Thanks 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.tsx
  • src/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

Comment thread src/utils/status.tsx
@chioarub chioarub marked this pull request as ready for review June 16, 2026 12:32
coderabbitai[bot]
coderabbitai Bot previously approved these changes Jun 16, 2026
coderabbitai[bot]
coderabbitai Bot previously approved these changes Jun 16, 2026

@jatmn jatmn left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 to OPENAI_API_BASE for the displayed OpenAI-compatible base URL, but that value is still passed through the generic pushRedactedProperty path, which only masks whole values that exactly match a known secret. A legacy base URL such as https://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 the OPENAI_API_BASE alias case.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 win

Clear 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

📥 Commits

Reviewing files that changed from the base of the PR and between 0031b31 and dff893a.

📒 Files selected for processing (2)
  • src/utils/status.routes.test.ts
  • src/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.ts
  • src/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.ts
  • src/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 OpenClaude

Thanks 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.ts
  • src/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!

Comment thread src/utils/status.tsx
coderabbitai[bot]
coderabbitai Bot previously approved these changes Jun 16, 2026
@chioarub chioarub requested a review from jatmn June 16, 2026 20:12

@jatmn jatmn left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 to redactionSource, but pushRedactedProperty() only masks values that exactly equal a configured secret or look like standalone secret tokens. If OPENROUTER_API_KEY=sk-or-SECRET-VALUE-123 and OPENAI_MODEL=prefix-sk-or-SECRET-VALUE-123-suffix, /status still prints prefix-sk-or-SECRET-VALUE-123-suffix in 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_BASE in /status
    src/utils/status.tsx:157
    The new status path now falls back to OPENAI_API_BASE, but pushRedactedUrlProperty only calls redactUrlForDisplay(), which redacts userinfo and query parameters while preserving the URL fragment. With OPENAI_API_BASE=https://openrouter.ai/api/v1?api_key=querysecret#access_token=fragsecret, /status displays https://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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between cd12cef and b39a0b6.

📒 Files selected for processing (4)
  • src/utils/status.routes.test.ts
  • src/utils/status.tsx
  • src/utils/urlRedaction.test.ts
  • src/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.ts
  • src/utils/urlRedaction.ts
  • src/utils/status.routes.test.ts
  • src/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.ts
  • src/utils/urlRedaction.ts
  • src/utils/status.routes.test.ts
  • src/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.ts
  • src/utils/status.routes.test.ts
**

⚙️ CodeRabbit configuration file

**: # Contributing to OpenClaude

Thanks 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.ts
  • src/utils/urlRedaction.ts
  • src/utils/status.routes.test.ts
  • src/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!

Comment thread src/utils/urlRedaction.test.ts
coderabbitai[bot]
coderabbitai Bot previously approved these changes Jun 16, 2026
@chioarub chioarub requested a review from jatmn June 16, 2026 23:32

@jatmn jatmn left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 because buildPropertiesWithProvider() 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" runs GitHub route remains clear and then the real OpenRouter test; the latter fails with Provider route as undefined instead of OpenRouter. 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.

jatmn
jatmn previously approved these changes Jun 17, 2026

@jatmn jatmn left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution. I do not see any actionable issues from my review.

@kevincodex1 LGTM

@kevincodex1

Copy link
Copy Markdown
Contributor

please rebase and fix conflicts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants