fix(ci): cache downloaded linters on ECS runners - #9001
Merged
Conversation
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
yiliang114
marked this pull request as ready for review
August 12, 2026 12:02
Collaborator
Collaborator
Author
|
Closeout update: the shared cache is now treated as untrusted. Every archive is copied into the job-private directory and checked against a repository-pinned SHA-256 before extraction; a mismatch is replaced through the verified cold-download path. The focused cache regression, Prettier, ESLint, build, and workspace typecheck pass locally. Full CI is running on 0d6cfa3. |
Collaborator
|
Thanks for the PR! Template looks good ✓ — all sections present, including the restored Environment section and the Chinese summary. Problem: observed and quantified, not theoretical. The cited runs check out: run Direction: aligned — this hardens the repo's own CI gate against a measured outage. No CHANGELOG signal applies to internal CI infra. Size: not applicable — only Approach: scope feels right. Cache versioned archives on the persistent host, and verify a job-local copy against repository-pinned SHA-256 before extracting — the minimal fix that removes the per-job dependency on live GitHub Releases while ensuring a shared cache is never executed on trust alone. Heavier alternatives (actions/cache, an OSS mirror) would add infrastructure for the same outcome. Nothing unrelated in the diff. Risk: no elevated risk signals — changed files don't match any high-risk path. Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ —— 各部分齐全，包括恢复的 Environment 部分和中文说明。 问题：已观测且有量化数据，不是理论问题。引用的运行记录经核实无误：run 方向：对齐——这是在加固仓库自身 CI 门禁，对抗有量化数据的故障。内部 CI 基础设施不适用 CHANGELOG 信号。 规模：不适用——仅改动 方案：范围合理。在持久化宿主机上按版本缓存归档，解压前用仓库内固定的 SHA-256 校验 job 本地副本——既消除了每个 job 对实时 GitHub Releases 的依赖，又确保共享缓存绝不会在未经校验的情况下被执行，是能解决问题的最小改动。更重的替代方案（actions/cache、OSS 镜像）会为同样的结果引入额外基础设施。diff 中没有无关改动。 风险：无升级风险信号——改动文件不匹配任何高风险路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
Code reviewMy independent take on "ECS runners can't reliably reach GitHub Releases" would have been exactly this shape: a persistent per-host archive cache keyed by tool version, with integrity verification of a job-local copy before anything is extracted. The PR matches that and gets the details right:
What the previous pass could not settle, this one did: I re-verified all six pinned SHA-256 digests against the official release artifacts — all match, and the actionlint ones also agree with that release's own The two new tests execute the generated installer for real and pin the load-bearing claims: a validator-passing archive with the wrong digest is rejected without polluting the cache, a verified cache hit installs with zero downloads, and an unpersistable cache target warns but still installs. Remove the guard logic and they fail. CI ran them green: Non-blocking: old versioned archives accumulate in Testing evidence (the PR's own CI — PR code not run locally)Per gate rules I did not build or execute PR code; the evidence below is the PR's own CI at the reviewed commit, plus the independent digest re-check above. Nothing left unverified this pass. The decisive check is
中文说明代码审查我对"ECS runner 无法可靠访问 GitHub Releases"的独立方案正是这个形状：宿主机上按工具版本持久缓存归档，任何解压之前先对 job 本地副本做完整性校验。PR 与之一致，且细节到位：
上一轮无法核实的一点，这一轮补齐了：六个固定的 SHA-256 摘要已逐一与官方发布产物比对——全部一致，actionlint 的三个还与该 release 自带的 两个新测试真实执行了生成的安装脚本，钉住了关键声明：摘要错误但能通过校验器的归档会被拒绝且不会污染缓存；已验证的缓存命中在安装时零下载；缓存目标不可写时只告警、安装照常。删掉守卫逻辑这些测试就会失败。CI 中全绿： 非阻塞项：旧版本归档会在 测试证据（PR 自身的 CI——未在本地运行 PR 代码）按门禁规则未构建或执行 PR 代码；以上证据来自该提交上 PR 自身的 CI，加上独立的摘要复核。本轮没有留下未验证项。 决定性检查是 CI 结论见上表（全绿）。 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
|
Confidence: 4/5 — clean across every stage; the single remaining nit is the unpruned cache growth, which the PR already acknowledges in Risk & Scope. Stepping back: the problem is real and quantified — 17 failed Test attempts in 24h on the ECS egress path, and I verified the cited failing runs myself. The solution is the smallest one that works, and strictly safer than what it replaces: the old code executed whatever CI at the reviewed commit is fully green (Qwen Code CI run 31621143501, including the self-hosted ECS Test leg), so approval is pinned to it now rather than deferred. 中文说明信心：4/5 —— 各阶段均干净；唯一剩余的小问题是缓存无清理，PR 已在 Risk & Scope 中声明。 退一步看：问题真实且有量化——ECS 出口路径上 24 小时内 17 次 Test 失败，引用的失败运行我也亲自核实过。方案是能解决问题的最小方案，并且严格优于被替换的旧代码：旧代码对 该提交上的 CI 已全绿（Qwen Code CI run 31621143501，含自托管 ECS Test 分支），因此直接批准并钉在该提交上，不再延迟。 — Qwen Code · qwen3.8-max Reviewed at |
qwen-code-ci-bot
previously approved these changes
Aug 12, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not explored to full depth (tool budget reached): Change summary: This PR caches versioned actionlint/shell...: npm install + actually running npm run test:scripts was not executed (the worktree has no node_modules). I relied on direct module simulation instead.; Change summary: This PR caches versioned actionlint/shell...: full npm run test:scripts execution — the worktree has no node_modules (vitest unresolvable), so I validated the test harness by direct module simulation in….
— qwen3.8-max via Qwen Code /review (v0.21.10)
Collaborator
Author
|
@qwen-code /review |
Contributor
|
Qwen Code review request accepted. Review is queued in workflow run. |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not explored to full depth (tool budget reached): Change under review: PR #9001 caches versioned actionlint...: none — all checks I started were completed within budget.; Change under review: PR #9001 caches versioned actionlint...: could not run the vitest suite ( npm run test:scripts ) — the review worktree has no node_modules and a full monorepo install was out of budget; the shell tes…; Change under review: PR #9001 caches versioned actionlint...: could not check pre-existing unresolved Critical threads on the PR — no access to PR comments from this session.; Change under review: PR #9001 caches versioned actionlint...: could not run the actual vitest suite ( scripts/tests/lint.test.js ) or ESLint/typecheck — no node_modules in the review worktree and npm ci was not viable;….
— qwen3.8-max via Qwen Code /review (v0.21.10)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not explored to full depth (tool budget reached): Change summary: PR #9001 caches versioned actionlint/shel...: could not run the actual vitest suite ( scripts/tests/vitest.config.ts ) because node_modules is not installed in this worktree — covered by the standalone harn…; Change summary: PR #9001 caches versioned actionlint/shel...: could not run the repository's own vitest suite ( npm run test:scripts ) because the worktree has no installed dependencies; the standalone harness covered the ….
— qwen3.8-max via Qwen Code /review (v0.21.10)
Collaborator
Author
|
Closeout status at ee7a70e: all required CI jobs pass, including the self-hosted ECS Test job and Web Shell smoke, and no review thread remains unresolved. @chiga0 @doudouOUC, the latest head is ready for re-review. |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not explored to full depth (tool budget reached): Change under review: PR #9001 (QwenLM/qwen-code) caches v...: none — all planned checks completed within budget.; Change under review: PR #9001 (QwenLM/qwen-code) caches v...: none (≈6 of ~39 tool calls used; all planned checks completed).; You are review agent reverse-audit — Reverse audit agen...: could not verify the remaining five SHA-256 pins (actionlint darwin_amd64/darwin_arm64, shellcheck linux.x86_64/darwin.x86_64/darwin.aarch64) against upstream —…; You are review agent reverse-audit — Reverse audit agen...: did not run a real end-to-end node scripts/lint.js --setup download (same egress restriction); installer behavior verified via the PR's hermetic fake-curl sui…; You are review agent reverse-audit — Reverse audit agen...: Windows-gate behavior assessed by code reading only (the path-logic test ran on Linux here; getLinters() throws on win32 before any shell is generated, and th…, and 6 more.
— qwen3.8-max via Qwen Code /review (v0.21.10)
Collaborator
Author
|
Closeout from resolve-pr-comments automation: Changed: PR body now includes the repository template's Chinese details section. No code changes. Pending: none found in this pass. |
Collaborator
Author
|
Updated the PR body to restore the current template Environment section. No code changes. |
Collaborator
Author
|
@qwen-code /triage |
Collaborator
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 64 passed · 0 failed · 64 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：64 通过 · 0 失败 · 64 总计 Verification reportPR #9001 Deep Verification —
|
| cell | arm | scenario | oracle | result |
|---|---|---|---|---|
| C1 | head | cold cache, real network | exit 0; binaries run (actionlint --version = 1.7.12); cache sha == pin; 2 downloads |
✅ 7/7 |
| C2 | head | warm cache, offline | exit 0; cache-sourced binary runs; 0 downloads | ✅ 3/3 |
| C3 | base | cold cache, real network | exit 0; 2 downloads; no cache dir written | ✅ 3/3 |
| C4 | base | warm host, offline | fails (control red): exit 1, curl: (6) + Failed to install actionlint on stderr, no binary |
✅ 4/4 |
| C5 | head | planted valid-tgz with wrong digest + fixture net | replaced with pinned bytes; planted bytes never executed; exactly 1 re-download | ✅ 4/4 |
| C6 | head | planted wrong digest + offline | fails closed: exit 1, plant never extracted, cache untouched | ✅ 3/3 |
| C7 | head | cache target path is a directory | exit 0; Warning: could not persist … on stderr; target untouched; job-local binary runs |
✅ 4/4 |
| C8 | head | corrupt download | exit 1; SHA-256 mismatch … expected 8aca8db9…, got 64c53703…; nothing persisted to cache |
✅ 3/3 |
| C9 | head | second warm job, offline | exit 0; per-run extraction dirs (102/107) coexist; 0 downloads | ✅ 3/3 |
The load-bearing flip: C2/C9 (head, offline) exit 0 with zero downloads vs C4 (base, offline) exit 1 — the exact failure mode from the PR's motivation (17 attempts dead on github.com:443 unreachable) no longer fails once one verified download has landed on a host. Witness: 02-ab-head-vs-base-offline-flip.png.
Secondary claim — integrity pins: all 6 pinned SHA-256s match the real upstream archives downloaded from the exact URLs lint.js constructs, and the linux archives contain the claimed tool versions (actionlint 1.7.12, shellcheck 0.11.0 verified by executing them). Witness: 01-sha256-pins-match-upstream.png.
Can the cache mechanism fire in production?
- The consuming jobs (
Install lintersin the full profile,GitHub CI helper checksin thegithub_ci_onlyprofile, both inci.yml) run directly on the runner — nocontainer:key — so$HOMEis the host's. On the self-hosted ECS lane the same job file already persists${HOME}/.cache/qwen-code/npmacross runs (production precedent for exactly this persistence assumption); the linter cache lands at${HOME}/.cache/qwen-code/linters(XDG override honored, covered by the PR's unit test). - On the kill-switch fallback (
ubuntu-latest, ephemeral) every run is a cold miss — behavior identical to base, no harm. - Archive filenames are versioned, so a version bump selects a fresh entry; stale entries are never read again. Retained size is ~8 MB per platform pair — the tradeoff the description names, confirmed negligible.
- Writer census (repo-wide grep):
lint.jsis the only writer ofqwen-code/linters. Concurrent first-downloads on one host are safe: both writers verify against the same pinned SHA before publishing viamktemp+ atomicrenameSync; readerscpthe final name, which rename swaps atomically. (A SIGKILL between mktemp and rename can orphan a*.XXXXXXsibling; readers never touch it — benign litter, not a correctness path.)
Reviewer Test Plan, per step
- Cold cache downloads and installs the pinned versions → C1 (+ executed
--versionoracles). Pass. - Validator-passing archive with the wrong digest in the shared cache is replaced → C5. Pass.
- Without outbound download access, a later job installs from the verified cache with a distinct extraction directory → C2 + C9. Pass.
- Cache target unavailable → verified job-local install still succeeds without replacing the target → C7. Pass.
All four steps were executable; none was structurally unreachable.
Findings
No blocking findings. One informational, pre-existing observation (not introduced by this PR, not a merge condition):
- Info (pre-existing): unused linter-version env vars in
ci.yml.ACTIONLINT_VERSION,SHELLCHECK_VERSION,YAMLLINT_VERSIONare declared at workflow level (lines 46–48) but no step consumes them; the versions are hardcoded inscripts/lint.js(the only two grep hits for each name are the declarations themselves). Present on the base tip as well. The only risk is a future version bump in one place making the other stale — a comment or consumption would close it, but this PR is not the right vehicle.
Not covered
- Windows ECS gate (
ecs-win): no Windows environment in this container.getPlatformArch()still throws lazily as before and the new test isskipIf(win32); the new top-levelgetLinterCacheDir()call uses onlyhomedir()/join(portable), but is untested on Windows here. - Per-commit attribution: depth-2 shallow checkout — 1 of the 6 PR commits is locally reachable (grafted). Verified the aggregate
HEAD^1..HEADdiff only. - Repo-wide gates (full ESLint,
prettier --write ., the remaining ~50scripts/testsfiles): not re-run; CI covers them and the PR touches two files, for which targeted gates ran (below). - The yamllint install step: container has no pip3, so it was shimmed identically on both arms; that installer is byte-identical between base and head.
- The real ECS egress failure cannot be reproduced on demand; the offline shim reproduces the wire shape (downloads that cannot complete), not the network-side cause. Likewise the HOME-persistence premise rests on the pre-existing npm-cache step in the same job file, not on a measurement of the ECS hosts.
- Mutation matrix scope: three guards of the new installer (cache-copy verification, best-effort write fallback, checksum enforcement) — each killed; no survivors to adjudicate. Guards outside the new code path (e.g.
test -x) were not mutated.
Methodology
Environment: CI verify container (node:22-bookworm, Node v22.23.2), merge-ref checkout at depth 2; outbound network available, used only to fetch the six upstream archives for pin verification and the C1/C3 cold cells. Harnesses are mock-free with respect to the unit under test — real node, real /bin/sh (dash), real tar/xz, real filesystem; the only substitution is the network endpoint, swapped at the PATH seam (curl shims), which is exactly the surface the production flake hits. The base arm is the HEAD^1 blob of scripts/lint.js verbatim (sha256 f66a089b… vs head b2348dfb…). The vitest suite ran at head (4/4) and under four point mutations in a scratch copy (M0 control green; M1 cache-verify removed, M2 write-failure-fatal, M3 verify-gutted all red with expected-vs-actual assertion failures — the test is not vacuous and each guard is pinned). Generated installer bash was checked with bash -n and with the actual shellcheck 0.11.0 binary recovered from the verified upstream fixture; prettier/ESLint gates on the changed files were proven live by planted violations before their green results were cited. Two harness bugs were found and fixed during the round (a relative symlink that silently bypassed the curl shims, and execFileSync dropping stderr in exit-0 cells); the discarded run's numbers were never reported, and the final figures above come from the fixed harness's re-runs, which are also the captured witnesses. Raw per-cell stdout/stderr: logs/; harness scripts: harness/; fixtures (real upstream bytes): fixtures/.
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
qwen-code-ci-bot
approved these changes
Aug 13, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM — verified end-to-end: all six digests match the upstream releases, the guard tests pin the behavior, and the self-hosted ECS Test job ran this commit green with linter setup down from 135s timeouts to ~0.25s. ✅
yiliang114
enabled auto-merge
August 13, 2026 05:10
ytahdn
approved these changes
Aug 13, 2026
Collaborator
|
Released in v0.21.11. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
This PR keeps versioned actionlint and shellcheck release archives in a persistent runner cache while preserving a separate extraction directory for every run, attempt, and job.
Before a cached archive is trusted, it is copied into the current job's private temporary directory and those copied bytes are checked against a platform-specific SHA-256 pinned in this repository. A missing or mismatched cache is downloaded with bounded retries, checksum-verified, and archive-validated. When the shared cache is writable, the verified archive is atomically published through a random temporary file; a cache write failure emits a warning and does not block the job-local installation. Extraction always uses the verified job-local copy.
Why it's needed
The self-hosted Linux gate previously downloaded both archives from GitHub Releases for every PR. In the 24 hours ending at 2026-08-12 09:55 UTC, 17 Test attempts failed before lint or tests because one of those downloads could not connect to
github.com:443; seven failed on actionlint and ten on shellcheck. PR #8976 reproduced the same failure again after actionlint downloaded successfully and the immediately following shellcheck download timed out.The failures spanned multiple logical runners and three ECS machines, including two downloads failing on the same physical host one millisecond apart. Retries commonly succeeded, isolating the failure to the shared GitHub egress path rather than PR code or the release artifacts.
After the first successful download of each version/platform archive on a host, later jobs no longer require live GitHub Releases access. The pinned digest also prevents a stale, corrupt, or planted shared-cache archive from being executed by a later job.
Reviewer Test Plan
How to verify
Confirm a cold cache downloads and installs the pinned actionlint and shellcheck versions. Place a validator-passing archive with the wrong digest in the shared cache and confirm it is replaced. Then remove outbound download access and confirm a later job installs from the verified cache while retaining a distinct extraction directory. Finally, make the cache target unavailable and confirm the verified job-local installation still succeeds without replacing the target.
Evidence (Before & After)
Before: run
31583913822, job94073230536, failed downloading actionlint after 135 seconds. The next #8976 run,31585951791, job94079756489, downloaded actionlint and then failed downloading shellcheck after 133 seconds.After: the focused linter tests pass 4/4, including corrupt-download rejection, mismatched-cache replacement, offline cache reuse, and best-effort cache persistence. Prettier, ESLint, the full build, and the full workspace typecheck also pass locally. The previous PR head completed the self-hosted ECS Test job with linter setup in about 0.43 seconds; the current head is covered by this PR's active CI run.
Tested on
Environment (optional)
N/A.
Risk & Scope
Linked Issues
Related: #8976
中文说明
本 PR 在自托管 runner 上缓存 actionlint 和 shellcheck 发布包，同时每次使用前校验仓库内固定的 SHA-256，减少 GitHub Releases 网络波动导致的 CI 失败。缓存写入失败不会阻断 job，本地验证和 CI 证据见上文。