Skip to content

filter job 一旦失败,Test Core / Build Core / Dogfood 会全部 skipped 而分支保护判为通过 —— 隐式 success() 今天已第三次咬人 #4928

Closed

Description

@os-zhuang

问题

ci.yml 的 7 个下游 job 都是这个形状:

  test:
    needs: filter
    if: needs.filter.outputs.core == 'true'

if: 里没有状态函数,GitHub 会隐式包一层 success()。于是 filter 这个前置 job 一旦失败(checkout 抖动、dorny/paths-filter 故障、10 分钟超时),下游 test / temporal-conformance / dogfood / dogfood-verify / build-core / build-docs / console-pin 全部被跳过。

而聚合闸门无条件接受 skipped:

case "$result" in
  success|skipped|cancelled) echo "Test Core gate satisfied ($result)." ;;

skipped 在分支保护里算通过。合起来:一次 filter 的偶发失败 → 零测试运行 → Test Core / Dogfood Regression Gate 报「gate satisfied」→ PR 全绿可合。而且这个过程没有任何红色信号。

作者意图已经写在代码里了,只是没覆盖到这一层

filter 的 outputs 本身已经做了 fail-safe:

outputs:
  core: "${{ steps.changes.outputs.core || 'true' }}"

—— 存疑就全跑。但这个兜底只覆盖「输出值缺失」,没覆盖「job 整体失败」。job 挂掉时 needs.filter.outputs.core 是空串,'' == 'true' 为假,再叠加隐式 success(),两条路都通向跳过。

dogfood-gate 对 cancelled 的处理是这类推理的正例(有实验记录 run 30271824408,说明真实失败会 dominate 聚合结果所以接受 cancelled 是安全的)。skipped 缺的正是同一层论证:它无法区分「路径过滤器说没碰核心代码」和「过滤器自己炸了」。

建议改法

把下游条件从「显式为真才跑」改成「显式为假才跳」,与 || 'true' 的意图完全一致:

    if: ${{ !cancelled() && needs.filter.outputs.core != 'false' }}

filter 失败时 outputs 为空 → '' != 'false' → 跑全量;!cancelled() 压过隐式 success()。存疑就全跑,和作者原本的取舍一模一样。

这是今天同一个 GitHub 语义陷阱的第三次出现

# 位置 后果
1 release.yml 的发布完整性守卫(#4900) changesets 步骤失败后守卫被跳过,恰恰漏掉最需要它的场景
2 release.yml 的 docker job(#4900) 包已公开之后才发生的故障,把镜像一起带走
3 ci.yml 的 7 个核心 job(本 issue) 核心闸门零运行却报通过

值得沉淀成一条可检查的规则:任何「上游失败时仍须执行」的 step/job,if: 必须显式带状态函数(!cancelled() / always())。这条可以静态扫出来 —— 我审计时就是这么找到全部三处的。

Activity

  1. added
    toolingFix lands in tooling, not a product package — enters pm:queue only with Unblocks: or a named surface
    on Aug 3, 2026
  2. added
    target:v17Release blocker for v17 — on the board until fixed, dropped as no longer valid, or accepted for GA
    on Aug 4, 2026
  3. added
    pm:queueReady for the PM dispatch loop
    pm:dispatchedDispatched to a dev agent by /pm-dispatch
    domain:devxDomain lane — seat card indexed by label:pm:seat
    on Aug 4, 2026
  4. self-assigned this
    on Aug 4, 2026
  5. os-zhuang commented on Aug 4, 2026

    @os-zhuang
    ContributorAuthor

    认领:PM 循环第 1 轮补位(v17 D 组工作集)
    会话:session_01FTszibd6C8sUCCZnM4VcrL
    分支:claude/issue-4928-implicit-success-guard
    Worktree:objectstack-issue-4928
    域:domain:devx(门禁类 —— 落点是 .github/workflows/ci.yml,按锚定规则取落点包而非标题词汇)
    文件面:.github/workflows/ci.yml(7 处下游 job 的 if: 行)、可能新增一处静态扫描门禁及其测试

    前提已实锤仍在,实测 origin/main:

    .github/workflows/ci.yml:97   if: needs.filter.outputs.core == 'true'
    .github/workflows/ci.yml:325  if: needs.filter.outputs.core == 'true'
    .github/workflows/ci.yml:513  if: needs.filter.outputs.core == 'true'
    .github/workflows/ci.yml:642  if: needs.filter.outputs.core == 'true'
    .github/workflows/ci.yml:770  if: needs.filter.outputs.core == 'true'
    .github/workflows/ci.yml:917  if: needs.filter.outputs.docs == 'true'
    .github/workflows/ci.yml:1003 if: needs.filter.outputs.console == 'true'
    

    七处全部无状态函数(git grep -c "success()" -- .github/workflows/ci.yml 零命中),隐式 success() 成立。共享基础设施 churn 核查(Operational note 8):ci.yml 在 main 上最近一次改动是 b850318,与本单无关,无在飞冲突;git ls-remote | grep 4928 无分支。

    与在飞两单(#4971 spec/src/shared/**、#4749 spec/src/ui/**)文件面完全不相交。


    Generated by Claude Code

  6. os-zhuang commented on Aug 5, 2026

    @os-zhuang
    ContributorAuthor

    PM 复核(第 1 轮):ACCEPT → PR #5344

    会话 session_01FTszibd6C8sUCCZnM4VcrL。逐条对 GitHub 与 origin/main 核验:

    通过项

    • 七处 if: 全改,且各自保留自己的输出名(core ×5、docs、console)—— 没有盲目 pattern-replace,这是我派发时点名要求逐 job 核对布尔逻辑的地方。
    • 真值表五个状态,只有 S3 改变行为,其余四态逐字节同行为。共享基础设施改动的爆炸半径应当就是这么小。更要紧的是它处理了一个我没想到的二义性:「失败 job 的 outputs 到底是空串还是 || 'true' 照常渲染」有两种读法,新条件在两种读法下都跑('' != 'false' 与 'true' != 'false' 均真),旧条件在两种读法下都跳(隐式 success() 压过一切)—— 修法不依赖于这个语义细节的判定结果,这比赌一种读法正确稳得多。
    • 我额外要求的「聚合闸门那一半」已实现,且理由正确:test-gate / dogfood-gate 把 filter 纳入 needs,某腿 skipped 而 filter 未成功时判红。dev 明确指出该分支在新 if: 下不可达,仍保留作常驻不变量断言 ——「它守的失败模式是静默的,而那七行 if: 距离被人手滑改回去只有一次编辑」。这正是我要的那半,不是敷衍。cancelled 两侧仍放行,CI: Dogfood Regression Gate 把 cancelled 当失败 —— 每次连续推送都产生一条假红 #3668 的 run-lifecycle 论证原样保留。
    • 验证方式对得起「CI 配置无法用本地测试证明」这个约束:
      • actionlint 改前改后都零发现,并跑了反向对照 —— 故意把 docs 拼成 doc、把 test-gate 的 needs 去掉 filter,actionlint 立刻两条都点名。绿色因此不是空转,它确实在逐条类型检查这些表达式。这是「零命中必须用确定存在的邻近词反查」那条纪律用在门禁上的正确形态。
      • 从 YAML 里抽出 run: 原文交给真 bash 执行(不是重写一份等价逻辑),test-gate 跑 4×4 全交叉,dogfood-gate 另确认不会短路。
    • 并发核查按 Operational note 8 重跑,ci.yml 在飞期间无人改动,不存在静默 revert。
    • PM 独立核实了 PR 的三处声明:publish-smoke.yml:98 确为 if: needs.resolve.outputs.run == 'true' 同形状的唯一存量违规;空 frontmatter changeset 确是仓内既有约定(ci-cache-tier1-optimizations.md 同款);release.yml:342 的 !cancelled() && needs.release.outputs.published == 'true' 确是同形状的生产先例。

    拆单判断正确(#5343,已入队 domain:devx)

    把「沉淀成静态规则」拆出去而不是塞进本 PR,理由站得住:落地它要先决定 publish-smoke.yml 的 resolve 失败时怎么兜底 —— 那个 job 的 outputs 没有 || 'true',且 ref 也由它算出,照搬「存疑就全跑」会让 checkout 落到错的 ref 跑一趟 45 分钟的 job。这属于那个 workflow 自己的取舍,不该由本单顺手拍板。#5343 里的全仓审计表还做对了一件事:把规则限定在 job 级 + needs.*.outputs.*,明确排除 step 级读自己 steps.*.outputs.* 的情形 —— 否则门禁一出生就带着一长串豁免名单,离 #4690 的反模式只有一步。验收条件里也钉了「输入缺失必须大声失败」。

    其中一条未核实项它如实标了出来留给接手人:publish-smoke / packed-tarballs 这个 commit status 在 resolve 挂掉时可能连 pending 都不会写,若它是 release PR 的必需检查,后果与「skipped 算通过」不同形。存疑写成存疑,没有猜。

    CI 全绿后转 ready 入队。


    Generated by Claude Code

  7. os-zhuang commented on Aug 5, 2026

    @os-zhuang
    ContributorAuthor

    复核补记:dev 回报里有一条 PR 正文没写、但是本单最强的证据,记在这里以免丢失 ——

    PR #5344 验证了它自己。 因为这个 PR 改的就是 ci.yml,而 ci.yml 命中全部三个路径过滤器,所以它自己那一轮 CI 就是在新条件下跑的:filter → success,七个下游 job 全部被调度而非跳过(Test Core 1-3/3、Dogfood Regression Gate 1-3/3、Dogfood Verify CLI、Temporal Conformance、Build Core、Build Docs、Console Pin Gate)。这是真值表里的 S1 状态在生产 Actions 上的实测确认,不是模拟。

    诚实边界 dev 也标了:它改的两个聚合闸门在收工时还排在各自 matrix 后面没跑完,所以那两个的实测判决不在本次报告里 —— 它们的逻辑由第 (4) 项(从 YAML 抽出 run: 原文交真 bash 跑 4×4 全交叉)覆盖。把「哪些是实测、哪些是模拟」分清楚,而不是笼统说「验证通过」,正是这单该有的写法。

    判决不变:ACCEPT。CI 全绿后入队(当前 GraphQL 配额 0/5000,draft→ready 与 auto-merge 均为 GraphQL-only,已排队等恢复,不重试)。


    Generated by Claude Code

  8. os-zhuang commented on Aug 5, 2026

    @os-zhuang
    ContributorAuthor

    已落地 —— PR #5344 经合并队列合入 main(b8add3758)

    会话 session_01FTszibd6C8sUCCZnM4VcrL。按纪律落地之后再核一次落地判据本身(队列的合并同样走 os-regen 驱动,静默吞并在那一步一样能发生),实测 origin/main:

    $ git grep -c "!cancelled() && needs.filter.outputs" origin/main -- .github/workflows/ci.yml
    8                      # 7 个下游 job + filter outputs 上的契约注释块
    
    $ git grep -c "if: needs.filter.outputs.*== 'true'" origin/main -- .github/workflows/ci.yml
    0                      # 旧形状零残留
    

    修复完整落地,没有一侧被吞。

    这个信任洞从现在起关上了:filter job 失败(checkout 抖动、dorny/paths-filter 故障、10 分钟超时)不再让 test / temporal-conformance / dogfood / dogfood-verify / build-core / build-docs / console-pin 七个核心闸门集体跳过、而分支保护把 skipped 当通过。真值表五态里只有这一态变了行为,其余四态逐字节同行为。

    两个聚合闸门(test-gate / dogfood-gate)另把 filter 纳入 needs,某腿 skipped 而 filter 未成功时判红 —— 该分支在新 if: 下不可达,保留作常驻不变量断言,因为它守的失败模式是静默的,而那七行 if: 距离被人手滑改回去只有一次编辑。

    沉淀成静态规则那半在 #5343(已入队 domain:devx),含全仓审计表与 publish-smoke.yml:98 这个唯一存量违规的取舍说明。


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

ci/cddomain:devxDomain lane — seat card indexed by label:pm:seatpm:dispatchedDispatched to a dev agent by /pm-dispatchpm:queueReady for the PM dispatch looptarget:v17Release blocker for v17 — on the board until fixed, dropped as no longer valid, or accepted for GAtoolingFix lands in tooling, not a product package — enters pm:queue only with Unblocks: or a named surface

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions