ci: optimize test and build pipeline#1107
Conversation
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
📝 WalkthroughWalkthrough本次变更新增仓库根目录的 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the project to Bun 1.3.10, modifies Dockerfiles to use bun ci, and significantly enhances the Vitest test configuration by introducing environment variable support for timeouts, worker limits, and parallelism. The review feedback identifies critical issues in the Dockerfiles where bun ci will fail because the lockfile is no longer being copied into the image. Additionally, there is a recommendation to export shared helper functions in the test configuration to eliminate code duplication between the base and global Vitest configs.
| COPY package.json ./ | ||
| RUN bun ci |
There was a problem hiding this comment.
The command bun ci requires a lockfile (bun.lockb) to be present in the working directory to function correctly. However, the current COPY instruction only includes package.json. This will cause the Docker build to fail because bun ci cannot verify the dependencies against a lockfile.
Additionally, the PR description mentions making builds work "without tracked lockfiles". If the project does not track bun.lockb, bun ci is not the appropriate command; you should use bun install instead. If you do track the lockfile, ensure it is copied into the image.
COPY package.json bun.lockb* ./
RUN bun ci
| COPY package.json ./ | ||
| RUN bun install | ||
| RUN bun ci |
There was a problem hiding this comment.
Similar to the root Dockerfile, bun ci will fail here because the lockfile is not copied. Please ensure bun.lockb is included in the COPY command to allow bun ci to verify the installation.
COPY package.json bun.lockb* ./
RUN bun ci
| function parsePositiveInt(value: string | undefined, fallback: number): number { | ||
| if (!value) return fallback; | ||
| const parsed = Number.parseInt(value, 10); | ||
| return Number.isFinite(parsed) && parsed > 0 ? parsed : fallback; | ||
| } | ||
|
|
||
| function parseWorkerLimit(value: string | undefined, fallback: number | string): number | string { | ||
| if (!value) return fallback; | ||
| if (/^\d+%$/.test(value)) return value; | ||
| return parsePositiveInt(value, typeof fallback === "number" ? fallback : 2); | ||
| } |
There was a problem hiding this comment.
The helper functions parsePositiveInt and parseWorkerLimit are duplicated in vitest.config.ts. To improve maintainability and adhere to the DRY (Don't Repeat Yourself) principle, consider exporting these helpers from this base configuration file and importing them in vitest.config.ts instead of redefining them.
| function parsePositiveInt(value: string | undefined, fallback: number): number { | |
| if (!value) return fallback; | |
| const parsed = Number.parseInt(value, 10); | |
| return Number.isFinite(parsed) && parsed > 0 ? parsed : fallback; | |
| } | |
| function parseWorkerLimit(value: string | undefined, fallback: number | string): number | string { | |
| if (!value) return fallback; | |
| if (/^\d+%$/.test(value)) return value; | |
| return parsePositiveInt(value, typeof fallback === "number" ? fallback : 2); | |
| } | |
| export function parsePositiveInt(value: string | undefined, fallback: number): number { | |
| if (!value) return fallback; | |
| const parsed = Number.parseInt(value, 10); | |
| return Number.isFinite(parsed) && parsed > 0 ? parsed : fallback; | |
| } | |
| export function parseWorkerLimit(value: string | undefined, fallback: number | string): number | string { | |
| if (!value) return fallback; | |
| if (/^\d+%$/.test(value)) return value; | |
| return parsePositiveInt(value, typeof fallback === "number" ? fallback : 2); | |
| } |
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
vitest.config.ts (1)
9-24: 与tests/vitest.base.ts实现重复，建议复用同一组解析助手
parsePositiveInt（Line 9-13）与parseWorkerLimit（Line 15-19）已经在tests/vitest.base.ts中存在等价实现。两处重复后续只要任一处调整（例如对负数、0%、空白字符的处理）就会出现行为漂移。建议从tests/vitest.base.ts导出后在此处直接 import，defaultMaxWorkers保留在本文件即可。建议改造
tests/vitest.base.ts中将解析助手改为导出：-function parsePositiveInt(value: string | undefined, fallback: number): number { +export function parsePositiveInt(value: string | undefined, fallback: number): number { ... -function parseWorkerLimit(value: string | undefined, fallback: number | string): number | string { +export function parseWorkerLimit(value: string | undefined, fallback: number | string): number | string {
vitest.config.ts复用：-import { sharedResolve } from "./tests/vitest.base"; +import { parsePositiveInt, parseWorkerLimit, sharedResolve } from "./tests/vitest.base"; @@ -function parsePositiveInt(value: string | undefined, fallback: number): number { - if (!value) return fallback; - const parsed = Number.parseInt(value, 10); - return Number.isFinite(parsed) && parsed > 0 ? parsed : fallback; -} - -function parseWorkerLimit(value: string | undefined, fallback: number | string): number | string { - if (!value) return fallback; - if (/^\d+%$/.test(value)) return value; - return parsePositiveInt(value, typeof fallback === "number" ? fallback : 8); -} - function defaultMaxWorkers(): number { const workerBudget = Math.floor(availableParallelism() * 0.75); return Math.min(8, Math.max(2, workerBudget)); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@vitest.config.ts` around lines 9 - 24, parsePositiveInt and parseWorkerLimit duplicate helpers already implemented in tests/vitest.base.ts; remove these local implementations and import the exported parsePositiveInt and parseWorkerLimit from tests/vitest.base.ts to avoid divergent behavior, keeping defaultMaxWorkers in this file unchanged; update any type expectations so parseWorkerLimit's fallback typing matches its exported signature and replace local calls to parsePositiveInt/parseWorkerLimit with the imported functions.tests/vitest.base.ts (1)
33-48: LGTM，但与vitest.config.ts中的实现高度重复
parsePositiveInt与parseWorkerLimit同时存在于本文件与vitest.config.ts（见该文件 Line 9-19）。建议把这三个解析助手集中导出（例如从本文件export），让vitest.config.ts直接复用，避免后续两处实现漂移。具体重构建议放在vitest.config.ts的评论中。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/vitest.base.ts` around lines 33 - 48, The three helper functions parsePositiveInt, parseWorkerLimit, and parseBoolean are duplicated in vitest.config.ts; export them from this file and have vitest.config.ts import and reuse them to avoid drift. Concretely, add named exports for parsePositiveInt, parseWorkerLimit, and parseBoolean in this file and remove the duplicate implementations from vitest.config.ts, replacing them with imports that reference the exported symbols so both places use the single shared implementation.package.json (1)
125-125: 确保@typescript/native-preview版本在 bun.lock 中锁定
7.0.0-dev.20260425.1已确认在 npm registry 上可解析。由于该版本属于每日发布的 dev tag，建议通过bun.lock锁定具体的包哈希，以保证团队成员和 CI/CD 环境中的构建可重现。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` at line 125, package.json currently references "@typescript/native-preview": "7.0.0-dev.20260425.1" but the bun.lock must pin the exact package hash for reproducible builds; regenerate/ensure bun.lock contains the resolved integrity by running bun install (or bun add `@typescript/native-preview`@7.0.0-dev.20260425.1) locally and committing the updated bun.lock, verifying the entry for "@typescript/native-preview" includes the concrete lock/hash rather than only the version tag so CI and teammates get the same artifact..github/workflows/pr-check.yml (1)
36-46: LGTM，附一点 cold-cache 风险提示缓存与
bun ci设置正确。Install dependencies步骤 5 分钟限时对热缓存非常充裕；但首次跑（缓存 miss）+ 网络抖动情况下（npm registry 偶发延迟、平台特定二进制如sharp拉取等）可能逼近上限。如果后续观察到偶发超时，可以把这一步的timeout-minutes略放宽到 8–10，或保留现值并依赖restore-keys的 fallback。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/pr-check.yml around lines 36 - 46, The workflow's "Install dependencies" step sets timeout-minutes: 5 which may be too short on cache misses or network/registry delays; update the "Install dependencies" step to increase timeout-minutes to 8–10 (or make it configurable) so bun ci has more time on cold-cache runs, and optionally document the reason near the step and keep the existing cache/restore-keys logic unchanged to preserve hot-cache performance.tests/unit/public-status/config-publisher.test.ts (1)
119-119: 单元测试超时设置不一致第 119、173 行超时被提升至
30_000，但第 278 行的同类用例仍保留20_000，第 227、328 行则无显式覆盖（走基础配置默认值）。如果 30s 是为了应对 CI 慢盘/动态 import 抖动，建议统一所有用例（或抽到vitest.base.ts的testTimeout），避免某个用例在压力下偶发翻车而其它用例已被放宽。另：这是单元测试文件（所有外部依赖均通过
vi.mock/vi.hoistedmock 掉），30s 超时偏长，可能掩盖未来真正的性能回归——可以在 CI 稳定后再评估是否回收到更小的值。♻️ 建议统一为相同超时
- }, 20_000); + }, 30_000);（应用到第 278 行；或对所有四个用例去除显式超时，由
vitest.base.ts集中管理）Also applies to: 173-173, 278-278
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/public-status/config-publisher.test.ts` at line 119, Several unit tests in this file use inconsistent per-test timeouts (e.g., the snippet "}, 30_000);" appears at lines like 119 and 173 while other cases at 278 use 20_000 and some (227, 328) rely on the default); make them consistent by either (A) setting the same timeout for all these tests to 30_000 (change the trailing "}, 20_000);" occurrences and add "}, 30_000);" where missing) or (B) remove all per-test timeout overrides in this file so they inherit the centralized testTimeout from vitest.base.ts—apply the chosen approach to the test cases referenced around lines 119, 173, 227, 278, and 328..github/workflows/test.yml (1)
220-226: 缩窄 Next.js 缓存 key 的 hashFiles 范围，提升命中率
hashFiles('**/*.js', '**/*.jsx', '**/*.ts', '**/*.tsx', 'next.config.*')的广泛模式会扫描整个仓库，包括tests/、scripts/、vitest.config.ts、drizzle.config.ts等与 Next.js 构建无关的文件。任何 PR 修改这些文件都会导致主 key 失效，缓存命中率低下。建议收敛到实际影响 Next.js 构建的文件：
♻️ 缩窄 hashFiles 范围
- key: ${{ runner.os }}-nextjs-${{ hashFiles('bun.lock') }}-${{ hashFiles('**/*.js', '**/*.jsx', '**/*.ts', '**/*.tsx', 'next.config.*') }} + key: ${{ runner.os }}-nextjs-${{ hashFiles('bun.lock') }}-${{ hashFiles('src/**/*.{js,jsx,ts,tsx}', 'next.config.ts', 'tsconfig.json', 'postcss.config.mjs') }}这样只会在
src/目录下源代码、TypeScript 配置、Next.js 配置或样式配置有变化时更新 key，避免不相关的文件变更触发缓存失效。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/test.yml around lines 220 - 226, 当前使用的 hashFiles 范围过宽（hashFiles('**/*.js','**/*.jsx','**/*.ts','**/*.tsx','next.config.*')）会把仓库内不相关文件纳入 key，导致缓存命中率低；请在 .github/workflows/test.yml 中调整 actions/cache@v4 的 key/restore-keys，改为只对影响 Next.js 构建的文件取哈希（例如 src/**/*.{js,jsx,ts,tsx}、next.config.*、tsconfig.json、tailwind.config.*、postcss.config.* 等），保持 path 为 ${{ github.workspace }}/.next/cache 并同步更新 restore-keys 模式以匹配新 key，从而避免无关文件变更使缓存失效。
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/dev.yml:
- Line 16: CI currently fails because the workflow's "bun ci" step requires a
lockfile but the repository lacks bun.lock or bun.lockb; generate the lockfile
locally with bun install (or bun generate-lockfile), ensure bun.lock or
bun.lockb is not ignored by .gitignore, add and commit the lockfile to the repo,
then push so the "bun ci" step in the workflow can run successfully.
In `@Dockerfile`:
- Around line 4-5: The Dockerfiles invoke "bun ci" but only COPY package.json
and the repo lacks bun.lock, so image builds fail; either generate and commit
bun.lock (run bun install locally), then update Dockerfile, deploy/Dockerfile
and deploy/Dockerfile.dev to COPY bun.lock (and package.json) before running bun
ci, or change the two failing files (Dockerfile and deploy/Dockerfile) to use
"bun install" instead of "bun ci" to avoid requiring a lockfile; ensure
deploy/Dockerfile.dev stays consistent if already using bun install.
---
Nitpick comments:
In @.github/workflows/pr-check.yml:
- Around line 36-46: The workflow's "Install dependencies" step sets
timeout-minutes: 5 which may be too short on cache misses or network/registry
delays; update the "Install dependencies" step to increase timeout-minutes to
8–10 (or make it configurable) so bun ci has more time on cold-cache runs, and
optionally document the reason near the step and keep the existing
cache/restore-keys logic unchanged to preserve hot-cache performance.
In @.github/workflows/test.yml:
- Around line 220-226: 当前使用的 hashFiles
范围过宽（hashFiles('**/*.js','**/*.jsx','**/*.ts','**/*.tsx','next.config.*')）会把仓库内不相关文件纳入
key，导致缓存命中率低；请在 .github/workflows/test.yml 中调整 actions/cache@v4 的
key/restore-keys，改为只对影响 Next.js 构建的文件取哈希（例如
src/**/*.{js,jsx,ts,tsx}、next.config.*、tsconfig.json、tailwind.config.*、postcss.config.*
等），保持 path 为 ${{ github.workspace }}/.next/cache 并同步更新 restore-keys 模式以匹配新
key，从而避免无关文件变更使缓存失效。
In `@package.json`:
- Line 125: package.json currently references "@typescript/native-preview":
"7.0.0-dev.20260425.1" but the bun.lock must pin the exact package hash for
reproducible builds; regenerate/ensure bun.lock contains the resolved integrity
by running bun install (or bun add
`@typescript/native-preview`@7.0.0-dev.20260425.1) locally and committing the
updated bun.lock, verifying the entry for "@typescript/native-preview" includes
the concrete lock/hash rather than only the version tag so CI and teammates get
the same artifact.
In `@tests/unit/public-status/config-publisher.test.ts`:
- Line 119: Several unit tests in this file use inconsistent per-test timeouts
(e.g., the snippet "}, 30_000);" appears at lines like 119 and 173 while other
cases at 278 use 20_000 and some (227, 328) rely on the default); make them
consistent by either (A) setting the same timeout for all these tests to 30_000
(change the trailing "}, 20_000);" occurrences and add "}, 30_000);" where
missing) or (B) remove all per-test timeout overrides in this file so they
inherit the centralized testTimeout from vitest.base.ts—apply the chosen
approach to the test cases referenced around lines 119, 173, 227, 278, and 328.
In `@tests/vitest.base.ts`:
- Around line 33-48: The three helper functions parsePositiveInt,
parseWorkerLimit, and parseBoolean are duplicated in vitest.config.ts; export
them from this file and have vitest.config.ts import and reuse them to avoid
drift. Concretely, add named exports for parsePositiveInt, parseWorkerLimit, and
parseBoolean in this file and remove the duplicate implementations from
vitest.config.ts, replacing them with imports that reference the exported
symbols so both places use the single shared implementation.
In `@vitest.config.ts`:
- Around line 9-24: parsePositiveInt and parseWorkerLimit duplicate helpers
already implemented in tests/vitest.base.ts; remove these local implementations
and import the exported parsePositiveInt and parseWorkerLimit from
tests/vitest.base.ts to avoid divergent behavior, keeping defaultMaxWorkers in
this file unchanged; update any type expectations so parseWorkerLimit's fallback
typing matches its exported signature and replace local calls to
parsePositiveInt/parseWorkerLimit with the imported functions.
🪄 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: b9011335-851b-4274-9f48-a7acb5245c82
📒 Files selected for processing (15)
.bun-version.github/workflows/dev.yml.github/workflows/pr-check.yml.github/workflows/release.yml.github/workflows/test.ymlDockerfiledeploy/Dockerfilepackage.jsontests/configs/e2e.config.tstests/configs/integration.config.tstests/configs/public-status.integration.config.tstests/integration/public-status/config-publish.test.tstests/unit/public-status/config-publisher.test.tstests/vitest.base.tsvitest.config.ts
| jobs: | ||
| dev-build-deploy: | ||
| runs-on: ubuntu-latest | ||
| timeout-minutes: 90 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -HI -t f '^bun\.lock$' -x ls -l {}
fd -HI -t f '^bun\.lockb$' -x ls -l {}Repository: ding113/claude-code-hub
Length of output: 49
缺少 bun.lock 文件，导致工作流将在 bun ci 步骤失败
虽然工作流配置本身正确（Bun 版本锁定、缓存设置、超时设置合理），但仓库中未找到 bun.lock 或 bun.lockb 文件。bun ci 命令需要 lockfile 存在且使用 --frozen-lockfile 标志，缺失时会直接失败。
需要先提交 bun.lock 文件至仓库，工作流才能正常运行。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/dev.yml at line 16, CI currently fails because the
workflow's "bun ci" step requires a lockfile but the repository lacks bun.lock
or bun.lockb; generate the lockfile locally with bun install (or bun
generate-lockfile), ensure bun.lock or bun.lockb is not ignored by .gitignore,
add and commit the lockfile to the repo, then push so the "bun ci" step in the
workflow can run successfully.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bc18638a3b
ℹ️ 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".
| - name: 📦 Install dependencies | ||
| run: bun install | ||
| timeout-minutes: 5 | ||
| run: bun ci |
There was a problem hiding this comment.
bun ci without a committed lockfile
This step now runs bun ci, but this repository does not commit bun.lock (no lockfile exists in this commit), and Bun’s CI/CD docs define bun ci as frozen-lockfile install intended for a committed lockfile. On fresh checkouts this can break dependency installation (or at least remove reproducibility guarantees), and the same pattern is now repeated across multiple workflows and Dockerfiles in this commit.
Useful? React with 👍 / 👎.
| uses: actions/cache@v4 | ||
| with: | ||
| path: ~/.bun/install/cache | ||
| key: ${{ runner.os }}-bun-${{ hashFiles('bun.lock') }} |
There was a problem hiding this comment.
The cache key is derived from hashFiles('bun.lock'), but this repo does not track bun.lock; in GitHub Actions, hashFiles returns an empty string when no files match, so the key becomes effectively constant (<os>-bun-). That prevents key rotation when dependencies change and undermines the intended CI speedup because updated package cache contents are not saved under a new key.
Useful? React with 👍 / 👎.
🧪 测试结果
总体结果: ✅ 所有测试通过 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| COPY package.json ./ | ||
| RUN bun ci |
There was a problem hiding this comment.
bun ci (= bun install --frozen-lockfile) requires a lockfile to produce a reproducible install. Without copying bun.lock into the deps stage, Bun either errors or falls back to a regular install, meaning Docker images may resolve different package versions on each build. The CI workflows correctly reference bun.lock via hashFiles('bun.lock'), so the file is tracked — it should be COPY'd here.
| COPY package.json ./ | |
| RUN bun ci | |
| COPY package.json bun.lock ./ | |
| RUN bun ci |
Prompt To Fix With AI
This is a comment left during a code review.
Path: Dockerfile
Line: 4-5
Comment:
**Lockfile not copied — non-deterministic dependency resolution in Docker**
`bun ci` (= `bun install --frozen-lockfile`) requires a lockfile to produce a reproducible install. Without copying `bun.lock` into the `deps` stage, Bun either errors or falls back to a regular install, meaning Docker images may resolve different package versions on each build. The CI workflows correctly reference `bun.lock` via `hashFiles('bun.lock')`, so the file is tracked — it should be COPY'd here.
```suggestion
COPY package.json bun.lock ./
RUN bun ci
```
How can I resolve this? If you propose a fix, please make it concise.| COPY package.json ./ | ||
| RUN bun install | ||
| RUN bun ci |
There was a problem hiding this comment.
Same as the root Dockerfile: bun ci needs bun.lock present to enforce a frozen install. Without it, each Docker build may resolve package versions independently of what the lockfile pins. Since bun.lock is tracked in git (CI caches it via hashFiles('bun.lock')), it should be COPY'd before bun ci.
| COPY package.json ./ | |
| RUN bun install | |
| RUN bun ci | |
| COPY package.json bun.lock ./ | |
| RUN bun ci |
Prompt To Fix With AI
This is a comment left during a code review.
Path: deploy/Dockerfile
Line: 8-9
Comment:
**Lockfile not copied — non-deterministic dependency resolution in Docker**
Same as the root `Dockerfile`: `bun ci` needs `bun.lock` present to enforce a frozen install. Without it, each Docker build may resolve package versions independently of what the lockfile pins. Since `bun.lock` is tracked in git (CI caches it via `hashFiles('bun.lock')`), it should be COPY'd before `bun ci`.
```suggestion
COPY package.json bun.lock ./
RUN bun ci
```
How can I resolve this? If you propose a fix, please make it concise.| function parseWorkerLimit(value: string | undefined, fallback: number | string): number | string { | ||
| if (!value) return fallback; | ||
| if (/^\d+%$/.test(value)) return value; | ||
| return parsePositiveInt(value, typeof fallback === "number" ? fallback : 8); | ||
| } |
There was a problem hiding this comment.
parsePositiveInt and parseWorkerLimit are now defined in both vitest.config.ts and tests/vitest.base.ts. The inner numeric fallback in parseWorkerLimit differs between the two files: 8 here vs 2 in vitest.base.ts. While neither call-site currently passes a string fallback (so the branch is effectively dead), the duplication will cause subtle confusion if the functions drift further. Consider exporting them from tests/vitest.base.ts and importing them here.
Prompt To Fix With AI
This is a comment left during a code review.
Path: vitest.config.ts
Line: 15-19
Comment:
**Duplicated helpers with diverging fallback constants**
`parsePositiveInt` and `parseWorkerLimit` are now defined in both `vitest.config.ts` and `tests/vitest.base.ts`. The inner numeric fallback in `parseWorkerLimit` differs between the two files: `8` here vs `2` in `vitest.base.ts`. While neither call-site currently passes a string fallback (so the branch is effectively dead), the duplication will cause subtle confusion if the functions drift further. Consider exporting them from `tests/vitest.base.ts` and importing them here.
How can I resolve this? If you propose a fix, please make it concise.![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| uses: actions/cache@v4 | ||
| with: | ||
| path: ~/.bun/install/cache | ||
| key: ${{ runner.os }}-bun-${{ hashFiles('bun.lock') }} |
There was a problem hiding this comment.
[Medium] [PERFORMANCE-ISSUE] Bun cache key is effectively constant -- cache never refreshes when dependencies change
hashFiles('bun.lock') references a file that is not tracked in this repository. Since bun.lock does not exist at checkout time, hashFiles returns an empty string, making the primary cache key identical to the restore-key prefix (${{ runner.os }}-bun-). This means:
- First CI run: cache miss, cache saved with the constant key.
- All subsequent runs: exact key match (cache hit), the post-save step is skipped.
- When
package.jsonchanges (new deps), the stale cache is restored and never updated -- new packages are re-downloaded on every run.
The same issue appears in all four workflow files:
.github/workflows/test.yml(lines 34, 70, 135, 209).github/workflows/dev.yml(line 72).github/workflows/pr-check.yml(line 40).github/workflows/release.yml(line 209)
Suggested fix:
key: ${{ runner.os }}-bun-${{ hashFiles('package.json') }}Using package.json ensures the cache key changes when dependencies are added/updated, triggering a cache miss and a fresh save. The restore-keys prefix match will still restore the old cache as a warm start.
There was a problem hiding this comment.
Code Review Summary
This PR optimizes CI/test pipelines by pinning Bun, adding caching and timeouts, tuning Vitest parallelism, and updating Dockerfiles. The changes are well-scoped to tooling with no product behavior changes. One medium-severity performance issue was identified.
PR Size: M
- Lines changed: 191 (171 additions, 20 deletions)
- Files changed: 15
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 0 | 0 | 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 |
| Performance | 0 | 0 | 1 | 0 |
Medium Priority Issues (Should Fix)
-
Ineffective Bun cache key across all 4 workflow files -
hashFiles('bun.lock')references a file not tracked in this repository. Sincebun.lockdoes not exist at checkout time, the hash is constant, making the cache key identical to the restore-key prefix. After the first CI run saves the cache, all subsequent runs get a cache hit and the cache is never refreshed whenpackage.jsondependencies change. Fix: replacehashFiles('bun.lock')withhashFiles('package.json')in all cache key definitions.Affected files:
.github/workflows/test.yml(lines 34, 70, 135, 209).github/workflows/dev.yml(line 72).github/workflows/pr-check.yml(line 40).github/workflows/release.yml(line 209)
Note: The Next.js cache key in
test.ymlline 224 also includeshashFiles('bun.lock')but is partially effective since it also hashes source files (**/*.{js,jsx,ts,tsx},next.config.*). Thebun.lockcomponent there is dead weight but not harmful.
Review Coverage
- Logic and correctness - Clean
- Security (OWASP Top 10) - Clean
- Error handling - Clean
- Type safety - Clean
- Documentation accuracy - Clean
- Test coverage - Adequate (no new product features; test config changes are reasonable)
- Code clarity - Good
Automated review by Claude AI
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
🧪 测试结果
总体结果: ✅ 所有测试通过 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/test.yml:
- Around line 30-40: The CI currently runs bun install without a frozen lockfile
while bun.lock is ignored, risking non-reproducible builds; either commit
bun.lock, restore the --frozen-lockfile flag on the "Install dependencies" steps
(all occurrences of the step named "Install dependencies") and update the cache
key hashFiles(...) to include 'bun.lock', or if you intentionally omit the
lockfile document that decision in the PR description; apply the chosen fix for
every matching "Install dependencies" step (lines referenced in the review).
- Around line 146-152: The CI step that runs Vitest is passing two positional
test paths which filter and override the 13-file list in the
integration.config.ts testFiles; update the "Run integration tests" step so the
intended full suite runs: either remove the positional arguments (the two
entries "tests/integration/usage-ledger.test.ts" and
"tests/integration/my-usage-imported-ledger.test.ts") so Vitest uses testFiles
from --config, or deliberately trim integration.config.ts.testFiles to only
include those two if you only want them in this job, or add separate CI jobs
that explicitly run the other 11 tests (e.g., create jobs that run vitest with
those specific file paths or configs) to ensure no declared tests are silently
skipped.
🪄 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: 1964ee31-50fd-41dd-8893-9e5ce4890ccc
📒 Files selected for processing (8)
.github/workflows/dev.yml.github/workflows/pr-check.yml.github/workflows/release.yml.github/workflows/test.ymlDockerfiletests/unit/public-status/config-publisher.test.tstests/vitest.base.tsvitest.config.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- Dockerfile
- .github/workflows/pr-check.yml
- .github/workflows/dev.yml
- tests/unit/public-status/config-publisher.test.ts
- .github/workflows/release.yml
- vitest.config.ts
- tests/vitest.base.ts
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
🧪 测试结果
总体结果: ✅ 所有测试通过 |
|
All actionable review comments have been addressed and re-verified. The remaining CodeRabbit inline comments now show "Addressed in commits 44a2d6d to 6529bdd", and all required checks are passing. @coderabbitai review |
|
🧠 Learnings used✅ Actions performedReview triggered.
|
github-project-automation
Bot
moved this from Backlog
to Done
in Claude Code Hub Roadmap
1 participant
Summary
.bun-versionand update@typescript/native-previewto7.0.0-dev.20260425.1for faster type checking.bun installpaths for workflows and Docker because this repo intentionally ignores lockfiles.Verification
/tmp/cch-ci-timing-baseline-3785825/tmp/cch-ci-verify-final-1777123355/tmp/cch-oracle-fix-integration-1777126125/ci_integration.log(2 passed,19 passed)/tmp/cch-public-status-verify-1777123966/public_status.log(3 passed,18 passed)/tmp/cch-e2e-verify-1777123991/e2e.log(3 passed,36 passed)/tmp/cch-review-fix-precommit-rerun-1777128170(build,lint,lint:fix,typecheck,testall exit 0;548 files,5189 tests)/tmp/cch-review-fix-docker-1777128396(deploy/root no-cache builds passed withRUN bun install)/tmp/cch-review-ci-integration-1777129283/ci_integration_config.log(2 passed,19 passed)/tmp/cch-review-second-fix-precommit-1777129308(build,lint,lint:fix,typecheck,testall exit 0;548 files,5189 tests)bg_e48f5f72; blocking findings fixed before PR.Notes
develop, but this repository has nodevelopbranch and project instructions specify PR targetdev, so this PR targetsdev.bun.lock/bun.lockbare intentionally ignored by this repository, so CI/Docker use non-frozenbun installand cache onpackage.jsonplus.bun-versioninstead of lockfiles.Greptile Summary
This PR pins Bun to
1.3.10via.bun-version, bumps@typescript/native-previewto the April 25 nightly, adds job-level timeouts and Bun/Next.js caches to all four CI workflows, switches tobun install(no lockfile) consistently across Docker and workflows, and tunes Vitest worker/concurrency/parallelism settings to reduce DB and Redis contention in stateful test suites.The refactoring of
vitest.config.tsto import shared helpers fromtests/vitest.base.ts(rather than duplicating them) is a clean improvement. One concern:integration.config.tsnow lists only 2 test files where it previously listed 12, sobun run test:integrationsilently skips auth, webhook, notification, and provider integration tests going forward.Confidence Score: 5/5
Safe to merge; all changes are tooling and CI configuration with no product behavior impact.
No P0 or P1 findings. The single P2 (integration test scope reduction in
bun run test:integration) is a coverage/discoverability concern but does not affect production code or CI pass/fail gates since those tests were already excluded from CI runs before this PR.tests/configs/integration.config.ts — the pruned test file list may mislead developers running local integration tests.
Important Files Changed
bun install; matches stated policy of not tracking lockfiles.bun run test:integrationnow covers only 2 files.Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[push / PR] --> B{Workflow} B --> C[test.yml] B --> D[pr-check.yml] B --> E[dev.yml] B --> F[release.yml] C --> G[quality job\ntimeout: 15m] C --> H[unit-tests job\ntimeout: 15m] C --> I[integration-tests job\ntimeout: 25m] C --> J[api-tests job\ntimeout: 35m] G --> K[bun install\nno lockfile] H --> K I --> K J --> K K --> L[(Bun pkg cache\nhashFiles package.json + .bun-version)] I --> M[vitest run\n--config integration.config.ts\n2 test files\nfileParallelism: false] J --> N[bun run build\nNext.js cache] N --> O[vitest run\n--config e2e.config.ts\nfileParallelism: false] style M fill:#f9f,stroke:#c66 style L fill:#bbf,stroke:#66cPrompt To Fix All With AI
Reviews (3): Last reviewed commit: "ci: document no-lock install strategy" | Re-trigger Greptile