Skip to content

feat: expose self-service quota windows#1111

Open
dofastted wants to merge 2 commits into
ding113:devfrom
dofastted:pr/dev-quota-windows-20260426
Open

feat: expose self-service quota windows#1111
dofastted wants to merge 2 commits into
ding113:devfrom
dofastted:pr/dev-quota-windows-20260426

Conversation

@dofastted
Copy link
Copy Markdown
Contributor

@dofastted dofastted commented Apr 26, 2026

Summary

Split from #1106 for dev review. This PR exposes pre-computed quota windows through the self-service getMyQuota API, enabling API key users to query their own quota status across multiple time windows (5h, daily, weekly, monthly, total) without requiring a web UI session.

Related PRs

Problem

Previously, the getMyQuota endpoint only returned raw limit and current usage values, requiring API consumers to manually calculate remaining quotas and percentages across different time windows. Additionally, readonly API keys (Bearer tokens with canLoginWebUi: false) could not access self-service quota endpoints in opaque session-token mode, limiting programmatic quota monitoring for service accounts.

Solution

1. Quota Windows API Enhancement

Added a new quotaWindows object to getMyQuota response with pre-computed values for all time periods:

Window Fields Computed
5h limitUsd, usedUsd, remainingUsd, usedPercent, remainingPercent, isUnlimited, isExhausted
daily Same as above
weekly Same as above
monthly Same as above
total Same as above

Additional top-level convenience fields added:

  • todayUsedUsd, todayRemainingUsd, todayUsedPercent, todayRemainingPercent - Key daily window
  • remainingPercent - Minimum remaining percentage across all bounded windows
  • providerGroup, rpmLimit, concurrentSessions

2. Readonly Bearer Key Support

Endpoints using allowReadOnlyAccess now accept Bearer API keys even when the proxy is in opaque session-token mode:

  • New src/lib/auth/readonly-access.ts - Utility functions for readonly key detection
  • Modified src/app/v1/_lib/proxy/auth-guard.ts - Tracks readonlyAccess flag in auth state
  • Modified src/lib/auth.ts - Supports readonly bearer fallback in opaque mode

3. Readonly Data Redaction

For security, readonly keys receive redacted responses:

  • Quota: userAllowedModels and userAllowedClients are hidden
  • Usage logs: endpoint field is nullified

4. Public Examples

Added 6 quota extractor example files in both docs/examples/ and public/examples/:

  • api-key-quota-extractor.js - Basic implementation with Chinese field names
  • api-key-quota-extractor-compatible.js - Fallback-compatible version with safe null handling
  • api-key-quota-extractor-direct.js - Direct mapping without fallback logic
  • api-key-quota-extractor-daily.js - Daily window focused
  • api-key-quota-extractor-weekly.js - Weekly window focused
  • api-key-quota-extractor-total.js - Total quota focused

The examples demonstrate how to call /api/actions/my-usage/getMyQuota with a Bearer token and extract quota information for external integrations.

5. Public Path Updates

Added /system-status and /examples to public paths in src/proxy.ts, allowing access to examples without authentication.

Changes

Core Changes

  • src/actions/my-usage.ts - Added quota window computation, new MyUsageQuota fields
  • src/app/api/actions/[...route]/route.ts - OpenAPI schema for new quota window fields
  • src/app/v1/_lib/proxy/auth-guard.ts - Added readonlyAccess flag to auth state
  • src/app/v1/_lib/proxy/session.ts - Extended AuthState interface

New Files

  • src/lib/auth/readonly-access.ts - Readonly key detection utilities
  • src/lib/my-usage/readonly-redaction.ts - Response redaction for readonly keys
  • docs/examples/api-key-quota-extractor*.js (6 files)
  • public/examples/api-key-quota-extractor*.js (3 files)

Supporting Changes

  • src/lib/auth.ts - Opaque mode readonly bearer fallback
  • src/proxy.ts - Public path additions

Test Updates

  • tests/api/my-usage-readonly.test.ts - Database-backed quota window tests
  • tests/unit/actions/my-usage-concurrent-inherit.test.ts - Monthly fallback as total tests
  • tests/unit/auth/auth-scoped-session-branches.test.ts - New test for readonly session branches
  • tests/configs/my-usage.config.ts - Coverage configuration update

Breaking Changes

None. All changes are additive:

  • New response fields in getMyQuota - existing consumers unaffected
  • New public paths - no security impact
  • New optional auth behavior - existing auth flows unchanged

Testing

Automated Tests

./node_modules/.bin/vitest run tests/api/my-usage-readonly.test.ts tests/unit/actions/my-usage-concurrent-inherit.test.ts tests/unit/auth/auth-scoped-session-branches.test.ts --reporter=verbose --testTimeout=30000

Verification Commands

node --check docs/examples/api-key-quota-extractor.js
node --check docs/examples/api-key-quota-extractor-compatible.js
node --check docs/examples/api-key-quota-extractor-direct.js
node --check public/examples/api-key-quota-extractor.js
node --check public/examples/api-key-quota-extractor-compatible.js
node --check public/examples/api-key-quota-extractor-direct.js
npm run typecheck

Note: tests/api/my-usage-readonly.test.ts skips database-backed cases when DATABASE_URL is not configured in the worktree.

Exclusions (in #1106, not here)

  • Temporary key groups batch management
  • User-to-key config sync
  • Billing multiplier correction
  • Status route aliases
  • Deployment scripts

Checklist

  • Code follows project conventions
  • Self-review completed
  • Examples validated with node --check
  • Type check passes
  • Tests pass locally (with database)

Description enhanced by Claude AI

Greptile Summary

This PR exposes pre-computed quota windows (5h, daily, weekly, monthly, total) through getMyQuota, adds readonly Bearer key support for self-service endpoints in opaque session mode, and includes redaction utilities plus six example extractor scripts. All new response fields are additive.

  • The allowLegacyReadOnlyBearer fallback in validateAuthToken (opaque mode only) calls validateKey with allowReadOnlyAccess: true, which bypasses the canLoginWebUi check for all keys — not just readonly ones — so any raw API key can now directly authenticate against all my-usage routes in opaque mode, broader than the PR description states.
  • The API integration test "总额度缺失时应回退 monthly" asserts keyLimitTotalUsd === 7 and limitTotalUsd === 7 after setting limitTotalUsd: null in the DB; this contradicts both the code and the unit test that explicitly verifies no monthly fallback, and will fail against a real database (tracked from prior review).

Confidence Score: 4/5

Safe to merge with low risk — no data-loss or security exploit, but the opaque-mode auth scope is wider than described and one API test will fail against a real DB.

No P0 findings. The carried-over P1 (failing API test assertions expecting monthly-total fallback) was flagged in prior rounds and is unresolved. New P2s are non-blocking. Score capped at 4 by the outstanding P1.

tests/api/my-usage-readonly.test.ts and src/lib/auth.ts

Important Files Changed

Filename Overview
src/actions/my-usage.ts Core quota window computation added; usedTotalUsd switches semantics (key-level vs user-level) when total limit is absent, which may confuse API consumers.
src/lib/auth.ts New allowLegacyReadOnlyBearer fallback in opaque mode allows any key (not just readonly keys) to authenticate directly for all my-usage routes — broader than the PR description states.
src/lib/my-usage/readonly-redaction.ts Redacts userAllowedModels/userAllowedClients to [] and endpoint to null for readonly keys; empty-array-vs-redacted ambiguity flagged in prior round.
src/app/v1/_lib/proxy/session.ts Adds readonlyAccess: boolean to AuthState; field is populated but not yet consumed in the proxy layer in this PR.
tests/api/my-usage-readonly.test.ts Two new DB-backed test suites; the "total-fallback" test asserts keyLimitTotalUsd === 7 and limitTotalUsd === 7 while setting limitTotalUsd: null in the DB — these assertions contradict the code and will fail against a real database.
src/lib/api/action-adapter-openapi.ts allowLegacyReadOnlyBearer is hardcoded for the my-usage module, enabling raw API keys in opaque mode for all self-service routes — broader scope than readonly-only keys.

Comments Outside Diff (3)

  1. src/actions/my-usage.ts, line 917-929 (link)

    P2 Endpoint filter is applied to DB query even for readonly keys

    findUsageLogsForKeySlim receives endpoint: filters.endpoint directly. A readonly caller can pass endpoint: "/v1/chat/completions" and, by comparing result counts across requests, enumerate which endpoints they've hit — even though endpoint is nulled out in the response by redactReadonlyLogs. The same applies to getMyUsageLogsBatch. If the redaction goal is to prevent endpoint disclosure, the filter should be stripped (or ignored) for readonly sessions before it reaches the query layer.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: src/actions/my-usage.ts
    Line: 917-929
    
    Comment:
    **Endpoint filter is applied to DB query even for readonly keys**
    
    `findUsageLogsForKeySlim` receives `endpoint: filters.endpoint` directly. A readonly caller can pass `endpoint: "/v1/chat/completions"` and, by comparing result counts across requests, enumerate which endpoints they've hit — even though `endpoint` is nulled out in the response by `redactReadonlyLogs`. The same applies to `getMyUsageLogsBatch`. If the redaction goal is to prevent endpoint disclosure, the filter should be stripped (or ignored) for readonly sessions before it reaches the query layer.
    
    How can I resolve this? If you propose a fix, please make it concise.
  2. src/actions/my-usage.ts, line 791-811 (link)

    P2 usedPercent and remainingPercent use different rounding bases

    usedPercent is computed from the raw window.used divided by limitUsd (already rounded), while remainingPercent is computed from remainingUsd (also rounded). Because remainingUsd = round2(clamp(limit - used)), the two percentages can sum to a value slightly different from 100 due to the double-rounding effect. This is cosmetic, but for quota displays where users often expect usedPercent + remainingPercent == 100, it may cause confusion. Using consistent unrounded intermediate values for both computations would eliminate the discrepancy.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: src/actions/my-usage.ts
    Line: 791-811
    
    Comment:
    **`usedPercent` and `remainingPercent` use different rounding bases**
    
    `usedPercent` is computed from the raw `window.used` divided by `limitUsd` (already rounded), while `remainingPercent` is computed from `remainingUsd` (also rounded). Because `remainingUsd = round2(clamp(limit - used))`, the two percentages can sum to a value slightly different from 100 due to the double-rounding effect. This is cosmetic, but for quota displays where users often expect `usedPercent + remainingPercent == 100`, it may cause confusion. Using consistent unrounded intermediate values for both computations would eliminate the discrepancy.
    
    How can I resolve this? If you propose a fix, please make it concise.
  3. tests/api/my-usage-readonly.test.ts, line 1462-1464 (link)

    P1 Test assertions expect monthly-fallback values that the code doesn't implement

    The three assertions below set limitTotalUsd: null in the DB for both the key and user, yet expect the response to contain the monthly limit values (7, 9, 7). The code path is keyLimitTotalUsd = key.limitTotalUsd ?? null (no fallback), and effectiveTotal is computed with both candidates having null limits, so effectiveTotal.limit is null. These assertions will fail when the test runs against a real database.

    The concurrent-inherit unit test added in the same PR ("总额度为空时 lifetime window 不应回退到月额度") explicitly verifies the opposite — that quotaWindows.total.isUnlimited === true when limitTotalUsd is null — which is consistent with the code but contradicts these API-test expectations.

    All three assertions here should likely be toBeNull() unless the monthly-fallback logic is being added elsewhere (e.g., in feat: add temporary key groups and self-service quota windows #1106) and these expectations are premature.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: tests/api/my-usage-readonly.test.ts
    Line: 1462-1464
    
    Comment:
    **Test assertions expect monthly-fallback values that the code doesn't implement**
    
    The three assertions below set `limitTotalUsd: null` in the DB for both the key and user, yet expect the response to contain the monthly limit values (7, 9, 7). The code path is `keyLimitTotalUsd = key.limitTotalUsd ?? null` (no fallback), and `effectiveTotal` is computed with both candidates having `null` limits, so `effectiveTotal.limit` is `null`. These assertions will fail when the test runs against a real database.
    
    The concurrent-inherit unit test added in the same PR ("总额度为空时 lifetime window 不应回退到月额度") explicitly verifies the **opposite** — that `quotaWindows.total.isUnlimited === true` when `limitTotalUsd` is null — which is consistent with the code but contradicts these API-test expectations.
    
    All three assertions here should likely be `toBeNull()` unless the monthly-fallback logic is being added elsewhere (e.g., in #1106) and these expectations are premature.
    
    How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
This is a comment left during a code review.
Path: src/lib/auth.ts
Line: 294-300

Comment:
**`allowLegacyReadOnlyBearer` allows any key, not just readonly keys, in opaque mode**

The new fallback path (lines 298–300) is only reached when `mode` is neither `"legacy"` nor `"dual"` — i.e., pure opaque mode — and the session store lookup already failed. When it does fire, `validateKey` is called with `{ allowReadOnlyAccess: true }`, which makes `canUseReadonlyAccess` return `true` for **all** keys regardless of `canLoginWebUi`. So regular user keys (with `canLoginWebUi: true`) can also bypass the opaque session requirement and authenticate directly for every `my-usage` route.

The PR description says this is intended only for "readonly Bearer keys (Bearer tokens with `canLoginWebUi: false`)", but the guard in `validateKey` doesn't enforce that restriction when `allowReadOnlyAccess` is `true`. If the intent is truly limited to readonly keys, the condition should additionally verify `isReadonlyKey` before calling `validateKey`, or `validateKey` should only grant access when the resolved key is actually readonly.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: src/actions/my-usage.ts
Line: 767-773

Comment:
**`usedTotalUsd` is `Math.max(keyUsed, userUsed)` when both limits are null — semantically inconsistent with bounded windows**

When `effectiveTotal` has no bounded candidate (both `keyLimitTotalUsd` and `userLimitTotalUsd` are null), `resolveEffectiveQuotaWindow` returns `used = Math.max(keyTotalCost, userTotalCost, 0)`. For all bounded windows the selected candidate's own `used` value is returned. The unit test in `my-usage-concurrent-inherit.test.ts` explicitly asserts `usedTotalUsd === 40` (the user-level cost) even though the key-level cost is only 12.

This means `usedTotalUsd` silently switches from key-scoped semantics to user-level semantics when the total limit is absent. An API consumer that compares `usedTotalUsd` to `quotaWindows.total.isUnlimited` to decide how to display progress will see an unexpectedly large "used" figure. Consider either always returning the key-level usage or documenting the switch explicitly in the field description.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: src/app/v1/_lib/proxy/session.ts
Line: 38

Comment:
**`readonlyAccess` field is set but never consumed in the proxy layer**

`readonlyAccess` is populated in `auth-guard.ts` for every successful authentication, but no code in the v1 proxy routes reads or acts on it in this PR. The field appears to be infrastructure for #1106's features (redaction, routing restrictions). If that's the case, it's a no-op here — worth a comment noting the intended consumer so it isn't silently removed or confused with the `allowReadOnlyAccess` concept in the actions layer.

How can I resolve this? If you propose a fix, please make it concise.

Reviews (3): Last reviewed commit: "chore: format code (pr-dev-quota-windows..." | Re-trigger Greptile

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 26, 2026

📝 Walkthrough

Walkthrough

此拉取请求新增多种 API 密钥配额提取示例文件、引入只读访问判定与脱敏工具、扩展 my-usage 配额模型与路由响应 schema、修改认证会话以携带 readonlyAccess,并新增/更新相关测试与代理公开路径。

Changes

Cohort / File(s) Summary
配额提取器示例(docs)
docs/examples/api-key-quota-extractor.js, docs/examples/api-key-quota-extractor-direct.js, docs/examples/api-key-quota-extractor-total.js, docs/examples/api-key-quota-extractor-daily.js, docs/examples/api-key-quota-extractor-weekly.js, docs/examples/api-key-quota-extractor-compatible.js
新增多个示例提取器,每个导出 requestextractor(response),统一对 /api/actions/my-usage/getMyQuota 的 POST 请求、响应验证、数值/布尔强制及多窗口配额映射与 extra 字段构造。
配额提取器示例(public)
public/examples/api-key-quota-extractor.js, public/examples/api-key-quota-extractor-direct.js, public/examples/api-key-quota-extractor-compatible.js
在 public 下新增与 docs 对应的示例实现,导出相同 requestextractor,行为与 docs 版本一致以供示例/演示使用。
My-usage 类型与返回行为扩展
src/actions/my-usage.ts
新增 MyUsageQuotaWindow 类型并扩展 MyUsageQuota,加入 providerGroup、quotaWindows(5H/daily/weekly/monthly/total)、today/overall 派生字段、并发字段、remaining/unit/reset 元数据;getMyQuota 与日志接口的返回现在经过只读脱敏处理(调用 redactReadonlyQuota / redactReadonlyLogs)。
只读访问判定与脱敏工具
src/lib/auth/readonly-access.ts, src/lib/my-usage/readonly-redaction.ts
新增 isReadonlyKeycanUseReadonlyAccess 辅助函数;新增 redactReadonlyQuotaredactReadonlyLogs,在只读密钥场景下隐藏/替换敏感字段并记录被脱敏字段。
认证会话与守卫调整
src/app/v1/_lib/proxy/auth-guard.ts, src/app/v1/_lib/proxy/session.ts, src/lib/auth.ts
Auth 验证路径现在返回/承载 readonlyAccess 字段;引入 allowLegacyReadOnlyBearer 选项以在特定模块(如 my-usage)允许旧版 Bearer 作为只读回退;validateKey/getSession 的只读判定集中为新 helper。
路由 schema 与代理公开路径
src/app/api/actions/[...route]/route.ts, src/proxy.ts
扩展 getMyQuota 的响应 Zod/OpenAPI schema(新增 quotaWindows 子模式、RPM/并发/用户允许列表/重置元数据/today 派生字段、unit 固定为 "USD" 等);将 /examples/system-status 列入公开 allowlist 并调整 proxy matcher 以排除 /examples
测试与配置更新
tests/api/my-usage-readonly.test.ts, tests/unit/actions/my-usage-concurrent-inherit.test.ts, tests/unit/auth/auth-scoped-session-branches.test.ts, tests/configs/my-usage.config.ts
更新/新增测试覆盖只读脱敏、配额窗口计算、总额回退逻辑、只读会话分支与日志端点隐藏,并将新测试加入 my-usage 测试配置。

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • ding113
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 5.26% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The pull request title 'feat: expose self-service quota windows' directly and clearly describes the main feature being added—exposing pre-computed quota windows through the self-service getMyQuota API.
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.
Description check ✅ Passed PR 描述详细说明了功能目的、解决方案、核心变更和示例,与代码变更内容相关且提供了足够的上下文信息。

✏️ 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.

Comment on lines +13 to +17
return {
...quota,
userAllowedModels: [],
userAllowedClients: [],
};
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Empty array is ambiguous — cannot distinguish redacted from unrestricted

userAllowedModels: [] and userAllowedClients: [] are returned for readonly keys, but the API consumer has no way to tell whether [] means "no model/client restrictions configured" or "data was redacted." A consumer that checks userAllowedModels.length === 0 to decide whether any filtering is applied will silently draw the wrong conclusion for every readonly-key caller. Consider returning null (or a dedicated redacted: true sentinel) to signal that the field was intentionally hidden.

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/lib/my-usage/readonly-redaction.ts
Line: 13-17

Comment:
**Empty array is ambiguous — cannot distinguish redacted from unrestricted**

`userAllowedModels: []` and `userAllowedClients: []` are returned for readonly keys, but the API consumer has no way to tell whether `[]` means "no model/client restrictions configured" or "data was redacted." A consumer that checks `userAllowedModels.length === 0` to decide whether any filtering is applied will silently draw the wrong conclusion for every readonly-key caller. Consider returning `null` (or a dedicated `redacted: true` sentinel) to signal that the field was intentionally hidden.

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines +1 to +3
({
request: {
url: "{{baseUrl}}/api/actions/my-usage/getMyQuota",
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Duplicate extractor files across docs/examples and public/examples

docs/examples has 6 extractor variants while public/examples carries only 3 of them (compatible, direct, main), with identical content. Any future update that touches one location must also be manually applied to the other, which is an easy source of drift. Consider keeping a single source of truth (e.g., only public/examples) and generating or symlinking the docs copies, or at minimum document why two locations are needed.

Prompt To Fix With AI
This is a comment left during a code review.
Path: docs/examples/api-key-quota-extractor-compatible.js
Line: 1-3

Comment:
**Duplicate extractor files across `docs/examples` and `public/examples`**

`docs/examples` has 6 extractor variants while `public/examples` carries only 3 of them (compatible, direct, main), with identical content. Any future update that touches one location must also be manually applied to the other, which is an easy source of drift. Consider keeping a single source of truth (e.g., only `public/examples`) and generating or symlinking the `docs` copies, or at minimum document why two locations are needed.

How can I resolve this? If you propose a fix, please make it concise.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces detailed quota window tracking and implements readonly access redaction for the usage API. It adds support for 5h, daily, weekly, monthly, and total quota calculations, along with utility functions to redact sensitive information like allowed models and request endpoints for restricted keys. The review feedback correctly identifies a logic error where the lifetime quota calculation incorrectly falls back to the monthly limit, which could lead to false exhaustion reports if lifetime costs exceed the monthly cap.

Comment thread src/actions/my-usage.ts Outdated
Comment on lines 365 to 370
function resolveTotalLimitWithMonthlyFallback(params: {
totalLimit: number | null | undefined;
monthlyLimit: number | null | undefined;
}): number | null {
return params.totalLimit ?? params.monthlyLimit ?? null;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

The fallback logic in resolveTotalLimitWithMonthlyFallback is potentially dangerous when applied to the total quota window. If a user has no lifetime limit (limitTotalUsd is null) but has a monthly limit, this function will return the monthly limit. When this is used in resolveEffectiveQuotaWindow against the lifetime cost (keyTotalCost), it will incorrectly report the quota as exhausted if the lifetime cost exceeds the monthly limit, even if the user still has monthly quota remaining. The total window should represent the lifetime quota and should not fall back to the monthly limit unless the intention is to treat the monthly limit as a lifetime cap, which is usually not the case.

function resolveTotalLimit(limit: number | null | undefined): number | null {
  return limit ?? null;
}

Comment thread src/actions/my-usage.ts Outdated
Comment on lines +631 to +638
const keyLimitTotalUsd = resolveTotalLimitWithMonthlyFallback({
totalLimit: key.limitTotalUsd,
monthlyLimit: key.limitMonthlyUsd,
});
const userLimitTotalUsd = resolveTotalLimitWithMonthlyFallback({
totalLimit: user.limitTotalUsd,
monthlyLimit: user.limitMonthlyUsd,
});
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

As noted in the helper function, using a monthly fallback for the total limit calculation can lead to incorrect 'exhausted' states for the total quota window if the lifetime cost exceeds the monthly limit. It is safer to use the actual total limit for the lifetime window.

    const keyLimitTotalUsd = key.limitTotalUsd ?? null;
    const userLimitTotalUsd = user.limitTotalUsd ?? null;

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: da8c9babca

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/lib/auth.ts Outdated
Comment on lines +298 to +299
if (options?.allowReadOnlyAccess && tokenKind === "legacy") {
return validateKey(token, options);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Restrict legacy-token fallback to self-service actions

This fallback makes opaque mode accept any raw API key whenever allowReadOnlyAccess is true, which now applies beyond the my-usage self-service endpoints. For example, usage-logs/getUsageLogs is marked allowReadOnlyAccess: true in src/app/api/actions/[...route]/route.ts (line 1040), and its action only scopes non-admin users by userId (not current key) in src/actions/usage-logs.ts (line 307), so a holder of one readonly key can read logs for sibling keys in the same account in opaque mode—a data-scope expansion that was previously blocked.

Useful? React with 👍 / 👎.

Comment thread src/actions/my-usage.ts Outdated
Comment on lines +674 to +676
const concurrentSessions = Math.max(keyConcurrent, userKeyConcurrent);
const concurrentSessionsLimit =
effectiveKeyConcurrentLimit > 0 ? effectiveKeyConcurrentLimit : null;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Align concurrent session usage with the reported limit

The new summary fields mix scopes: concurrentSessions uses Math.max(keyConcurrent, userKeyConcurrent), but concurrentSessionsLimit uses the effective key limit. When a user has many sessions on other keys, this can report concurrentSessions > concurrentSessionsLimit even though the current key is under its own cap, causing false “over-limit” signals for clients that consume these fields from getMyQuota.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 6

🧹 Nitpick comments (5)
src/app/api/actions/[...route]/route.ts (1)

1102-1208: Schema 与 MyUsageQuota 接口对齐良好

新增的 myUsageQuotaWindowSchema 与每窗口字段、quotaWindows 嵌套结构、today* / remainingPercent 派生字段、以及 userAllowedModels / userAllowedClients(非 nullable 数组,便于只读脱敏返回 [])均与 src/actions/my-usage.tsMyUsageQuota 接口和 redactReadonlyQuota 行为一致。

可选建议:unit: z.literal("USD") 将响应强制锁定为 "USD"。如果未来要支持多币种或与其它端点的 currencyCode: z.string() 统一,建议改为 z.string()(或一个币种枚举)以避免后续不兼容变更。当前如果 action 始终输出 USD 则无需改动。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/app/api/actions/`[...route]/route.ts around lines 1102 - 1208, The
responseSchema currently locks unit to z.literal("USD"); if you intend to
support other currencies or align with existing currencyCode fields, change the
unit field in the responseSchema from z.literal("USD") to a more flexible type
(e.g., z.string() or a currency enum) so it matches the MyUsageQuota interface
and future multi-currency needs; locate the unit declaration inside the created
route's responseSchema (alongside quotaWindows and myUsageQuotaWindowSchema) and
update it accordingly, keeping myUsageQuotaWindowSchema and other field types
unchanged if the action still emits USD.
public/examples/api-key-quota-extractor-compatible.js (1)

1-94: docs/examples/api-key-quota-extractor-compatible.js 内容重复,建议消除维护负担

逻辑与 docs/examples/ 下的同名 extractor 完全一致(含 limitTotalUsd 回退至 limitMonthlyUsd、本地计算 percent 兜底、isValid 包含 response.ok 校验等)。本 PR 同时新增了多份 docs/examples/*public/examples/* 的「成对副本」,未来一处修复另一处容易遗漏(已经存在过类似漂移风险)。

可选改进方案:

  • 方案 A:以 public/examples/ 为权威源,构建期或 prebuild 脚本将其复制/软链到 docs/examples/
  • 方案 B:让 docs/examples/ 仅保留一个 README 指向 public/examples/ 的实际示例。

不阻塞本 PR 合入,留待后续清理。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@public/examples/api-key-quota-extractor-compatible.js` around lines 1 - 94,
This PR adds a duplicate of the same extractor logic (the extractor function and
its helpers like toNumber, formatPercent, percent, and variables such as
limitTotalUsd) into two locations which creates maintenance drift; pick one
canonical source (prefer public/examples) and remove the duplicate in the other
location, then ensure the docs build references the canonical file (either by
adding a prebuild step to copy/symlink public/examples -> docs/examples or by
replacing the docs/examples copy with a small README that points to the
public/examples extractor); update any import/usage references if needed and
keep the extractor function and its helper names unchanged to avoid breaking
callers.
tests/api/my-usage-readonly.test.ts (1)

565-642: LGTM:总额度回退 + 只读日志 endpoint 脱敏

新用例同时覆盖三件事:limitTotalUsdlimitMonthlyUsd 回退、Key/User 取较小值、getMyUsageLogs 返回 endpoint: null 验证 redactReadonlyLogs。建议补充一条断言例如 expect(quotaData.remainingTotalUsd).toBeCloseTo(5.5, 6)(7 - 1.5 = 5.5)以锁定回退后的余额数值,避免仅断言类型时回归被掩盖。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/api/my-usage-readonly.test.ts` around lines 565 - 642, Add a precise
numeric assertion for the post-fallback remaining total in the test "总额度缺失时应回退
monthly,且只读日志隐藏 endpoint": after fetching quotaData (the object asserted by
expect(quotaData.*)), add expect(quotaData.remainingTotalUsd).toBeCloseTo(7 -
1.5, 6) (i.e., 5.5) to lock the balance calculation; this uses the existing
created message costUsd "1.5000" and the key limitTotalUsd 7 to ensure the
fallback & subtraction logic in getMyQuota is validated.
docs/examples/api-key-quota-extractor-total.js (1)

38-39: 回退校验同样未嵌套 toNumber(与 *-daily.js 一致的小问题)

Line 39 的 toNumber(total.remainingPercent, data.remainingPercent) 把上游 data.remainingPercent 直接当回退,未做有限数值校验。当前 schema 返回 number | null,影响有限。建议与 api-key-quota-extractor-daily.js 同步处理:

-      remainingPercent: toNumber(total.remainingPercent, data.remainingPercent),
+      remainingPercent: toNumber(total.remainingPercent, toNumber(data.remainingPercent, null)),
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/examples/api-key-quota-extractor-total.js` around lines 38 - 39, The
fallback for remainingPercent is using data.remainingPercent directly without
validating it through toNumber; update the call so remainingPercent is passed
through toNumber for proper numeric validation (mirror the behavior from
api-key-quota-extractor-daily.js). Locate the expression
toNumber(total.remainingPercent, data.remainingPercent) and change it to call
toNumber on the fallback (e.g., toNumber(total.remainingPercent,
toNumber(data.remainingPercent, null)) or equivalent safe numeric coercion) so
both total.remainingPercent and data.remainingPercent are validated before
assignment.
docs/examples/api-key-quota-extractor-daily.js (1)

35-39: 回退路径校验不一致(可选优化)

Lines 35-37 对扁平字段做了嵌套的 toNumber 校验(例如 toNumber(daily.remainingUsd, toNumber(data.remainingDailyUsd, null))),保证了回退值也是有限数值。但 Lines 38-39 直接把 data.todayUsedPercent / data.todayRemainingPercent 作为回退,没有走 toNumber。当前 API 返回 number | null,运行无虞,但与同文件其他字段的防御性写法不一致;后续若上游字段语义变更(例如返回 NaN),该路径会泄漏出去。

建议的小幅修正
-      usedPercent: toNumber(daily.usedPercent, data.todayUsedPercent),
-      remainingPercent: toNumber(daily.remainingPercent, data.todayRemainingPercent),
+      usedPercent: toNumber(daily.usedPercent, toNumber(data.todayUsedPercent, null)),
+      remainingPercent: toNumber(daily.remainingPercent, toNumber(data.todayRemainingPercent, null)),
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/examples/api-key-quota-extractor-daily.js` around lines 35 - 39, The
fallback check for percent fields is inconsistent: wrap the percent fallbacks
with the same defensive toNumber pattern used elsewhere so non-finite upstream
values won't leak; specifically update the usedPercent and remainingPercent
entries to call toNumber(daily.usedPercent, toNumber(data.todayUsedPercent,
null)) and toNumber(daily.remainingPercent, toNumber(data.todayRemainingPercent,
null)) (matching the pattern used for remaining/total/used fields and using
toNumber for the data.* fallback).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/examples/api-key-quota-extractor-weekly.js`:
- Around line 31-46: The returned object incorrectly marks isValid true when the
HTTP response failed because it doesn't check response.ok and also introduces an
extra ok field unlike other extractors; update the return shape to match the
other extractors by removing the ok property and change isValid to include
response.ok (e.g. isValid: response && response.ok === true &&
toBoolean(data.keyIsEnabled, true) && toBoolean(data.userIsEnabled, true)) so
failures are not treated as valid (refer to the isValid behavior in
api-key-quota-extractor.js and api-key-quota-extractor-compatible.js).

In `@src/actions/my-usage.ts`:
- Around line 717-757: The flat USD fields (limit5hUsd/used5hUsd/remaining5hUsd
… limitTotalUsd/usedTotalUsd/remainingTotalUsd and remaining) are using raw
effectiveXxx values while quotaWindows (built by buildQuotaWindow) is already
rounded to 2 decimals; unify precision by replacing the direct effectiveXxx.*
and overallRemaining usage with the rounded values from quotaWindows (e.g.,
quotaWindows.fiveHour.limit/used/remaining, quotaWindows.daily.*,
quotaWindows.total.*) or by passing each effective value through the existing
round2 helper before serializing; update the fields
limit5hUsd/used5hUsd/remaining5hUsd …
limitTotalUsd/usedTotalUsd/remainingTotalUsd and remaining to use those rounded
values so the response uses one consistent two-decimal precision.

In `@src/lib/auth.ts`:
- Around line 298-300: The new legacy fallback branch (tokenKind === "legacy" &&
options?.allowReadOnlyAccess) should emit an audit log and enforce explicit
read-only capability checks: when you call validateKey(token, options) from
validateAuthToken, add a logger.warn() noting a legacy fallback was used
(include token id or masked token info) and then verify the resolved key's
capabilities (via canUseReadonlyAccess / canLoginWebUi fields) before accepting
it — if the key has elevated privileges (e.g., canLoginWebUi or other
non-read-only flags) either reject or downgrade/strip those privileges; also
consider adding a unit-test for validateAuthToken to cover legacy fallback
logging and capability enforcement.

In `@src/lib/my-usage/readonly-redaction.ts`:
- Around line 1-32: getMyUsageLogsBatchFull currently uses
scrubUsageLogsBatchForReadonly which does not redact the endpoint field, causing
inconsistent behavior with getMyUsageLogsBatch and getMyUsageLogs that use
redactReadonlyLogs; update getMyUsageLogsBatchFull to call redactReadonlyLogs on
its output (or modify scrubUsageLogsBatchForReadonly to set endpoint: null) so
that endpoint is consistently redacted for readonly keys — reference functions:
getMyUsageLogsBatchFull, scrubUsageLogsBatchForReadonly, redactReadonlyLogs and
the endpoint field.

In `@src/proxy.ts`:
- Around line 16-17: PUBLIC_PATH_PATTERNS contains a redundant "/examples" entry
that is already excluded by the matcher, and "/system-status" is inconsistent
with the PR description and lacks the same special handling as "/status"; either
remove "/examples" from PUBLIC_PATH_PATTERNS or stop excluding "examples" in the
matcher so proxyHandler will handle it (pick one), and decide whether
"/system-status" should be treated like status aliases—if so add it alongside
"/status" handling where the x-cch-public-status header is set (mirror the logic
in the status handling block), otherwise remove "/system-status" from
PUBLIC_PATH_PATTERNS and update the PR description to reflect its exclusion.

In `@tests/unit/auth/auth-scoped-session-branches.test.ts`:
- Around line 70-85: The test falsely passes because runWithAuthSession relies
on globalThis.__cchAuthSessionStorage being initialized to create a scoped
context; since tests don't import the session-storage module,
__cchAuthSessionStorage is undefined and runWithAuthSession just calls the
callback, never exercising the scoped readonly branch in
getSession/getScopedAuthContext. Fix by initializing the
AsyncLocalStorage-backed session storage before each test (or importing the
auth-session-storage module) so globalThis.__cchAuthSessionStorage is set;
specifically modify the test setup to initialize the storage in beforeEach (so
runWithAuthSession creates a scoped context) and then call
runWithAuthSession(session, () => getSession({ allowReadOnlyAccess: false }), {
allowReadOnlyAccess: true }) to ensure the scoped readonly permission branch in
getScopedAuthContext/getSession is executed.

---

Nitpick comments:
In `@docs/examples/api-key-quota-extractor-daily.js`:
- Around line 35-39: The fallback check for percent fields is inconsistent: wrap
the percent fallbacks with the same defensive toNumber pattern used elsewhere so
non-finite upstream values won't leak; specifically update the usedPercent and
remainingPercent entries to call toNumber(daily.usedPercent,
toNumber(data.todayUsedPercent, null)) and toNumber(daily.remainingPercent,
toNumber(data.todayRemainingPercent, null)) (matching the pattern used for
remaining/total/used fields and using toNumber for the data.* fallback).

In `@docs/examples/api-key-quota-extractor-total.js`:
- Around line 38-39: The fallback for remainingPercent is using
data.remainingPercent directly without validating it through toNumber; update
the call so remainingPercent is passed through toNumber for proper numeric
validation (mirror the behavior from api-key-quota-extractor-daily.js). Locate
the expression toNumber(total.remainingPercent, data.remainingPercent) and
change it to call toNumber on the fallback (e.g.,
toNumber(total.remainingPercent, toNumber(data.remainingPercent, null)) or
equivalent safe numeric coercion) so both total.remainingPercent and
data.remainingPercent are validated before assignment.

In `@public/examples/api-key-quota-extractor-compatible.js`:
- Around line 1-94: This PR adds a duplicate of the same extractor logic (the
extractor function and its helpers like toNumber, formatPercent, percent, and
variables such as limitTotalUsd) into two locations which creates maintenance
drift; pick one canonical source (prefer public/examples) and remove the
duplicate in the other location, then ensure the docs build references the
canonical file (either by adding a prebuild step to copy/symlink public/examples
-> docs/examples or by replacing the docs/examples copy with a small README that
points to the public/examples extractor); update any import/usage references if
needed and keep the extractor function and its helper names unchanged to avoid
breaking callers.

In `@src/app/api/actions/`[...route]/route.ts:
- Around line 1102-1208: The responseSchema currently locks unit to
z.literal("USD"); if you intend to support other currencies or align with
existing currencyCode fields, change the unit field in the responseSchema from
z.literal("USD") to a more flexible type (e.g., z.string() or a currency enum)
so it matches the MyUsageQuota interface and future multi-currency needs; locate
the unit declaration inside the created route's responseSchema (alongside
quotaWindows and myUsageQuotaWindowSchema) and update it accordingly, keeping
myUsageQuotaWindowSchema and other field types unchanged if the action still
emits USD.

In `@tests/api/my-usage-readonly.test.ts`:
- Around line 565-642: Add a precise numeric assertion for the post-fallback
remaining total in the test "总额度缺失时应回退 monthly,且只读日志隐藏 endpoint": after fetching
quotaData (the object asserted by expect(quotaData.*)), add
expect(quotaData.remainingTotalUsd).toBeCloseTo(7 - 1.5, 6) (i.e., 5.5) to lock
the balance calculation; this uses the existing created message costUsd "1.5000"
and the key limitTotalUsd 7 to ensure the fallback & subtraction logic in
getMyQuota is validated.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 3a21a371-560f-4ceb-8e9b-09adc5eea6a8

📥 Commits

Reviewing files that changed from the base of the PR and between 757a8fb and da8c9ba.

📒 Files selected for processing (21)
  • docs/examples/api-key-quota-extractor-compatible.js
  • docs/examples/api-key-quota-extractor-daily.js
  • docs/examples/api-key-quota-extractor-direct.js
  • docs/examples/api-key-quota-extractor-total.js
  • docs/examples/api-key-quota-extractor-weekly.js
  • docs/examples/api-key-quota-extractor.js
  • public/examples/api-key-quota-extractor-compatible.js
  • public/examples/api-key-quota-extractor-direct.js
  • public/examples/api-key-quota-extractor.js
  • src/actions/my-usage.ts
  • src/app/api/actions/[...route]/route.ts
  • src/app/v1/_lib/proxy/auth-guard.ts
  • src/app/v1/_lib/proxy/session.ts
  • src/lib/auth.ts
  • src/lib/auth/readonly-access.ts
  • src/lib/my-usage/readonly-redaction.ts
  • src/proxy.ts
  • tests/api/my-usage-readonly.test.ts
  • tests/configs/my-usage.config.ts
  • tests/unit/actions/my-usage-concurrent-inherit.test.ts
  • tests/unit/auth/auth-scoped-session-branches.test.ts

Comment thread docs/examples/api-key-quota-extractor-weekly.js
Comment thread src/actions/my-usage.ts Outdated
Comment thread src/lib/auth.ts Outdated
Comment on lines +298 to +300
if (options?.allowReadOnlyAccess && tokenKind === "legacy") {
return validateKey(token, options);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# 列出所有传入 allowReadOnlyAccess: true 的调用点,确认仅限只读 action / 自助页面使用
rg -nP --type=ts -C2 'allowReadOnlyAccess\s*:\s*true'

Repository: ding113/claude-code-hub

Length of output: 17361


🏁 Script executed:

#!/bin/bash
# 检查当前没有显式传 allowReadOnlyAccess 的 getSession/validateAuthToken 调用,确保默认行为没变
rg -nP --type=ts -C2 '\b(getSession|validateAuthToken|validateSession|getSessionWithDualRead)\s*\('

Repository: ding113/claude-code-hub

Length of output: 50379


🏁 Script executed:

sed -n '290,320p' src/lib/auth.ts

Repository: ding113/claude-code-hub

Length of output: 918


🏁 Script executed:

# Check if there's logging around the legacy token fallback
rg -A5 -B5 'tokenKind.*legacy' src/lib/auth.ts

Repository: ding113/claude-code-hub

Length of output: 442


🏁 Script executed:

# Check validateKey implementation and how it handles allowReadOnlyAccess
rg -A20 'async function validateKey\(' src/lib/auth.ts | head -40

Repository: ding113/claude-code-hub

Length of output: 656


🏁 Script executed:

# Search for logging around the legacy token fallback
rg -n 'logger\|console\.' src/lib/auth.ts | grep -A2 -B2 '298\|300'

Repository: ding113/claude-code-hub

Length of output: 49


🏁 Script executed:

# Check if there's any logger statement in validateAuthToken or nearby
rg -n 'logger\.' src/lib/auth.ts | head -20

Repository: ding113/claude-code-hub

Length of output: 201


🏁 Script executed:

# Examine the full validateAuthToken function to see the complete context
sed -n '266,310p' src/lib/auth.ts | cat -n

Repository: ding113/claude-code-hub

Length of output: 1819


🏁 Script executed:

# Check all places that call validateAuthToken or getSession with allowReadOnlyAccess in production (non-test) code
rg -n 'allowReadOnlyAccess.*true' src/ --type=ts | grep -v test

Repository: ding113/claude-code-hub

Length of output: 2773


🏁 Script executed:

# Look at the action metadata for those endpoints to confirm they are readonly
rg -B10 'allowReadOnlyAccess: true' src/app/api/actions/\[\.\.\.route\]/route.ts | grep -E 'summary|allowReadOnlyAccess'

Repository: ding113/claude-code-hub

Length of output: 1035


🏁 Script executed:

# Check the login route context to understand the allowReadOnlyAccess: true usage
sed -n '200,220p' src/app/api/auth/login/route.ts

Repository: ding113/claude-code-hub

Length of output: 643


🏁 Script executed:

# Verify if there's any logging elsewhere for legacy token detection
rg -n 'legacy.*token\|tokenKind' src/lib/auth.ts | head -20

Repository: ding113/claude-code-hub

Length of output: 49


🏁 Script executed:

# Check the login route to understand why it uses allowReadOnlyAccess: true
sed -n '185,230p' src/app/api/auth/login/route.ts

Repository: ding113/claude-code-hub

Length of output: 1339


🏁 Script executed:

# Check if there are any other places calling validateAuthToken directly in production
rg -n 'validateAuthToken\(' src/ --type=ts | grep -v test | grep -v node_modules

Repository: ding113/claude-code-hub

Length of output: 323


🏁 Script executed:

# Check the action-adapter-openapi to see how allowReadOnlyAccess is passed down
sed -n '310,325p' src/lib/api/action-adapter-openapi.ts

Repository: ding113/claude-code-hub

Length of output: 743


🏁 Script executed:

# Check if there's any documentation or comment about the allowReadOnlyAccess behavior
rg -B5 -A5 'allowReadOnlyAccess.*readonly' src/lib/auth/readonly-access.ts

Repository: ding113/claude-code-hub

Length of output: 49


🏁 Script executed:

# Verify canUseReadonlyAccess function to understand how it validates readonly keys
rg -A15 'function.*canUseReadonlyAccess\|export.*canUseReadonlyAccess' src/lib/auth/readonly-access.ts

Repository: ding113/claude-code-hub

Length of output: 49


🏁 Script executed:

# Find and examine the readonly-access.ts file
cat src/lib/auth/readonly-access.ts

Repository: ding113/claude-code-hub

Length of output: 428


🏁 Script executed:

# Check validateKey to see how canUseReadonlyAccess is used
sed -n '150,200p' src/lib/auth.ts | cat -n

Repository: ding113/claude-code-hub

Length of output: 1798


🏁 Script executed:

# Search for canUseReadonlyAccess in the codebase
rg -n 'canUseReadonlyAccess' src/lib/auth.ts

Repository: ding113/claude-code-hub

Length of output: 255


🏁 Script executed:

# Check line 231 in validateKey to see the canUseReadonlyAccess check
sed -n '220,240p' src/lib/auth.ts

Repository: ding113/claude-code-hub

Length of output: 599


🏁 Script executed:

# Verify if there are any checks that prevent non-readonly actions from using allowReadOnlyAccess
rg -n 'allowReadOnlyAccess.*false' src/ --type=ts | grep -v test | head -10

Repository: ding113/claude-code-hub

Length of output: 350


🏁 Script executed:

# Check if login route should be using allowReadOnlyAccess
git log --oneline -n 20 src/app/api/auth/login/route.ts 2>/dev/null || echo "Git history not available"

Repository: ding113/claude-code-hub

Length of output: 116


🏁 Script executed:

# Verify if there are any other non-readonly endpoints using allowReadOnlyAccess
rg -B15 'allowReadOnlyAccess: true' src/app/api/actions/\[\.\.\.route\]/route.ts | rg 'summary|allowReadOnlyAccess' | grep -A1 -B1 '获取用户列表\|获取活跃'

Repository: ding113/claude-code-hub

Length of output: 218


🏁 Script executed:

# Check the action-adapter to confirm allowReadOnlyAccess is the action metadata value being passed
sed -n '300,330p' src/lib/api/action-adapter-openapi.ts

Repository: ding113/claude-code-hub

Length of output: 1260


🏁 Script executed:

# Understand the purpose of allowReadOnlyAccess in login route - check if it's needed for readonly keys to login
sed -n '150,220p' src/app/api/auth/login/route.ts | grep -A30 'validateKey'

Repository: ding113/claude-code-hub

Length of output: 421


🏁 Script executed:

# Check if there's a test that clarifies the login route behavior with readonly keys
rg -A10 'login.*readonly\|readonly.*login' tests/ --type=ts -i | head -30

Repository: ding113/claude-code-hub

Length of output: 49


🏁 Script executed:

# Check what happens after login validation - does the redirect depend on canLoginWebUi?
sed -n '220,260p' src/app/api/auth/login/route.ts

Repository: ding113/claude-code-hub

Length of output: 1381


🏁 Script executed:

# Check the complete login flow to understand the allowReadOnlyAccess: true usage
sed -n '140,160p' src/app/api/auth/login/route.ts

Repository: ding113/claude-code-hub

Length of output: 736


🏁 Script executed:

# Find the complete login redirect logic after session validation
sed -n '240,290p' src/app/api/auth/login/route.ts

Repository: ding113/claude-code-hub

Length of output: 1758


🏁 Script executed:

# Check what getLoginRedirectTarget returns and if readonly keys get redirected
rg -A10 'getLoginRedirectTarget' src/app/api/auth/login/route.ts

Repository: ding113/claude-code-hub

Length of output: 816


🏁 Script executed:

# Verify getLoginRedirectTarget behavior for readonly keys
sed -n '238,248p' src/lib/auth.ts

Repository: ding113/claude-code-hub

Length of output: 470


🏁 Script executed:

# Double-check: does readonly user get different redirect than dashboard user?
rg -A5 'readonly_user' src/app/api/auth/login/route.ts

Repository: ding113/claude-code-hub

Length of output: 214


Opaque 模式下 legacy Bearer 回退的实际安全范围已得验证,但缺乏审计日志

新分支 if (options?.allowReadOnlyAccess && tokenKind === "legacy") 的行为确实如初步分析:在 opaque 模式下任何持有 legacy bearer 的请求都会走 validateKey(token, options)。由于 canUseReadonlyAccess 的逻辑(allowReadOnlyAccess=true 时返回 true),包括 canLoginWebUi=true 的全权限 key 也会被接受。

现状分析:

  1. 当前用途已绑定到只读端点:action-adapter 在调用 validateAuthToken 时直接传递 action 元数据的 allowReadOnlyAccess 值,而所有标记 allowReadOnlyAccess: true 的 action 均为只读操作(getMyQuota、getMyUsageLogs 等)。
  2. 登录路由的用法是安全的/api/auth/loginallowReadOnlyAccess: true 是为了让只读 key 通过认证,随后 getLoginRedirectTarget() 会根据 canLoginWebUi 字段将只读用户重定向至 /my-usage 而非 /dashboard
  3. 隐性保护但不牢固:虽然当前所有调用点都恰当地使用了此参数,但如果未来某个非只读 action 在不加验证的情况下添加 allowReadOnlyAccess: true,legacy fallback 会接受全权限 key,从而削弱 opaque 模式的权限隔离。

建议:

  • 为 legacy token fallback 命中点添加 logger.warn() 以便生产环境监控回退使用量
  • 若计划扩展 allowReadOnlyAccess: true 到更多 action,考虑在 action adapter 层或 validateAuthToken 中补充显式的权限降级检查,确保 legacy fallback 不会意外接受超出预期的权限级别
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/lib/auth.ts` around lines 298 - 300, The new legacy fallback branch
(tokenKind === "legacy" && options?.allowReadOnlyAccess) should emit an audit
log and enforce explicit read-only capability checks: when you call
validateKey(token, options) from validateAuthToken, add a logger.warn() noting a
legacy fallback was used (include token id or masked token info) and then verify
the resolved key's capabilities (via canUseReadonlyAccess / canLoginWebUi
fields) before accepting it — if the key has elevated privileges (e.g.,
canLoginWebUi or other non-read-only flags) either reject or downgrade/strip
those privileges; also consider adding a unit-test for validateAuthToken to
cover legacy fallback logging and capability enforcement.

Comment thread src/lib/my-usage/readonly-redaction.ts
Comment thread src/proxy.ts
Comment on lines +16 to +17
"/system-status",
"/examples",
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

/examples 同时出现在两处导致 PUBLIC_PATH_PATTERNS 条目冗余,并请确认 /system-status 是否在本 PR 范围内

  • 第 136 行的 matcher 已经排除 examples,意味着对 /examples/... 的请求根本不会进入 proxyHandler,第 17 行将其加入 PUBLIC_PATH_PATTERNS 实际上是 dead code(除非将来有 /[locale]/examples 这样的路由需求,但 public/examples/*.js 是静态资源,不会带 locale 前缀)。建议二选一:要么从 matcher 中保留处理(移除 matcher 的 examples 排除),要么从 PUBLIC_PATH_PATTERNS 移除 /examples
  • 第 16 行新增的 /system-status 与 PR 描述中"Intentionally excludes: ... status aliases"看似矛盾。请确认是否应当与 status 别名一起从本 PR 中拆出,或在 PR 描述中更新该项排除说明。另外 /status 在第 51-61 行有 locale 前缀的特殊处理(设置 x-cch-public-status 头),/system-status 没有对应处理,这是有意的吗?

Also applies to: 134-136

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/proxy.ts` around lines 16 - 17, PUBLIC_PATH_PATTERNS contains a redundant
"/examples" entry that is already excluded by the matcher, and "/system-status"
is inconsistent with the PR description and lacks the same special handling as
"/status"; either remove "/examples" from PUBLIC_PATH_PATTERNS or stop excluding
"examples" in the matcher so proxyHandler will handle it (pick one), and decide
whether "/system-status" should be treated like status aliases—if so add it
alongside "/status" handling where the x-cch-public-status header is set (mirror
the logic in the status handling block), otherwise remove "/system-status" from
PUBLIC_PATH_PATTERNS and update the PR description to reflect its exclusion.

Comment thread tests/unit/auth/auth-scoped-session-branches.test.ts
@dofastted dofastted force-pushed the pr/dev-quota-windows-20260426 branch from da8c9ba to 8487473 Compare April 26, 2026 05:04
@dofastted
Copy link
Copy Markdown
Contributor Author

Updated after review:\n\n- quota window now only uses lifetime total limits; it no longer falls back to monthly limits. Monthly limits remain represented by the window.\n- readonly quota responses now include so clients can distinguish hidden model/client restrictions from unrestricted empty arrays.\n\nVerification after update:\n-
RUN v4.1.4 /mnt/x/project/claude-code-hub
API started at http://127.0.0.1:51206/

stdout | tests/unit/actions/my-usage-concurrent-inherit.test.ts

Vitest 测试环境初始化...

测试配置:

stdout | tests/unit/auth/auth-scoped-session-branches.test.ts

Vitest 测试环境初始化...

测试配置:

stdout | tests/unit/auth/auth-scoped-session-branches.test.ts

Vitest 测试环境清理...

Vitest 测试环境清理完成

✓ |0| tests/unit/auth/auth-scoped-session-branches.test.ts > auth scoped session branches > rejects scoped readonly session when caller tries to access it without readonly permission 173ms
✓ |0| tests/unit/auth/auth-scoped-session-branches.test.ts > auth scoped session branches > allows legacy bearer token fallback in opaque mode for readonly self-service 2ms
↓ |0| tests/api/my-usage-readonly.test.ts > my-usage API:只读 Key 自助查询 > 未携带 auth-token:my-usage 端点应返回 401
↓ |0| tests/api/my-usage-readonly.test.ts > my-usage API:只读 Key 自助查询 > 只读 Key:允许访问 my-usage 端点和其他 allowReadOnlyAccess 端点
↓ |0| tests/api/my-usage-readonly.test.ts > my-usage API:只读 Key 自助查询 > Bearer-only:仅 Authorization 也应可查询 my-usage 和其他 allowReadOnlyAccess 端点
↓ |0| tests/api/my-usage-readonly.test.ts > my-usage API:只读 Key 自助查询 > 只读 Key:只能查询当前 key 日志里出现过的 IP 详情
↓ |0| tests/api/my-usage-readonly.test.ts > my-usage API:只读 Key 自助查询 > 总额度缺失时应回退 monthly,且只读日志隐藏 endpoint
↓ |0| tests/api/my-usage-readonly.test.ts > my-usage API:只读 Key 自助查询 > 今日统计:应与 message_request 数据一致,并排除 warmup 与其他 Key 数据
↓ |0| tests/api/my-usage-readonly.test.ts > my-usage API:只读 Key 自助查询 > getMyStatsSummary:未认证返回 401
↓ |0| tests/api/my-usage-readonly.test.ts > my-usage API:只读 Key 自助查询 > getMyStatsSummary:基础聚合统计,排除 warmup,区分 key/user breakdown
↓ |0| tests/api/my-usage-readonly.test.ts > my-usage API:只读 Key 自助查询 > getMyStatsSummary:日期范围过滤
↓ |0| tests/api/my-usage-readonly.test.ts > my-usage API:只读 Key 自助查询 > imported ledger:full batch / filters / IP / summary / quota 在全局 message_request 非空时仍可用
↓ |0| tests/api/my-usage-readonly.test.ts > my-usage API:只读 Key 自助查询 > imported ledger:mixed old/new data 在 readonly full batch 与 summary 中不重复计数
✓ |0| tests/unit/actions/my-usage-concurrent-inherit.test.ts > getMyQuota - concurrent limit inheritance > Key 并发为 0 时应回退到 User 并发上限 6345ms
stdout | tests/unit/actions/my-usage-concurrent-inherit.test.ts

Vitest 测试环境清理...

Vitest 测试环境清理完成

✓ |0| tests/unit/actions/my-usage-concurrent-inherit.test.ts > getMyQuota - concurrent limit inheritance > Key 并发为正数时应优先使用 Key 自身上限 0ms
✓ |0| tests/unit/actions/my-usage-concurrent-inherit.test.ts > getMyQuota - concurrent limit inheritance > Key=0 且 User=0 时应返回 0(无限制) 0ms
✓ |0| tests/unit/actions/my-usage-concurrent-inherit.test.ts > getMyQuota - concurrent limit inheritance > 总额度为空时应使用月额度作为 usage API 返回的总额度 0ms
✓ |0| tests/unit/actions/my-usage-concurrent-inherit.test.ts > getMyQuota - concurrent limit inheritance > 存在总额度时 usage API 应优先返回总额度 0ms

Test Files 2 passed | 1 skipped (3)
Tests 7 passed | 11 skipped (18)
Start at 13:04:27
Duration 11.35s (transform 1.80s, setup 526ms, import 2.32s, tests 6.53s, environment 10.53s)\n-

claude-code-hub@0.7.0 typecheck
tsgo -p tsconfig.json --noEmit

@dofastted
Copy link
Copy Markdown
Contributor Author

Correction for the previous comment formatting. Updated after review:\n\n- The total quota window now only uses lifetime total limits; it no longer falls back to monthly limits. Monthly limits remain represented by the monthly window.\n- readonly quota responses now include readonlyRedactedFields so clients can distinguish hidden model/client restrictions from unrestricted empty arrays.\n\nVerification after update:\n- Vitest targeted quota/auth tests passed with testTimeout=30000.\n- npm run typecheck passed.

@dofastted dofastted force-pushed the pr/dev-quota-windows-20260426 branch from 8487473 to 3e41208 Compare April 26, 2026 05:12
@dofastted
Copy link
Copy Markdown
Contributor Author

Second review update pushed. Additional changes:\n\n- Raw legacy API-key fallback in opaque mode is now limited to my-usage self-service action routes. Other allowReadOnlyAccess routes do not accept raw legacy Bearer keys through this fallback.\n- Weekly quota extractor now treats failed HTTP responses as invalid.\n- Flat quota fields now use the same rounded values as quotaWindows.\n- Full-format readonly usage logs now redact endpoint consistently.\n- Added schema entry for readonlyRedactedFields.\n\nVerification:\n- node --check for daily, total, and weekly extractor examples passed.\n- Targeted my-usage/auth Vitest passed: 8 passed, 11 skipped because local database is not configured.\n- npm run typecheck passed.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

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/app/api/actions/[...route]/route.ts (1)

1119-1208: ⚠️ Potential issue | 🟡 Minor

响应 Schema 缺少 readonlyRedactedFields 字段声明

MyUsageQuota 接口(src/actions/my-usage.ts Line 264)已声明 readonlyRedactedFields?: string[],并由 redactReadonlyQuota 在只读 Key 场景下实际写入 ["userAllowedModels", "userAllowedClients"](测试 tests/api/my-usage-readonly.test.ts Line 478、626 也对此进行断言)。但当前 OpenAPI 响应 schema 未声明该字段,OpenAPI 文档/Scalar/类型生成的 SDK 会丢失这一关键提示,调用方无法据此判断哪些数组是被脱敏过的还是用户原本就为空。

建议在 schema 中补上:

♻️ 建议的 schema 修正
       userAllowedModels: z.array(z.string()),
       userAllowedClients: z.array(z.string()),
+      readonlyRedactedFields: z
+        .array(z.string())
+        .optional()
+        .describe("只读 Key 场景下被脱敏的字段列表(仅当请求方为只读 Key 时返回)"),

       expiresAt: z.string().nullable(),
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/app/api/actions/`[...route]/route.ts around lines 1119 - 1208, The
OpenAPI responseSchema is missing the optional readonlyRedactedFields field
present on the MyUsageQuota interface and set by redactReadonlyQuota (tests
assert this behavior), so add readonlyRedactedFields?: string[] to
responseSchema (a nullable/optional array of strings) to match MyUsageQuota and
ensure SDKs/doc generation include which fields were redacted (refer to
responseSchema, MyUsageQuota, and redactReadonlyQuota to align semantics and
tests).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tests/api/my-usage-readonly.test.ts`:
- Around line 425-475: The test assertions for total quota are inconsistent with
the implementation: either change the tests or implement a fallback so total
limits fall back to monthly when total is missing. Update
src/actions/my-usage.ts by changing how keyLimitTotalUsd and userLimitTotalUsd
are derived (currently using key.limitTotalUsd ?? null and user.limitTotalUsd ??
null) to prefer limitTotalUsd then fallback to limitMonthlyUsd (e.g.,
keyLimitTotalUsd = key.limitTotalUsd ?? key.limitMonthlyUsd ?? null; same for
userLimitTotalUsd) before calling resolveEffectiveQuotaWindow; then run tests
and adjust the failing assertion in tests/api/my-usage-readonly.test.ts only if
you intentionally want the other behavior (make quotaData.limitTotalUsd null
instead of 35) to keep test and implementation consistent.

---

Outside diff comments:
In `@src/app/api/actions/`[...route]/route.ts:
- Around line 1119-1208: The OpenAPI responseSchema is missing the optional
readonlyRedactedFields field present on the MyUsageQuota interface and set by
redactReadonlyQuota (tests assert this behavior), so add
readonlyRedactedFields?: string[] to responseSchema (a nullable/optional array
of strings) to match MyUsageQuota and ensure SDKs/doc generation include which
fields were redacted (refer to responseSchema, MyUsageQuota, and
redactReadonlyQuota to align semantics and tests).
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: df7b9183-e6f6-4250-80f1-cc8b6d023130

📥 Commits

Reviewing files that changed from the base of the PR and between da8c9ba and 8487473.

📒 Files selected for processing (21)
  • docs/examples/api-key-quota-extractor-compatible.js
  • docs/examples/api-key-quota-extractor-daily.js
  • docs/examples/api-key-quota-extractor-direct.js
  • docs/examples/api-key-quota-extractor-total.js
  • docs/examples/api-key-quota-extractor-weekly.js
  • docs/examples/api-key-quota-extractor.js
  • public/examples/api-key-quota-extractor-compatible.js
  • public/examples/api-key-quota-extractor-direct.js
  • public/examples/api-key-quota-extractor.js
  • src/actions/my-usage.ts
  • src/app/api/actions/[...route]/route.ts
  • src/app/v1/_lib/proxy/auth-guard.ts
  • src/app/v1/_lib/proxy/session.ts
  • src/lib/auth.ts
  • src/lib/auth/readonly-access.ts
  • src/lib/my-usage/readonly-redaction.ts
  • src/proxy.ts
  • tests/api/my-usage-readonly.test.ts
  • tests/configs/my-usage.config.ts
  • tests/unit/actions/my-usage-concurrent-inherit.test.ts
  • tests/unit/auth/auth-scoped-session-branches.test.ts
✅ Files skipped from review due to trivial changes (6)
  • src/lib/auth/readonly-access.ts
  • public/examples/api-key-quota-extractor-direct.js
  • docs/examples/api-key-quota-extractor-weekly.js
  • src/lib/my-usage/readonly-redaction.ts
  • docs/examples/api-key-quota-extractor.js
  • docs/examples/api-key-quota-extractor-total.js
🚧 Files skipped from review as they are similar to previous changes (9)
  • tests/configs/my-usage.config.ts
  • public/examples/api-key-quota-extractor.js
  • src/app/v1/_lib/proxy/auth-guard.ts
  • tests/unit/auth/auth-scoped-session-branches.test.ts
  • docs/examples/api-key-quota-extractor-compatible.js
  • docs/examples/api-key-quota-extractor-daily.js
  • src/proxy.ts
  • tests/unit/actions/my-usage-concurrent-inherit.test.ts
  • src/lib/auth.ts

Comment on lines +425 to +475
expect(quotaData.usedMonthlyUsd).toBeTypeOf("number");
expect(quotaData.limit5hUsd).toBe(10);
expect(quotaData.limitDailyUsd).toBe(15);
expect(quotaData.limitWeeklyUsd).toBe(25);
expect(quotaData.limitMonthlyUsd).toBe(35);
expect(quotaData.limitTotalUsd).toBe(35);
expect(quotaData.todayUsedUsd).toBeCloseTo(0.01, 6);
expect(quotaData.todayRemainingUsd).toBeCloseTo(14.99, 6);
expect(quotaData.todayUsedPercent).toBeCloseTo(0.07, 6);
expect(quotaData.todayRemainingPercent).toBeCloseTo(99.93, 6);
expect(quotaData.remainingPercent).toBeCloseTo(99.9, 6);
expect(quotaData.quotaWindows).toMatchObject({
fiveHour: {
period: "5h",
limitUsd: 10,
usedUsd: 0.01,
remainingUsd: 9.99,
usedPercent: 0.1,
remainingPercent: 99.9,
isUnlimited: false,
isExhausted: false,
},
daily: {
period: "daily",
limitUsd: 15,
usedUsd: 0.01,
remainingUsd: 14.99,
usedPercent: 0.07,
remainingPercent: 99.93,
isUnlimited: false,
isExhausted: false,
},
weekly: {
period: "weekly",
limitUsd: 25,
usedUsd: 0.01,
remainingUsd: 24.99,
},
monthly: {
period: "monthly",
limitUsd: 35,
usedUsd: 0.01,
remainingUsd: 34.99,
},
total: {
period: "total",
limitUsd: null,
usedUsd: 0.01,
remainingUsd: null,
},
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# 验证 Key / User 类型 + repository 里是否存在 limitTotalUsd → limitMonthlyUsd 的回退映射

echo "=== 1. Key 类型定义 ==="
fd -t f 'key.ts' src/types --exec rg -nP -C2 'limitTotalUsd|limit_total_usd' {}

echo "=== 2. User 类型定义 ==="
fd -t f 'user.ts' src/types --exec rg -nP -C2 'limitTotalUsd|limit_total_usd' {}

echo "=== 3. Key 仓储中 limitTotalUsd 的处理 ==="
fd -t f -e ts . src/repository --exec rg -nP -C3 'limitTotalUsd' {}

echo "=== 4. Drizzle schema 中相关列的默认值 ==="
fd -t f 'schema.ts' src/drizzle --exec rg -nP -C2 'limit_total_usd|limitTotalUsd' {}

echo "=== 5. 是否存在显式的 monthly 回退到 total 的代码 ==="
rg -nP -C2 '(limitTotalUsd|limit_total_usd)\s*\?\?\s*[a-zA-Z_]*[Mm]onthly|monthly.{0,30}total|total.{0,30}monthly'

Repository: ding113/claude-code-hub

Length of output: 21296


🏁 Script executed:

# Check the my-usage.ts implementation around the key lines mentioned in review
echo "=== Lines 620-660 in my-usage.ts (extract limits, build quota windows) ==="
sed -n '620,660p' src/actions/my-usage.ts | cat -n

echo -e "\n=== Lines 700-730 in my-usage.ts (response assembly) ==="
sed -n '700,730p' src/actions/my-usage.ts | cat -n

echo -e "\n=== resolveEffectiveQuotaWindow function ==="
rg -nP -A15 'function resolveEffectiveQuotaWindow|const resolveEffectiveQuotaWindow' src/actions/my-usage.ts

echo -e "\n=== buildQuotaWindow function ==="
rg -nP -A20 'function buildQuotaWindow|const buildQuotaWindow' src/actions/my-usage.ts

Repository: ding113/claude-code-hub

Length of output: 5238


🏁 Script executed:

# Now check the test setup - what limits are being set before the assertions at lines 425-475
echo "=== Test setup (lines 1-150 or earlier) to see what limits are configured ==="
sed -n '1,200p' tests/api/my-usage-readonly.test.ts | cat -n

echo -e "\n=== Look for the test context around line 425 (the assertion that's in question) ==="
sed -n '350,430p' tests/api/my-usage-readonly.test.ts | cat -n

Repository: ding113/claude-code-hub

Length of output: 10503


🏁 Script executed:

# Let me look at the exact test lines 425-475 (the quotaWindows assertion part)
sed -n '425,480p' tests/api/my-usage-readonly.test.ts | cat -n

Repository: ding113/claude-code-hub

Length of output: 2232


🏁 Script executed:

# Check the second test mentioned in the review (lines 566-644) to see if it also has this pattern
echo "=== Lines 550-600 (setup for second test) ==="
sed -n '550,600p' tests/api/my-usage-readonly.test.ts | cat -n

echo -e "\n=== Lines 600-650 (assertions for second test) ==="
sed -n '600,650p' tests/api/my-usage-readonly.test.ts | cat -n

Repository: ding113/claude-code-hub

Length of output: 4187


🏁 Script executed:

# Check the response schema/interface to see how keyLimitTotalUsd and userLimitTotalUsd are defined
echo "=== Search for MyUsageResponse or similar response type definition ==="
rg -nP -A30 'interface MyUsageResponse|type MyUsageResponse|interface.*[Qq]uota.*[Rr]esponse' src/actions/my-usage.ts

echo -e "\n=== Check if there's any post-processing of effectiveTotal.limit ==="
sed -n '700,750p' src/actions/my-usage.ts | cat -n

Repository: ding113/claude-code-hub

Length of output: 2361


🏁 Script executed:

# Look for where keyLimitTotalUsd and userLimitTotalUsd are added to the response
echo "=== Full response object assembly (search for keyLimitTotalUsd in response) ==="
rg -nP -B3 -A3 'keyLimitTotalUsd|userLimitTotalUsd' src/actions/my-usage.ts

echo -e "\n=== Check if there's a fallback happening in the Key/User objects themselves ==="
echo "Looking for computed properties or getters in Key/User interfaces..."
rg -nP -B2 -A10 'get limitTotalUsd|limitTotalUsd.*=' src/types/key.ts src/types/user.ts | head -80

Repository: ding113/claude-code-hub

Length of output: 2381


🏁 Script executed:

# Search for where the fallback logic might be - in repository findById or similar
echo "=== Look for findKeyById and how limitTotalUsd is handled ==="
rg -nP -B5 -A15 'findKeyById|function findById' src/repository/key.ts | head -100

echo -e "\n=== Look for user findById ==="
rg -nP -B5 -A15 'findUserById|function findById' src/repository/user.ts | head -100

Repository: ding113/claude-code-hub

Length of output: 3364


🏁 Script executed:

# Check if toKey or toUser transformers add fallback logic
echo "=== Check toKey and toUser transformers ==="
rg -nP -B5 -A30 'function toKey|const toKey = ' src/repository/_shared/transformers.ts

echo -e "\n=== Check toUser transformer ==="
rg -nP -B5 -A30 'function toUser|const toUser = ' src/repository/_shared/transformers.ts

Repository: ding113/claude-code-hub

Length of output: 3403


🏁 Script executed:

# Get the full toUser transformer to see limitTotalUsd handling
echo "=== Full toUser transformer (lines 12-60) ==="
sed -n '12,75p' src/repository/_shared/transformers.ts | cat -n

Repository: ding113/claude-code-hub

Length of output: 3574


🏁 Script executed:

# Look more carefully - maybe the fallback happens in the test expectations
# Let me check if there's some logic that makes limitTotalUsd fall back to limitMonthlyUsd
echo "=== Search for any 'limitMonthlyUsd' reference near 'limitTotalUsd' fallback context ==="
rg -nP -C5 'limitTotalUsd.*limitMonthlyUsd|limitMonthlyUsd.*limitTotalUsd' src/

echo -e "\n=== Check the learning note from NieiR about limitTotalUsd ==="
echo "The learning said: limitTotalUsd preserves null to match database schema"
echo "Let me verify if the test comment says 'should fallback'"

echo -e "\n=== Check the test description/comment ==="
sed -n '560,570p' tests/api/my-usage-readonly.test.ts | cat -n

Repository: ding113/claude-code-hub

Length of output: 819


🏁 Script executed:

# The test description says "总额度缺失时应回退 monthly" which means "should fallback to monthly when total is missing"
# But the implementation doesn't seem to have this fallback. Let me check if maybe there's 
# a computed property or getter that does this fallback

echo "=== Check Key type for any computed properties ==="
cat src/types/key.ts | cat -n

echo -e "\n=== Check User type for any computed properties ==="
cat src/types/user.ts | cat -n

Repository: ding113/claude-code-hub

Length of output: 10638


Test 中的 limitTotalUsd 断言与实现不符,且存在逻辑矛盾

第一个测试(第 425-475 行)的矛盾

测试未为 user/key 设置 limitTotalUsd,仅设置了 limitMonthlyUsd(user: 35,key: 40)。但随后的断言相互矛盾:

  • Line 430:expect(quotaData.limitTotalUsd).toBe(35); ← 期望值为 35
  • Lines 469-474:quotaWindows.total.limitUsd: null ← 期望值为 null

这两个断言由同一数据源 effectiveTotal.limit 派生,无法同时为 35 和 null。

第二个测试(第 566-644 行)期望的回退逻辑不存在

测试名称明确:「总额度缺失时应回退 monthly」,并在以下行断言:

  • Line 621:expect(quotaData.keyLimitTotalUsd).toBe(7);
  • Line 622:expect(quotaData.userLimitTotalUsd).toBe(9);
  • Line 623:expect(quotaData.limitTotalUsd).toBe(7);

此测试将 limitTotalUsd 设置为 null,但期望回退到 limitMonthlyUsd(key: 7,user: 9)。

然而,实现中不存在这个回退逻辑

src/actions/my-usage.ts 第 625-647 行:

const keyLimitTotalUsd = key.limitTotalUsd ?? null;        // 无月度回退
const userLimitTotalUsd = user.limitTotalUsd ?? null;      // 无月度回退
const effectiveTotal = resolveEffectiveQuotaWindow([
  { limit: keyLimitTotalUsd, used: keyTotalCost },
  { limit: userLimitTotalUsd, used: userTotalCost },
]);

resolveEffectiveQuotaWindow 会在两个候选值都为 null 时返回 limit: null,不会回退到 monthly。同样,transformers 中也不存在这个回退。

需要确认

  1. 是否应该为 test 第 425-475 行修正断言(将 line 430 改为 .toBeNull(),或将 total 的 limitUsd 改为 35)?
  2. 是否应该在 my-usage.ts 中添加「limitTotalUsd 缺失时回退到 limitMonthlyUsd」的逻辑来满足 test 第二部分的期望?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/api/my-usage-readonly.test.ts` around lines 425 - 475, The test
assertions for total quota are inconsistent with the implementation: either
change the tests or implement a fallback so total limits fall back to monthly
when total is missing. Update src/actions/my-usage.ts by changing how
keyLimitTotalUsd and userLimitTotalUsd are derived (currently using
key.limitTotalUsd ?? null and user.limitTotalUsd ?? null) to prefer
limitTotalUsd then fallback to limitMonthlyUsd (e.g., keyLimitTotalUsd =
key.limitTotalUsd ?? key.limitMonthlyUsd ?? null; same for userLimitTotalUsd)
before calling resolveEffectiveQuotaWindow; then run tests and adjust the
failing assertion in tests/api/my-usage-readonly.test.ts only if you
intentionally want the other behavior (make quotaData.limitTotalUsd null instead
of 35) to keep test and implementation consistent.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 50ec0145f5

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/actions/my-usage.ts
Comment on lines +918 to +921
logs: redactReadonlyLogs(
mapMyUsageLogEntries(result, settings.billingModelSource),
session.key
),
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Strip endpoint filter for readonly log requests

The new endpoint redaction is applied only after fetching logs, so a readonly caller can still infer which endpoints were used by probing endpoint filters and comparing total/result size. In getMyUsageLogs (and similarly getMyUsageLogsBatch), filters.endpoint is sent to the query before redactReadonlyLogs(...) nulls the field, which defeats the intended endpoint concealment for readonly access paths introduced in this change.

Useful? React with 👍 / 👎.

Comment on lines +620 to +622
expect(quotaData.keyLimitTotalUsd).toBe(7);
expect(quotaData.userLimitTotalUsd).toBe(9);
expect(quotaData.limitTotalUsd).toBe(7);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Fix total-limit assertions to match null-limit behavior

These assertions expect monthly fallback values for total limits even though this implementation computes total-window limits directly from limitTotalUsd and returns null when both key/user totals are unset. With DSN enabled, this test will fail against the current production logic and create false negatives in DB-backed runs; the expectations should be null unless total→monthly fallback is actually implemented.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

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 (2)
src/actions/my-usage.ts (1)

1003-1066: ⚠️ Potential issue | 🟠 Major

只读 key 仍可通过 endpoint 过滤参数与 getMyAvailableEndpoints 探测端点信息

getMyUsageLogsBatchFull(Line 1003-1015)以及 getMyUsageLogs(Line 909)/getMyUsageLogsBatch(Line 957)会把调用方传入的 endpoint 过滤参数透传到底层查询,对只读 key 同样生效;同时 getMyAvailableEndpoints(Line 1048-1066)直接返回 getDistinctEndpointsForKey() 的完整端点列表,不做任何只读脱敏。

这意味着:即便响应里 endpoint 字段已被置 null,持有只读 key 的调用方仍可:

  1. 直接调用 getMyAvailableEndpoints 拿到所有访问过的端点;或
  2. 反复带不同 endpoint 参数请求 list 接口,根据返回数量推断该 key 是否在该端点上有流量。

如果 readonly redaction 的目标是阻止只读 token 泄漏端点信息,这两条路径会让响应侧的 endpoint: null 失效。建议在 isReadonlyKey(session.key) 为真时:

  • getMyUsageLogs / getMyUsageLogsBatch / getMyUsageLogsBatchFull 中忽略 filters.endpoint(或在执行前置空);
  • getMyAvailableEndpoints 中直接返回 [](或返回到长度上的 sentinel)。
建议改造(示意)
+import { isReadonlyKey } from "@/lib/auth/readonly-access";
+
 export async function getMyAvailableEndpoints(): Promise<ActionResult<string[]>> {
   const tError = await getErrorTranslator();
   try {
     const session = await getSession({ allowReadOnlyAccess: true });
     if (!session) {
       return { ok: false, error: tError("UNAUTHORIZED"), errorCode: ERROR_CODES.UNAUTHORIZED };
     }

+    if (isReadonlyKey(session.key)) {
+      return { ok: true, data: [] };
+    }
+
     const endpoints = await getDistinctEndpointsForKey(session.key.key);
     return { ok: true, data: endpoints };

getMyUsageLogs / getMyUsageLogsBatch / getMyUsageLogsBatchFull 内类似处理:

-    const result = await findUsageLogsForKeySlim({
+    const effectiveEndpoint = isReadonlyKey(session.key) ? undefined : filters.endpoint;
+    const result = await findUsageLogsForKeySlim({
       keyString: session.key.key,
       sessionId: filters.sessionId,
       startTime: dateRange.startTime,
       endTime: dateRange.endTime,
       model: filters.model,
       statusCode: filters.statusCode,
       excludeStatusCode200: filters.excludeStatusCode200,
-      endpoint: filters.endpoint,
+      endpoint: effectiveEndpoint,
       minRetryCount: filters.minRetryCount,
       page,
       pageSize,
     });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/actions/my-usage.ts` around lines 1003 - 1066, The issue: readonly keys
can still probe endpoint info because endpoint filters are honored and
getMyAvailableEndpoints returns all endpoints; fix by detecting readonly keys
(use isReadonlyKey(session.key) after getSession) and when true: clear/ignore
any incoming endpoint filter before calling
findReadonlyUsageLogsBatchForKey/findUsage functions in getMyUsageLogs,
getMyUsageLogsBatch, and getMyUsageLogsBatchFull, and change
getMyAvailableEndpoints to return an empty array (or length-sentinel) instead of
calling getDistinctEndpointsForKey when isReadonlyKey(session.key) is true;
ensure you reference getSession, isReadonlyKey, getDistinctEndpointsForKey,
findReadonlyUsageLogsBatchForKey and the three getMyUsageLogs* functions when
making the changes.
src/app/v1/_lib/proxy/session.ts (1)

33-40: ⚠️ Potential issue | 🟡 Minor

4 个测试文件的 AuthState 构造缺少 readonlyAccess 字段,需要补充

虽然 auth-guard.ts 中的构造点已正确设置 readonlyAccess 字段,但以下测试文件仍缺少此字段:

  • tests/unit/proxy/pricing-no-price.test.ts (第 179 行)
  • tests/unit/proxy/response-handler-lease-decrement.test.ts (第 238 行)
  • tests/unit/proxy/response-handler-endpoint-circuit-isolation.test.ts (第 158 行)
  • tests/integration/billing-model-source.test.ts (第 225 行)

由于 tsconfig.json 排除了 tests/** 目录的类型检查,这些遗漏未被静态检查发现。所有 setAuthState() 调用需要添加 readonlyAccess 字段,保持与接口定义一致。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/app/v1/_lib/proxy/session.ts` around lines 33 - 40, Update all test calls
that create an AuthState object via setAuthState to include the missing
readonlyAccess boolean field so the test fixtures match the AuthState interface;
specifically, locate the test setups in pricing-no-price.test,
response-handler-lease-decrement.test,
response-handler-endpoint-circuit-isolation.test and billing-model-source.test
and add readonlyAccess (true or false as appropriate for the scenario) to the
AuthState literal, ensuring consistency with the auth-guard.ts construction and
the AuthState interface definition.
♻️ Duplicate comments (2)
tests/api/my-usage-readonly.test.ts (2)

566-644: ⚠️ Potential issue | 🔴 Critical

用例名称与断言仍按旧"回退月额度"语义编写,需对齐新行为

  • 用例名"总额度缺失时应回退 monthly"描述的是旧行为;
  • Line 620-622:keyLimitTotalUsd → 7userLimitTotalUsd → 9limitTotalUsd → 7 — 这些都是把 limitMonthlyUsd 当成 limitTotalUsd 的回退。

src/actions/my-usage.ts Line 626-627 现在严格使用 key.limitTotalUsd ?? null,没有 monthly fallback,因此实际返回值会是 null/null/null。该用例同样会在 DSN 配置好的环境下失败。

建议改名并更新断言:

建议改造
-  test("总额度缺失时应回退 monthly,且只读日志隐藏 endpoint", async () => {
+  test("总额度缺失时 total 窗口为 unlimited,且只读日志隐藏 endpoint", async () => {
@@
-    expect(quotaData.keyLimitTotalUsd).toBe(7);
-    expect(quotaData.userLimitTotalUsd).toBe(9);
-    expect(quotaData.limitTotalUsd).toBe(7);
-    expect(quotaData.remainingTotalUsd).toBeTypeOf("number");
+    expect(quotaData.keyLimitTotalUsd).toBeNull();
+    expect(quotaData.userLimitTotalUsd).toBeNull();
+    expect(quotaData.limitTotalUsd).toBeNull();
+    expect(quotaData.remainingTotalUsd).toBeNull();
+    expect((quotaData.quotaWindows as Record<string, { isUnlimited: boolean }>).total.isUnlimited).toBe(true);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/api/my-usage-readonly.test.ts` around lines 566 - 644, The test title
and assertions still assume the old "fallback to monthly" behavior; update the
test name string and assertions to reflect that key.limitTotalUsd and
user.limitTotalUsd no longer fall back to monthly. Rename the test description
(the string passed to test(...)) to indicate no monthly fallback, and change the
assertions referencing quotaData.keyLimitTotalUsd, quotaData.userLimitTotalUsd,
and quotaData.limitTotalUsd to expect null (instead of 7/9/7); keep the other
expectations (readonlyRedactedFields, userAllowedModels/Clients,
remainingTotalUsd type check, and log endpoint null) as-is. Ensure you update
the test that constructs quotaData (the variable quotaData) and its
corresponding expect(...) calls so they match the new behavior.

426-478: ⚠️ Potential issue | 🔴 Critical

断言相互矛盾:limitTotalUsdquotaWindows.total.limitUsd 取自同一来源,不可能同时为 35 和 null

实现里(src/actions/my-usage.ts Line 715)limitTotalUsd 直接赋值为 quotaWindows.total.limitUsd,二者必然相等。

  • Line 430:expect(quotaData.limitTotalUsd).toBe(35); — 期望走"月额度回退"
  • Line 469-474:total: { ..., limitUsd: null, ..., remainingUsd: null } — 期望"无总额度限制"

而本用例的 setup(Line 352-372)只设置了 limitMonthlyUsd,没有设置 limitTotalUsd。按 PR 现行的"total 不再回退到 monthly"的修订,quotaWindows.total.limitUsd 应当为 null,因此 Line 430 的 toBe(35) 会失败。

这条断言应是先前 fallback 行为的遗留,建议更新:

建议改造
-    expect(quotaData.limitTotalUsd).toBe(35);
+    expect(quotaData.limitTotalUsd).toBeNull();
+    expect(quotaData.remainingTotalUsd).toBeNull();
+    expect(quotaData.usedTotalUsd).toBeTypeOf("number");

由于该 describedescribe.skipIf(!process.env.DSN) 包裹,CI 默认未配置 DSN 时会被跳过,可能因此本地未捕获到这次回归。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/api/my-usage-readonly.test.ts` around lines 426 - 478, The test
contains contradictory assertions: quotaData.limitTotalUsd is asserted as 35
while quotaWindows.total.limitUsd is asserted as null, but limitTotalUsd is
derived from quotaWindows.total.limitUsd (see assignment to limitTotalUsd).
Update the test to reflect the new behavior by changing the assertion for
quotaData.limitTotalUsd to expect null (or match quotaWindows.total.limitUsd)
and remove any legacy fallback expectation to 35 so quotaData.limitTotalUsd and
quotaWindows.total.limitUsd remain consistent.
🧹 Nitpick comments (7)
public/examples/api-key-quota-extractor-compatible.js (1)

79-79: remainingDailyUsd || 0 / remaining5hUsd || 0 在不限额时语义略糊。

当对应窗口不限额(limitDailyUsd / limit5hUsdnull)时,remainingDailyUsd / remaining5hUsd 也为 null,经过 || 0 后传入 percent(0, null),依赖 percent 内部 total > 0 守卫返回 null,目前结果正确但耦合较紧。如果后续修改 percent 的判定(例如改用 != null),就会得到 0% 而非 -。建议直接传 null 让守卫显式处理,更稳健也更易读。

♻️ 建议
-      todayRemainingPercent: toNumber(
-        data.todayRemainingPercent,
-        toNumber(daily.remainingPercent, percent(remainingDailyUsd || 0, limitDailyUsd))
-      ),
+      todayRemainingPercent: toNumber(
+        data.todayRemainingPercent,
+        toNumber(daily.remainingPercent, percent(remainingDailyUsd ?? 0, limitDailyUsd)),
+      ),
@@
-      extra: "5H剩余:" + formatPercent(toNumber(fiveHour.remainingPercent, percent(remaining5hUsd || 0, limit5hUsd)))
-        + "/日剩余:" + formatPercent(toNumber(daily.remainingPercent, percent(remainingDailyUsd || 0, limitDailyUsd)))
+      extra: "5H剩余:" + formatPercent(toNumber(fiveHour.remainingPercent, percent(remaining5hUsd ?? 0, limit5hUsd)))
+        + "/日剩余:" + formatPercent(toNumber(daily.remainingPercent, percent(remainingDailyUsd ?? 0, limitDailyUsd)))

Also applies to: 87-88

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@public/examples/api-key-quota-extractor-compatible.js` at line 79, The call
sites using toNumber(daily.remainingPercent, percent(remainingDailyUsd || 0,
limitDailyUsd)) (and the similar uses for remaining5hUsd/limit5hUsd) rely on `||
0` which masks the "unlimited" case and couples behavior to `percent`'s internal
guard; change these call sites to pass null instead of 0 when the corresponding
limit is null (i.e., compute percent(remainingDailyUsd, limitDailyUsd) or
percent(remaining5hUsd, limit5hUsd) and ensure you only coerce toNumber as
before), so the `percent` function can explicitly handle null limits and return
the intended sentinel instead of producing 0%.
docs/examples/api-key-quota-extractor.js (1)

1-67: 建议补齐对象字面量的尾随逗号以符合 Biome 规范。

按仓库编码规范(Biome:double quotes / trailing commas / 2-space indent / 100 字符宽度),多处对象字面量末尾缺少尾随逗号,例如第 8 行("User-Agent": "cc-switch/1.0" 后)、第 10 行(body: "{}" 后)、第 64 行(extra: ... 后)、第 66 行(extractor 函数体所在属性后)等。建议统一补齐,避免后续 biome check / 格式化报错。

♻️ 建议的修改示例
     headers: {
       "Authorization": "Bearer {{apiKey}}",
       "Content-Type": "application/json",
-      "User-Agent": "cc-switch/1.0"
+      "User-Agent": "cc-switch/1.0",
     },
-    body: "{}"
+    body: "{}",
   },
       remainingDaily: toNumber(daily.remainingUsd, toNumber(data.remainingDailyUsd, null)),
       extra: "5H剩余:" + formatPercent(fiveHour.remainingPercent)
         + "/日剩余:" + formatPercent(toNumber(daily.remainingPercent, data.todayRemainingPercent))
         + "/周剩余:" + formatPercent(weekly.remainingPercent)
         + "/月剩余:" + formatPercent(monthly.remainingPercent)
-        + "/总剩余:" + formatPercent(toNumber(total.remainingPercent, data.remainingPercent))
+        + "/总剩余:" + formatPercent(toNumber(total.remainingPercent, data.remainingPercent)),
     };
-  }
+  },
 })

As per coding guidelines: "Code must be formatted according to Biome standards: double quotes, trailing commas, 2-space indent, 100 character width".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/examples/api-key-quota-extractor.js` around lines 1 - 67, The object
literals in this file (notably the request.headers object with "User-Agent", the
request object after body, the returned object from extractor including the
extra property and the extractor property itself) are missing trailing commas
which violates Biome formatting rules; add trailing commas to all multiline
object and array literals and ensure double quotes and 2-space indentation are
preserved (e.g., update the request.headers block, the outer request object, and
the returned object inside extractor such as the extra, remainingTotal,
remaining5h, remainingDaily, and final extractor property) so biome
check/formatting passes.
src/actions/my-usage.ts (2)

333-353: usedPercentremainingPercent 使用不同的分母/分子精度

usedPercent 基于未圆整的 window.used 与已圆整的 limitUsd 计算,remainingPercent 基于已圆整的 remainingUsd 与已圆整的 limitUsd 计算。这会导致 usedPercent + remainingPercent 在多数场景下不精确等于 100(相差 0.01 量级)。如果客户端做强等式校验会出现毛刺。

建议统一使用 usedUsd(即 round 后的值)作为分子:

建议改造
-    usedPercent: hasPositiveLimit ? round2((window.used / limitUsd) * 100) : null,
+    usedPercent: hasPositiveLimit ? round2((usedUsd / limitUsd) * 100) : null,

或者保留当前实现并在文档/Schema 中明确"两者之和并非恒为 100"。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/actions/my-usage.ts` around lines 333 - 353, In buildQuotaWindow the
usedPercent calculation mixes unrounded window.used with rounded limitUsd,
causing small inconsistencies vs remainingPercent; change usedPercent to use the
rounded usedUsd as the numerator (i.e. round2(window.used) already stored in
usedUsd) and compute usedPercent as hasPositiveLimit ? round2((usedUsd /
limitUsd) * 100) : null, keeping the existing hasPositiveLimit and rounding
logic and similarly ensure remainingPercent stays using remainingUsd/limitUsd so
both percentages use the same rounded denominator and consistent numerators (and
preserve null/zero checks and isUnlimited/isExhausted behavior).

733-741: resetMode/resetTimedailyResetMode/dailyResetTime 重复

resetMode/resetTime(Line 736-737)与上方的 dailyResetMode/dailyResetTime(Line 734-735)取自同一字段,但在响应里成对暴露,命名隐式让人误以为是某种"周期级"或"总额度级"的重置配置。

如果这是为了对外契约的稳定性(例如对应另一类客户端字段),建议加一行注释说明它是 daily 重置的别名;否则建议在新的字段层面收敛或者暴露真正与 5h/weekly/monthly 对应的重置时间。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/actions/my-usage.ts` around lines 733 - 741, The response currently
exposes duplicate fields resetMode/resetTime and dailyResetMode/dailyResetTime
(they both derive from key.dailyResetMode/key.dailyResetTime) which is
confusing; update the code that builds this object (the block setting expiresAt,
dailyResetMode, dailyResetTime, resetMode, resetTime, remaining, unit — and the
function resolveOverallRemaining/quotaWindows nearby) to either (A) remove the
redundant resetMode/resetTime properties and only expose
dailyResetMode/dailyResetTime, or (B) keep resetMode/resetTime but add an inline
comment in the returned object indicating they are aliases of
dailyResetMode/dailyResetTime for backward-compatibility, or (C) if you intend
multi-period resets, compute and populate resetMode/resetTime from the actual
period fields instead of key.dailyResetMode/key.dailyResetTime; pick one option
and implement it consistently so the API surface is not ambiguous.
src/lib/my-usage/readonly-redaction.ts (1)

13-19: 可选:将脱敏字段列表抽取为常量,避免重复字面量

readonlyRedactedFields 中列出的字段名与上面赋空数组的字段名是同一来源,未来新增脱敏字段时需要在两处同步修改,容易出现漏改导致客户端误判。

建议改造
+const READONLY_QUOTA_REDACTED_FIELDS = ["userAllowedModels", "userAllowedClients"] as const;
+
 export function redactReadonlyQuota<T extends MyUsageQuota>(
   quota: T,
   key: Pick<Key, "canLoginWebUi">
 ): T {
   if (!isReadonlyKey(key)) {
     return quota;
   }

   return {
     ...quota,
     userAllowedModels: [],
     userAllowedClients: [],
-    readonlyRedactedFields: ["userAllowedModels", "userAllowedClients"],
+    readonlyRedactedFields: [...READONLY_QUOTA_REDACTED_FIELDS],
   };
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/lib/my-usage/readonly-redaction.ts` around lines 13 - 19, Extract the
repeated field names into a single constant (e.g., REDACTED_FIELDS) and use that
constant both to set the empty arrays for userAllowedModels and
userAllowedClients and to populate readonlyRedactedFields; specifically, create
a const (array of "userAllowedModels" and "userAllowedClients") near the top of
the module, replace the inline literal arrays assigned to
userAllowedModels/userAllowedClients with references to that constant (or copies
if mutation is a concern), and set readonlyRedactedFields to that same constant
so the source of truth for redacted field names is centralized.
tests/unit/actions/my-usage-concurrent-inherit.test.ts (1)

113-223: 测试覆盖到位,但用例间 mock 状态可能相互污染

新增的两个用例正确反映了"总额度不再回退到月额度"的行为:当 keyLimitTotalUsd/userLimitTotalUsd 均为 nullquotaWindows.total.isUnlimited === true,存在显式总额度时按更严格的一方返回。

不过,vi.clearAllMocks() 仅清除调用历史,并不会重置由 mockResolvedValue 注入的实现。Line 171-184 在第一个新增用例里把 sumKeyQuotaCostsById / sumUserQuotaCosts 改成了非零成本,这些实现会泄漏到后面"存在总额度时…"用例(Line 202-223)。当前后者只校验 limitTotalUsd,所以仍然通过;但若以后扩展为校验 used/remaining,则容易随用例顺序漂移。建议在 beforeEach 里追加:

建议改造
   beforeEach(() => {
     vi.clearAllMocks();
+    // 重置成本聚合 mock 的实现,避免用例间相互影响
+    statisticsMock.sumKeyQuotaCostsById.mockResolvedValue({
+      cost5h: 0,
+      costDaily: 0,
+      costWeekly: 0,
+      costMonthly: 0,
+      costTotal: 0,
+    });
+    statisticsMock.sumUserQuotaCosts.mockResolvedValue({
+      cost5h: 0,
+      costDaily: 0,
+      costWeekly: 0,
+      costMonthly: 0,
+      costTotal: 0,
+    });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/unit/actions/my-usage-concurrent-inherit.test.ts` around lines 113 -
223, The tests leak mock implementations between cases because
vi.clearAllMocks() only clears call history; add a reset of mock implementations
in the beforeEach for this suite by either calling vi.resetAllMocks() (to fully
reset mocks) or explicitly re-stubbing statisticsMock.sumKeyQuotaCostsById and
statisticsMock.sumUserQuotaCosts to their default resolved values (e.g., zeros)
after getSessionMock is set; target symbols: beforeEach, vi.clearAllMocks,
vi.resetAllMocks, statisticsMock.sumKeyQuotaCostsById,
statisticsMock.sumUserQuotaCosts, getSessionMock.
tests/api/my-usage-readonly.test.ts (1)

433-435: 百分比断言隐式依赖了 usedPercentremainingPercent 的不对称舍入

todayUsedPercent 期望 0.07todayRemainingPercent 期望 99.93,恰好之和为 100。但 buildQuotaWindowusedPercent 用未圆整的 window.used(0.01)做分子,remainingPercent 用已圆整的 remainingUsd(14.99)做分子。由于 0.01/15*100 ≈ 0.0666... 圆整为 0.0714.99/15*100 ≈ 99.9333... 圆整为 99.93,本用例下恰好仍合 100。

如果按我在 src/actions/my-usage.ts:333-353 评论中建议把 usedPercent 也换成圆整后的 usedUsd 作分子,0.01/15*100 则圆整为 0.07,等式仍然成立;建议两侧同时调整,避免后续出现"usedPercent + remainingPercent ≠ 100"的诡异场景。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/api/my-usage-readonly.test.ts` around lines 433 - 435, 在
buildQuotaWindow 中修复百分比不对称舍入:不要用未四舍五入的 window.used 计算
usedPercent,而是先按现有逻辑生成已四舍五入的 usedUsd(与 remainingUsd 一致的舍入策略),然后用 usedUsd 作为
usedPercent 的分子计算百分比(remainingPercent 继续用 remainingUsd),确保 usedPercent 与
remainingPercent 的分子都来自相同的舍入结果以避免两者之和不等于
100(相关标识符:buildQuotaWindow、usedPercent、remainingPercent、usedUsd、remainingUsd、window.used)。
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@public/examples/api-key-quota-extractor-compatible.js`:
- Around line 47-50: The code currently falls back to monthly values when total
fields are null (e.g., limitTotalUsd = toNumber(data.limitTotalUsd,
limitMonthlyUsd)), which causes quotaWindows.total with isUnlimited:true to be
polluted by monthly limits; update the logic so that when quotaWindows (and
specifically quotaWindows.total / total.limitUsd, total.remainingUsd,
total.usedUsd) is present you respect its null/isUnlimited semantics and do NOT
pass monthly fallbacks into toNumber; only use the old monthly fallback (e.g.,
limitMonthlyUsd) when quotaWindows is entirely missing/undefined — change uses
of toNumber(data.limitTotalUsd, limitMonthlyUsd) and the related
toNumber(total.limitUsd, limitTotalUsd) calls to first check for the presence of
quotaWindows.total (or total.isUnlimited) and skip the fallback if
quotaWindows.total exists.

---

Outside diff comments:
In `@src/actions/my-usage.ts`:
- Around line 1003-1066: The issue: readonly keys can still probe endpoint info
because endpoint filters are honored and getMyAvailableEndpoints returns all
endpoints; fix by detecting readonly keys (use isReadonlyKey(session.key) after
getSession) and when true: clear/ignore any incoming endpoint filter before
calling findReadonlyUsageLogsBatchForKey/findUsage functions in getMyUsageLogs,
getMyUsageLogsBatch, and getMyUsageLogsBatchFull, and change
getMyAvailableEndpoints to return an empty array (or length-sentinel) instead of
calling getDistinctEndpointsForKey when isReadonlyKey(session.key) is true;
ensure you reference getSession, isReadonlyKey, getDistinctEndpointsForKey,
findReadonlyUsageLogsBatchForKey and the three getMyUsageLogs* functions when
making the changes.

In `@src/app/v1/_lib/proxy/session.ts`:
- Around line 33-40: Update all test calls that create an AuthState object via
setAuthState to include the missing readonlyAccess boolean field so the test
fixtures match the AuthState interface; specifically, locate the test setups in
pricing-no-price.test, response-handler-lease-decrement.test,
response-handler-endpoint-circuit-isolation.test and billing-model-source.test
and add readonlyAccess (true or false as appropriate for the scenario) to the
AuthState literal, ensuring consistency with the auth-guard.ts construction and
the AuthState interface definition.

---

Duplicate comments:
In `@tests/api/my-usage-readonly.test.ts`:
- Around line 566-644: The test title and assertions still assume the old
"fallback to monthly" behavior; update the test name string and assertions to
reflect that key.limitTotalUsd and user.limitTotalUsd no longer fall back to
monthly. Rename the test description (the string passed to test(...)) to
indicate no monthly fallback, and change the assertions referencing
quotaData.keyLimitTotalUsd, quotaData.userLimitTotalUsd, and
quotaData.limitTotalUsd to expect null (instead of 7/9/7); keep the other
expectations (readonlyRedactedFields, userAllowedModels/Clients,
remainingTotalUsd type check, and log endpoint null) as-is. Ensure you update
the test that constructs quotaData (the variable quotaData) and its
corresponding expect(...) calls so they match the new behavior.
- Around line 426-478: The test contains contradictory assertions:
quotaData.limitTotalUsd is asserted as 35 while quotaWindows.total.limitUsd is
asserted as null, but limitTotalUsd is derived from quotaWindows.total.limitUsd
(see assignment to limitTotalUsd). Update the test to reflect the new behavior
by changing the assertion for quotaData.limitTotalUsd to expect null (or match
quotaWindows.total.limitUsd) and remove any legacy fallback expectation to 35 so
quotaData.limitTotalUsd and quotaWindows.total.limitUsd remain consistent.

---

Nitpick comments:
In `@docs/examples/api-key-quota-extractor.js`:
- Around line 1-67: The object literals in this file (notably the
request.headers object with "User-Agent", the request object after body, the
returned object from extractor including the extra property and the extractor
property itself) are missing trailing commas which violates Biome formatting
rules; add trailing commas to all multiline object and array literals and ensure
double quotes and 2-space indentation are preserved (e.g., update the
request.headers block, the outer request object, and the returned object inside
extractor such as the extra, remainingTotal, remaining5h, remainingDaily, and
final extractor property) so biome check/formatting passes.

In `@public/examples/api-key-quota-extractor-compatible.js`:
- Line 79: The call sites using toNumber(daily.remainingPercent,
percent(remainingDailyUsd || 0, limitDailyUsd)) (and the similar uses for
remaining5hUsd/limit5hUsd) rely on `|| 0` which masks the "unlimited" case and
couples behavior to `percent`'s internal guard; change these call sites to pass
null instead of 0 when the corresponding limit is null (i.e., compute
percent(remainingDailyUsd, limitDailyUsd) or percent(remaining5hUsd, limit5hUsd)
and ensure you only coerce toNumber as before), so the `percent` function can
explicitly handle null limits and return the intended sentinel instead of
producing 0%.

In `@src/actions/my-usage.ts`:
- Around line 333-353: In buildQuotaWindow the usedPercent calculation mixes
unrounded window.used with rounded limitUsd, causing small inconsistencies vs
remainingPercent; change usedPercent to use the rounded usedUsd as the numerator
(i.e. round2(window.used) already stored in usedUsd) and compute usedPercent as
hasPositiveLimit ? round2((usedUsd / limitUsd) * 100) : null, keeping the
existing hasPositiveLimit and rounding logic and similarly ensure
remainingPercent stays using remainingUsd/limitUsd so both percentages use the
same rounded denominator and consistent numerators (and preserve null/zero
checks and isUnlimited/isExhausted behavior).
- Around line 733-741: The response currently exposes duplicate fields
resetMode/resetTime and dailyResetMode/dailyResetTime (they both derive from
key.dailyResetMode/key.dailyResetTime) which is confusing; update the code that
builds this object (the block setting expiresAt, dailyResetMode, dailyResetTime,
resetMode, resetTime, remaining, unit — and the function
resolveOverallRemaining/quotaWindows nearby) to either (A) remove the redundant
resetMode/resetTime properties and only expose dailyResetMode/dailyResetTime, or
(B) keep resetMode/resetTime but add an inline comment in the returned object
indicating they are aliases of dailyResetMode/dailyResetTime for
backward-compatibility, or (C) if you intend multi-period resets, compute and
populate resetMode/resetTime from the actual period fields instead of
key.dailyResetMode/key.dailyResetTime; pick one option and implement it
consistently so the API surface is not ambiguous.

In `@src/lib/my-usage/readonly-redaction.ts`:
- Around line 13-19: Extract the repeated field names into a single constant
(e.g., REDACTED_FIELDS) and use that constant both to set the empty arrays for
userAllowedModels and userAllowedClients and to populate readonlyRedactedFields;
specifically, create a const (array of "userAllowedModels" and
"userAllowedClients") near the top of the module, replace the inline literal
arrays assigned to userAllowedModels/userAllowedClients with references to that
constant (or copies if mutation is a concern), and set readonlyRedactedFields to
that same constant so the source of truth for redacted field names is
centralized.

In `@tests/api/my-usage-readonly.test.ts`:
- Around line 433-435: 在 buildQuotaWindow 中修复百分比不对称舍入:不要用未四舍五入的 window.used 计算
usedPercent,而是先按现有逻辑生成已四舍五入的 usedUsd(与 remainingUsd 一致的舍入策略),然后用 usedUsd 作为
usedPercent 的分子计算百分比(remainingPercent 继续用 remainingUsd),确保 usedPercent 与
remainingPercent 的分子都来自相同的舍入结果以避免两者之和不等于
100(相关标识符:buildQuotaWindow、usedPercent、remainingPercent、usedUsd、remainingUsd、window.used)。

In `@tests/unit/actions/my-usage-concurrent-inherit.test.ts`:
- Around line 113-223: The tests leak mock implementations between cases because
vi.clearAllMocks() only clears call history; add a reset of mock implementations
in the beforeEach for this suite by either calling vi.resetAllMocks() (to fully
reset mocks) or explicitly re-stubbing statisticsMock.sumKeyQuotaCostsById and
statisticsMock.sumUserQuotaCosts to their default resolved values (e.g., zeros)
after getSessionMock is set; target symbols: beforeEach, vi.clearAllMocks,
vi.resetAllMocks, statisticsMock.sumKeyQuotaCostsById,
statisticsMock.sumUserQuotaCosts, getSessionMock.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c488f081-0df6-460e-be6a-cbe6de9698dc

📥 Commits

Reviewing files that changed from the base of the PR and between 8487473 and 50ec014.

📒 Files selected for processing (22)
  • docs/examples/api-key-quota-extractor-compatible.js
  • docs/examples/api-key-quota-extractor-daily.js
  • docs/examples/api-key-quota-extractor-direct.js
  • docs/examples/api-key-quota-extractor-total.js
  • docs/examples/api-key-quota-extractor-weekly.js
  • docs/examples/api-key-quota-extractor.js
  • public/examples/api-key-quota-extractor-compatible.js
  • public/examples/api-key-quota-extractor-direct.js
  • public/examples/api-key-quota-extractor.js
  • src/actions/my-usage.ts
  • src/app/api/actions/[...route]/route.ts
  • src/app/v1/_lib/proxy/auth-guard.ts
  • src/app/v1/_lib/proxy/session.ts
  • src/lib/api/action-adapter-openapi.ts
  • src/lib/auth.ts
  • src/lib/auth/readonly-access.ts
  • src/lib/my-usage/readonly-redaction.ts
  • src/proxy.ts
  • tests/api/my-usage-readonly.test.ts
  • tests/configs/my-usage.config.ts
  • tests/unit/actions/my-usage-concurrent-inherit.test.ts
  • tests/unit/auth/auth-scoped-session-branches.test.ts
✅ Files skipped from review due to trivial changes (5)
  • src/lib/auth/readonly-access.ts
  • docs/examples/api-key-quota-extractor-total.js
  • public/examples/api-key-quota-extractor.js
  • docs/examples/api-key-quota-extractor-direct.js
  • docs/examples/api-key-quota-extractor-weekly.js
🚧 Files skipped from review as they are similar to previous changes (7)
  • tests/configs/my-usage.config.ts
  • docs/examples/api-key-quota-extractor-daily.js
  • docs/examples/api-key-quota-extractor-compatible.js
  • public/examples/api-key-quota-extractor-direct.js
  • src/app/v1/_lib/proxy/auth-guard.ts
  • src/app/api/actions/[...route]/route.ts
  • src/lib/auth.ts

Comment on lines +47 to +50
const limitMonthlyUsd = toNumber(data.limitMonthlyUsd, null);
const limitTotalUsd = toNumber(data.limitTotalUsd, limitMonthlyUsd);
const usedTotalUsd = toNumber(data.usedTotalUsd, toNumber(data.usedMonthlyUsd, 0));
const remainingTotalUsd = limitTotalUsd === null ? null : round2(Math.max(limitTotalUsd - usedTotalUsd, 0));
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

total 在“新服务端 + total 不限额”场景会被月度上限污染。

当新版服务端返回 quotaWindows.total 且其为不限额(limitUsd/remainingUsdnullisUnlimited: true),但同时存在 limitMonthlyUsd 时:

  • 第 48 行:limitTotalUsd = toNumber(data.limitTotalUsd, limitMonthlyUsd),在 data.limitTotalUsdnull 时会回退到月度上限。
  • 第 71-73 行:toNumber(total.limitUsd, limitTotalUsd) 由于 total.limitUsdnull,最终 total/remaining/used 字段会被填成基于月度的数值。

这与本 PR 的语义(total 仅代表生命周期累计,不再回退到月度)相违背,可能让客户端把“总额不限”错误地展示为“总额=月度限额”。建议在新格式下信任 quotaWindows.totalisUnlimited 语义,仅在确实缺失 quotaWindows 时才走旧的月度回退。

♻️ 参考改法(仅在 quotaWindows 缺失时才回退)
-    const limitMonthlyUsd = toNumber(data.limitMonthlyUsd, null);
-    const limitTotalUsd = toNumber(data.limitTotalUsd, limitMonthlyUsd);
-    const usedTotalUsd = toNumber(data.usedTotalUsd, toNumber(data.usedMonthlyUsd, 0));
-    const remainingTotalUsd = limitTotalUsd === null ? null : round2(Math.max(limitTotalUsd - usedTotalUsd, 0));
+    const hasQuotaWindows = data.quotaWindows && typeof data.quotaWindows === "object";
+    const limitMonthlyUsd = toNumber(data.limitMonthlyUsd, null);
+    const limitTotalUsd = toNumber(
+      data.limitTotalUsd,
+      hasQuotaWindows ? null : limitMonthlyUsd,
+    );
+    const usedTotalUsd = toNumber(
+      data.usedTotalUsd,
+      hasQuotaWindows ? 0 : toNumber(data.usedMonthlyUsd, 0),
+    );
+    const remainingTotalUsd =
+      limitTotalUsd === null ? null : round2(Math.max(limitTotalUsd - usedTotalUsd, 0));
@@
-      remaining: toNumber(total.remainingUsd, remainingTotalUsd),
-      total: toNumber(total.limitUsd, limitTotalUsd),
-      used: toNumber(total.usedUsd, usedTotalUsd),
+      remaining: total.isUnlimited ? null : toNumber(total.remainingUsd, remainingTotalUsd),
+      total: total.isUnlimited ? null : toNumber(total.limitUsd, limitTotalUsd),
+      used: toNumber(total.usedUsd, usedTotalUsd),

Also applies to: 71-73

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@public/examples/api-key-quota-extractor-compatible.js` around lines 47 - 50,
The code currently falls back to monthly values when total fields are null
(e.g., limitTotalUsd = toNumber(data.limitTotalUsd, limitMonthlyUsd)), which
causes quotaWindows.total with isUnlimited:true to be polluted by monthly
limits; update the logic so that when quotaWindows (and specifically
quotaWindows.total / total.limitUsd, total.remainingUsd, total.usedUsd) is
present you respect its null/isUnlimited semantics and do NOT pass monthly
fallbacks into toNumber; only use the old monthly fallback (e.g.,
limitMonthlyUsd) when quotaWindows is entirely missing/undefined — change uses
of toNumber(data.limitTotalUsd, limitMonthlyUsd) and the related
toNumber(total.limitUsd, limitTotalUsd) calls to first check for the presence of
quotaWindows.total (or total.isUnlimited) and skip the fallback if
quotaWindows.total exists.

@github-actions github-actions Bot mentioned this pull request Apr 27, 2026
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Backlog

Development

Successfully merging this pull request may close these issues.

1 participant