Skip to content

fix: restore dashboard API compatibility#1145

Merged
ding113 merged 4 commits into
devfrom
fix/page-api-regressions-20260430T072940Z
Apr 30, 2026
Merged

fix: restore dashboard API compatibility#1145
ding113 merged 4 commits into
devfrom
fix/page-api-regressions-20260430T072940Z

Conversation

@ding113

@ding113 ding113 commented Apr 30, 2026

Copy link
Copy Markdown
Owner

Summary

Fixes dashboard/API regressions introduced by the v1 REST management migration where multiple dashboard pages rendered empty, threw Date-related client errors, or hit v1 validation failures.

Reproduced issues

  • Users page crashed with TypeError: S?.getTime is not a function because v1 JSON transport returned date strings while legacy dashboard components expected Date instances.
  • Provider dashboard/settings/status-page surfaces lost hidden legacy provider types (claude-auth, gemini-cli) after public v1 schemas filtered them out.
  • Provider endpoint hidden-type stats hit public batch schemas and returned 400.
  • Rate-limit dashboard returned 400 from /api/v1/dashboard/rate-limit-stats due Date values flowing through raw SQL parameters.
  • Rate-limit dashboard also hit /api/v1/users:search?limit=5000 400 because the v1 user search limit was capped below the legacy action/repository contract.

Fixes

  • Revive dashboard user date fields in the v1 action client and keep action/v1 transport JSON-safe.
  • Add dashboard compatibility handling for provider and provider-endpoint REST reads/writes while keeping hidden provider types out of public schemas.
  • Avoid public batch endpoint validation for hidden provider endpoint stats by using compat endpoint reads and local aggregation.
  • Convert rate-limit stats time filters to ISO strings with explicit ::timestamptz casts.
  • Restore /api/v1/users:search and filter-search limit support up to 5000.

Agent Browser QA

Local target: http://localhost:13500

dashboard errs 0 cErrs 0 http4+ 0
users errs 0 cErrs 0 http4+ 0
providers-dashboard errs 0 cErrs 0 http4+ 0
providers-settings errs 0 cErrs 0 http4+ 0
status-page-settings errs 0 cErrs 0 http4+ 0
availability errs 0 cErrs 0 http4+ 0
logs errs 0 cErrs 0 http4+ 0
rate-limits errs 0 cErrs 0 http4+ 0
quotas-providers errs 0 cErrs 0 http4+ 0
quotas-users errs 0 cErrs 0 http4+ 0
settings-prices errs 0 cErrs 0 http4+ 0

Local verification

  • bunx vitest run tests/unit/users-action-get-users-compat.test.ts tests/unit/api/v1/schema-serialization.test.ts tests/unit/api/v1/api-client-actions.test.ts tests/api/v1/users/users.test.ts tests/api/v1/providers/providers.read.test.ts tests/api/v1/provider-endpoints/provider-endpoints.test.ts tests/api/v1/dashboard/dashboard.test.ts tests/unit/repository/rate-limit-stats-query.test.ts --reporter=verbose
  • bun run build
  • bun run lint
  • bun run lint:fix
  • bun run lint
  • bun run typecheck
  • bun run test

Note: bun run build still prints existing Next/Turbopack warnings about node:net imports in Edge runtime traces, but the build exits successfully.

Greptile Summary

This PR fixes a set of dashboard/API regressions introduced by the v1 REST management migration: user date fields crashing dashboard components, hidden provider types (claude-auth, gemini-cli) disappearing from provider/endpoint surfaces, rate-limit stats returning 400 due to raw Date SQL parameters, and user search limit caps blocking legacy callers. The core strategy is a X-CCH-Dashboard-Compat: 1 request header (admin-only) that unlocks internal schemas server-side, while the client action layer adds the header automatically and revives ISO date strings back to Date instances.

Confidence Score: 5/5

Safe to merge; all findings are P2 style/quality suggestions with no present runtime defects.

Only P2 findings: a type-lie in toRequiredActionTransportDate (no runtime crash because the client handles null) and the 5000-user search limit being wider than the dashboard-only intent. The compat-header approach is properly gated behind admin role checks, the Drizzle migration for stats queries is correct, and date normalization logic is well-covered by tests.

src/lib/api/v1/schemas/users.ts — limit increase affects both public search endpoints, not just the dashboard path.

Important Files Changed

Filename Overview
src/actions/users.ts Adds toActionTransportUserDisplay to serialize Date fields to ISO strings for JSON transport; toRequiredActionTransportDate lies about its Date return type and can actually return null.
src/app/api/v1/resources/provider-endpoints/handlers.ts Adds isDashboardCompatRequest guard and internal schemas to allow hidden provider types through the compat header path; logic is consistent and admin-gated.
src/app/api/v1/resources/providers/handlers.ts Mirrors compat-header approach from provider-endpoints; adds early return in loadVisibleProviders for dashboard requests; pre-existing HIDDEN_PROVIDER_TYPES.has() call is unchanged.
src/lib/api-client/v1/actions/users.ts Adds normalizeUsers/normalizeDate to revive JSON date strings to Date instances on the client; handles null, string, and numeric epoch inputs correctly.
src/repository/statistics.ts Migrates raw SQL string concatenation to Drizzle ORM DSL; ISO string + ::timestamptz cast correctly avoids passing raw Date objects through the parameterized query layer.
src/lib/api/v1/schemas/users.ts Raises UserFilterSearchQuerySchema limit from 100 to 5000 — shared by both :search and :filter-search endpoints, opening large unbounded fetches to any admin caller.
src/lib/api/v1/_shared/serialization.ts Extends serializeDates to handle Date-like objects via duck typing and guard invalid dates in dateToIsoString; toJSON recursion guard (jsonValue !== value) prevents simple cycles.
src/lib/api/v1/_shared/constants.ts Adds DASHBOARD_COMPAT_HEADER, PUBLIC_PROVIDER_TYPE_VALUES, and INTERNAL_PROVIDER_TYPE_VALUES; consolidates provider type enumeration cleanly.

Sequence Diagram

sequenceDiagram
    participant Dashboard as Dashboard (browser)
    participant ActionClient as API Client (v1 actions)
    participant Server as Hono Route Handler
    participant Action as Server Action
    participant DB as Database

    Dashboard->>ActionClient: getProviders() / getProviderEndpoints()
    ActionClient->>Server: GET /api/v1/providers<br/>X-CCH-Dashboard-Compat: 1
    Server->>Server: isDashboardCompatRequest(c)<br/>checks header + admin role
    alt compat + admin
        Server->>Action: getProviders() [all types]
        Action->>DB: SELECT providers
        DB-->>Action: rows (incl. claude-auth, gemini-cli)
        Action-->>Server: ProviderDisplay[]
        Server-->>ActionClient: {items: [...all types...]}
    else public
        Server->>Action: getProviders() [filtered]
        Action->>DB: SELECT providers
        DB-->>Action: rows
        Action-->>Server: ProviderDisplay[]
        Server->>Server: filterVisibleProviderTypes()
        Server-->>ActionClient: {items: [...public types only...]}
    end
    ActionClient-->>Dashboard: ProviderDisplay[]

    Dashboard->>ActionClient: getUsersBatchCore()
    ActionClient->>Server: GET /api/v1/users
    Server->>Action: getUsersBatchCore()
    Action->>Action: toActionTransportUserDisplay()<br/>(Date to ISO string for JSON)
    Action-->>Server: UserDisplay[] (dates as strings)
    Server-->>ActionClient: {users: [...]}
    ActionClient->>ActionClient: normalizeUsers()<br/>(ISO string to Date)
    ActionClient-->>Dashboard: UserDisplay[] (dates as Date objects)
Loading
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
src/actions/users.ts:44-47
**`toRequiredActionTransportDate` return type is a type-lie**

`toRequiredActionTransportDate` is typed as `(value: Date): Date` but can actually return `null` (via the `as unknown as Date` cast) when the input date is invalid. If any call site (now or in the future) relies on the returned value being a real `Date` — e.g., calling `.getTime()` on it — it will throw `TypeError: null.getTime is not a function` at runtime. The client-side fallback `?? new Date(0)` is only present for `createdAt`; if the same server-side helper is ever reused in a context without that guard, silent `null`-as-`Date` values will propagate.

Consider making the type honest:

```suggestion
function toRequiredActionTransportDate(value: Date): string | null {
  const timestamp = value.getTime();
  return Number.isFinite(timestamp) ? value.toISOString() : null;
}
```

### Issue 2 of 2
src/lib/api/v1/schemas/users.ts:98
**Search limit raised to 5000 on both public endpoints**

`UserFilterSearchQuerySchema` is shared by both `/api/v1/users:search` and `/api/v1/users:filter-search`. Raising `max` from 100 → 5000 was intended to unblock the rate-limit dashboard (`limit=5000`), but it also opens the same ceiling to any admin calling either endpoint directly. A deployment with tens of thousands of users could produce a very large unstreamed JSON payload on each call. If the intent is only to allow the dashboard path to request more users, consider keeping the public schema capped at 100 and adding a separate dashboard-scoped schema (similar to the compat pattern applied to providers), or at minimum add a server-side timeout or row limit to protect the query.

Reviews (4): Last reviewed commit: "fix: filter hidden provider types in ven..." | Re-trigger Greptile

@coderabbitai

coderabbitai Bot commented Apr 30, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

本次变更:引入仪表板兼容请求头并在客户端/服务端透传该头;集中/规范用户相关日期字段的传输序列化(ISO/null);扩展路由与处理器以支持内部 providerType 值并在兼容模式下放宽可见性检查;将部分原生 SQL 改为 Drizzle 类型化表达式;补充/更新大量测试。

Changes

Cohort / File(s) Summary
常量与共享类型
src/lib/api/v1/_shared/constants.ts
新增导出常量 DASHBOARD_COMPAT_HEADERPUBLIC_PROVIDER_TYPE_VALUESINTERNAL_PROVIDER_TYPE_VALUES
路由与模式(提供商/端点)
src/app/api/v1/resources/providers/router.ts, src/app/api/v1/resources/provider-endpoints/router.ts, src/lib/api/v1/schemas/_common.ts
路由/模式扩展以支持兼容的 providerType 字段;ProviderTypeSchema 改为使用共享常量枚举值并附 OpenAPI 元数据。
处理程序(提供商/端点)
src/app/api/v1/resources/providers/handlers.ts, src/app/api/v1/resources/provider-endpoints/handlers.ts
新增 dashboard-compat 分支:使用内部 Zod 模式解析,兼容模式下绕过隐藏 providerType 的可见性过滤并调整对不可见端点的 404/授权行为;新增 isDashboardCompatRequest 帮助函数。
API 客户端兼容层
src/lib/api-client/v1/actions/_compat.ts
兼容包装函数签名新增可选 options 参数并在提供时转发到底层 apiClient;保留并合并 onResponse 行为以返回 { body, headers }
API 客户端(providers / provider-endpoints / users)
src/lib/api-client/v1/actions/providers.ts, src/lib/api-client/v1/actions/provider-endpoints.ts, src/lib/api-client/v1/actions/users.ts
在相关请求中注入 DASHBOARD_COMPAT_HEADER: "1"provider-endpoints 客户端增加客户端侧 providerType 过滤;users 客户端将返回的日期字段归一化为 Date 或 null(并在兼容路径序列化为 ISO)。
Action 层用户序列化
src/actions/users.ts
getUsersBatchCore 改为通过新的 transport-serialization(如 toActionTransportUserDisplay)将顶层与 key 级别的日期字段转换为传输友好形式(ISO 字符串或 null)。
日期序列化工具与模式
src/lib/api/v1/_shared/serialization.ts, src/lib/api/v1/schemas/users.ts
serializeDates 扩展以识别类 Date 值并优先使用对象的 toJSON() 返回值进行序列化;UserFilterSearchQuerySchema.limit 上限由 100 调整为 5000。
仓储/查询构造
src/repository/statistics.ts
getRateLimitEventStats 查询改用 Drizzle 类型化 SQL 表达式(sqlandeq 等),时间过滤使用 ISO 字符串并带 ::timestamptz
生成类型文档
src/lib/api-client/v1/openapi-types.gen.ts
仅更新 providerType 参数的 JSDoc/描述文本(无类型或枚举变更)。
测试更新与新增
多处测试 tests/...
tests/api/v1/provider-endpoints/provider-endpoints.test.ts, tests/api/v1/providers/providers.read.test.ts, tests/api/v1/users/users.test.ts, tests/unit/...
新增/修改测试以覆盖仪表板兼容头行为、用户日期序列化(含无效日期)、扩大用户搜索 limit、以及改写后的速率限制 SQL 构造与序列化行为;增加若干兼容性与授权边界测试。

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 1.87% 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 标题准确总结了本次PR的主要变化:修复仪表板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 description is directly related to changeset, detailing specific regressions fixed and implementation strategy.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/page-api-regressions-20260430T072940Z

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.

@github-actions github-actions Bot added bug Something isn't working area:statistics area:core size/L Large PR (< 1000 lines) labels Apr 30, 2026

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

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.

Code Review

This pull request introduces a dashboard compatibility layer via a new X-CCH-Dashboard-Compat header, allowing the V1 API to handle 'hidden' provider types that were previously filtered out. The changes span API handlers, Zod schemas, and client-side actions to conditionally permit these types. Additionally, the PR improves date serialization/deserialization for transport, increases user search limits, and refactors the rate-limit statistics repository to use type-safe SQL construction. Feedback highlights opportunities to remove redundant client-side filtering and aggregation logic in the provider-endpoints client, as the server-side handlers now natively support these operations when the compatibility header is present.

Comment on lines +73 to +75
const providerTypeQuery = hiddenProviderTypes.has(input.providerType ?? "")
? undefined
: input.providerType;

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.

medium

Suppressing the providerType query parameter for hidden types in getProviderEndpoints is unnecessary and leads to over-fetching data from the server. The server-side handler in src/app/api/v1/resources/provider-endpoints/handlers.ts now correctly handles hidden types when the X-CCH-Dashboard-Compat header is present. Passing the provider type directly allows the server to perform the filtering, which is more efficient than fetching all endpoints for a vendor and filtering them on the client.

  const providerTypeQuery = input.providerType;

Comment thread src/lib/api-client/v1/actions/provider-endpoints.ts
Comment on lines +29 to +37
VendorTypeQuerySchema,
} from "@/lib/api/v1/schemas/provider-endpoints";

const INTERNAL_PROVIDER_TYPE_VALUES = [
"claude",
"claude-auth",
"codex",
"gemini",
"gemini-cli",

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 INTERNAL_PROVIDER_TYPE_VALUES duplicated across handler files

The exact same array is defined verbatim in both src/app/api/v1/resources/provider-endpoints/handlers.ts and src/app/api/v1/resources/providers/handlers.ts. If a new internal provider type is introduced, both files must be updated in sync. Moving this constant to @/lib/api/v1/_shared/constants.ts alongside HIDDEN_PROVIDER_TYPES would eliminate the duplication and make it easier to maintain.

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/api/v1/resources/provider-endpoints/handlers.ts
Line: 29-37

Comment:
**`INTERNAL_PROVIDER_TYPE_VALUES` duplicated across handler files**

The exact same array is defined verbatim in both `src/app/api/v1/resources/provider-endpoints/handlers.ts` and `src/app/api/v1/resources/providers/handlers.ts`. If a new internal provider type is introduced, both files must be updated in sync. Moving this constant to `@/lib/api/v1/_shared/constants.ts` alongside `HIDDEN_PROVIDER_TYPES` would eliminate the duplication and make it easier to maintain.

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

Comment thread src/actions/users.ts
Comment on lines 70 to 88
providerType?: string;
dashboard?: boolean;
}) {
const providerTypeQuery = hiddenProviderTypes.has(input.providerType ?? "")
? undefined
: input.providerType;
return apiGet<{ items?: ProviderEndpoint[] }>(
`/api/v1/provider-vendors/${input.vendorId}/endpoints${searchParams({
providerType: input.providerType,
providerType: providerTypeQuery,
dashboard: input.dashboard,
})}`
).then(unwrapItems);
})}`,
dashboardCompatOptions
)
.then(unwrapItems)
.then((items) =>
input.providerType ? items.filter((item) => item.providerType === input.providerType) : items
);
}

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 Over-fetching when resolving hidden-type endpoint stats

For hidden provider types the providerType query param is intentionally stripped from the request URL, so the server returns all endpoints for the vendor regardless of type. The client then filters locally via items.filter(item => item.providerType === input.providerType). A vendor with large numbers of mixed-type endpoints will always transfer the full set just to discard the non-matching rows. This is acceptable for a compat layer, but noting it so the team is aware of the N-per-vendor fan-out and the unbounded response size in pathological cases.

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/lib/api-client/v1/actions/provider-endpoints.ts
Line: 70-88

Comment:
**Over-fetching when resolving hidden-type endpoint stats**

For hidden provider types the `providerType` query param is intentionally stripped from the request URL, so the server returns all endpoints for the vendor regardless of type. The client then filters locally via `items.filter(item => item.providerType === input.providerType)`. A vendor with large numbers of mixed-type endpoints will always transfer the full set just to discard the non-matching rows. This is acceptable for a compat layer, but noting it so the team is aware of the N-per-vendor fan-out and the unbounded response size in pathological cases.

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

@github-actions

Copy link
Copy Markdown
Contributor

🧪 测试结果

测试类型 状态
代码质量
单元测试
集成测试
API 测试

总体结果: ✅ 所有测试通过

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (1)
src/app/api/v1/resources/provider-endpoints/handlers.ts (1)

32-59: ⚡ Quick win

避免在 compat 路径里再维护一份 providerType 白名单。

这里把公开类型和隐藏类型手工拼成了第二份枚举。后续只要主 schema 新增或更名 provider type,这些 compat 分支就会先开始返回 400,把这次修复过的兼容层再次变成回归点。更稳妥的是从同一个权威来源派生,或者至少复用共享常量来组装这些内部 schema。

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

In `@src/app/api/v1/resources/provider-endpoints/handlers.ts` around lines 32 -
59, The internal enum duplication (INTERNAL_PROVIDER_TYPE_VALUES and
InternalProviderTypeSchema) should be replaced by deriving from the
authoritative public provider-type source instead of hand-maintaining a second
list; update the code that defines INTERNAL_PROVIDER_TYPE_VALUES /
InternalProviderTypeSchema and any schemas that extend it
(InternalProviderEndpointListQuerySchema, InternalProviderEndpointCreateSchema,
InternalBatchVendorEndpointStatsSchema, InternalVendorTypeQuerySchema,
InternalVendorTypeManualOpenSchema, InternalVendorTypeBodySchema) to import or
reuse the shared provider-type constant or base schema (the public ProviderType
values/schema) and then add any hidden-only types (e.g. "claude-auth",
"gemini-cli") programmatically (e.g. union or extend) so the internal schemas
are always derived from the single authoritative source rather than a manually
duplicated array.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/app/api/v1/resources/provider-endpoints/handlers.ts`:
- Around line 510-512: The current isDashboardCompatRequest(c: Context) trusts
only the X-CCH-Dashboard-Compat header (DASHBOARD_COMPAT_HEADER), allowing
clients to spoof access to hidden provider types; change it to require a
server-trust signal instead (e.g., authenticated session attribute,
role/permission check, or an internal-only route flag) by updating
isDashboardCompatRequest to consult a verified property on Context such as
c.state.session.isInternal, c.state.user?.roles.includes('internal') or
c.auth.hasPermission('dashboard:compat') (or an explicit internalRoute boolean)
and drop the raw header-only check; if you still need to accept the header from
trusted internal services, validate it against the authenticated service
identity in Context rather than trusting it standalone.

In `@src/lib/api-client/v1/actions/provider-endpoints.ts`:
- Around line 181-199: This code creates unbounded parallel requests by
Promise.all over input.vendorIds calling getProviderEndpoints for each vendorId;
limit concurrency (e.g., use a concurrency-controlled mapper like p-map or an
async pool to cap simultaneous calls to a reasonable number such as 5–10) or
batch vendorIds into chunks and perform sequential batches instead of firing all
at once, and ideally move the hidden-type aggregation back to the server so you
can request aggregated stats for multiple vendorIds in one call; update the
block that currently maps input.vendorIds (the Promise.all +
getProviderEndpoints usage) to use the chosen concurrency/batching approach and
ensure the returned totals/healthy/unhealthy/unknown calculations remain the
same.

---

Nitpick comments:
In `@src/app/api/v1/resources/provider-endpoints/handlers.ts`:
- Around line 32-59: The internal enum duplication
(INTERNAL_PROVIDER_TYPE_VALUES and InternalProviderTypeSchema) should be
replaced by deriving from the authoritative public provider-type source instead
of hand-maintaining a second list; update the code that defines
INTERNAL_PROVIDER_TYPE_VALUES / InternalProviderTypeSchema and any schemas that
extend it (InternalProviderEndpointListQuerySchema,
InternalProviderEndpointCreateSchema, InternalBatchVendorEndpointStatsSchema,
InternalVendorTypeQuerySchema, InternalVendorTypeManualOpenSchema,
InternalVendorTypeBodySchema) to import or reuse the shared provider-type
constant or base schema (the public ProviderType values/schema) and then add any
hidden-only types (e.g. "claude-auth", "gemini-cli") programmatically (e.g.
union or extend) so the internal schemas are always derived from the single
authoritative source rather than a manually duplicated array.
🪄 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: 684be430-9dad-4250-b595-c4a6d8084643

📥 Commits

Reviewing files that changed from the base of the PR and between 7e2a722 and e0a9cb5.

📒 Files selected for processing (19)
  • src/actions/users.ts
  • src/app/api/v1/resources/provider-endpoints/handlers.ts
  • src/app/api/v1/resources/providers/handlers.ts
  • src/lib/api-client/v1/actions/_compat.ts
  • src/lib/api-client/v1/actions/provider-endpoints.ts
  • src/lib/api-client/v1/actions/providers.ts
  • src/lib/api-client/v1/actions/users.ts
  • src/lib/api/v1/_shared/constants.ts
  • src/lib/api/v1/_shared/serialization.ts
  • src/lib/api/v1/schemas/users.ts
  • src/repository/statistics.ts
  • tests/api/v1/dashboard/dashboard.test.ts
  • tests/api/v1/provider-endpoints/provider-endpoints.test.ts
  • tests/api/v1/providers/providers.read.test.ts
  • tests/api/v1/users/users.test.ts
  • tests/unit/api/v1/api-client-actions.test.ts
  • tests/unit/api/v1/schema-serialization.test.ts
  • tests/unit/repository/rate-limit-stats-query.test.ts
  • tests/unit/users-action-get-users-compat.test.ts

Comment thread src/app/api/v1/resources/provider-endpoints/handlers.ts
Comment thread src/lib/api-client/v1/actions/provider-endpoints.ts Outdated

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e0a9cb5f5e

ℹ️ 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 on lines +148 to +149
if (hiddenInput) {
return toActionResult(loadHiddenVendorEndpointStats(hiddenInput));

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 Use batch endpoint for hidden vendor endpoint stats

When providerType is hidden, this branch bypasses /api/v1/provider-vendors/endpoint-stats:batch and calls getProviderEndpoints once per vendor via Promise.all. In the dashboard flow, batches can include up to 500 vendor IDs, so this introduces an N-request fan-out that can dramatically increase latency and trigger request failures under load. The same commit already added hidden-type parsing support for compat requests on the batch stats route, so this fallback now creates a performance regression without a functional requirement.

Useful? React with 👍 / 👎.

@github-actions github-actions Bot left a comment

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.

Code Review Summary

This PR effectively restores dashboard/API compatibility broken by the v1 REST management migration. The fixes address date serialization, hidden provider types, and rate-limit stats queries. Code is well-tested and the architecture appropriately uses a header-based compatibility layer.

PR Size: L

  • Lines changed: 954 (829 additions, 125 deletions)
  • Files changed: 19

Note: This is a large but focused PR fixing regressions across 5 independent areas. The changes are cohesive and splitting would add unnecessary overhead given the interrelated nature of the dashboard compatibility fixes.

Issues Found

Category Critical High Medium Low
Logic/Bugs 0 0 0 0
Security 0 0 0 0
Error Handling 0 0 1 0
Types 0 0 0 0
Standards 0 0 1 0
Tests 0 0 0 0
Simplification 0 0 0 0

Medium Priority Issues (Should Fix)

  1. toISOString() without invalid-Date guard (src/actions/users.ts:953)

    • Functions toActionTransportDate and toRequiredActionTransportDate call toISOString() without validating the Date is valid. Invalid dates throw RangeError. The existing dateToIsoString helper in serialization.ts shows the correct pattern with Number.isFinite(timestamp) validation.
    • Suggested fix: Add guard: const ts = value.getTime(); return (Number.isFinite(ts) ? value.toISOString() : null) as unknown as Date;
  2. INTERNAL_PROVIDER_TYPE_VALUES duplication (src/app/api/v1/resources/provider-endpoints/handlers.ts:32-39, src/app/api/v1/resources/providers/handlers.ts:44-51)

    • The same array is defined in both files. Per CLAUDE.md conventions, move this to src/lib/api/v1/_shared/constants.ts alongside HIDDEN_PROVIDER_TYPES for maintainability.

Positive Observations

  • Good test coverage: New unit and integration tests cover date serialization, dashboard compatibility headers, and rate-limit query changes
  • Proper error handling: Rate-limit stats query properly refactored to use Drizzle ORM with type-safe parameters
  • Security conscious: Dashboard compat header (X-CCH-Dashboard-Compat) provides clean separation between public and internal schemas
  • Date serialization: Cross-realm Date handling and toJSON support added to serializeDates

Review Coverage

  • Logic and correctness - Fixes correctly address the 5 regression areas
  • Security (OWASP Top 10) - No vulnerabilities identified
  • Error handling - Proper logging and error propagation
  • Type safety - Appropriate Zod schema extensions for internal types
  • Documentation accuracy - Comments match code behavior
  • Test coverage - Adequate unit and integration tests
  • Code clarity - Clear separation of concerns with dashboard compat layer

Automated review by Claude AI

Comment thread src/lib/api-client/v1/actions/users.ts
const hiddenInput = parseHiddenVendorEndpointStatsInput(input);
if (hiddenInput) {
return toActionResult(loadHiddenVendorEndpointStats(hiddenInput));
}

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.

[MEDIUM] [PERFORMANCE-ISSUE] Hidden provider endpoint stats fall back to N+1 requests

Evidence (src/lib/api-client/v1/actions/provider-endpoints.ts:149):
return toActionResult(loadHiddenVendorEndpointStats(hiddenInput));

Why this is a problem: loadHiddenVendorEndpointStats issues one getProviderEndpoints request per vendorId (via Promise.all). That can significantly slow the dashboard and increase server load when vendorIds is large, despite a batch endpoint existing.

The v1 server handler already accepts hidden provider types under the dashboard compat header (InternalBatchVendorEndpointStatsSchema), so the batch endpoint should be usable for hidden provider types as well.

Guideline: "[PERFORMANCE-ISSUE] - N+1 queries, memory leaks, inefficient algorithms".

Suggested fix (prefer the batch endpoint; optionally keep a fallback only for older servers):

export function batchGetVendorTypeEndpointStats(input: unknown) {
  return toActionResult(
    apiPost<VendorTypeEndpointStats[]>(
      "/api/v1/provider-vendors/endpoint-stats:batch",
      input,
      dashboardCompatOptions
    )
  );
}

@github-actions github-actions Bot left a comment

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.

Code Review Summary

This PR introduces multiple compatibility shims (dashboard-compat header, date rehydration, hidden provider type handling) across server handlers, client actions, and repository queries. The scope is broad for a regression fix and increases the chance of subtle behavioral drift unless the remaining edge cases and performance tradeoffs are tightened.

PR Size: L

  • Lines changed: 954
  • Files changed: 19
  • Split suggestion: (1) user date transport/rehydration, (2) provider/provider-endpoint dashboard-compat + hidden types, (3) rate-limit stats query + serialization changes.

Issues Found

Category Critical High Medium Low
Logic/Bugs 0 0 1 0
Security 0 0 0 0
Error Handling 0 0 0 0
Types 0 0 0 0
Comments/Docs 0 0 0 0
Tests 0 0 0 0
Simplification 0 0 1 0

Critical Issues (Must Fix)

None.

High Priority Issues (Should Fix)

  • (Confidence 90) [LOGIC-BUG] normalizeDate drops numeric timestamp 0 -> null (src/lib/api-client/v1/actions/users.ts:88).
  • (Confidence 85) [PERFORMANCE-ISSUE] Hidden provider endpoint stats fan out into N+1 requests instead of batch (src/lib/api-client/v1/actions/provider-endpoints.ts:149).

Review Coverage

  • Logic and correctness
  • Security (OWASP Top 10)
  • Error handling
  • Type safety
  • Documentation accuracy
  • Test coverage
  • Code clarity

Automated review by Codex AI

@github-actions github-actions Bot left a comment

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.

  • Applied PR label size/L.
  • Posted 2 inline review comments:
    • src/lib/api-client/v1/actions/users.ts:88[LOGIC-BUG] normalizeDate drops numeric timestamp 0.
    • src/lib/api-client/v1/actions/provider-endpoints.ts:149[PERFORMANCE-ISSUE] hidden provider endpoint stats fan out into N+1 requests instead of using the batch endpoint.
  • Submitted the required “Code Review Summary” review comment on PR #1145.

@github-actions

Copy link
Copy Markdown
Contributor

🧪 测试结果

测试类型 状态
代码质量
单元测试
集成测试
API 测试

总体结果: ✅ 所有测试通过

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/app/api/v1/resources/provider-endpoints/handlers.ts`:
- Around line 67-70: The details endpoint getProviderVendor is returning
sanitizeProviderEndpointData(result.data) without applying visibility filtering,
so hidden providerTypes can leak for compat requests; modify the
getProviderVendor handler to detect compat requests with
isDashboardCompatRequest(c) and, when true, pass result.data through
filterVisibleProviderTypes before calling sanitizeProviderEndpointData (mirror
the logic used in the list handler), ensuring the same filtered data is returned
for /provider-vendors/{id}.

In `@src/app/api/v1/resources/provider-endpoints/router.ts`:
- Around line 47-71: The route-level schema
DashboardCompatProviderTypeRouteSchema currently restricts providerType to
PUBLIC_PROVIDER_TYPE_VALUES which blocks dashboard-compat requests that use
hidden/internal types (e.g., claude-auth, gemini-cli) before handlers run;
update DashboardCompatProviderTypeRouteSchema to accept the broader set (use
INTERNAL_PROVIDER_TYPE_VALUES or a union of PUBLIC_PROVIDER_TYPE_VALUES and
INTERNAL_PROVIDER_TYPE_VALUES) so routes like
ProviderEndpointListRouteQuerySchema, ProviderEndpointCreateRouteSchema,
BatchVendorEndpointStatsRouteSchema, VendorTypeRouteQuerySchema,
VendorTypeManualOpenRouteSchema and VendorTypeBodyRouteSchema can validate
compat requests without rejecting hidden types, or alternatively create a
separate route-level schema for dashboard-compat routes that uses
INTERNAL_PROVIDER_TYPE_VALUES.

In `@src/app/api/v1/resources/providers/router.ts`:
- Around line 63-81: The route-level Zod schemas (ProviderListRouteQuerySchema,
ProviderUnifiedTestRouteSchema, ProviderTypeRouteQuerySchema,
ProviderFetchUpstreamModelsRouteSchema) currently constrain providerType to
PUBLIC_PROVIDER_TYPE_VALUES via DashboardCompatProviderTypeRouteSchema, which
prevents hidden types from reaching handlers; change the route registration to
select a looser schema when DASHBOARD_COMPAT_HEADER is present (e.g., in
createRoute check request.headers[DASHBOARD_COMPAT_HEADER] and use the
InternalProviderListQuerySchema / InternalProviderUnifiedTestSchema or a
permissive z.string() providerType schema instead) so requests with hidden
provider types (handled by isDashboardCompatRequest()) are not blocked by
pre-handler validation. Ensure the referenced symbols are used: modify
createRoute, ProviderListRouteQuerySchema, ProviderUnifiedTestRouteSchema,
ProviderTypeRouteQuerySchema, ProviderFetchUpstreamModelsRouteSchema,
DashboardCompatProviderTypeRouteSchema, isDashboardCompatRequest(),
InternalProviderListQuerySchema and InternalProviderUnifiedTestSchema.
🪄 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: 0f552ff9-07b6-4c89-ad6b-bf2aa25c9294

📥 Commits

Reviewing files that changed from the base of the PR and between e0a9cb5 and 7b6129d.

📒 Files selected for processing (13)
  • src/actions/users.ts
  • src/app/api/v1/resources/provider-endpoints/handlers.ts
  • src/app/api/v1/resources/provider-endpoints/router.ts
  • src/app/api/v1/resources/providers/handlers.ts
  • src/app/api/v1/resources/providers/router.ts
  • src/lib/api-client/v1/actions/provider-endpoints.ts
  • src/lib/api-client/v1/openapi-types.gen.ts
  • src/lib/api/v1/_shared/constants.ts
  • src/lib/api/v1/schemas/_common.ts
  • tests/api/v1/provider-endpoints/provider-endpoints.test.ts
  • tests/api/v1/providers/providers.read.test.ts
  • tests/unit/api/v1/api-client-actions.test.ts
  • tests/unit/users-action-get-users-compat.test.ts
✅ Files skipped from review due to trivial changes (2)
  • src/lib/api-client/v1/openapi-types.gen.ts
  • src/lib/api/v1/schemas/_common.ts
🚧 Files skipped from review as they are similar to previous changes (3)
  • tests/api/v1/providers/providers.read.test.ts
  • src/lib/api/v1/_shared/constants.ts
  • tests/unit/api/v1/api-client-actions.test.ts

Comment thread src/app/api/v1/resources/provider-endpoints/handlers.ts
Comment thread src/app/api/v1/resources/provider-endpoints/router.ts
Comment thread src/app/api/v1/resources/providers/router.ts
@github-actions

Copy link
Copy Markdown
Contributor

🧪 测试结果

测试类型 状态
代码质量
单元测试
集成测试
API 测试

总体结果: ✅ 所有测试通过

@github-actions

Copy link
Copy Markdown
Contributor

🧪 测试结果

测试类型 状态
代码质量
单元测试
集成测试
API 测试

总体结果: ✅ 所有测试通过

@ding113
ding113 merged commit d62d9ea into dev Apr 30, 2026
21 checks passed
@github-project-automation github-project-automation Bot moved this from Backlog to Done in Claude Code Hub Roadmap Apr 30, 2026
@ding113
ding113 deleted the fix/page-api-regressions-20260430T072940Z branch May 13, 2026 06:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:core area:statistics bug Something isn't working size/L Large PR (< 1000 lines)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant