Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1479,6 +1479,10 @@ jobs:
echo "Using persistent npm cache at ${cache_dir}"
du -sh "${cache_dir}" 2>/dev/null || true

- name: 'Disk floor gate (self-hosted)'
if: "${{ steps.ci_profile.outputs.ci_profile == 'full' && runner.environment == 'self-hosted' }}"
run: 'bash .github/scripts/check-disk-floor.sh "${GITHUB_WORKSPACE}" "${RUNNER_TEMP:-/tmp}"'
Comment on lines +1482 to +1484

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] R2-1: The disk-floor gate invocation this PR adds is the fourth byte-identical copy in this workflow (test line 477, web_shell_e2e_smoke line 903, this one, integration_cli line 1699), but the contract test pins only this copy. If a later edit softens any of the other three — appending || true or adding continue-on-error: true to silence an intermittent disk-full flake, say — every test stays green while that job's ENOSPC admission check is silently disabled, because nothing asserts their shape. The repo already solved this exact shape twice: .github/actions/verify-checkout-head keeps the four checkout guards from drifting while each caller keeps its own run condition, and this same test file's guardCalls/nodeCalls loops pin every call site of those steps — the file's own comment says those call sites "must be pinned too, or a revert to the inline pre-PR script keeps the suite green".

Witness:

MUTANT (append `|| true` to the test job's copy, ci.yml:477):
  Test Files 1 passed (1) / Tests 13 passed (13)   ← gate silently disabled, all green
SAME MUTANT + loop over the four jobs' gate steps:
  Test Files 1 failed (1) / Tests 1 failed | 12 passed (13)
  AssertionError: test disk floor gate invocation:
  - run: 'bash .github/scripts/check-disk-floor.sh "${GITHUB_WORKSPACE}" "${RUNNER_TEMP:-/tmp}"'
  + run: 'bash .github/scripts/check-disk-floor.sh "${GITHUB_WORKSPACE}" "${RUNNER_TEMP:-/tmp}" || true'

Either move the invocation into a composite action (each caller keeps its own if:, as verify-checkout-head does), or keep the inline copies and extend the contract assertions into a loop over all four jobs' Disk floor gate (self-hosted) steps, mirroring guardCalls:

for (const jobName of ['test', 'web_shell_e2e_smoke', 'integration_no_ak', 'integration_cli']) {
  const gate = getWorkflowStep(getWorkflowJob(workflow, jobName), 'Disk floor gate (self-hosted)');
  expect(gate).toContain(
    'run: \'bash .github/scripts/check-disk-floor.sh "${GITHUB_WORKSPACE}" "${RUNNER_TEMP:-/tmp}"\'',
  );
  expect(gate).not.toContain('continue-on-error');
  expect(gate).not.toContain('|| true');
}

The fix must keep every gate invoked after its job's Checkout step — .github/scripts/check-disk-floor.sh:9: "Checkout populates this script first, so the gate cannot protect checkout." Whichever shape is chosen, the loop (or the composite action's single pinned body) must go red if any one call site reverts to a softened or differently-invoked copy — please remove one call site's coverage and confirm the mutation turns the suite red.

中文说明

本 PR 新增的磁盘门槛调用是该 workflow 中第四份逐字节相同的副本(test 477 行、web_shell_e2e_smoke 903 行、本处、integration_cli 1699 行),但契约测试只固定了本副本。若日后有人软化其他三份之一——比如为消除偶发磁盘满导致的 flake 而追加 || true 或加上 continue-on-error: true——所有测试仍为绿色,而该 job 的 ENOSPC 准入检查已被静默禁用,因为没有任何断言约束它们的形状。仓库对同类形状已有两个现成机制:.github/actions/verify-checkout-head 使四处 checkout 守卫不会漂移、且各调用方保留自己的运行条件;同一测试文件的 guardCalls/nodeCalls 循环固定了那些步骤的所有调用点——文件自己的注释写明这些调用点"也必须被固定,否则回退到 PR 前的内联脚本仍会让套件保持绿色"。

实测:把 test job 的副本(ci.yml:477)追加 || trueTests 13 passed (13)——门槛被静默禁用,全绿;加上"对四个 job 的门槛步骤循环断言"后:1 failed | 12 passed,断言错误指明被软化的 run: 行。探针双向翻转。

修复可二选一:把调用移入复合 action(各调用方保留自己的 if:,与 verify-checkout-head 一致);或保留内联副本,把契约断言扩展为对四个 job 的 Disk floor gate (self-hosted) 步骤的循环(仿照 guardCalls)。

修复不得违反的前提:任何共享机制仍须在各 job 的 Checkout 之后调用——.github/scripts/check-disk-floor.sh:9:"Checkout populates this script first, so the gate cannot protect checkout."。无论采用哪种形式,删除任一调用点的覆盖都必须使测试变红——请移除某一处覆盖并回放该变异以确认。

— qwen3.8-max via Qwen Code /review (v0.22.3)


- name: 'Install Dependencies'
if: "${{ steps.ci_profile.outputs.ci_profile == 'full' }}"
env:
Expand Down
21 changes: 21 additions & 0 deletions scripts/tests/no-ak-integration-ci.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,7 @@ describe('no-AK integration CI wiring', () => {
for (const stepName of [
'Setup Node.js (hosted)',
'Use pre-installed Node.js (self-hosted)',
'Disk floor gate (self-hosted)',
'Install Dependencies',
'Run required no-AK integration gate',
]) {
Expand All @@ -232,6 +233,26 @@ describe('no-AK integration CI wiring', () => {
).toContain("steps.ci_profile.outputs.ci_profile == 'full'");
}

const diskFloorGate = getWorkflowStep(
gateJob,
'Disk floor gate (self-hosted)',
);
expect(diskFloorGate).toContain(
"if: \"${{ steps.ci_profile.outputs.ci_profile == 'full' && runner.environment == 'self-hosted' }}\"",
);
expect(diskFloorGate).toContain(
'run: \'bash .github/scripts/check-disk-floor.sh "${GITHUB_WORKSPACE}" "${RUNNER_TEMP:-/tmp}"\'',
);
expect(diskFloorGate).not.toContain('continue-on-error');
expect(diskFloorGate).not.toContain('|| true');
expect(diskFloorGate).not.toContain('env:');
Comment on lines +246 to +248

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] R2-3: This block pins the disk-floor gate's hard-fail shape, but nothing in the suite pins the Verify checkout includes expected head commit guard after Checkout for integration_no_ak — the guard call there is asserted on uses: and expected_sha content only, while the Windows job pins exactly that ordering (expect(guardUseIndex).toBeGreaterThan(windowsCheckoutIndex)). Moving the guard above Checkout in this job escapes every assertion (measured: 13/13 green, and none of the other workflow contract suites — 246 tests — catches it either). On a warm runner the guard would then verify the previous run's stale workspace against the expected SHA instead of the fresh checkout — defeating exactly the stale-checkout detection it exists for — and on a fresh runner the repo-local ./ action does not resolve at all.

Witness:

MUTANT (guard above Checkout in integration_no_ak): Test Files 1 passed (1) / Tests 13 passed (13)
MUTANT + guard-ordering pin: AssertionError: expected 7621 to be less than 7408
INTACT + guard-ordering pin: Tests 13 passed (13)

Add beside the pins this PR adds:

expect(gateJob.indexOf("name: 'Checkout'")).toBeLessThan(
  gateJob.indexOf("uses: './.github/actions/verify-checkout-head'"),
);

The suite's own Windows-section comment states the contract this pin encodes: "Repository-local ./ actions resolve from the job workspace, so the checkout must precede them." With the pin in place, the guard-above-checkout reorder must turn this test red — please apply that mutation and confirm.

中文说明

这一段固定了磁盘门槛步骤的硬失败形状,但整个套件都没有为 integration_no_ak 固定 Verify checkout includes expected head commit 守卫在 Checkout 之后的顺序——该 job 中的守卫调用只断言了 uses:expected_sha 内容,而 Windows job 恰好固定了这一顺序(expect(guardUseIndex).toBeGreaterThan(windowsCheckoutIndex))。把该守卫移到本 job 的 Checkout 之上可以逃过所有断言(实测:13/13 全绿;其他 246 个 workflow 契约测试也不会捕获)。在常驻 runner 上,守卫会拿上一次运行遗留的工作区与期望 SHA 比对,而不是新鲜的 checkout——恰恰废掉了它存在的意义(陈旧检出检测);在全新 runner 上,仓库本地 ./ action 根本无法解析。

实测:变异(守卫移到 Checkout 之上)→ 13/13 全绿;变异 + 守卫顺序固定 → 红(expected 7621 to be less than 7408);原样 + 新固定 → 13/13 绿。

修复须遵守测试文件 Windows 部分自己写下的契约:"Repository-local ./ actions resolve from the job workspace, so the checkout must precede them."。加上固定后,守卫移到 Checkout 之上的重排必须使本测试变红——请回放该变异确认。

— qwen3.8-max via Qwen Code /review (v0.22.3)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] R1-1: (fix-induced) The env: negation the round-1 fix added reads as coverage against env-based gate-neutering, but it only sees this step's own slice: a job-level or workflow-level env: zeroing DISK_FLOOR_MIN_FREE_KB / DISK_FLOOR_MIN_FREE_INODES still disables the floor with every assertion here green. GitHub Actions merges workflow → job → step env into the gate step, and the script's validator accepts 0 (.github/scripts/check-disk-floor.sh:25-36), so with both floors at zero the breach comparison is unsatisfiable and the gate emits a normal-looking sample and exits 0 on any host — e.g. after an "emergency unblock" during a fleet-wide disk incident the exact ENOSPC-mid-install failure (#10035) this PR exists to prevent returns, with this test reporting the env vector fully covered. The premise the fix must not violate: the step-scoped negation here stays step-scoped (ci.yml legitimately carries env: blocks on neighbouring steps), so the new assertions pin the override names out of the whole workflow instead.

Witness:

MUTANT (job-level env: DISK_FLOOR_MIN_FREE_KB: '0', DISK_FLOOR_MIN_FREE_INODES: '0'):
  script on saturated dir: exit 0, sample DISKFLOOR ... floor_kb[0] floor_inodes[0]
  (huge-floor control arm: ::error::Disk floor breached, exit 1)
  suite: Tests 13 passed (13)
MUTANT + whole-workflow name pins: Test Files 1 failed (1) / Tests 1 failed | 12 passed (13)
  AssertionError: ... not to contain 'DISK_FLOOR_MIN_FREE_KB'
INTACT + whole-workflow name pins: Tests 13 passed (13)

Pin the override names out of the whole workflow (the script's documented override stays available for manual/local runs):

expect(workflow).not.toContain('DISK_FLOOR_MIN_FREE_KB');
expect(workflow).not.toContain('DISK_FLOOR_MIN_FREE_INODES');

(or at minimum assert both names absent from gateJob, which covers job-level and every step-level env in the job). The fix must pin the names out of the CI workflow only, not remove the script's documented override support (.github/scripts/check-disk-floor.sh:14-16: "Floors, checked per directory's filesystem (env-overridable)"). With the assertions in place, adding the job-level zero-env block must turn this test red — please apply that mutation and confirm.

中文说明

(fix-induced)第 1 轮修复加入的 env: 负向固定看似覆盖了"通过 env 使门槛失效"的路径,但它只能看到该步骤自身的切片:在 job 级或 workflow 级 env: 中把 DISK_FLOOR_MIN_FREE_KB / DISK_FLOOR_MIN_FREE_INODES 置 0,仍能在本文件所有断言保持绿色的情况下禁用磁盘门槛。GitHub Actions 会把 workflow → job → step 的 env 合并进门槛步骤,且脚本的校验器接受 0.github/scripts/check-disk-floor.sh:25-36),于是两个地板均为 0 时,突破比较永远不成立,门槛会输出看似正常的采样并在任何主机上以 0 退出——例如在一次全集群磁盘事故中作为"紧急放行"加上之后,本 PR 要消除的"安装中途 ENOSPC"(#10035)会原样返回,而本测试还报告 env 路径已被完全覆盖。修复必须遵守的前提:此处的步骤级负向固定保持步骤级作用域(ci.yml 中相邻步骤合法地携带 env: 块),因此新断言改为把覆盖开关名固定出整个 workflow。

实测:变异(job 级 env 置 0)→ 脚本对饱和目录退出 0(样本 DISKFLOOR ... floor_kb[0] floor_inodes[0];巨值地板对照组报 ::error::Disk floor breached 并退出 1),套件 13/13 全绿;变异 + 全 workflow 名称固定 → 1 failed | 12 passedAssertionError: ... not to contain 'DISK_FLOOR_MIN_FREE_KB');原样 + 新固定 → 13/13 绿。

修复只应把这两个覆盖开关名固定出 CI workflow,不要移除脚本文档化的覆盖支持(.github/scripts/check-disk-floor.sh:14-16:"Floors, checked per directory's filesystem (env-overridable)");或至少断言这两个名称不出现在 gateJob 中(覆盖 job 级及该 job 内所有步骤级 env)。加上断言后,加入 job 级置 0 env 块必须使本测试变红——请回放该变异确认。

— qwen3.8-max via Qwen Code /review (v0.22.3)

expect(gateJob.indexOf("id: 'ci_profile'")).toBeLessThan(
gateJob.indexOf("name: 'Disk floor gate (self-hosted)'"),
);
Comment thread
yiliang114 marked this conversation as resolved.
expect(
gateJob.indexOf("name: 'Disk floor gate (self-hosted)'"),
).toBeLessThan(gateJob.indexOf("name: 'Install Dependencies'"));
Comment on lines +252 to +254

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] R2-2: These ordering pins bound the gate between ci_profile and Install Dependencies, but nothing pins it after Checkout — and the gate runs the workspace-relative path bash .github/scripts/check-disk-floor.sh, whose own header says "Checkout populates this script first". A reorder that keeps ci_profile above the gate while moving Checkout below it — e.g. Checkout moved down between the gate and Install Dependencies — makes the gate execute before checkout with every assertion here green (measured: the suite still passes 13/13). On the warm self-hosted pool this job targets — its workspace persists between runs, which is exactly why the job opens with Restore workspace ownership / Clean stale .qwen before checkout — the pre-checkout gate would execute the previous run's check-disk-floor.sh, and since this job checks out PR heads those stale bytes are contributor code; on a fresh runner the reordered step instead fails at runtime with "No such file or directory". This file already pins the same contract for the Windows job's ./-relative references.

Witness:

MUTANT (Checkout moved below the gate): Test Files 1 passed (1) / Tests 13 passed (13)
MUTANT + checkout-ordering pin: AssertionError: expected 10376 to be less than 10122
INTACT + checkout-ordering pin: Tests 13 passed (13)
Suggested change
expect(
gateJob.indexOf("name: 'Disk floor gate (self-hosted)'"),
).toBeLessThan(gateJob.indexOf("name: 'Install Dependencies'"));
expect(gateJob.indexOf("name: 'Checkout'")).toBeLessThan(
gateJob.indexOf("name: 'Disk floor gate (self-hosted)'"),
);
expect(
gateJob.indexOf("name: 'Disk floor gate (self-hosted)'"),
).toBeLessThan(gateJob.indexOf("name: 'Install Dependencies'"));

The pin must encode gate-after-checkout, never the reverse (.github/scripts/check-disk-floor.sh:9). With the pin in place, moving Checkout below the gate must turn runs the no-AK integration script as its own check on PRs and the merge queue red — please apply that mutation and confirm.

中文说明

这些顺序断言把门槛限定在 ci_profileInstall Dependencies 之间,但没有把它固定在 Checkout 之后——而门槛执行的是工作区相对路径 bash .github/scripts/check-disk-floor.sh,脚本头部明确写着 "Checkout populates this script first"。若某次重排保持 ci_profile 在门槛之上、却把 Checkout 移到门槛之下(例如把 Checkout 下移到门槛与 Install Dependencies 之间),门槛会在 checkout 之前执行,而这里的所有断言仍为绿色(实测:套件仍 13/13 全绿)。在本 job 所用的常驻 self-hosted 池上,工作区在运行之间持久存在——这正是该 job 以 Restore workspace ownership / Clean stale .qwen before checkout 开头的原因——checkout 之前执行的门槛会运行上一次运行遗留的 check-disk-floor.sh,而本 job 检出的是 PR head,这些遗留字节来自贡献者代码;在全新 runner 上,重排后的步骤则会在运行时以 "No such file or directory" 失败。本文件已为 Windows job 的 ./ 相对引用固定过同类契约。

实测:变异(Checkout 移到门槛之下)→ 13/13 全绿;变异 + 新增顺序固定 → 红(expected 10376 to be less than 10122);原样 + 新固定 → 13/13 绿。

该固定必须表达"门槛在 Checkout 之后",绝不能相反(.github/scripts/check-disk-floor.sh:9)。加上固定后,把 Checkout 移到门槛之下必须使 runs the no-AK integration script as its own check on PRs and the merge queue 变红——请回放该变异确认。

— qwen3.8-max via Qwen Code /review (v0.22.3)


const gateStep = getWorkflowStep(
gateJob,
'Run required no-AK integration gate',
Expand Down
Loading