Skip to content

ci: add pull request validation workflow - #113

Merged
yxsj245 merged 1 commit into
GSManagerXZ:mainfrom
xiwangly2:feat/ci-cd-workflows
Oct 2, 2026
Merged

yxsj245 merged 1 commit into
GSManagerXZ:mainfrom
xiwangly2:feat/ci-cd-workflows

Conversation

@xiwangly2

Copy link
Copy Markdown
Contributor

Summary

This PR adds a separate CI workflow for regular contribution validation, based on upstream/main and independent from the current riscv64 Draft PR.

Changes

  • Add .github/workflows/ci.yml for PR, main push, and manual validation.
  • Run server npm ci, npm test, and npm run build.
  • Run client npm ci, npm test, and npm run build.
  • Add a root project-scripts job for package script syntax checks and build version resolution.
  • Switch release package workflow dependency installs from npm install to npm ci and cache all lockfiles.
  • Update CI/CD docs to match the actual workflow files.
  • Fix the existing client chunk-upload test baseline so frontend tests can run reliably in CI.

Validation

  • git diff --check
  • workflow YAML parse check for .github/workflows/ci.yml and .github/workflows/build.yml
  • node --check scripts/package.js
  • node --check scripts/resolve-build-version.js
  • node scripts/resolve-build-version.js
  • cd server && npm test
  • cd server && npm run build
  • cd client && npm test
  • cd client && npm run build

Note: client build still reports the existing Vite chunk/static-dynamic import warnings, but the build exits successfully.

@xiwangly2
xiwangly2 force-pushed the feat/ci-cd-workflows branch from 9f251eb to eb23f4e Compare October 1, 2026 12:06
@xiwangly2

Copy link
Copy Markdown
Contributor Author

Rebased this CI PR onto upstream bb66efb.

Validation performed locally after rebase:

  • git diff --check -> passed.
  • node --check scripts/package.js -> passed.
  • node --check scripts/resolve-build-version.js -> passed.

No workflow conflicts were required; this keeps the CI changes current with upstream's latest build artifact naming changes.

@yxsj245

yxsj245 commented Oct 1, 2026

Copy link
Copy Markdown
Collaborator

PR #113 审查报告:ci: add pull request validation workflow

  • 审查对象:ci: add pull request validation workflow #113
  • 作者:xiwangly2 | 状态:open(非草稿)
  • base:GSManagerXZ:main @ bb66efb736f13f8eff0f1831d2a623cd97437426
  • head:xiwangly2:feat/ci-cd-workflows @ eb23f4e96ea6e28ef33cb1e2e14304b77ddd3a03(本地 fetch 已验证一致)
  • merge-base:bb66efb(head 基于 bb66efb,快进基线)
  • 上游 main 当前 tip:3f82101ccf843d8da9e8ce0cea43843be987aa60(base 之后新增 6 个提交)
  • 审查快照:2026-10-02(commit 差异范围 bb66efb...eb23f4e,1 个提交,7 文件,+214/-70)

结论:暂不可合并

理由:平台报告 mergeable: false / mergeable_state: dirty,本地试合并上游 main (3f82101) 复现了同一处内容冲突:

  • client/src/utils/__tests__/chunkUpload.preservation.test.ts(CONFLICT content,UU)

需要作者 rebase 解决冲突后复审。除此之外,在已审查范围内未发现阻塞缺陷;PR 在 GitHub 上触发的 3 个必需检查全部成功。

按模块改动汇总

1. 新增 .github/workflows/ci.yml(+106)

  • 触发:pull_request(main) 的 opened/synchronize/reopened/ready_for_review、push(main)、workflow_dispatch。
  • 三个独立 job:
    • server:npm ci → npm test → npm run build(working-directory: server)
    • client:npm ci → npm test → npm run build(working-directory: client)
    • project-scripts:npm ci → node --check scripts/package.js 与 scripts/resolve-build-version.js → 运行版本解析
  • 安全性良好:permissions: contents: read(最小权限)、pull_request 触发(非 pull_request_target,fork PR 无凭据暴露面)、未使用任何 secrets、concurrency 按 PR/分支取消旧运行。
  • 用户可见影响:贡献者的 PR 会自动获得服务端/客户端测试与构建校验反馈。

2. 修改 .github/workflows/build.yml(+14/-10)

  • 发布打包工作流依赖安装从 npm install 改为 npm ci(root/client/server),并为 setup-node 缓存声明三个 lockfile 路径。
  • 影响与注意:npm ci 更可复现,但要求 lockfile 与 package.json 严格同步,否则构建会失败(这属于该工作流本身的严格化意图)。已同步缓存键,避免只缓存根 lockfile 的问题。

3. 测试基线修复(与 CI 可运行性配套)

  • client/src/utils/chunkUpload.ts(1 行):注释"默认50MB"改为"默认20MB",与代码中既有的 DEFAULT_CHUNK_SIZE = 20 * 1024 * 1024 对齐(仅注释,无行为变化)。
  • chunkUpload.preservation.test.ts:测试基线阈值 10MB→20MB、分片 50MB→20MB(同步代码现状);属性2 范围缩小到 21–120MB、numRuns 10→5 以保 CI 稳定;abort 用例增加 maxRetries: 1、console spy、补 await uploadPromise。
  • chunkUpload.fault.test.ts:将 merge 请求的 fetch mock 提前挂载(cleanupUpload 开头即 mock 初始化请求),替代轮询后才 mock 的方式,消除时序脆弱性。

4. 文档(2 个文件)

  • docs/GitHub-Actions-多架构构建说明.md、docs/PR自动语法检查说明.md:更新为与实际 workflow 命令一致(npm ci、测试/构建命令、脚本检查)。

按严重级别的问题

P1(阻塞合并,但非代码缺陷)

  1. 与上游 main 存在合并冲突 — client/src/utils/__tests__/chunkUpload.preservation.test.ts
    • 触发条件:将 PR 合并到 main @ 3f82101(平台与本地一致复现)。
    • 根因:main 侧在 3f82101 中已独立合入几乎相同的基线修复(10MB→20MB、50MB→20MB),但实现方式不同:
      • main 侧:期望值用计算式(EXPECTED_CHUNKS = Math.ceil(FILE_SIZE / DEFAULT_CHUNK_SIZE)),属性2 保留 500MB 上限、numRuns 10、超时放宽至 120000ms,并加了实现同步说明注释。
      • PR 侧:期望值硬编码(10 个分片),属性2 缩小到 120MB、numRuns 5、超时 60000ms。
    • 影响:无法合并;且两边对"CI 稳定性"的处理策略不同,需作者择一或融合(main 侧计算式期望更稳健,PR 侧的范围缩小更省时)。
    • 建议:作者 rebase 到最新 main;preservation.test.ts 建议以上游 main 版本为基底,仅叠加本 PR 特有的改进(abort 用例的 maxRetries: 1、console spy、await uploadPromise)。fault.test.ts 两边改动位置不同、可自动合并,无碍。

P2/P3

验证表

项目 命令 / 来源 结果
head SHA 一致性 git fetch origin pull/113/head + git rev-parse ✅ FETCH_HEAD == eb23f4e
base 祖先关系 git merge-base --is-ancestor bb66efb 3f82101 ✅ 通过(base 是 main 历史前缀)
合并兼容性 本地 git merge --no-commit 3f82101(已 abort 还原) ❌ preservation.test.ts 内容冲突(与平台 dirty 一致)
GitHub CI(head eb23f4e) check-runs API ✅ Project scripts / Server test and build / Client test and build 全部 success
相关单元测试(本地) npx vitest run src/utils/__tests__/chunkUpload.fault.test.ts src/utils/__tests__/chunkUpload.preservation.test.ts(Node v24.16.0,vitest 5.x) ✅ 2 文件 10 测试全通过(23.16s)
client 构建(本地) npm run build --prefix client ✅ 成功(14.74s,既有 chunk 体积警告,PR 描述已披露)
脚本语法(本地) node --check scripts/package.js / scripts/resolve-build-version.js;node scripts/resolve-build-version.js ✅ 全部 exit 0(版本解析 3.13.32-2-geb23f4e)
依赖安装(本地) npm ci --prefix client ✅ exit 0(npm audit 报既有依赖告警,非本 PR 引入)
server 测试/构建(本地) 未执行 PR 无 server 改动;平台 CI 已通过,风险低

未验证项与剩余风险

  • 本地 Node 为 v24.16.0,CI 固定 22.16.0,存在小版本差异(无 Node 版本相关代码改动,风险低)。
  • workflow 在合并到 main 后的 push 触发路径(push: branches: [main])无法在 PR 上预演,需合并后观察首轮运行。
  • 未运行 server 本地测试(PR 无 server 改动,以平台 CI 为准)。
  • 合并冲突解决后的 rebase 结果未审查(尚未产生新提交)。

结论绑定的提交快照

  • 本结论仅对 base bb66efb + head eb23f4e 有效。head 更新(含 rebase 解决冲突后)需要重新审查并更新本报告。

审查资源与清理状态

  • 审查分支:review/pr-113-eb23f4e-r1(本地,未推送)
  • 审查 worktree:C:\Users\17737\Code\Opensource\GameServerManager3\dsh-pr-review-wt\pr-113-eb23f4e
  • 状态记录:.git/dsh-pr-reviews/pr-113-eb23f4e-r1/state.json
  • 合并状态:未合并;worktree 与分支保留,待作者 rebase 后复审或确认合并后清理。
  • 备注:worktree 中因本地验证产生 client/node_modules、client/dist 等未跟踪产物,清理时若见异常文件应先确认(这些是验证产物,非 PR 内容)。

@xiwangly2
xiwangly2 force-pushed the feat/ci-cd-workflows branch from eb23f4e to 8e00a27 Compare October 2, 2026 06:23
@xiwangly2

Copy link
Copy Markdown
Contributor Author

Rebased this CI PR onto upstream 3f82101 and resolved the remaining conflict in client/src/utils/__tests__/chunkUpload.preservation.test.ts.

Conflict resolution followed the review suggestion:

  • Kept upstream/main's computed expected chunk counts and broader preservation-test baseline.
  • Re-applied this PR's abort-test stability fixes: maxRetries: 1, console warning/error spies, and awaiting the upload promise before test exit.

Validation after rebase:

  • git diff --check upstream/main...HEAD -> passed.
  • node --check scripts/package.js -> passed.
  • node --check scripts/resolve-build-version.js -> passed.
  • node scripts/resolve-build-version.js -> resolved 3.13.32-8-g8e00a27.
  • npm ci --prefix client -> passed.
  • npm test --prefix client -- --run src/utils/__tests__/chunkUpload.fault.test.ts src/utils/__tests__/chunkUpload.preservation.test.ts -> 2 files / 10 tests passed.
  • npm run build --prefix client -> passed, with existing Vite chunk-size/dynamic-import warnings only.

@xiwangly2
xiwangly2 force-pushed the feat/ci-cd-workflows branch from 8e00a27 to 3d36b99 Compare October 2, 2026 11:06
@xiwangly2

Copy link
Copy Markdown
Contributor Author

Rebased this CI PR onto upstream dba015e after #112 and #115 landed. No conflicts were required during this rebase.

Validation after rebase:

  • git diff --check upstream/main...HEAD -> passed.
  • node --check scripts/package.js -> passed.
  • node --check scripts/resolve-build-version.js -> passed.
  • node scripts/resolve-build-version.js -> resolved 3.13.32-19-g3d36b99.
  • npm test --prefix client -- --run src/utils/__tests__/chunkUpload.fault.test.ts src/utils/__tests__/chunkUpload.preservation.test.ts -> 2 files / 10 tests passed.
  • npm test --prefix server -> 9 suites / 80 tests passed.
  • npm run build --prefix server -> passed.
  • npm run build --prefix client -> passed, with existing Vite chunk-size/dynamic-import warnings only.

@yxsj245
yxsj245 merged commit cce1074 into GSManagerXZ:main Oct 2, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants