Skip to content

ci(shard-timings): give the write-back step every variable it expands, and rehearse it on pull_request - #19933

Merged
os-support-ai merged 2 commits into
mainfrom
claude/issue-18341-shard-timings-writeback-env
Sep 24, 2026
Merged

os-support-ai merged 2 commits into
mainfrom
claude/issue-18341-shard-timings-writeback-env

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

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: chore: version packages #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

…, and rehearse it on pull_request

The step that pushes the refresh branch and opens the PR expanded RUN_COUNT
and RUNS in its commit message under set -u, but its env: exported only
GH_TOKEN, RUN_ID and HEAD_SHA, so the scheduled refresh died at git commit
after the dataset had been computed. Export all four generate outputs, as
the compose step already does.

The pull_request dry run was a separate, mutually exclusive step, so the
script that writes never ran before a merge. Fold the two into one step
that runs on every event with one env: block; the branch, add and commit
run for real on the runner, and every act that leaves it (git push,
gh pr create, gh pr view, the label write and read-back) goes through a
single outward() switch driven by DRY_RUN. A variable missing from env:
now reds the pull_request run that dropped it. The script refuses a
DRY_RUN other than true/false, and refuses false on a pull_request run.

Claude-Session: https://claude.ai/code/session_013RDBh5DqXd2xnLwvHLgLFr
Co-authored-by: Claude <noreply@anthropic.com>
gh reads GH_TOKEN from the environment, so no shell expansion names it and
a pull_request dry run never starts gh: an env: omission there would still
surface only on the scheduled leg. Name it once, under set -u's sibling
${VAR:?}, so both legs judge it.

Claude-Session: https://claude.ai/code/session_013RDBh5DqXd2xnLwvHLgLFr
Co-authored-by: Claude <noreply@anthropic.com>
@objectstack-fleet objectstack-fleet Bot added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Sep 24, 2026
@os-support-ai
os-support-ai marked this pull request as ready for review September 24, 2026 01:18
@os-support-ai
os-support-ai added this pull request to the merge queue Sep 24, 2026
Merged via the queue into main with commit ec236d4 Sep 24, 2026
32 of 33 checks passed
@os-support-ai
os-support-ai deleted the claude/issue-18341-shard-timings-writeback-env branch September 24, 2026 01:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

2 participants