ci: 加一道 workflow 结构门 —— 挡住「job 名字在、但什么都不会做」 - #848
Conversation
起因是 2026-08-13 实测出来的一次真实事故,不是设想:把两个各自新增一个 job 的分支合到一起时,git 把它们共享的 runs-on / timeout-minutes / steps: / - uses: actions/checkout@v4 判成公共上下文、只保留一份(那几行逐字相同)。 按「冲突块取并集」解完之后,前一个 job 只剩一行 name,五个 step 全归了后一个。 🔴 那个文件是合法 YAML。GitHub 接受它,job 名字出现在检查列表里、显示绿色 —— 一道从不执行的门,和一道执行且通过的门,在 PR 页面上长得一模一样。 而当时仓里所有门都是绿的,因为没有任何一道门检查 workflow 文件本身 (实测 .github/ / scripts/ / tests/ 里 yamllint|actionlint 命中 = 0)。 判据:每个 job 要么有 runs-on + 非空 steps,要么是 reusable-workflow 调用 (只有 uses)。后者本仓当前一个都没有(7 个 workflow / 14 个 job 全是 steps 形态),放行它是因为那是 GitHub 的合法写法,将来有人用不该被拦。 验证:注入那个真实坏形态(把 agent-node-unit 的 runs-on/steps 抽走), 注入后 YAML 仍可解析 ✓（正是问题所在) 门红在 [no-runs-on] 与 [empty-steps] 两条上,退出码 1 复原后回绿,workflows_scanned=7 jobs_checked=14 problems=0 边界写在脚本头与 workflow 注释里:它挡的是「这个 job 是个空壳」,不是「这个 job 是对的」。真要完整校验该上 actionlint,这个是零依赖的最小兜底。 单独一个 workflow 文件、不动 qa.yml。qa.yml 眼下有五个 PR 在改,往里塞会让 合并更难 —— 而这道门本身就是为了少制造这类问题。
![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: e11a2eb4c3
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if bad: | ||
| print(file=sys.stderr) | ||
| print(" 最常见的成因是合并时把两个相邻 job 的公共部分只保留了一份 ——", file=sys.stderr) | ||
| print(" 见 docs/pre-pr-selfcheck.md §14。解法不是「冲突块取并集」,", file=sys.stderr) |
There was a problem hiding this comment.
When this check detects a malformed job, its only remediation pointer is docs/pre-pr-selfcheck.md §14, but a repository-wide search of the reviewed commit shows that this file does not exist. Consequently, the new failure path directs contributors to a dead document instead of the promised merge-repair procedure; add the guide in this change or point the message to an existing tracked document.
AGENTS.md reference: AGENTS.md:L23-L24
Useful? React with 👍 / 👎.
| - uses: actions/setup-python@v5 | ||
| with: | ||
| python-version: '3.12' | ||
| - run: pip install --quiet pyyaml |
There was a problem hiding this comment.
Every cold run resolves the latest available pyyaml release, so the parser and therefore this gate's result can change or fail without any repository change; it also prevents reconstructing the CI environment from the checked-in inputs. Record an exact version, preferably through a locked requirements file with hashes, and install from that reproducible input.
AGENTS.md reference: AGENTS.md:L25-L25
Useful? React with 👍 / 👎.
| structure: | ||
| runs-on: ubuntu-latest | ||
| steps: |
There was a problem hiding this comment.
When the malformed job is this workflow's own sole structure job—for example, the same merge accident removes its runs-on and steps block—the checker is never invoked, and no independent workflow in this change validates the damaged file. Thus the exact empty-shell failure this gate is intended to catch can still appear green when it affects workflow-structure.yml; invoke the checker from a second independent gate or otherwise arrange mutual validation.
Useful? React with 👍 / 👎.
三条全部成立。 ① P1 死链:失败信息指向 docs/pre-pr-selfcheck.md §14,而那个文件不在 main 上 (它在 #815 的分支里,未合)。实测 git ls-tree origin/main 命中 0。 改法不是等 #815 合,是把要点直接写进失败信息 —— 门的错误提示不该依赖另一个 未合的 PR。文件头那处引用也去掉了。 ② 未钉依赖:pip install --quiet pyyaml 每次冷跑都解析成当时最新版,同一个 commit 在不同时间可能拿到不同解析器。这正是本仓在 npm 侧用 npm ci 取代 npm install 的同一个理由(#841 / #842)—— 我上午刚给别的包做过这件事, 转头在自己的 PR 里犯了。 改为 --require-hashes 从 .github/scripts/requirements-workflow-structure.txt 安装,钉 pyyaml==6.0.2,53 个 sha256 取自 PyPI 的 release 元数据(脚本拉的, 不是手写的)。 ③ 门无法自检:如果同一次合并把 workflow-structure.yml 自己的 structure job 弄成空壳,那个 workflow 就再也不会被调用 —— 而空壳 job 正是它要抓的东西。 审查这条抓得准。 改法:把同一个检查也挂进 no-memory-slugs.yml。选它是因为它的触发含 '**/*.yml',任何 workflow 文件的改动都会到那里,所以两个文件互为兜底。 验证:把 workflow-structure.yml 的 structure job 抽成只剩 name(仍是合法 YAML),同一个脚本报 [no-runs-on] + [empty-steps],退出 1;复原后回绿。
|
三条全部成立,已修( ① P1 死链 —— 而且我不该用「等 #815 合」来解失败信息指向 改法不是等 #815:一道门的错误提示不该依赖另一个未合的 PR。 把要点直接写进失败信息: 文件头那处引用也去掉了。 ② 未钉依赖 —— 这条最难看
这正是本仓在 npm 侧用 改为按锁文件安装:
③ 门无法自检 —— 这条抓得最准如果同一次合并把 改法:把同一个检查也挂进 验证(把 一句自评这三条里有两条(①的死链、②的未钉依赖)是我在别处刚讲过、转头自己违反的。第三条是我完全没想到的角度 —— 一道检查空壳的门,自己也可能变成空壳。 |
|
独立深审结论：MAJOR / DO-NOT-MERGE as-is（exact head 负向 fixture 实测发现结构检查可假绿：
同一批非法 YAML 用 actionlint 均会报错，而当前 checker 得到 只读审查；未改代码、未 approve/merge/deploy。 |
独立深审判 MAJOR / DO-NOT-MERGE,列了四条。我用负向 fixture 逐条实跑,
**四条全部成立**,这道门确实会放行非法 workflow:
反例 旧版结果
─────────────────────────────────────────────────
jobs: {j1: {uses: }}（值为 null） 绿
steps: "this is not a list" 绿
缺顶层 on: 绿
job 级 uses + runs-on（误缩进） 绿
→ 四个反例一起跑:jobs_checked=3 problems=0 退出码 0
成因逐条:
1. `if "uses" in job: continue` —— **只看键在不在**。`uses:`(None)也算。
现在要求它是非空字符串。
2. `if not job.get("steps")` —— 只测真值。非空字符串是真值。
现在要求非空**列表**。
3. 根本没检查顶层 `on:` —— 一个永远不会被触发的 workflow 也算「结构完好」。
现在缺 on 直接红。
4. 只查「uses 是不是非空字符串」还不够:真正的 reusable 调用**不会同时带
runs-on / steps**。两者并存的典型来源正是把 step 级的
`- uses: actions/checkout@v4` 误缩进到了 job 级。现在这种也红。
第 4 条是我修第 1 条之后**仍然漏掉**的 —— 加严「uses 必须是字符串」并不能
挡住「uses 是合法字符串但这个 job 其实是 runner job」。是重跑第四个反例才发现。
加严后重跑(见证红):四个反例 problems=4、退出码 1,每条给出各自的失败原因:
[bad-uses] / [bad-steps] / [no-trigger] / [uses-with-runner-keys]
反向复核(见证绿):真仓库 8 个 workflow / 15 个 job / problems=0,没有误伤。
另修第四条:`requirements-workflow-structure.txt` 此前不在任何一个 workflow 的
`paths:` 里 —— 只改锁文件(比如 bump pyyaml)的 PR 不会触发这两道门。
现已加进 workflow-structure.yml 与 no-memory-slugs.yml 的 pull_request/push paths。
🔴 记一个我自己的过程错误:第一次加 paths 时,我用「文件里有没有这个字符串」
做「是否已包含」的判据,而那个串本来就出现在 `pip install -r` 那一行 ——
于是误判成「已含」直接跳过,YAML 解析出来 paths 里根本没有。
改成按**解析后的 paths 列表**判才对。判据要落在你真正关心的那个结构上。
关于「可考虑由 actionlint 承担完整 Actions schema」:同意方向,本次没做 ——
那是换实现,不是修 bug,应当单独提。这道门的定位仍是零依赖的最小兜底,
它的文件头也已写明「不做 actionlint 那样的完整校验」。
这条 MAJOR 钉的是过期 head,现状已不适用(复核,附证据)我在做一次全仓审计时发现:本 PR 的 一、判定当时说的
也就是旧实现里的 二、当前 head 的实现已经不是那样
并且同一份文件里补齐了几类结构判据( 三、当前状态我另外在别处独立验过这道门会红(不是恒绿):在 四、建议请原评审人确认后把这条 MAJOR 撤回或降级,并在评论里钉当前 head。
(同一天 (只读: |
|
这道门抓的东西是真的,而且我把它拿到今天的
而它要防的那个失效形状,今晚我在别处又遇到了一次同族的:「job 存在」「job 会被触发」「job 拦得住合并」是三件不同的事,而这道门管的是更靠前的一件 —— 「job 里到底有没有东西可执行」。docstring 里那句总结很准:
三条小意见,都不阻塞,但第一条建议合之前顺手改。 1. 🔴 job 没写
|
本 PR 新增的 job 没有写 `name:`:
jobs:
structure:
runs-on: ubuntu-latest
于是 GitHub 上的 check 名会退化成 job id `structure` —— 泛到看不出是哪一道门。
**这正是 2026-08-18 之前四个 workflow 的 job 全叫 `scan` 造成的局面**:
分支保护里的 required check 只能按这个名字写,重名/泛名时指的是哪一道无法确定,
按名字统计覆盖率还会把几道门算成一道。`docs-integrity.yml` 里为此专门留了注释。
🔴 而本 PR 的立论是「**一道不执行的 job,和一道执行且通过的 job,在 PR 页面上
长得一模一样**」—— 一个认不出是谁的 check 名,是同一个问题的另一面:
门在跑,但看的人分不清跑的是哪一道。
补上 `name: workflow-structure`。复核:全仓 26 个 job,26 个不同的名字。
顺带扫了一遍:另有 4 个 job 也没写显式 `name`(`e2e` / `rename-ghost-gate` /
`published-pins` / `release-tag`),但它们的 job id 本身就是描述性的,当 check 名
够用;而且它们已经是既有的 check 名,改名会让引用它们的地方(以及历史)对不上。
**只改这一个新加的。**
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
合了。CI 里这道门真跑了,数字在: 我做了见红,三种坏法三种红不能只信「它在干净树上是绿的」。三次变异(每次都还原, 🔴 A 那个变异的 YAML 是合法的, C 是分母承重那格:目录塌了会红,而不是「扫到 0 个 job,0 个问题,绿」。 🔴 我在合并时补了一样东西 —— 它恰好是这个 PR 讲的问题的另一面本 PR 新增的 job 没有写 jobs:
structure:
runs-on: ubuntu-latest于是 GitHub 上的 check 名会退化成 job id 而这正是 2026-08-18 之前四个 workflow 的 job 全叫 本 PR 的立论是「一道不执行的 job 和一道执行且通过的 job,在 PR 页面上长得一模一样」—— 一个认不出是谁的 check 名,是同一个问题的另一面:门在跑,但看的人分不清跑的是哪一道。 补上 顺带扫了一遍:另有 4 个 job 也没写显式 两条附带确认① 依赖是 ② 单独一个 workflow 文件、不动 一条后续(不阻塞)这个脚本没有 |
2 participants
补上一道仓里一直没有的检查:每个 workflow 的每个 job,是不是真的会执行点什么。
起因是一次实测出来的事故,不是设想
2026-08-13,把两个各自新增一个 job 的分支合到一起时,git 把它们共享的这几行判成公共上下文、只保留一份 —— 因为它们逐字相同:
按「冲突块取并集」解完之后:
🔴 这个文件是合法 YAML。 GitHub 接受它,
doc-source-pins会作为一个什么都不做的 job 出现在检查列表里、显示绿色 —— 一道从不执行的门,和一道执行且通过的门,在 PR 页面上长得一模一样。而当时仓库里所有的门都是绿的,因为没有任何一道门检查 workflow 文件本身:
判据
每个 job 要么有
runs-on+ 非空steps,要么是 reusable-workflow 调用(只有uses)。第二种形态本仓当前一个都没有(7 个 workflow / 14 个 job 全是
steps形态),放行它是因为那是 GitHub 的合法写法,将来有人用不该被这道门拦住。验证
把那个真实的坏形态注入
qa.yml(抽走agent-node-unit的runs-on/steps):🔴 边界(写在脚本头和 workflow 注释里)
它挡的是「这个 job 是个空壳」,不是「这个 job 是对的」。 不检查 job 会不会通过、命令对不对、
uses是否存在、表达式语法、矩阵展开。真要完整校验该上 actionlint —— 这个是零依赖的最小兜底,不是它的替代。为什么是独立 workflow 文件而不是加进 qa.yml
qa.yml眼下有五个 PR 在改(#798 / #801 / #803 / #843 / #846,协调分析见 #803)。往里再塞一个只会让合并更难 —— 而这道门本身就是为了少制造这类问题。 独立文件在结构上就不会与它们冲突。失败信息里直接指向
docs/pre-pr-selfcheck.md §14(那条记的就是这次事故的根因与正确解法),不让人再去翻评论。