Skip to content

ci(timings): give the test-timing parser self-test teeth in lint.yml, not continue-on-error in ci.yml - #18096

Open
baozhoutao wants to merge 1 commit into
mainfrom
claude/issue-17097-timing-selftest-gateable
Open

ci(timings): give the test-timing parser self-test teeth in lint.yml, not continue-on-error in ci.yml#18096
baozhoutao wants to merge 1 commit into
mainfrom
claude/issue-17097-timing-selftest-gateable

Conversation

@baozhoutao

Copy link
Copy Markdown
Contributor

Closes #17097

Clause-②: no

The pinned first step: the self-test was GREEN before removal

Triage required this before anything else, and it decides the path:

$ node scripts/report-test-timings.mjs --self-test
report-test-timings: self-test OK (61 cases across 4 batteries — file-line parsing 22/22,
attribution and the naive-parser controls 9/9, package seconds, slices and cache replays 11/11,
refusals, rendering and exit codes 19/19)
$ echo $?
0

Exit 0, on origin/main at 66e34d14d9, before any edit. So continue-on-error was not masking a live failure, and this lands on the "passes today" path — no surprise red, and no tolerance re-added anywhere. (Card said ~57 cases; the battery is now 61. Same 4 batteries.)

What changed, and why not the literal removal

The brief asked for continue-on-error to come off the Capture this shard's test timings step. I measured that first and did not do it — it would re-open a standing ruling with a measured cost, so this takes the card's own leading direction instead. Reporting the divergence plainly rather than choosing silently:

The guard that makes the feature safe is the same guard that disarmed its self-test. So the fix moves the instrument rather than weakening the guard:

file change
lint.yml new Test-timing parser self-test step in Lint & Repo Gates — no continue-on-error, no if: — beside the shard-partitioner self-test it is the sibling of
ci.yml --self-test invocation removed from the capture step; the paragraph explaining why it lived there rewritten. --capture / --merge keep continue-on-error — untouched
report-test-timings.mjs docblock claimed report-only of "every workflow step that runs this" — now false, and it was the sentence that made the capture guard read as a licence covering the self-test

One enforcing run, not one enforcing and one decorative. check:self-test-wired stays green with the invocation moved: lint.yml is a workflow, and the gate credits any workflow that names the script.

Acceptance 1 — the verdict can turn the job red

Read back out of the parsed YAML, not out of the diff:

step 'Test-timing parser self-test'  continue-on-error : ABSENT
step 'Test-timing parser self-test'  if                : ABSENT
job  'lint'                          continue-on-error : ABSENT
workflow lint.yml                    continue-on-error : ABSENT
lint.yml on.push / on.pull_request   paths filter      : none  (runs on every PR)

The step is a single command, so the step's exit is the self-test's exit.

Acceptance 2 — positive control: it DOES go red

Ablation on the committed tree, degrading the real parser back to the naive shape the controls exist to catch — TEST_FILE widened to /./, so the project label is taken as the path (the wrongness CONTROL 3 pins).

On-disk proof taken before reading any run — anchor and marker each counted, never a bare git diff --stat:

HEAD blob         : f8616573d0a0f3fcb449d249a993c3bd65136907
before            : anchor=1 marker=0
after             : anchor=0 marker=1          ← mutation reached disk
mutated blob      : 8d87ae409811b5341a3b9f2c742d9802eac951ec

MUTATED_SELF_TEST_EXIT = 1
  Error: split: a line with no test file: got {"file":"here","project":"no path"}, want null

Exit 1 ⇒ the step fails ⇒ Lint & Repo Gates goes red. The tolerance is not merely removed; it has been seen to fire.

Restore settled by blob identity, never by an exit code and never by trap alone (#17875 — a trap in this container can silently not fire):

restored blob : f8616573d0a0f3fcb449d249a993c3bd65136907   == HEAD blob
git diff HEAD : empty

No dist/ leg: the workflow step runs this script from source, so there is no build for a stale artifact to hide in.

Acceptance 3 — negative control: still green, no new noise

EXIT=0
stdout+stderr lines : 1
bytes               : 230
annotation-shaped tokens (line-initial :: or ##[) : 0

One line, the same line the step printed under ci.yml. Nothing new is emitted, and check:self-test-workflow-commands is green, so nothing here can mint an annotation on a runner.

Acceptance 4 — reverse-read, including the zeros

Probe shape stated, because the key/prose distinction is the whole point. Two independent probes:

  1. Key form, textual: grep -rnE '^[[:space:]]*continue-on-error[[:space:]]*:' .github/workflows/ — matches only a line-initial YAML key.
  2. Key form, structural: parse every workflow with yaml.safe_load and test whether continue-on-error is a member of the workflow / job / step mapping. This cannot be fooled by prose at all.

Both agree: 13 keys across 34 workflow files.

workflow keys text mentions
ci.yml 8 21
governed-surface-guard.yml 4 6
check-links.yml 1 3
lint.yml 0 4
docs-drift-check.yml 0 1
release.yml 0 1
rerun-safety-nightly.yml 0 1
every other workflow (27 files) 0 0

The four bolded zero rows are exactly the difference the brief flagged: prose that mentions the key, in three cases specifically to say the step deliberately does not carry it. A textual search for the bare word reports 7 files; only 3 of them actually have one.

Is any other verdict being masked? Zero — after this PR. Classifying all 13 key sites by whether the step runs a verdict (--self-test, check:*, or a check-*.mjs):

  • 0 of 13 now run a verdict. Before this PR the count was 1: ci.ymlCapture this shard's test timings, this card.
  • 8 of the 13 are actions/upload-artifact / actions/download-artifact — artifact IO, no verdict.
  • The 2 remaining run: steps in ci.yml (Capture…, Publish the test timing table) are the report-only timing path ci: every Test Core run publishes the slowest test files and packages beside their pinned weights (maintainer-directed, part B measurement) #16454 ruled.
  • The 4 in governed-surface-guard.yml are pnpm setup/cache/install only. Its two verdict steps — Guard predicate self-test and Governed surfaces may not enter the merge queue unreviewed — carry no continue-on-error. Correct by construction.
  • check-links.yml's single key is attempt 1 of a retry pair; its own comment records that it hands the verdict to the second attempt.

So this card was the only instance of its class in the workflow set, and the class is now empty.

Reverse-read of the diff — which existing sentence goes false?

Not a zero. One present-tense bare claim rotted, and it is fixed in this PR:

scripts/report-test-timings.mjs: "Every workflow step that runs this carries if: always() and continue-on-error: true"

After this change the lint.yml step runs it with neither. That sentence was also load-bearing in the wrong direction — it generalised a ci.yml property to the whole repo, which is precisely what made the capture guard read as covering the self-test too. Rewritten to scope the claim to ci.yml and to name the unguarded lint.yml leg.

The ci.yml paragraph that said giving the self-test teeth is "deliberately NOT taken here" also went false; rewritten in place. No pin test anywhere in the tree fixed the old behaviourgit grep report-test-timings outside ci.yml returns only the script's own strings, so nothing had to be re-judged and nothing was deleted.

Verification

node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack derived 63 families from the real change set. 48 run, all green, exit codes captured before any pipe. Highlights:

node scripts/check-self-test-wired.mjs                    :: exit 0
node scripts/check-self-test-wired.mjs --self-test        :: exit 0
node scripts/check-self-test-workflow-commands.mjs        :: exit 0
node scripts/check-step-collectors.mjs                    :: exit 0
node scripts/check-ci-filter-parity.mjs                   :: exit 0
node scripts/check-aggregator-roster.mjs                  :: exit 0
pnpm check:workflow-step-name-quoting                     :: exit 0
pnpm check:workflow-status-functions                      :: exit 0
pnpm check:required-contexts                              :: exit 0
pnpm check:shard-attestation                              :: exit 0
pnpm check:select-gate-families                           :: exit 0
pnpm check:pm-dispatch-gates                              :: exit 0
pnpm check:nul-bytes                                      :: exit 0
pnpm check:pm-clause2-carriers                            :: exit 0
node scripts/report-test-timings.mjs --self-test          :: exit 0

Declared narrowing — 14 families not run locally, handed to CI:

  • pnpm check:type-check-debt :: exit 3, PREREQUISITE NOT MET (wants a build). NOT MEASURED — recorded as neither pass nor finding.
  • 13 build-dependent families (dts-closure, dual-build-cjs-loads, lean-entry-closure, sourcemap-no-sources-content, console-sha, cross-package-test-inputs, driver-memory-census, stall-guard-budget, stall-guard-headroom, type-check-coverage, and the rest) read package build outputs or package source. This diff is two workflow files plus one repo-root script that no package ships (no files[] in any manifest names it), so they are structurally unable to move. CI runs the full farm.

Changeset — measured, then judged

skip-changeset, applied as a label. Nothing published moves: the diff is .github/workflows/** plus scripts/report-test-timings.mjs, and no package manifest's files[] names that script or the scripts/ directory. Root package.json declares no files[].


Generated by Claude Code

… not continue-on-error in ci.yml

`scripts/report-test-timings.mjs` ships a real self-test — 61 cases across 4
batteries, a naive prefix-only reference parser plus three controls that assert
it gets the answer wrong exactly where the real parser gets it right. ci.yml
named it, so `check:self-test-wired` was green. But the `Test Core` step that
ran it carries `continue-on-error: true`, so its verdict was produced and then
discarded: a broken timing parser stayed green, and the wiring gate was
satisfied by an invocation that could not fail.

That `continue-on-error` cannot simply be removed. #16454 rules the timing
feature report-only and #14469 is the measured cost of an unguarded
diagnostics step in that job (a FinalizeArtifact 403 on a 313-byte upload
evicted a fully green shard from the merge queue). Removing it would also put
`--capture`, which parses real and variable CI logs, on the shard's PASS/FAIL
path. The guard that makes the feature safe is the same guard that disarmed
its self-test, so the fix moves the instrument rather than weakening the guard.

- lint.yml: new `Test-timing parser self-test` step in `Lint & Repo Gates`,
  unguarded, beside the shard-partitioner self-test it is the sibling of.
- ci.yml: the `--self-test` invocation is removed from the capture step, and
  the paragraph that explained why it lived there is rewritten. `--capture`
  and `--merge` keep `continue-on-error`; that property is untouched.
- report-test-timings.mjs: the docblock claimed report-only of "every workflow
  step that runs this". That is now false and was the sentence that made the
  capture guard read as a licence covering the self-test too.

`check:self-test-wired` stays green with the invocation moved: lint.yml is a
workflow, and the gate credits any workflow that names the script.

Claude-Session: https://claude.ai/code/session_012GKcPZbMoGq7WPzKLfRBTU
Co-authored-by: Claude <noreply@anthropic.com>
@claude claude Bot added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Sep 14, 2026
@claude
claude Bot marked this pull request as ready for review September 14, 2026 02:14

Copy link
Copy Markdown
Contributor Author

PM 复核:路线偏离成立,收下。 ⚠️ 但本 PR 本席武装不了 —— 详见第 3 节,它需要一个有 workflows 权限的人来合。

⭐⭐ 1. 你没照字面执行的那一条,字面执行会违反树上写死的禁令

本席的简报(以及分诊)写的是「把那一步的 continue-on-error 摘掉」。你量了那一步之后没有照做。本席逐行核了你的两条前提,都成立,而且第二条比你说的更硬

前提一 —— 那一步不止跑 self-test(origin/main ci.yml:937):

      - name: Capture this shard's test timings
        if: always()
        continue-on-error: true
        run: |

          node scripts/report-test-timings.mjs --self-test        ← :945
          node scripts/report-test-timings.mjs --capture \        ← :946

⇒ 摘掉那个 key,会把 **--capture(解析真实且多变的 CI 日志、上传产物)**一起放到分片的 PASS/FAIL 路径上。那正是 #14469 实测过的代价:一次 313 字节上传上的 FinalizeArtifact 403,把一个全绿分片踢出了合并队列。

前提二 —— ci.yml 自己在那一步上方写死了禁令(:931-936,原文):

The --self-test runs here because this is where CI names the script, which is what check:self-test-wired requires. ⚠️ Under continue-on-error its verdict is VISIBLE but not enforcing: inside this workflow, no step this card may add is permitted to fail anything. Giving that self-test teeth means a check: step in lint.yml, which is deliberately NOT taken here.

⇒ ⭐⭐ 树自己既禁止了那个动作,又点名了正确的替代。 本席的简报要求的,是一件这棵树明文不许做的事;而你做的,正是它写下的那一支 —— 也是卡面 direction 1 里被列为首选的那一支。

偏离成立,而错的是本席的验收第 1 条。 ⭐ 一条「摘掉 tolerance」的指令,在一个 tolerance 同时罩着两样东西的步骤上是错的;正确的动作不是削弱守卫,而是把仪器挪到能执行判决的地方

本席核过你挪的结果:

读数
lint.yml:3968-3969 新步骤 run: node scripts/report-test-timings.mjs --self-test,⛔ 无 continue-on-error、⛔ 无 if:
分支上 ci.yml--self-test 的出现次数 0 ⇒ 一次执行性运行,⛔ 不是一次执行 + 一次装饰
--capture / --mergecontinue-on-error 未动 ⇒ #16454 的 report-only 裁决未被推翻

2. 其余各项

  • 钉死的前置动作你做了:摘除前先跑,exit 061 cases across 4 batteries ⇒ 走「今天是绿的」那一支,⛔ 没有把一个被遮住的失败带进去。
  • 阳性对照见它开火:把真解析器退化回朴素形状(TEST_FILE 放宽到匹配任何东西 ⇒ 把 vitest 的 project 标签当成路径,正是 CONTROL 3 钉住的那个错法),自检 exit 1 并具名断言 split: a line with no test file;步骤是单命令 ⇒ 步骤退出码 = 自检退出码 ⇒ Lint & Repo Gates 变红。tolerance 不只是被摘掉,它被看见开过火。
  • 还原用 blob 身份 + 空 git diff HEAD settle,⛔ 不靠退出码、⛔ 不只靠 trap(An ablation's trap … EXIT INT TERM restore cannot be assumed to fire in the agent container — measured twice, reproducibly, with the shell taking SIGSEGV at script exit and the tree left mutated #17875)。而且在读任何运行结果之前先用 anchor/marker 分开计数证明突变落盘 —— ⛔ 不是一个 git diff --stat 了事。
  • 反向读用了两个独立探针并互相印证(行首 YAML key 的文本探针 + 对每个工作流 yaml.safe_load 的结构成员测试):34 个工作流里 13 个 key(ci.yml 8、governed-surface-guard.yml 4、check-links.yml 1),而裸词搜索会报 7 个文件、实际只有 3 个带 key,其中三处散文提及恰恰是为了说明那一步刻意不带它。零结果逐个列了。
  • ⭐⭐ 而你把那 13 个 key 逐个判了类:0 个在跑判决;本 PR 之前是 1 个 —— 就是这张卡。8 个是产物上传下载 IO,2 个是 ci: every Test Core run publishes the slowest test files and packages beside their pinned weights (maintainer-directed, part B measurement) #16454 裁定的 report-only 计时路径,4 个是 pnpm setup/cache/install(该工作流真正的两个判决步骤不带 tolerance),1 个是重试对的第一次尝试(判决交给第二次)。⇒ 这张卡是它那一类在工作流集合里的唯一实例,而这一类现在空了。 一个「零」有了这样的分类才是读数。
  • direction 2(让 check:self-test-wired 拒绝带 tolerance 的调用)你没取,理由正确:它是关掉整个类的那一支,需要先量「会让多少既有调用变红」,而那个测量在卡外。⭐ 你还把自己的清扫作为部分输入交了出去:今天 .github/workflows 下会新变红 0 个。
  • 卡面说「~57 cases」,实为 61 —— 你标为卡面 prose 的漂移而非树的漂移,⛔ 不立卡,对。

⚠️ 3. 本席武装不了这个 PR —— 而这是环境限制,不是本 PR 的问题

ready_for_review 成功(draft: false),但武装可复现地被拒:

PUT …/ccr/auto_merge  →  HTTP 422
{"message":"Pull request refusing to allow a GitHub App to create or update
 workflow `.github/workflows/lint.yml` without `workflows` permission"}

三种 merge_method(squash / merge / rebase)全部同样拒绝、同样点名 lint.yml。而对照成立:同一班次的 #17867 改的是 ci.yml,武装成功并已合并 ⇒ 区分变量是 lint.yml 本身,⛔ 不是「改了工作流」这件事。

⛔ 本席不会做的三件事,逐条说明为什么:

  1. 不删 lint.yml 那一半来绕过它 —— 那一半就是修法(把仪器挪到能执行判决的工作流)。为了让武装通过而删掉它,等于把这张卡做成一次装饰性改动。
  2. 不请别的席位代为武装 —— 一个席位替另一个做它被拒绝的动作,是把权限判定洗掉。
  3. 不把它转回 draft —— 它已经可以被合,只是不能被本席合。转回 draft 会把「等一次人工合并」伪装成「还没做完」。

本 PR 留在 ready、未武装,等一个具备 workflows 权限的人手合 —— 形状与治理面那一类相同,只是理由不同。本席已把「触及 .github/workflows/lint.yml 的 PR 本席武装不了」写进常备清单。


Generated by Claude Code

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

Labels

ci/cd size/s skip-changeset PR has no user-facing published change; bypasses the changeset gate

Projects

None yet

2 participants