Skip to content

Instrument coderouter usage and failures - #9725

Merged
lawrencecchen merged 1 commit into
mainfrom
feat/coderouter-observability
Aug 6, 2026
Merged

lawrencecchen merged 1 commit into
mainfrom
feat/coderouter-observability

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Aug 6, 2026 •

Copy link
Copy Markdown
Contributor

Adds privacy-bounded, non-blocking PostHog events for coderouter account/session/model lifecycles; per-user/team streaming token totals and subscription cost basis; Sentry lifecycle breadcrumbs with stronger scrubbing; actionable API errors; and bounded safe-read retries.

Privacy:

  • no prompt/output, email, provider account ID, credential, token, header, or body properties
  • streaming response remains pass-through with only a bounded rolling tail used to extract aggregate usage
  • PostHog delivery uses post-response work, a 2s timeout, and one deduplicated retry

Validation:

  • bun run typecheck
  • bun test tests/coderouter*.test.ts (43 pass)

View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Instruments coderouter requests, accounts, and sessions with privacy-safe analytics and Sentry breadcrumbs, and extracts streaming token usage without retaining prompts or outputs. Improves reliability and clarity with safer retries/timeouts and actionable API errors across Codex and OpenCode routing.

  • New Features

    • Added PostHog events for account add/remove/status, session issue/revoke, auth rejects, and per-request model completion (grouped by coderouter_team).
    • Extracted usage via observeModelUsage for Codex and OpenCode (model + token counts only; bodies remain pass-through).
    • Returned clearer API errors with human messages, retryable hints, and retry-after where applicable.
  • Refactors

    • Introduced addCoderouterBreadcrumb and expanded breadcrumbs across auth, routing, refresh leasing, retries, requests, and analytics delivery with strict scrubbing.
    • Hardened provider interactions: transport failure handling, safe-read retry with breadcrumbs, 5–10s timeouts, and cooldowns on 429.
    • Improved OpenCode proxy: safe provider URL checks, nested model provider rewrite without secrets, and resilient config fetch.
    • Centralized analytics capture with bounded, deferred delivery and one deduplicated retry.
    • Extended route token auth and request context to include stackUserId.

Written for commit 5473136. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • New Features

    • Added model and token usage tracking for routed requests.
    • Added clearer request status details, including retry guidance for temporary failures.
    • Improved provider routing for nested model APIs.
  • Bug Fixes

    • Added timeouts and retry handling for account, provider, credential, and session operations.
    • Improved account and session error responses.
    • Strengthened protection of sensitive request, response, credential, and prompt data.
  • Observability

    • Added richer activity and request diagnostics for account, session, provider, and routing operations.

@coderabbitai

coderabbitai Bot commented Aug 6, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

CodeRouter routes and services now add analytics, Sentry breadcrumbs, structured retryable errors, request timeouts, route-token identities, and streamed model-usage telemetry. Tests cover privacy filtering, deferred delivery, usage extraction, nested secret removal, and updated authentication fixtures.

Changes

CodeRouter observability and reliability

Layer / File(s) Summary
Telemetry, privacy, and usage contracts
web/services/coderouter/analytics.ts, web/services/coderouter/observability.ts, web/services/coderouter/responseUsage.ts, web/services/sentry.ts, web/tests/coderouter-analytics.test.ts, web/tests/coderouter-sentry.test.ts
PostHog delivery, sensitive-property filtering, Sentry breadcrumbs, Sentry scrubbing, and streamed model-usage extraction were added with tests.
Account and session route handling
web/app/api/coderouter/accounts/..., web/app/api/coderouter/session/route.ts, web/services/coderouter/repository.ts, web/services/coderouter/requestContext.ts, web/tests/coderouter-models-proxy.test.ts, web/tests/coderouter-session-route.test.ts
Account and session operations now emit telemetry, return structured retry metadata, handle storage and token failures, and propagate stackUserId.
Codex proxy instrumentation
web/services/coderouter/codexProxy.ts
Codex requests now report authentication, routing, retries, upstream outcomes, model usage, request completion data, and timeout-controlled model requests.
OpenCode proxy routing
web/services/coderouter/opencodeProxy.ts, web/tests/coderouter-opencode-proxy.test.ts
OpenCode requests now use structured errors, transport reporting, usage observation, stack-user route identity, and nested provider secret removal.
Credential refresh and usage lifecycle
web/services/coderouter/providerFetch.ts, web/services/coderouter/refresh.ts, web/services/coderouter/usage.ts
Provider retries, credential refresh leases, refresh requests, and usage loading now emit breadcrumbs and use refresh timeouts.

Estimated code review effort: 5 (Critical) | ~90 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant codexProxy
  participant providerFetch
  participant UpstreamProvider
  participant captureCoderouterEvent
  participant addCoderouterBreadcrumb
  Client->>codexProxy: Codex request
  codexProxy->>providerFetch: Select account and fetch credentials
  providerFetch->>UpstreamProvider: Send models request
  UpstreamProvider-->>codexProxy: Stream response or failure
  codexProxy->>captureCoderouterEvent: Record completion metrics and usage
  codexProxy->>addCoderouterBreadcrumb: Record request outcome
  codexProxy-->>Client: Return streamed response or structured error
Loading

Possibly related PRs


Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (2 errors, 1 warning)

Check name Status Explanation Resolution
Cmux User-Facing Error Privacy ❌ Error New API error copy exposes upstream names and provider internals: Codex/OpenCode subscriptions, OpenCode configuration, provider list, and provider endpoint. Use generic cmux/coderouter error text and actions. Remove vendor names and provider/configuration details from API messages; keep them in sanitized telemetry.
Cmux Full Internationalization ❌ Error Added English API response messages in coderouter routes and Codex/OpenCode proxies bypass locale resolution; exact messages are absent from all 20 web/messages locale files. Use locale-backed next-intl/API messages and add translated entries in web/messages for every locale in web/i18n/routing.ts: en, ja, zh-CN, zh-TW, ko, de, es, fr, it, da, pl, ru, bs, ar, no, pt-BR, th, tr, km, uk.
Docstring Coverage ⚠️ Warning Docstring coverage is 11.54% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (22 passed)
Check name Status Explanation
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.
Cmux Swift Actor Isolation ✅ Passed The patch changes 19 TypeScript files only; the verified HEAD^..HEAD diff contains no .swift files, so the Swift actor-isolation check is not applicable.
Cmux Swift Blocking Runtime ✅ Passed PASS: The commit changes only 19 web TypeScript files; it contains no Swift, Package.swift, Xcode project, or other Swift runtime changes to assess.
Cmux Browser Automation Off-Main ✅ Passed The PR changes only web coderouter/Sentry files; neither browser automation source file changed, and the patch contains no browser socket or WebKit routing edits.
Cmux Expensive Synchronous Load ✅ Passed The pull request changes only web TypeScript files; it adds no Swift changes or Swift interactive-path loads covered by this check.
Cmux Cache Substitution Correctness ✅ Passed The diff does not replace a fresh persistence/history/undo/snapshot read; its only cache-like map coalesces in-flight usage reads, and the new stream tail is bounded and non-persisted.
Cmux No Hacky Sleeps ✅ Passed The PR adds no sleep, setTimeout, setInterval, polling, or wall-clock delay; new AbortSignal.timeout calls are bounded cancellation-aware network timeouts, and retries are immediate and tested.
Cmux Algorithmic Complexity ✅ Passed Changed production code uses linear scans or Map lookups, fixed-size retry/list loops, and provider/model traversal linear in total entries; streaming usage retains an explicit 256 KiB tail. No new...
Cmux Swift Concurrency ✅ Passed The complete PR diff changes 19 web files and contains zero Swift, Objective-C, or Xcode project changes; no cmux-owned Swift async patterns were introduced.
Cmux Swift @Concurrent ✅ Passed The pull request commit changes 19 web TypeScript files and no Swift files, so the Swift @concurrent check is not applicable.
Cmux Swift Package Boundaries ✅ Passed The PR changes 19 files, all under web/, with zero .swift paths or Swift package manifests; the Swift package-boundaries rule is not applicable.
Cmux Swiftpm Lockfiles ✅ Passed The PR diff changes only web coderouter and test files; it includes no Package.swift, Package.resolved, Xcode project, .gitignore, workflow, or dependency changes covered by this rule.
Cmux Swift Logging ✅ Passed The complete PR diff changes only 19 web TypeScript files; it adds no Swift files or Swift logging statements, so the Swift logging rule is not applicable.
Cmux Swiftui State Layout ✅ Passed The PR commit changes only web TypeScript and test files; it contains no Swift or SwiftUI paths, state, or layout changes.
Cmux Architecture Rethink ✅ Passed The diff changes only web TypeScript and tests; it contains no changed Swift paths, so the Swift architectural-rethink rule is not applicable.
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed The diff changes only web TypeScript files; it adds or materially changes no Swift window code, so the auxiliary-window close-shortcut rule is not applicable.
Cmux Source Artifacts ✅ Passed All 19 changed paths are TypeScript source or tests under web/app, web/services, and web/tests; no artifact directories, binaries, logs, caches, or build outputs appear in the diff.
Cmux No Test Or Debug Seam In Production Source ✅ Passed The PR changes only web TypeScript and web test files; it adds no Swift file or production Sources/ change covered by this check.
Cmux No Ambient Global State ✅ Passed The PR diff contains 19 TypeScript files and no Swift paths or Swift changes; the rule explicitly applies to production Swift code.
Title check ✅ Passed The title clearly summarizes the primary instrumentation and failure-handling changes in the pull request.
Description check ✅ Passed The description provides a detailed summary, privacy scope, and test validation, but omits several template sections.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/coderouter-observability

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.

@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: 5

🤖 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 `@web/app/api/coderouter/accounts/`[accountId]/route.ts:
- Around line 40-61: Replace all hard-coded client-visible coderouter messages
with keys from the locale-specific API error source, and add matching
translations for every supported locale. In
web/app/api/coderouter/accounts/[accountId]/route.ts:40-61 update both removal
responses; in web/app/api/coderouter/accounts/route.ts:117-131 update the
storage response; and in web/app/api/coderouter/session/route.ts:50-56, 77-83,
96-106, and 119-130 update each session response. In
web/services/coderouter/codexProxy.ts:32-38, 48-54, 163-169, and 341-345, and
web/services/coderouter/opencodeProxy.ts:39-87 and 117-122, localize every
public guidance/error message and require an explicit localized message rather
than falling back to the internal error code. Use next-intl or the established
locale API and preserve internal error codes separately from localized message
content.

In `@web/services/coderouter/codexProxy.ts`:
- Around line 244-255: Handle rejection from fetchProviderRead within the
account loop around the upstream fetch, including timeout failures after
retries; report the error through the existing route logging mechanism and
continue evaluating the next account instead of allowing the rejection to escape
as an unstructured route failure.

In `@web/services/coderouter/opencodeProxy.ts`:
- Around line 230-240: Update rewriteProviders so the nested provider object is
constructed only from an explicit allowlist of public fields, excluding apiKey,
token, access-token, refresh-token, headers, and other credential data. Preserve
the rewritten proxy api URL, but remove the spread of nestedProvider so raw
remote provider records cannot reach openCodeClientConfig.

In `@web/services/coderouter/providerFetch.ts`:
- Around line 11-16: Limit fetchProviderRead to at most two provider requests by
ensuring the catch around the initial request does not invoke request() after
the retry has already been attempted. Track or scope the attempt state across
the try/catch, and keep retry breadcrumbs reporting the correct attempt number.

In `@web/services/sentry.ts`:
- Line 4: Update SECRET_KEY in web/services/sentry.ts:4 to match both
providerAccountId and provider_account_id. In
web/tests/coderouter-sentry.test.ts:53-57, add provider_account_id to the event
fixture; in web/tests/coderouter-sentry.test.ts:75-85, assert it is redacted as
"[Filtered]".
🪄 Autofix

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: 5aed2138-941f-4f81-a5f7-f6dd8bde571e

📥 Commits

Reviewing files that changed from the base of the PR and between fe18ae9 and 9e2b324.

📒 Files selected for processing (18)
  • web/app/api/coderouter/accounts/[accountId]/route.ts
  • web/app/api/coderouter/accounts/route.ts
  • web/app/api/coderouter/session/route.ts
  • web/services/coderouter/analytics.ts
  • web/services/coderouter/codexProxy.ts
  • web/services/coderouter/observability.ts
  • web/services/coderouter/opencodeProxy.ts
  • web/services/coderouter/providerFetch.ts
  • web/services/coderouter/refresh.ts
  • web/services/coderouter/repository.ts
  • web/services/coderouter/requestContext.ts
  • web/services/coderouter/responseUsage.ts
  • web/services/coderouter/usage.ts
  • web/services/sentry.ts
  • web/tests/coderouter-analytics.test.ts
  • web/tests/coderouter-models-proxy.test.ts
  • web/tests/coderouter-sentry.test.ts
  • web/tests/coderouter-session-route.test.ts

Comment thread web/app/api/coderouter/accounts/[accountId]/route.ts
Comment thread web/services/coderouter/codexProxy.ts Outdated
Comment thread web/services/coderouter/opencodeProxy.ts
Comment thread web/services/coderouter/providerFetch.ts Outdated
Comment thread web/services/sentry.ts Outdated
@lawrencecchen
lawrencecchen force-pushed the feat/coderouter-observability branch from 9e2b324 to 5473136 Compare August 6, 2026 22:54
@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

@lawrencecchen
lawrencecchen merged commit b9a8e20 into main Aug 6, 2026
5 of 6 checks passed
@lawrencecchen
lawrencecchen deleted the feat/coderouter-observability branch August 6, 2026 23:00

@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: 11

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
web/tests/coderouter-opencode-proxy.test.ts (1)

12-34: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

Assert the nested apiKey value too.

"nested-secret" is not a substring of "nested-upstream-secret", so line 34 only proves the nested headers.authorization value was removed. If publicNestedProvider regressed and forwarded apiKey, this test would still pass. Assert both fixture values, and add a top-level credential field to cover the provider object itself.

💚 Proposed fix
-    expect(JSON.stringify(rewritten)).not.toContain("nested-secret");
+    const serialized = JSON.stringify(rewritten);
+    expect(serialized).not.toContain("nested-secret");
+    expect(serialized).not.toContain("nested-upstream-secret");
🤖 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 `@web/tests/coderouter-opencode-proxy.test.ts` around lines 12 - 34, Update the
test around publicNestedProvider to assert that both nested credential fixtures,
“nested-upstream-secret” and “nested-secret”, are absent from the rewritten
result, and add a top-level credential field to the provider fixture so the
provider object itself is covered.
🤖 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 `@web/services/coderouter/analytics.ts`:
- Around line 122-141: Update safeProperties to use an explicit allowlist of the
approved analytics property names shared by both proxies, including provider,
agent, outcome, status, retry/count fields, duration, model, token counts, cost
fields, path, and reason. Remove the SENSITIVE_PROPERTY check and safeTokenCount
exception, and only copy scalar values whose keys are on that allowlist.

In `@web/services/coderouter/codexProxy.ts`:
- Around line 410-416: Update agentFromUserAgent to match pi only as a delimited
product token rather than any substring, and evaluate the opencode token before
pi so it is classified correctly when both text patterns are present. Preserve
codex and other-agent classification behavior.

In `@web/services/coderouter/observability.ts`:
- Around line 24-33: Update reportCoderouterFailure() to use one
import("`@sentry/nextjs`") promise for both scope context and exception capture:
call scope.setContext("cmux", context) and Sentry.captureException(error)
together, and remove the separate Sentry.addBreadcrumb() flow before
reportError().

In `@web/services/coderouter/opencodeProxy.ts`:
- Around line 124-149: Extract a shared exported completion-event builder from
the duplicated payload in the opencode proxy, accepting provider, identity,
status, duration, and usage while preserving the existing schema and outcome
fields. Reuse this helper from both the opencode completion callback and
codexProxy’s captureModelRequest path, removing their provider-specific payload
duplication.
- Around line 39-54: Replace the hardcoded user-facing messages in
opencodeProxy.ts#L39-L54 for unauthorized, no_usable_account,
provider_unavailable, unknown_provider, and invalid_provider with per-request
locale lookups from the established next-intl or locale-specific message source,
adding every key to all catalogs under web/messages/ for locales in
web/i18n/routing.ts. Apply the same catalog-based resolution to the unauthorized
and no_usable_account jsonError messages in codexProxy.ts#L163-L169, preserving
the existing error codes and response behavior.
- Around line 13-29: Update openCodeClientConfig to use the same apiError
structured error contract as proxyOpenCodeRequest for unauthorized and
unavailable-account responses, and wrap remoteConfig in error handling that maps
provider failures and timeouts to the structured retryable 502 response.
Preserve the successful provider response and no-store cache header.
- Around line 245-260: The provider rewrite in rewriteProviders must not spread
the raw value into each openCodeClientConfig entry. Construct the top-level
provider object from an allowlist of safe fields required by OpenCode clients,
retain the conditional npm and models fields as applicable, and continue
removing the upstream api field while setting the proxy baseURL and routeToken
apiKey in options.

In `@web/services/coderouter/refresh.ts`:
- Line 216: Update the provider timeout used by postForm/postJson in
refreshProviderCredential to exceed the full credential refresh duration,
including provider work, encryptCredential, and completeRefreshLease; align
REFRESH_LEASE_MS so it covers that same end-to-end path and prevents another
client from claiming the lease while completion is still possible.

In `@web/services/coderouter/requestContext.ts`:
- Around line 34-42: Update the request authentication flow around
authenticateRouteToken so any supplied crt_ bearer token that fails validation
immediately returns the normal unauthorized response, without calling
resolveCodeRouterRequestContext or falling back to cookies/native
authentication. Add a regression test covering an invalid crt_ bearer alongside
valid ambient authentication and assert the request remains unauthorized.

In `@web/services/coderouter/responseUsage.ts`:
- Around line 36-39: Add a cancel handler alongside flush in the TransformStream
transformer so cancelled streams also invoke onComplete with usageFromTail(tail,
model). Preserve the existing tail decoding and completion behavior, and ensure
both normal completion and client-disconnect paths report usage outcomes.
- Around line 24-39: Update the tail handling in the response transform around
tail, transform, and flush to store recent decoded chunk strings in a bounded
queue instead of repeatedly concatenating and slicing the entire retained tail.
Enforce the MAX_TAIL_CHARS limit while retaining the newest content, then join
the queued chunks once during flush before calling usageFromTail; preserve model
extraction and decoder flushing behavior.

---

Outside diff comments:
In `@web/tests/coderouter-opencode-proxy.test.ts`:
- Around line 12-34: Update the test around publicNestedProvider to assert that
both nested credential fixtures, “nested-upstream-secret” and “nested-secret”,
are absent from the rewritten result, and add a top-level credential field to
the provider fixture so the provider object itself is covered.
🪄 Autofix

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: ff9ec259-e6a5-4b0c-a72d-c7a39357ae42

📥 Commits

Reviewing files that changed from the base of the PR and between fe18ae9 and 5473136.

📒 Files selected for processing (19)
  • web/app/api/coderouter/accounts/[accountId]/route.ts
  • web/app/api/coderouter/accounts/route.ts
  • web/app/api/coderouter/session/route.ts
  • web/services/coderouter/analytics.ts
  • web/services/coderouter/codexProxy.ts
  • web/services/coderouter/observability.ts
  • web/services/coderouter/opencodeProxy.ts
  • web/services/coderouter/providerFetch.ts
  • web/services/coderouter/refresh.ts
  • web/services/coderouter/repository.ts
  • web/services/coderouter/requestContext.ts
  • web/services/coderouter/responseUsage.ts
  • web/services/coderouter/usage.ts
  • web/services/sentry.ts
  • web/tests/coderouter-analytics.test.ts
  • web/tests/coderouter-models-proxy.test.ts
  • web/tests/coderouter-opencode-proxy.test.ts
  • web/tests/coderouter-sentry.test.ts
  • web/tests/coderouter-session-route.test.ts

Comment on lines +122 to +141
function safeProperties(
input: Readonly<Record<string, AnalyticsScalar | null | undefined>>,
): Record<string, AnalyticsScalar | { coderouter_team: string }> {
const output: Record<string, AnalyticsScalar | { coderouter_team: string }> =
{};
for (const [key, value] of Object.entries(input)) {
const safeTokenCount = /^(?:input|cached_input|output|total)_tokens$/.test(
key,
);
if (SENSITIVE_PROPERTY.test(key) && !safeTokenCount) continue;
if (
typeof value === "string" ||
typeof value === "number" ||
typeof value === "boolean"
) {
output[key] = value;
}
}
return output;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Prefer an allowlist over the sensitive-name denylist.

safeProperties drops properties by name pattern. This is fail-open: any future property whose name does not match SENSITIVE_PROPERTY reaches PostHog, even if it carries user content. The event property surface is small and fixed across both proxies (provider, agent, outcome, status, attempt_count, refresh_retry_count, duration_ms, model, token counts, actual_cost_usd, cost_basis, path, reason). An allowlist fails closed and removes the need for the safeTokenCount exception.

🛡️ Proposed allowlist
+const ALLOWED_PROPERTY = new Set([
+  "agent",
+  "attempt_count",
+  "actual_cost_usd",
+  "cached_input_tokens",
+  "cost_basis",
+  "duration_ms",
+  "input_tokens",
+  "model",
+  "outcome",
+  "output_tokens",
+  "path",
+  "provider",
+  "reason",
+  "refresh_retry_count",
+  "status",
+  "total_tokens",
+]);
+
 function safeProperties(
   input: Readonly<Record<string, AnalyticsScalar | null | undefined>>,
 ): Record<string, AnalyticsScalar | { coderouter_team: string }> {
   const output: Record<string, AnalyticsScalar | { coderouter_team: string }> =
     {};
   for (const [key, value] of Object.entries(input)) {
-    const safeTokenCount = /^(?:input|cached_input|output|total)_tokens$/.test(
-      key,
-    );
-    if (SENSITIVE_PROPERTY.test(key) && !safeTokenCount) continue;
+    if (!ALLOWED_PROPERTY.has(key)) continue;
     if (
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
function safeProperties(
input: Readonly<Record<string, AnalyticsScalar | null | undefined>>,
): Record<string, AnalyticsScalar | { coderouter_team: string }> {
const output: Record<string, AnalyticsScalar | { coderouter_team: string }> =
{};
for (const [key, value] of Object.entries(input)) {
const safeTokenCount = /^(?:input|cached_input|output|total)_tokens$/.test(
key,
);
if (SENSITIVE_PROPERTY.test(key) && !safeTokenCount) continue;
if (
typeof value === "string" ||
typeof value === "number" ||
typeof value === "boolean"
) {
output[key] = value;
}
}
return output;
}
const ALLOWED_PROPERTY = new Set([
"agent",
"attempt_count",
"actual_cost_usd",
"cached_input_tokens",
"cost_basis",
"duration_ms",
"input_tokens",
"model",
"outcome",
"output_tokens",
"path",
"provider",
"reason",
"refresh_retry_count",
"status",
"total_tokens",
]);
function safeProperties(
input: Readonly<Record<string, AnalyticsScalar | null | undefined>>,
): Record<string, AnalyticsScalar | { coderouter_team: string }> {
const output: Record<string, AnalyticsScalar | { coderouter_team: string }> =
{};
for (const [key, value] of Object.entries(input)) {
if (!ALLOWED_PROPERTY.has(key)) continue;
if (
typeof value === "string" ||
typeof value === "number" ||
typeof value === "boolean"
) {
output[key] = value;
}
}
return output;
}
🤖 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 `@web/services/coderouter/analytics.ts` around lines 122 - 141, Update
safeProperties to use an explicit allowlist of the approved analytics property
names shared by both proxies, including provider, agent, outcome, status,
retry/count fields, duration, model, token counts, cost fields, path, and
reason. Remove the SENSITIVE_PROPERTY check and safeTokenCount exception, and
only copy scalar values whose keys are on that allowlist.

Comment on lines +410 to +416
function agentFromUserAgent(value: string | null): string {
const normalized = value?.toLowerCase() ?? "";
if (normalized.includes("codex")) return "codex";
if (normalized.includes("pi")) return "pi";
if (normalized.includes("opencode")) return "opencode";
return "other";
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Fix the pi substring match.

normalized.includes("pi") matches any user agent that contains api, rapid, or spider. Those requests are labelled pi, and the opencode branch never runs for a user agent that also contains api. The agent dimension on coderouter_model_request_completed becomes wrong for most clients. Match a delimited product token instead, and test opencode before the shorter token.

🐛 Proposed fix
 function agentFromUserAgent(value: string | null): string {
   const normalized = value?.toLowerCase() ?? "";
   if (normalized.includes("codex")) return "codex";
-  if (normalized.includes("pi")) return "pi";
   if (normalized.includes("opencode")) return "opencode";
+  if (/(?:^|[\s/(])pi(?:[\s/)]|$)/.test(normalized)) return "pi";
   return "other";
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
function agentFromUserAgent(value: string | null): string {
const normalized = value?.toLowerCase() ?? "";
if (normalized.includes("codex")) return "codex";
if (normalized.includes("pi")) return "pi";
if (normalized.includes("opencode")) return "opencode";
return "other";
}
function agentFromUserAgent(value: string | null): string {
const normalized = value?.toLowerCase() ?? "";
if (normalized.includes("codex")) return "codex";
if (normalized.includes("opencode")) return "opencode";
if (/(?:^|[\s/(])pi(?:[\s/)]|$)/.test(normalized)) return "pi";
return "other";
}
🤖 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 `@web/services/coderouter/codexProxy.ts` around lines 410 - 416, Update
agentFromUserAgent to match pi only as a delimited product token rather than any
substring, and evaluate the opencode token before pi so it is classified
correctly when both text patterns are present. Preserve codex and other-agent
classification behavior.

Comment on lines +24 to +33
void import("@sentry/nextjs")
.then((Sentry) => {
Sentry.addBreadcrumb({
category: `coderouter.${category}`,
message,
level,
data: safeData,
});
})
.catch(() => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

ast-grep outline web/services/sentry.ts --items all --match 'reportError|scrubSentryEvent'
rg -n -C 10 'function reportError|const reportError|reportError\s*=' web/services web/app
rg -n -C 8 'reportCoderouterFailure|addCoderouterBreadcrumb' web/tests web/services/coderouter

Repository: manaflow-ai/cmux

Length of output: 40078


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

echo "== web/services/observability/report.ts =="
cat -n web/services/observability/report.ts

echo
echo "== web/services/**/*.ts tracked imports/usages of Sentry/reportError =="
rg -n -C 12 'reportError\(|addBreadcrumb|import.*Sentry|from ["'\''].+-sentry|`@sentry/nextjs`|`@sentry/nextjs`' web/services web/app \
  --glob '*.ts' --glob '*.tsx' | sed -n '1,240p'

echo
echo "== package Sentry Next versions =="
rg -n '"`@sentry/`(nextjs|react|node|browser)"' package.json pnpm-lock.yaml package-lock.json yarn.lock bun.lockb . 2>/dev/null || true

Repository: manaflow-ai/cmux

Length of output: 15759


Wire failure breadcrumbs into the same SDK path used by reportError().

reportCoderouterFailure() queues Sentry.addBreadcrumb() and then queues reportError(), but both reports live behind unawaited import("@sentry/nextjs") promises and do not share scope state. Capture the failure with scope.setContext("cmux", context) + Sentry.captureException(error) in one Sentry promise instead of a separate breadcrumb before reportError(); otherwise the failure breadcrumb may not belong to the captured event.

🤖 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 `@web/services/coderouter/observability.ts` around lines 24 - 33, Update
reportCoderouterFailure() to use one import("`@sentry/nextjs`") promise for both
scope context and exception capture: call scope.setContext("cmux", context) and
Sentry.captureException(error) together, and remove the separate
Sentry.addBreadcrumb() flow before reportError().

Comment on lines +13 to 29
export async function openCodeClientConfig(
request: Request,
): Promise<Response> {
const auth = await routeIdentity(request);
if (!auth) return Response.json({ error: "unauthorized" }, { status: 401 });
const resolved = await openCodeAccount(auth.teamId);
if (!resolved) return Response.json({ error: "no_usable_account" }, { status: 503 });
if (!resolved)
return Response.json({ error: "no_usable_account" }, { status: 503 });
const remote = await remoteConfig(resolved.credential.accessToken);
const provider = rewriteProviders(remote, auth.token);
return Response.json({ provider }, {
headers: { "cache-control": "no-store" },
});
return Response.json(
{ provider },
{
headers: { "cache-control": "no-store" },
},
);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Use the structured error path in openCodeClientConfig, and handle remoteConfig failures.

This entrypoint returns bare { error } bodies while proxyOpenCodeRequest returns { error, message, retryable } through apiError. Clients therefore see two error contracts from the same feature. Line 21 also calls remoteConfig with no try/catch, so a provider outage or the 5-second timeout produces an unhandled rejection and a framework 500 instead of the structured 502 that proxyOpenCodeRequest returns.

🩺 Proposed fix
   const auth = await routeIdentity(request);
-  if (!auth) return Response.json({ error: "unauthorized" }, { status: 401 });
+  if (!auth) {
+    return apiError(
+      "unauthorized",
+      "Your coderouter session expired or was revoked. Run `cr login` and retry.",
+      401,
+      false,
+    );
+  }
   const resolved = await openCodeAccount(auth.teamId);
-  if (!resolved)
-    return Response.json({ error: "no_usable_account" }, { status: 503 });
-  const remote = await remoteConfig(resolved.credential.accessToken);
+  if (!resolved) {
+    return apiError(
+      "no_usable_account",
+      "No healthy OpenCode subscription is available. Check `cr`, add an account with `cr add`, or retry shortly.",
+      503,
+      true,
+    );
+  }
+  let remote: Record<string, unknown>;
+  try {
+    remote = await remoteConfig(resolved.credential.accessToken);
+  } catch (error) {
+    reportCoderouterFailure("provider_usage", error, {
+      provider: "opencode-go",
+      operation: "config",
+    });
+    return apiError(
+      "provider_unavailable",
+      "OpenCode configuration is temporarily unavailable. Retry shortly.",
+      502,
+      true,
+    );
+  }
   const provider = rewriteProviders(remote, auth.token);

As per coding guidelines: "When behavior has multiple entrypoints, implement one shared action path and verify every entrypoint rather than duplicating logic."

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export async function openCodeClientConfig(
request: Request,
): Promise<Response> {
const auth = await routeIdentity(request);
if (!auth) return Response.json({ error: "unauthorized" }, { status: 401 });
const resolved = await openCodeAccount(auth.teamId);
if (!resolved) return Response.json({ error: "no_usable_account" }, { status: 503 });
if (!resolved)
return Response.json({ error: "no_usable_account" }, { status: 503 });
const remote = await remoteConfig(resolved.credential.accessToken);
const provider = rewriteProviders(remote, auth.token);
return Response.json({ provider }, {
headers: { "cache-control": "no-store" },
});
return Response.json(
{ provider },
{
headers: { "cache-control": "no-store" },
},
);
}
export async function openCodeClientConfig(
request: Request,
): Promise<Response> {
const auth = await routeIdentity(request);
if (!auth) {
return apiError(
"unauthorized",
"Your coderouter session expired or was revoked. Run `cr login` and retry.",
401,
false,
);
}
const resolved = await openCodeAccount(auth.teamId);
if (!resolved) {
return apiError(
"no_usable_account",
"No healthy OpenCode subscription is available. Check `cr`, add an account with `cr add`, or retry shortly.",
503,
true,
);
}
let remote: Record<string, unknown>;
try {
remote = await remoteConfig(resolved.credential.accessToken);
} catch (error) {
reportCoderouterFailure("provider_usage", error, {
provider: "opencode-go",
operation: "config",
});
return apiError(
"provider_unavailable",
"OpenCode configuration is temporarily unavailable. Retry shortly.",
502,
true,
);
}
const provider = rewriteProviders(remote, auth.token);
return Response.json(
{ provider },
{
headers: { "cache-control": "no-store" },
},
);
}
🤖 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 `@web/services/coderouter/opencodeProxy.ts` around lines 13 - 29, Update
openCodeClientConfig to use the same apiError structured error contract as
proxyOpenCodeRequest for unauthorized and unavailable-account responses, and
wrap remoteConfig in error handling that maps provider failures and timeouts to
the structured retryable 502 response. Preserve the successful provider response
and no-store cache header.

Source: Coding guidelines

Comment on lines +39 to +54
return apiError(
"unauthorized",
"Your coderouter session expired or was revoked. Run `cr login` and retry.",
401,
false,
);
}
const resolved = await openCodeAccount(auth.teamId);
if (!resolved) return Response.json({ error: "no_usable_account" }, { status: 503 });
const config = await remoteConfig(resolved.credential.accessToken);
if (!resolved) {
return apiError(
"no_usable_account",
"No healthy OpenCode subscription is available. Check `cr`, add an account with `cr add`, or retry shortly.",
503,
true,
);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟠 Major | ⚖️ Poor tradeoff

New API error copy is hardcoded English in both proxies. Both proxies add user-facing message strings as inline English literals, so the new error copy exists for one locale only. The repository rule requires API copy to read from next-intl or another locale-specific source and to be present for every locale in web/i18n/routing.ts and web/messages/.

  • web/services/coderouter/opencodeProxy.ts#L39-L54: move the apiError messages for unauthorized, no_usable_account, provider_unavailable, unknown_provider, and invalid_provider into the message catalogs and resolve them per request locale.
  • web/services/coderouter/codexProxy.ts#L163-L169: move the jsonError messages for unauthorized and no_usable_account into the same catalogs and resolve them the same way.

As per path instructions: "web UI, API response, rendered markdown, changelog, or user-facing data changes must read from next-intl or another locale-specific source and update every locale in web/i18n/routing.ts and web/messages/".

📍 Affects 2 files
  • web/services/coderouter/opencodeProxy.ts#L39-L54 (this comment)
  • web/services/coderouter/codexProxy.ts#L163-L169
🤖 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 `@web/services/coderouter/opencodeProxy.ts` around lines 39 - 54, Replace the
hardcoded user-facing messages in opencodeProxy.ts#L39-L54 for unauthorized,
no_usable_account, provider_unavailable, unknown_provider, and invalid_provider
with per-request locale lookups from the established next-intl or
locale-specific message source, adding every key to all catalogs under
web/messages/ for locales in web/i18n/routing.ts. Apply the same catalog-based
resolution to the unauthorized and no_usable_account jsonError messages in
codexProxy.ts#L163-L169, preserving the existing error codes and response
behavior.

Sources: Coding guidelines, Path instructions

Comment on lines +245 to +260
return [
[
id,
{
...value,
...(npm ? { npm } : {}),
api: undefined,
models,
options: {
...(isRecord(value.options) ? withoutSecrets(value.options) : {}),
baseURL: `https://coderouter.dev/api/coderouter/opencode/proxy/${encodeURIComponent(id)}`,
apiKey: routeToken,
},
},
],
];

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Inspect every place the rewritten provider payload is produced or asserted.
rg -n -C 6 'rewriteProviders|publicNestedProvider|withoutSecrets' --type=ts

Repository: manaflow-ai/cmux

Length of output: 154


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Tracked files matching opencodeProxy:"
git ls-files | rg 'opencodeProxy|coderouter' || true

echo
echo "Find opencodeProxy.ts:"
fd -a 'opencodeProxy\.ts$' . || true

echo
echo "Search for provider/session-related terms in ts/js:"
rg -n -C 4 'opencode|publicNestedProvider|rewriteProviders|withoutSecrets|openCodeClientConfig|console\.opencode|provider' --type=ts --type=js --type=tsx 2>/dev/null | head -n 240 || true

Repository: manaflow-ai/cmux

Length of output: 2026


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "opencodeProxy.ts outline:"
ast-grep outline web/services/coderouter/opencodeProxy.ts --view expanded | sed -n '1,220p' | head -n 220 || true

echo
echo "opencodeProxy.ts relevant sections:"
cat -n web/services/coderouter/opencodeProxy.ts | sed -n '1,330p'

echo
echo "Relevant test assertions:"
cat -n web/tests/coderouter-opencode-proxy.test.ts | sed -n '1,260p'

echo
echo "Provider config related sources:"
cat -n web/services/coderouter/types.ts | sed -n '1,260p'
cat -n web/services/coderouter/providerFetch.ts | sed -n '1,260p'

Repository: manaflow-ai/cmux

Length of output: 18811


Do not spread the raw provider object into openCodeClientConfig.

options keys are filtered, but rewriteProviders still copies every other top-level field from console.opencode.ai/api/config into the route-token holder. Build each top-level entry with an allowlist of safe provider fields needed by OpenCode clients, and still drop the upstream api endpoint before adding the route proxy URL.

🤖 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 `@web/services/coderouter/opencodeProxy.ts` around lines 245 - 260, The
provider rewrite in rewriteProviders must not spread the raw value into each
openCodeClientConfig entry. Construct the top-level provider object from an
allowlist of safe fields required by OpenCode clients, retain the conditional
npm and models fields as applicable, and continue removing the upstream api
field while setting the proxy baseURL and routeToken apiKey in options.

headers: { "content-type": "application/x-www-form-urlencoded" },
body: new URLSearchParams(body),
cache: "no-store",
signal: AbortSignal.timeout(10_000),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

rg -n -C 12 \
  '\b(claim|complete|fail|isTerminal|failureCode)\b|lease|TTL|AbortSignal\.timeout|TimeoutError|AbortError' \
  web/services/coderouter web/tests --glob '*.{ts,tsx}'

Repository: manaflow-ai/cmux

Length of output: 50373


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

# Locate the refresh implementation and focused credential refresh symbols without heavy output.
printf 'Candidate files:\n'
fd -a 'refresh\.(ts|tsx)$' web/services web/tests 2>/dev/null | sed 's#^\./##'

printf '\nFocused symbols in web/services/coderouter/refresh.ts:\n'
if [ -f web/services/coderouter/refresh.ts ]; then
  ast-grep outline web/services/coderouter/refresh.ts --match refreshProviderCredential --view expanded || true
  rg -n -C 8 'refreshProviderCredential|fetchProviderCred|isTerminal|failureCode|failure_message|claim|complete|lease|AbortSignal\.timeout|encrypt|complete' web/services/coderouter/refresh.ts web/tests --glob '*.{ts,tsx}'
fi

Repository: manaflow-ai/cmux

Length of output: 50373


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

printf 'refresh.ts file info:\n'
wc -l web/services/coderouter/refresh.ts
printf '\nrefresh.ts around 160-250:\n'
sed -n '140,270p' web/services/coderouter/refresh.ts | nl -ba -v140

printf '\ncredential refresh helper definitions/usages:\n'
rg -n -C 6 'function .*refresh|const .*refresh|refreshProviderCredential|fetchProviderCred|encrypt|complete|isTerminal|failureCode|claim|complete' web/services/coderouter web --glob '*.{ts,tsx}'

printf '\nall timeout values in refresh.ts:\n'
rg -n 'AbortSignal\.timeout|signal:' web/services/coderouter/refresh.ts

Repository: manaflow-ai/cmux

Length of output: 283


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

FILE=web/services/coderouter/refresh.ts
echo "refresh.ts file info:"
wc -l "$FILE"

echo
echo "refresh.ts around 140-260:"
sed -n '140,260p' "$FILE" | awk '{print NR+139": "$0}'

echo
echo "focused credential refresh symbols:"
rg -n -C 8 'refreshProviderCredential|fetchProviderCred|encrypt|complete|isTerminal|failureCode|claim|complete|AbortSignal\.timeout|signal:' "$FILE" web/services/websocket/websocketToken.ts web/services --glob '*.{ts,tsx}'

echo
echo "refresh.ts timeout occurrences:"
rg -n 'AbortSignal\.timeout|signal:' "$FILE"

Repository: manaflow-ai/cmux

Length of output: 50373


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

FILE=web/services/coderouter/repository.ts

echo "constants and credential refresh lease implementation:"
sed -n '1,45p' "$FILE"
sed -n '456,535p' "$FILE"

echo
echo "refresh constants/usages and tests:"
rg -n -C 8 'REFRESH_LEASE_MS|claimRefreshLease|completeRefreshLease|failRefreshLease|releaseRefreshLease|isTerminalRefreshError|refreshFailureCode|CodeRouterRefreshBusy|credential_refresh|provider_refresh|lastFailureCode|failureCode|lastFailureMessage' web/services/coderouter web/tests --glob '*.{ts,tsx}'

echo
echo "encryptCredential definition:"
rg -n -C 12 'encryptCredential|encryptCredential\(' web/services/coderouter --glob '*.{ts,tsx}'

Repository: manaflow-ai/cmux

Length of output: 50372


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

FILE=web/services/coderouter/refresh.ts
REPO=web/services/coderouter/repository.ts
TEST=web/tests/coderouter-refresh.test.ts

echo "refresh.ts relevant sections:"
sed -n '1,165p' "$FILE" | awk '{print NR": "$0}'
sed -n '257,272p' "$FILE" | awk '{print NR+256": "$0}'

echo
echo "encryption definitions for encryptCredential/decryptCredential:"
rg -n -C 12 'encryptCredential|decryptCredential|algorithm|aes|crypto|KMS|dataKey|encryptedDataKey|nonce|authTag' web/services/coderouter --glob '*.{ts,tsx}'

echo
echo "release/fail implementations:"
sed -n '519,572p' "$REPO" | awk '{print NR+518": "$0}'

echo
echo "refresh tests around invalid terminal error:"
sed -n '130,170p' "$TEST" | awk '{print NR+129": "$0}'

Repository: manaflow-ai/cmux

Length of output: 43329


Align the provider timeout with the credential refresh lease.

REFRESH_LEASE_MS is 30s, but postForm/postJson use AbortSignal.timeout(10_000) before encryptCredential calls KMS and completeRefreshLease writes the transaction. A slow provider can abort inside refreshProviderCredential, while the 30s lease is still allowed and another client can claim refresh. If the refresh completes after the timeout, completeRefreshLease can still race with a fresh claim. Keep the timeout longer than provider plus encryption/complete, and have the lease cover the full refresh path.

🤖 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 `@web/services/coderouter/refresh.ts` at line 216, Update the provider timeout
used by postForm/postJson in refreshProviderCredential to exceed the full
credential refresh duration, including provider work, encryptCredential, and
completeRefreshLease; align REFRESH_LEASE_MS so it covers that same end-to-end
path and prevents another client from claiming the lease while completion is
still possible.

Comment on lines 34 to 42
const authorization = request.headers.get("authorization");
const token = authorization?.match(/^Bearer\s+(\S+)$/i)?.[1];
if (token?.startsWith("crt_")) {
const routed = await authenticateRouteToken(token);
if (routed) return { ok: true, teamId: routed.teamId };
if (routed) {
return { ok: true, teamId: routed.teamId, stackUserId: routed.stackUserId };
}
}
const resolved = await resolveCodeRouterRequestContext(request, "use-or-manage");

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Reject an invalid route token without cookie fallback.

If authenticateRouteToken returns null, Line 42 can authorize the same request through cookie or native authentication. An expired or revoked explicit crt_ token must fail closed instead.

Return the normal unauthorized response when a supplied route token fails validation. Add a regression test that combines an invalid crt_ bearer with valid ambient authentication.

🤖 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 `@web/services/coderouter/requestContext.ts` around lines 34 - 42, Update the
request authentication flow around authenticateRouteToken so any supplied crt_
bearer token that fails validation immediately returns the normal unauthorized
response, without calling resolveCodeRouterRequestContext or falling back to
cookies/native authentication. Add a regression test covering an invalid crt_
bearer alongside valid ambient authentication and assert the request remains
unauthorized.

Source: Path instructions

Comment on lines +24 to +39
const decoder = new TextDecoder();
let tail = "";
let model: string | undefined;
return body.pipeThrough(
new TransformStream<Uint8Array, Uint8Array>({
transform(chunk, controller) {
controller.enqueue(chunk);
const text = decoder.decode(chunk, { stream: true });
const combined = `${tail.slice(-256)}${text}`;
model ??= stringField(combined, "model");
tail = `${tail}${text}`.slice(-MAX_TAIL_CHARS);
},
flush() {
tail = `${tail}${decoder.decode()}`.slice(-MAX_TAIL_CHARS);
onComplete(usageFromTail(tail, model));
},

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win

Stop rebuilding the 256 KiB tail on every chunk.

Line 34 concatenates the whole retained tail with the new chunk and then slices it. After the tail saturates, each chunk costs one allocation and copy of up to 262144 characters. Streamed model responses arrive as many small SSE chunks, so a single long response with a few thousand chunks produces hundreds of megabytes of string copying on the proxy request path. Complexity is O(chunks × MAX_TAIL_CHARS) instead of O(response bytes).

Keep the recent chunk strings in a bounded queue and join once in flush.

⚡ Proposed fix
   const decoder = new TextDecoder();
-  let tail = "";
+  const segments: string[] = [];
+  let tailLength = 0;
   let model: string | undefined;
   return body.pipeThrough(
     new TransformStream<Uint8Array, Uint8Array>({
       transform(chunk, controller) {
         controller.enqueue(chunk);
         const text = decoder.decode(chunk, { stream: true });
-        const combined = `${tail.slice(-256)}${text}`;
+        const previous = segments[segments.length - 1] ?? "";
+        const combined = `${previous.slice(-256)}${text}`;
         model ??= stringField(combined, "model");
-        tail = `${tail}${text}`.slice(-MAX_TAIL_CHARS);
+        segments.push(text);
+        tailLength += text.length;
+        while (
+          segments.length > 1 &&
+          tailLength - segments[0]!.length >= MAX_TAIL_CHARS
+        ) {
+          tailLength -= segments.shift()!.length;
+        }
       },
       flush() {
-        tail = `${tail}${decoder.decode()}`.slice(-MAX_TAIL_CHARS);
-        onComplete(usageFromTail(tail, model));
+        segments.push(decoder.decode());
+        onComplete(usageFromTail(segments.join("").slice(-MAX_TAIL_CHARS), model));
       },
     }),
   );

As per path instructions: "flag ... repeated sort/filter/map work in hot paths".

🤖 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 `@web/services/coderouter/responseUsage.ts` around lines 24 - 39, Update the
tail handling in the response transform around tail, transform, and flush to
store recent decoded chunk strings in a bounded queue instead of repeatedly
concatenating and slicing the entire retained tail. Enforce the MAX_TAIL_CHARS
limit while retaining the newest content, then join the queued chunks once
during flush before calling usageFromTail; preserve model extraction and decoder
flushing behavior.

Sources: Coding guidelines, Path instructions

Comment on lines +36 to +39
flush() {
tail = `${tail}${decoder.decode()}`.slice(-MAX_TAIL_CHARS);
onComplete(usageFromTail(tail, model));
},

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

Node.js TransformStream transformer cancel callback support version

💡 Result:

Support for the cancel callback in the TransformStream transformer was added in Node.js versions v21.5.0 and v20.14.0 [1][2][3]. This callback is a user-defined function invoked when either the readable side of the TransformStream is canceled or the writable side is aborted [1][2]. It receives a reason argument and is expected to return a promise that fulfills with undefined [1][2]. While this functionality was implemented in those versions, it was documented more recently in May 2026 [4][2]. Previously, users noted its absence in both the official documentation and the DefinitelyTyped @types/node definitions, despite the implementation existing in the codebase [5].

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Locate relevant files =="
fd -a 'responseUsage\.ts|engine\.json|package\.json|Dockerfile|compose\.(yaml|yml)$' . | sed 's#^\./##' | head -200

echo
echo "== responseUsage.ts =="
file=$(fd 'responseUsage\.ts' . | head -1)
if [ -n "${file:-}" ]; then
  wc -l "$file"
  cat -n "$file"
fi

echo
echo "== Search for ResponseUsage/API usages =="
rg -n "responseUsage|codexProxy|opencodeProxy|new ResponseUsage|Usage|onComplete|TransformStream" -S . --glob '!node_modules' --glob '!dist' --glob '!build' | head -300

echo
echo "== Runtime constraints =="
for f in .nvmrc package.json .node-version .tool-versions Dockerfile docker-compose.yml docker-compose.yaml compose.yml compose.yaml .github/workflows/*.yml .github/workflows/*.yaml; do
  [ -e "$f" ] || continue
  echo "--- $f"
  rg -n "node|NODE_VERSION|FROM node|setup-node|engines" "$f" || true
done

Repository: manaflow-ai/cmux

Length of output: 42832


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Observed usage wrapper imports/usages =="
rg -n "observeModelUsage|from .*responseUsage|responseUsage" web --glob '*.{ts,tsx}' | head -200

echo
echo "== Package/runtime/version declarations relevant to web =="
for f in web/package.json package.json; do
  if [ -e "$f" ]; then
    echo "--- $f"
    jq -r '.type as $type | .engines.node as $node | "type:"+$type+" node:"+$node // empty' "$f"
  fi
done

echo
echo "== ResponseUsage implementation with imports =="
wc -l web/services/coderouter/responseUsage.ts
sed -n '1,45p' web/services/coderouter/responseUsage.ts

echo
echo "== Candidate coderouter files =="
fd . web/services/coderouter -t f | sort

Repository: manaflow-ai/cmux

Length of output: 2991


Report usage when the stream is cancelled.

flush is the only completion path. If the client disconnects, the pipe is cancelled and onComplete never runs, so codexProxy and opencodeProxy emit no usage event for that request. Add a cancel handler so aborted requests still report an outcome.

The configured Node runtimes support TransformStream transformer cancel, with Node 20.x used in the broader app workflows and Node 22.x used in web/TUI workflows.

🤖 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 `@web/services/coderouter/responseUsage.ts` around lines 36 - 39, Add a cancel
handler alongside flush in the TransformStream transformer so cancelled streams
also invoke onComplete with usageFromTail(tail, model). Preserve the existing
tail decoding and completion behavior, and ensure both normal completion and
client-disconnect paths report usage outcomes.

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.

1 participant