Repository navigation
Commit ec236d4
ci(shard-timings): give the write-back step every variable it expands, and rehearse it on pull_request (#19933)
Fixes #18341
Clause-②: no
## What this changes
One file: `.github/workflows/shard-timings-refresh.yml`.
1. **The write-back step now exports every variable its script
expands.** Its `env:` gave `GH_TOKEN`, `RUN_ID`, `HEAD_SHA`; the commit
message also expands `RUN_COUNT` and `RUNS` under `set -euo pipefail`.
Both are added from the same `steps.generate.outputs` the compose step
already reads. The commit message is unchanged: `$RUN_COUNT` and `$RUNS`
stay in it (they are the dataset's provenance).
2. **Structural defense: the write and its `pull_request` rehearsal are
now ONE step with ONE `env:` and ONE script.** The old pair was gated
`github.event_name != 'pull_request'` / `== 'pull_request'`, so no PR
ever ran the script that writes. Now the step runs on every event (`if:
steps.compare.outputs.changed == 'true'`). On a PR run the branch, the
`git add` and the commit (whose message expands every provenance
variable) run for real on the runner. Every act that leaves the runner
goes through one shell function, `outward`, driven by `DRY_RUN: ${{
github.event_name == 'pull_request' }}`. That covers `git push`, `gh pr
create`, `gh pr view`, the label POST and its read-back. On a dry run
`outward` prints the fully expanded command and returns a stand-in
value. The shell expands arguments before `outward` is entered, so `set
-u` judges them on both legs. A key missing from `env:` now reds the PR
run that dropped it.
3. **The switch cannot be turned the wrong way.** The script refuses any
`DRY_RUN` other than `true`/`false`. It also refuses `false` when
`GITHUB_EVENT_NAME` is `pull_request`, and an absent `DRY_RUN` is itself
an unbound variable. `GH_TOKEN` is read by `gh`, not by the shell, so a
dry run would never notice it missing. The script names it once as
`${GH_TOKEN:?…}`, so both legs judge it too.
4. The old "Dry run — the pull request this would have opened" step is
folded into the same script (same summary text, one sentence updated).
The header's `pull_request` paragraph and the compose step's comment now
describe the new shape.
⛔ Not touched: `MAX_SHARD_OVER_MEAN`, `MAX_MEASURED_OVER_PREDICTED`,
`timeout-minutes`, the shard matrix, `FILE_SHARDED_PACKAGES`, or any
threshold. The diff has zero lines naming any of them.
## Measured: both directions (acceptance item 2)
Harness: each leg lifts the step's `run:` script out of the YAML (parsed
with `yaml`). It evaluates the step's OWN `env:` block into the
environment, using the values the failing scheduled run carried
(`RUN_ID=34808103618`, `RUN_COUNT=1`, `HEAD_SHA=a90a9f2679…`) and a fake
token. It adds only the runner defaults (`GITHUB_REPOSITORY`,
`GITHUB_EVENT_NAME`, `RUNNER_TEMP`, `GITHUB_STEP_SUMMARY`,
`GITHUB_OUTPUT`, `HOME`, `PATH`, `CI`) under `env -i`, and runs the
script with `bash -e`, the shell the job log shows for these steps. The
scratch repo matches the runner's checkout: a depth-1 clone of `main`
with `core.hooksPath=.githooks`, which the runner's `pnpm install`
registers (job log line: `git integration registered
(merge.os-regen.name, merge.os-regen.driver, core.hooksPath)`). So the
repo's real `pre-commit` and `pre-push` hooks run. PATH shims sit in
front of `gh` (records the call, answers a canned value) and `git push`
(records the call, forwards only when `origin` is a local bare repo).
Nothing left the machine.
Base script = blob `fe5a62ef1c` (`origin/main` `fdeeea0cc9`). Fixed
script = blob `2b997d121b` (this PR's head `d4ba97991e`).
| leg | script | event | env change | exit | reading |
|---|---|---|---|---|---|
| L1 | base | schedule | none (the shipped omission) | **1** | `line 9:
RUN_COUNT: unbound variable`: byte-for-byte the CI failure of run
34810389734. 0 pushes, 0 gh calls |
| L7 | base, the old dry-run step | pull_request | none | **0** |
control: the old rehearsal is green over the same defect |
| L2 | fixed | schedule | complete | **0** | commit made with the full
provenance body. Real `pre-push` ran (`✓ check:commit-card-trailers: 1
commit message(s) … carry no card relation`). Branch reached the local
origin. 5 recorded calls: `git push`, `gh pr create`, `gh pr view`,
label POST, label read-back |
| L3 | fixed | schedule | drop `RUNS` | **1** | `RUNS: unbound
variable`, no commit, 0 pushes |
| L4 | fixed | pull_request | complete | **0** | commit made locally.
**0** recorded calls, no branch on origin. Every outward act printed as
`DRY RUN, not executed: …`. Summary carries the dry-run section |
| L5 | fixed | pull_request | drop `RUN_COUNT` | **1** | `RUN_COUNT:
unbound variable`: **the PR leg now catches the defect class** |
| L8 | fixed | pull_request | drop `GH_TOKEN` | **1** | `GH_TOKEN: is
not set; gh would run unauthenticated.` |
| L6 | fixed | pull_request | force `DRY_RUN=false` | **1** |
`::error::DRY_RUN is 'false' on a pull_request run …`, 0 calls |
| L6b | fixed | schedule | drop `DRY_RUN` | **1** | `DRY_RUN: unbound
variable` |
| L6c | fixed | schedule | `DRY_RUN=yes` | **1** | `::error::DRY_RUN
must be 'true' or 'false', got 'yes'.` |
The exit codes were read from each run directly, never through a pipe.
The fix commits came first, and every leg ran against those committed
blobs.
## Audit of the whole file (acceptance item 3)
Method: every `run:` block parsed out of the YAML. A scanner that skips
single-quoted text and `${{ }}` lists each shell expansion (`$X`,
`${X}`, arithmetic names) and each `process.env.X` read by an inline
`node -e`. Each name is classified as: step `env:`, assigned in the
script, `$GITHUB_ENV` from an earlier step, or a runner default. The
file has no workflow-level or job-level `env:` and no `defaults:`.
Positive control: over the base file the scanner reports exactly two
UNRESOLVED names, `RUN_COUNT` and `RUNS` in the write step. Over this
PR's head it reports zero.
| step | `set -u` | step `env:` | expanded names not assigned in the
script | verdict |
|---|---|---|---|---|
| Get pnpm store directory | no | none | `GITHUB_ENV` (runner) | ok |
| Install dependencies | no | none | none | ok |
| Self-test the generator and the run selector | no | none | none
(`failed` is local) | ok |
| List the workspace | no | none | `RUNNER_TEMP` (runner) | ok |
| Choose a green, uncensored, un-replayed run | no | `GITHUB_TOKEN` |
`RUNNER_TEMP`, `GITHUB_OUTPUT`, `GITHUB_STEP_SUMMARY` (runner);
`GITHUB_TOKEN` read by the child script | ok |
| Regenerate the dataset… | **yes** | `GITHUB_TOKEN` | `GITHUB_TOKEN`
(step), `RUNNER_TEMP`, `GITHUB_REPOSITORY`, `GITHUB_OUTPUT` (runner);
`RUN_ID`, `RUN_COUNT` etc. are LOCAL here | ok |
| Compare against the committed dataset | no | none | `RUNNER_TEMP`,
`GITHUB_OUTPUT`, `GITHUB_STEP_SUMMARY` | ok |
| Predicted bins, before and after | no | none | `RUNNER_TEMP`,
`GITHUB_OUTPUT` | ok |
| Compose the pull request body | **yes** | `RUN_ID`, `RUNS`,
`RUN_COUNT`, `HEAD_SHA`, `PARTITIONER_EXIT`, `USED_PAT` | all six (step;
`PARTITIONER_EXIT` also defaulted `:-0`), `GITHUB_REPOSITORY`,
`RUNNER_TEMP`; node reads `RUNNER_TEMP`, `RUN_ID` | ok |
| Push the refresh branch… (base) | **yes** | `GH_TOKEN`, `RUN_ID`,
`HEAD_SHA` | **`RUN_COUNT`, `RUNS`: UNRESOLVED** | the defect |
| Push the refresh branch… (this PR) | **yes** | `DRY_RUN`, `GH_TOKEN`,
`RUN_ID`, `RUNS`, `RUN_COUNT`, `HEAD_SHA` | all step keys, plus
`GITHUB_EVENT_NAME`, `GITHUB_REPOSITORY`, `RUNNER_TEMP`,
`GITHUB_STEP_SUMMARY` (runner) | ok |
| Say what happened when nothing changed | no | none |
`GITHUB_STEP_SUMMARY`; its other values are `${{ }}` expressions,
substituted before bash | ok |
No second instance of the defect class exists in this file. After this
PR no step's execution depends on the event: `DRY_RUN` is the file's
only `github.event_name` test. The remaining legs split on data, not on
the event (`changed` true/false, the selector's exit 3), and the
selector's own `--self-test` already drives its exit-3 leg.
## Static read of what runs after the commit (never executed on GitHub;
⛔ not asserted to pass)
- **Credential in use:** on run 34810389734 the compose step's env
printed `USED_PAT: false`, so `GH_TOKEN` and the checkout credential
were the Actions `github.token`, with job `permissions` `contents:
write`, `pull-requests: write`, `actions: read`.
- **`git push origin claude/shard-timings-refresh-RUN_ID`:** it needs
`contents: write` (declared). `GET
/repos/…/rules/branches/claude/shard-timings-refresh-1` answers `[]`,
and the one repository ruleset (`main`) targets `~DEFAULT_BRANCH` only.
The repo's `pre-push` hook runs on the runner and passed in leg L2 on a
depth-1 clone. NOT MEASURED: classic branch-protection patterns (no read
path from this seat).
- **`gh pr create` with the Actions token:** this needs the repository
setting that lets GitHub Actions create pull requests. The seat cannot
read it (`GET /repos/…/actions/permissions/workflow` answers 403 through
the agent proxy). Circumstantial evidence: #17076 (`chore: version
packages`, open) was authored by `github-actions[bot]` on 2026-09-09,
and `release.yml` says changesets/action opens it with the default
token. NOT MEASURED for this workflow.
- **Label POST on `issues/N/labels` with `pull-requests: write` and no
`issues:` scope:** in-repo precedent is `pr-automation.yml`, whose
label-writing jobs declare exactly `contents: read` plus `pull-requests:
write`. The label exists (`GET /labels/skip-changeset` answers 200), so
no create is implied.
- Nothing found that would fail. The first live execution is still the
post-merge dispatch below.
## Owed after merge: acceptance item 4
Not triggered here, by design. After merge, dispatch the lane on `main`
and read the PR it opens:
```
gh workflow run shard-timings-refresh.yml --repo objectstack-ai/objectstack --ref main
```
(REST equivalent: `POST
/repos/objectstack-ai/objectstack/actions/workflows/shard-timings-refresh.yml/dispatches`
with body `{"ref":"main"}`.) `workflow_dispatch` evaluates `DRY_RUN` to
`false`, so that run writes.
## Changeset
None. The diff is `.github/` only and publishes nothing from any
package, so it takes route 2 of the `Check Changeset` gate, the
`skip-changeset` label. An empty-frontmatter changeset is rejected by
that gate. This PR does not apply labels; that is the seat's.
## Gates
`node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack
--commands` at `d4ba97991e` derived 40 commands, all run from this
worktree. All 40 exited 0 with their own verdict lines. `pnpm
check:pm-dispatch-gates` ran detached, as its header prescribes: `✓
dispatch-gates self-test: 1905 cases pass.` in 722.1s. `--ran`
reconciliation: `40 derived famil(ies) accounted for — 40 run, 0
NOT-MEASURED (a DERIVED zero — all 40 recorded an exit code and none of
them is 3)`. Also run: the five roster families the derivation flags as
living under `.github/workflows` (`check-platform-checklist-watchdog`
plus its self-test, `check-ci-filter-parity` plus its self-test,
`ci/scheduled-full-run --self-test`, `pr-labels --self-test`), all exit
0. No `actionlint` or other workflow linter exists in the repo's tooling
or on this container. NOT MEASURED: the type-check lanes and the six
workflow-valued families the derivation lists as CI-only.
## Acceptance notes
- The defense is only as strong as the PR run is visible. This lane's
`pull_request` run is not in the merge queue's required set, so its red
is advisory. Making it required is the maintainer's call, and this PR
does not do it.
- The rehearsal covers the write script whenever the PR run's
regeneration differs from the committed file. `measuredAt` is the run's
date, so that is every PR run except one on the same UTC day as a landed
refresh with identical inputs.
- A re-run after a partial success would meet the already-pushed
`claude/shard-timings-refresh-RUN_ID` branch and be refused as
non-fast-forward. This is not observed, only read.
## 维护者速读(草稿)
- **改了什么**:定时刷新分片时长数据集的工作流里,「推分支、开 PR」那一步补上了漏掉的两个变量;并且把它和 PR
上的演练合成同一步、同一套变量,只在对外动作(push、开 PR、打标签)前加了一个开关。
- **为什么改**:这一步从没在合并前跑过,第一次真跑就是 9 月 14
日的定时任务,算完数据后死在提交那一行,数据集因此一直没刷新。现在任何 PR
只要改到这条工作流,就会把这一步(含提交)真跑一遍,漏变量当场变红。
- **风险与代价(含回滚)**:PR 上多跑一次本地提交,不推送、不开 PR、不打标签;开关写错会直接报错而不是误推。回滚即还原这一个文件。
- **席位意见**:
- **你要做的**:合并本 PR(工作流改动需人工合);合并后手动触发一次 `workflow_dispatch`,确认它真开出刷新 PR。
---
_Generated by [Claude
Code](https://claude.ai/code/session_013RDBh5DqXd2xnLwvHLgLFr)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent fdeeea0 commit ec236d4
1 file changed
Lines changed: 112 additions & 39 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
139 | 139 | | |
140 | 140 | | |
141 | 141 | | |
142 | | - | |
143 | | - | |
144 | | - | |
145 | | - | |
146 | | - | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
147 | 150 | | |
148 | 151 | | |
149 | 152 | | |
| |||
517 | 520 | | |
518 | 521 | | |
519 | 522 | | |
520 | | - | |
521 | | - | |
| 523 | + | |
| 524 | + | |
| 525 | + | |
522 | 526 | | |
523 | 527 | | |
524 | 528 | | |
| |||
631 | 635 | | |
632 | 636 | | |
633 | 637 | | |
634 | | - | |
635 | | - | |
636 | | - | |
637 | | - | |
| 638 | + | |
| 639 | + | |
| 640 | + | |
| 641 | + | |
| 642 | + | |
| 643 | + | |
| 644 | + | |
| 645 | + | |
| 646 | + | |
| 647 | + | |
| 648 | + | |
| 649 | + | |
| 650 | + | |
| 651 | + | |
| 652 | + | |
| 653 | + | |
| 654 | + | |
| 655 | + | |
| 656 | + | |
| 657 | + | |
| 658 | + | |
| 659 | + | |
| 660 | + | |
| 661 | + | |
| 662 | + | |
| 663 | + | |
| 664 | + | |
638 | 665 | | |
| 666 | + | |
| 667 | + | |
| 668 | + | |
| 669 | + | |
639 | 670 | | |
| 671 | + | |
| 672 | + | |
| 673 | + | |
640 | 674 | | |
| 675 | + | |
| 676 | + | |
641 | 677 | | |
642 | 678 | | |
643 | 679 | | |
| 680 | + | |
| 681 | + | |
| 682 | + | |
| 683 | + | |
| 684 | + | |
| 685 | + | |
| 686 | + | |
| 687 | + | |
| 688 | + | |
| 689 | + | |
| 690 | + | |
| 691 | + | |
| 692 | + | |
| 693 | + | |
| 694 | + | |
| 695 | + | |
| 696 | + | |
| 697 | + | |
| 698 | + | |
| 699 | + | |
| 700 | + | |
| 701 | + | |
| 702 | + | |
| 703 | + | |
| 704 | + | |
| 705 | + | |
| 706 | + | |
| 707 | + | |
| 708 | + | |
| 709 | + | |
| 710 | + | |
| 711 | + | |
| 712 | + | |
| 713 | + | |
| 714 | + | |
| 715 | + | |
| 716 | + | |
644 | 717 | | |
645 | 718 | | |
646 | 719 | | |
| |||
651 | 724 | | |
652 | 725 | | |
653 | 726 | | |
654 | | - | |
| 727 | + | |
655 | 728 | | |
656 | | - | |
| 729 | + | |
| 730 | + | |
657 | 731 | | |
658 | 732 | | |
659 | | - | |
| 733 | + | |
660 | 734 | | |
661 | 735 | | |
662 | 736 | | |
663 | 737 | | |
664 | 738 | | |
665 | | - | |
666 | | - | |
| 739 | + | |
| 740 | + | |
667 | 741 | | |
668 | 742 | | |
669 | 743 | | |
670 | 744 | | |
671 | 745 | | |
672 | 746 | | |
673 | 747 | | |
674 | | - | |
| 748 | + | |
675 | 749 | | |
676 | | - | |
| 750 | + | |
677 | 751 | | |
678 | 752 | | |
679 | | - | |
| 753 | + | |
680 | 754 | | |
681 | | - | |
| 755 | + | |
682 | 756 | | |
683 | 757 | | |
684 | 758 | | |
685 | 759 | | |
686 | | - | |
687 | | - | |
688 | | - | |
689 | | - | |
690 | | - | |
691 | | - | |
692 | | - | |
693 | | - | |
694 | | - | |
695 | | - | |
696 | | - | |
697 | | - | |
698 | | - | |
699 | | - | |
700 | | - | |
701 | | - | |
702 | | - | |
703 | | - | |
704 | | - | |
| 760 | + | |
| 761 | + | |
| 762 | + | |
| 763 | + | |
| 764 | + | |
| 765 | + | |
| 766 | + | |
| 767 | + | |
| 768 | + | |
| 769 | + | |
| 770 | + | |
| 771 | + | |
| 772 | + | |
| 773 | + | |
| 774 | + | |
| 775 | + | |
| 776 | + | |
| 777 | + | |
705 | 778 | | |
706 | 779 | | |
707 | 780 | | |
| |||
0 commit comments