Skip to content

ci(pr-automation): run the ADR-0087 and major-bump steps past a designed Check Changeset red - #20976

Merged
objectstack-fleet[bot] merged 2 commits into
mainfrom
claude/issue-20784-changeset-steps-past-correction
Oct 1, 2026
Merged

objectstack-fleet[bot] merged 2 commits into
mainfrom
claude/issue-20784-changeset-steps-past-correction

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Fixes #20784

Clause-②: no

What changes

.github/workflows/pr-automation.yml, job changeset-check (Check Changeset). The three steps after Reject an empty-frontmatter changeset added by this PR now take:

if: >-
  !cancelled()
  && steps.labels.outputs.skip != 'true'
  && steps.labels_settled.outputs.skip != 'true'
  && steps.diffbase.outputs.merge_base != ''
  # the major guard alone adds:  && steps.allow_major.outputs.allow != 'true'
  • Require an ADR-0087 disposition on a declared-breaking changeset (check-adr-0087-registration.mjs)
  • Re-read this PR's allow-major label live (id allow_major; it only feeds the guard below)
  • Guard against accidental major bumps (launch window) (check-changeset-no-major.mjs, the level axis)

There is no always(), so a cancelled run still stops. The job stays red on the DELIBERATE CORRECTION class. It now also shows the ADR-0087 and level-axis verdicts on that same run.

The route-0 text that Require a changeset prints said "the steps after it are skipped". This change makes that false, so the text now says which step is still skipped and that the two verdict steps after it run.

The pin is the file's existing shape pin: scripts/check-empty-changeset.mjs --self-test, battery "The consumer: this gate's own CI step". It gains 11 cases, and the battery floor moves from 23 to 34. It asserts four things:

  • each of the three steps begins its if: with !cancelled() && and has no ||;
  • each of them requires steps.diffbase.outputs.merge_base != '';
  • no other step of the job names a status function;
  • nothing in the workflow uses always().

No gate-script logic changed. Only the self-test block and its floor did.

The three hypotheses, measured before building

H1: which steps follow the empty-changeset step. There are exactly three, the ones listed above. All three should run past its red. None of them is a consequence of that red: two are independent verdicts on the same diff, and the allow-major read is an input to one of them.

A second designed red hides the same three steps. On a correction-only PR (it adds no changeset of its own), route 0 makes Require a changeset fail by design. Both later gates diff with --diff-filter=AMR, so such a PR still gives them something to judge. The triage direction's literal form, !cancelled() && LABEL_CONDITIONS, runs the steps past both reds, and this PR keeps that reach. Fixture F2 below shows the verdicts that were being hidden.

H2: the outputs the later steps read. No later step reads an output of the empty-changeset step, which has no id. When that step is red, every step before it succeeded, so merge_base is set, labels.skip is false, and labels_settled.skip is false or unwritten (never 'true'). The later steps see correct inputs.

The literal form has a wider reach than that, though. It would also run the three steps past the unusable-base red (Require a usable diff base), where steps.diffbase.outputs.merge_base is the empty string. I measured all three changeset scripts on this tree with an empty base. Each one treats it as "no base" and answers against origin/main with exit 0:

node scripts/check-adr-0087-registration.mjs --base ""  -> exit 0  "this PR adds no declared-breaking changeset"
node scripts/check-changeset-no-major.mjs --base ""      -> exit 0  "Diffing HEAD from 3693a1b50 (merge base with origin/main)"
node scripts/check-empty-changeset.mjs --base ""         -> exit 0  (same fallback)

That verdict would be about a base this job had already refused to trust. So each step also requires merge_base != ''. This is the same step-level !cancelled() plus "the output that proves the input exists" pattern that release.yml and cut-rc.yml already use.

This is not the dispatch's stop condition: merge_base is not an output of the empty-changeset step. It is the minimal guard the literal form needs. No other input needs a guard. The two scripts import only node builtins and repo-local modules, so a failed pnpm install cannot change their verdict. An unwritten label output is not 'true', which is the enforcing direction.

H3: where the shape pin lives. It lives in scripts/check-empty-changeset.mjs --self-test, in the battery "The consumer: this gate's own CI step". That battery already pins this job's step conditions: the settling read, both label guards on every step that can fail, and the allow-major read. check-workflow-status-functions does not fit: its own scope note limits it to job-level if:.

check-changeset-no-major.mjs's wiring battery pins the guard step's run: and env:, and it passes unchanged.

Evidence

Ablation. Each leg ran once, on the committed tree, through scripts/ablation-replace.mjs. After every leg the tool proved the file was restored: its blob equals HEAD (e0b36ccdfa02) and git diff HEAD is empty.

Leg Mutation Self-test result
a ADR-0087 step without !cancelled() exit 1, 2 failures: the leading conjunct, and the closed set (found steps 13 and 14 instead of 12 to 14)
b major guard with base_error == '' in place of the merge_base read exit 1, 1 failure
c allow-major read with always() exit 1, 2 failures: the leading conjunct, and no always()
d Require a changeset also given !cancelled() exit 1, 1 failure: the closed set (found steps 10 and 12 to 14)
e whole workflow as it was at the base commit, i.e. without this change (trap-guarded swap; blob proven equal to HEAD after restore) exit 1, 7 failures: 3 leading conjunct, 3 merge_base, and the closed set (found none)

The first attempt at leg b did nothing. Its replacement text was a substring of its anchor, so the tool refused before running anything. The leg was re-run with a replacement that can be told apart.

Fixture run. This used a shared clone in scratch whose commits were never pushed. Each script ran with --base set to this branch's head.

Scenario empty-changeset ADR-0087 no-major
F1: the shape of PR #20780. It edits a pending note and adds its own minor changeset that carries a BREAKING banner and no disposition. exit 1 (foreign-changeset refusal) exit 1 (no disposition) exit 0
F2: correction only. It edits a pending note from minor to major and adds nothing. The count step would report 0, so route 0 turns Require a changeset red. exit 1 exit 1 (turned breaking, no disposition) exit 1 (majored in place)

Under the old conditions, none of the ADR-0087 or no-major verdicts in F1 or F2 would have run in CI.

Gates at HEAD 4888e25

node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack, with no paths, derived 52 commands from this diff. All 52 were run and every one exited 0.

dispatch-gates --ran reports: "52 derived famil(ies) accounted for, 52 run, 0 NOT-MEASURED". Among them:

exit 0  node scripts/check-empty-changeset.mjs --self-test      (170 assertions)
exit 0  pnpm check:changeset-gate-self-tests                    (empty-changeset, adr-0087 and no-major self-tests; no-major 339 assertions)
exit 0  pnpm check:workflow-status-functions
exit 0  pnpm check:workflow-step-name-quoting
exit 0  node scripts/check-self-test-workflow-commands.mjs  (+ --self-test)
exit 0  node scripts/check-step-collectors.mjs  (+ --self-test)
exit 0  pnpm check:nul-bytes
exit 0  pnpm check:pm-dispatch-gates                            (1976 cases; 1243 s on this box)

Narrowed lint. eslint's population is **/*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}, and .github/workflows/pr-automation.yml falls outside it. eslint itself reports "File ignored because no matching configuration was supplied". The --format json output has 2 entries: scripts/check-empty-changeset.mjs with 0 errors and 0 warnings, and the ignored YAML. Nothing in the config enables type-aware linting (no parserOptions.project, no typed rules), so this diff cannot change the verdict on any file it does not touch.

No changeset: this diff changes workflow wiring and a root scripts/ self-test, and neither publishes anything.

Acceptance notes

  • Live proof on GitHub is NOT MEASURED in this PR. A pull_request run takes its workflow from the PR's own merge ref. A correction-class PR will show both verdicts on one run only after this lands. PR fix(service-automation)!: the toggle door switches packaged flows only; a customer flow is refused, naming its status switch (#20726) #20780 would then need a new event such as a push or a main merge. A re-run is not enough: rerun_failed_jobs replays the original merge ref.
  • This PR's own Check Changeset run carries skip-changeset. That only exercises the exempt direction: the three steps must stay skipped when either label read exempts the PR.
  • Boundary, not widened: Reject an empty-frontmatter changeset added by this PR keeps the implicit success(). On a correction-only PR it is still skipped behind route 0, and route 0's own text says so.
  • Dormant, not filed: the three changeset scripts treat an explicit empty --base "" as absent and fall back to origin/main with exit 0. With the merge_base conjunct, CI never reaches that path. Carrier: none.
  • Base behind origin/main: this branch sits on 3693a1b50. No upstream commit since then touches either changed file or the three changeset scripts, and merge-tree is clean. CI judges the merge ref.

Generated by Claude Code

…d red

The Check Changeset job's DELIBERATE CORRECTION class is red by design
(route 0 on `Require a changeset`, and the empty-changeset step's
foreign-changeset refusal). Under GitHub's implicit success() that red
skipped the ADR-0087 disposition step, the live allow-major read and the
major guard, so the PRs most likely to carry a breaking changeset never
got either verdict in CI.

The three steps now name `!cancelled()` (never always()) and require
`steps.diffbase.outputs.merge_base != ''`: run past a red they would
otherwise also run past the unusable-base red, where every changeset
script reads `--base ""` as no base and answers against origin/main at
exit 0. The route-0 prose that said the steps after it are skipped is
corrected to say which one still is.

Claude-Session: https://claude.ai/code/session_017VaLJnYwhPsanVCe9dMCJU
Co-authored-by: Claude <noreply@anthropic.com>
…ast a designed red

The consumer battery now asserts that the ADR-0087 disposition step, the
live allow-major read and the major guard each lead their `if:` with
`!cancelled() &&` (no `||`), each require
`steps.diffbase.outputs.merge_base != ''`, that exactly those three steps
of the job name a status function, and that nothing in the workflow uses
always(). The battery's floor moves 23 -> 34 with the eleven new cases.

Claude-Session: https://claude.ai/code/session_017VaLJnYwhPsanVCe9dMCJU
Co-authored-by: Claude <noreply@anthropic.com>
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