Skip to content

A new action, tree-tested: skip a push's tests when its pull request already passed that tree - #209

Merged
askalf merged 5 commits into
mainfrom
tree-tested
Nov 12, 2026
Merged

askalf merged 5 commits into
mainfrom
tree-tested

Conversation

@askalf

@askalf askalf commented Nov 12, 2026 •

Copy link
Copy Markdown
Owner

A new action, actions/tree-tested, so a push to the default branch does not run the same build and tests a second time on files a pull request already tested.

Why

A pull request runs its workflow, merges, and the push to main runs the same workflow again, most often on exactly the same tree. On the askalf runner host, the last day's push runs were a large share of the CPU-heavy work (restore-desk's four-job CI alone is about 17 runner minutes a run), on a host whose runner slice spent a quarter of the time waiting on CPU.

The caller's word

GitHub does not record what a job's commands did with their inputs, so one thing cannot be read: whether a job runs the same commands on a push as on a pull request. The caller says so with same-on-pull-request: true; without it the answer is always false. It holds when nothing in the job depends on the event: no step or command that runs only on push, no matrix a push widens, one checkout of the event's own commit, and nothing after that checkout that moves the tests onto other files (a branch switch, a reset, a second checkout). What can be checked is checked below; the rest is the caller's word.

The rule

The action answers tested=true for the job that asks only when all of these hold, each read from the API with the job's token:

  1. The caller declared the job the same on push and on pull request.
  2. The event is a push to the default branch.
  3. A pull request of this repository was merged as this commit.
  4. That pull request's head has the same git tree as the pushed commit.
  5. The newest finished pull_request run of this workflow file at that head concluded success, and every job of its latest attempt, and every step of every job, concluded success, over all pages of jobs. A skipped job or step tested nothing (a step that runs only on push, a job a change did not need), so a run with one does not count.
  6. This job (found in its own run by its runner's name) has a name no other job of this run has, and exactly one job of that run has it. A job the pull request did not run is not there, and two jobs under one display name (a matrix whose entries share a name) cannot be told apart, so neither is taken.
  7. That job's log shows what its checkout made: exactly one actions/checkout step, exactly one "HEAD is now at" line in the whole log, and that line inside the checkout step reading "Merge into ". The head there must be the pull request's head, and that base must be inside the head by GitHub's compare. A merge of a base the head already holds is the head's own tree, so the checkout was exactly the pushed tree, whatever branch the pull request pointed at then and whatever main did afterwards. A second visible "HEAD is now at" line (a later detached checkout or hard reset, or a test that prints one) is caught. Not everything that moves a checkout prints that line: a switch to a branch prints "Switched to branch", and a quiet checkout or reset prints nothing. That the tests ran on what the checkout made is the caller's declaration, not something the log can show.

Anything else, and any read that fails, answers tested=false, and the job runs in full as before. The step never fails a job. It cannot turn a red run green; at worst it runs a test it could have skipped.

A caller puts it early in a job and guards the expensive steps:

      - id: tree
        uses: askalf/ci/actions/tree-tested@<sha>
        with:
          same-on-pull-request: true
      - run: npm test
        if: steps.tree.outputs.tested != 'true'

Replay

Run job by job against the last eight pushes of four repositories (read-only, 2026-11-12; only "this job" faked, everything else read from GitHub), it would have skipped 43 of 120 push jobs, 75 of 170 runner minutes: claude-re 22 of 25, restore-desk 38 of 87, dario 15 of 46. platform skips none: its job checks out a second repository at a pinned commit, and two checkouts are not taken. The rest were a moved main, a skipped job or step in the pull request run, a failure, a base the head did not hold, or a job with no checkout, all run in full.

Changes

  • scripts/tree-tested/check.mjs and check.test.mjs (57 checks against a stubbed GitHub, among them a retarget after the run, a change reverted on main between the run and the merge, a push-only matrix entry, a wider push matrix under one display name, a merge checkout followed by an ordinary-commit checkout, a merge line printed by a test, a skipped job past the first hundred, a skipped step in a green job, and a caller that did not declare the job the same on push).
  • actions/tree-tested/action.yml and main.mjs, a node24 action like scorecard-publish-check.
  • scripts/redline/pin.mjs: TREE_TESTED_ACTION joins ACTIONS, so the pin bump moves it; review.test.mjs counts it.
  • redline-pin-bump.yml watches the new paths; redline-self-test.yml and test-all.mjs run the new tests.
  • README table and CHANGELOG.

node test-all.mjs passes here apart from the two Windows-only checks in review.test.mjs that fail before this change too.

…already passed that tree

On a push to the default branch it says whether the pull request merged as that commit passed the same workflow, with every job green on its latest attempt, at a head with the same git tree. A caller guards its build and test steps on it. Anything short of that, and any failed read, runs the job in full.
…every step

A head with the pushed tree did not prove which tree its pull_request run tested: main could gain a change and revert it between the run and the merge. The check now also requires main as it was before the merge to be an ancestor of the head, so every base the run could have merged is inside it. It reads all pages of the run's jobs, and requires every step of every job to have run and passed, so a test step that runs only on push is never skipped on the word of a run that skipped it.

@sprayberry-redline sprayberry-redline left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: request changes. The action, pin-bump integration and stubbed tests are wired together, but the skip decision can accept a run that tested a different tree or a smaller test matrix. Those gaps need to be closed before callers can safely use the output to skip tests.

1. Blocking: scripts/tree-tested/check.mjs:141

const cmp = await get(io, env, `/repos/${f.repo}/compare/${parent}...${head}`);

Containment of the final main parent does not establish the base used by the selected pull_request run. For example, a head containing main can first target a release branch with additional changes and pass CI on that merge tree. Retargeting the PR to main does not trigger a default pull_request workflow (edited is not one of its default activity types). After merging, the pushed tree equals the head and this compare returns ahead, so the old successful run is accepted even though it tested the release merge tree, not the pushed tree. No later check verifies the run's actual base or tested merge tree.

Suggested fix:

Verify the actual merge commit/tree used by the selected run against the pushed tree, failing closed when that evidence is unavailable. Add a regression for a successful run before a base-branch retarget.

2. Blocking: scripts/tree-tested/check.mjs:163

return { tested: true, why: `#${pr.number}'s head ${head.slice(0, 7)} has this tree and holds main as it was before the merge, and ${f.workflowFile} passed there with every job and every step`, run: run.html_url ?? String(run.id) };

Every recorded PR job passing does not establish that the push's test coverage ran. A workflow can use an event-dependent matrix, such as one platform on pull_request and three platforms on push. The PR then has only successful jobs and steps, and all tree checks pass. Each of the three push jobs receives tested=true and skips its tests, including the two platforms never tested by the PR. The omitted matrix jobs do not appear as skipped jobs in the API, so jobsProblem cannot catch this.

Suggested fix:

Require evidence that the caller's job/matrix and guarded tests are equivalent to those executed in the prior run, or introduce an explicit restricted opt-in contract that excludes event-dependent coverage and stop promising unconditional safety. Add coverage for a push matrix with entries absent from the PR run.

rule:none

…re the same job there

Main's parent inside the head did not prove the base a pull_request run merged: a pull request tested against a release branch and then retargeted to main keeps its old green run. The check now reads, from the log of the job of the same name in that run, the merge its checkout made, and requires it to be the head merged into a base the head holds, which is the head's own tree. It also requires the job asking to be a job of that run, so a matrix entry a push expands and a pull request does not is never skipped. The parent compare is gone; the merge in the log replaces it.

@sprayberry-redline sprayberry-redline left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: request changes. The retarget regression now checks the base recorded in the prior job's log, and the tests cover missing matrix job names and paginated results. The action and pin-bump wiring are consistent, but job-name matching and a merge message in a log still do not establish that the guarded tests ran on the same inputs.

1. Blocking: scripts/tree-tested/check.mjs:209

const twins = theirs.jobs.filter((j) => j?.name === me.name);

The earlier matrix finding is only partly fixed: display names need not include matrix values. For example, a matrix job with explicit name: test can run only Linux on pull_request and Linux plus Windows on push. The prior run has exactly one job named test, so both push jobs match that one job and receive tested=true once the remaining checks pass. The Windows tests are skipped despite never running in the PR. The same problem applies to event-dependent test commands inside a successful step.

Suggested fix:

Require an explicit caller contract that identifies equivalent test inputs, including matrix dimensions, and excludes event-dependent coverage, or obtain and validate that equivalence before returning true. Add a regression with a fixed display name and a wider push matrix.

2. Blocking: scripts/tree-tested/check.mjs:124

const MERGE_LINE = /HEAD is now at [0-9a-f]{7,40} Merge ([0-9a-f]{40}) into ([0-9a-f]{40})/g;

Finding one matching message anywhere in the job log does not establish the checkout used by its tests. A job can check out the PR merge, then check out an older ordinary commit in the same directory before testing. The first checkout supplies the sole matching message; the second checkout's ordinary commit message is ignored. All steps can succeed, the recorded base can be inside the head, and decide returns tested=true even though the tests ran on the older tree. Output from test scripts can also supply this matching text, since it is not bound to a checkout step or verified commit.

Suggested fix:

Do not use an arbitrary log message as proof of the tested tree. Require trustworthy evidence tied to the test checkout and a restricted caller contract covering subsequent checkout changes, failing closed otherwise. Add a regression with a qualifying merge checkout followed by an ordinary-commit checkout before tests.

rule:none

…e bound to the one checkout

Two jobs under one display name, a matrix a push widens without changing the name, matched the pull request's single job; and a merge line anywhere in the log was taken, even with a later checkout of another commit or a test printing the same text. The job asking must now have a name no other job of its run has. The log must hold exactly one actions/checkout step and exactly one "HEAD is now at" line, inside that step, naming the pull request merge. What GitHub cannot show, that a job runs the same commands on push as on a pull request, the caller declares with same-on-pull-request: true; without it the answer is always false.

@sprayberry-redline sprayberry-redline left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: request changes. The caller opt-in, unique job-name checks and checkout-log regressions address the earlier findings, and the action is wired into the test and pin-bump paths. One safety claim about Git's output still needs correction before this ships.

1. Blocking: scripts/tree-tested/check.mjs:35

// exactly one "HEAD is now at" line in the whole log (git prints one for every checkout,
// switch or hard reset, so a later one, or a test that prints one, makes two), and that line

Git does not print this message for every checkout or switch. Switching to a local branch normally prints "Switched to branch ...", and quiet checkout or reset commands can suppress the message altogether. The parser counts only matching HEAD lines, so these subsequent changes are not detected. The same assertion appears in rule 6 of the PR description. The caller's restriction to one checkout is therefore essential, rather than something this log check can independently establish.

Suggested fix:

Remove the universal claim from the comment and PR description. Explain that the log check rejects additional visible HEAD messages, while the caller contract must exclude subsequent branch switches, resets and other changes to the tested checkout.

rule:misleading-text

Git does not print "HEAD is now at" for every change of a checkout: a switch to a branch prints "Switched to branch", and a quiet checkout or reset prints nothing. The comment no longer claims it does. The log check catches a second visible line; that nothing after the checkout moves the tests onto other files is part of the caller's declaration, and the comment and description now say so.

@sprayberry-redline sprayberry-redline left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: approve. The revised checkout explanation resolves the remaining finding by distinguishing visible log evidence from the caller's restriction on later checkout changes. The opt-in decision, regression tests, action entry point and test/pin-bump wiring remain consistent with that contract.

@askalf
askalf merged commit bbc8b21 into main Nov 12, 2026
13 of 14 checks passed
@askalf
askalf deleted the tree-tested branch November 12, 2026 03:40
askalf pushed a commit to askalf/pitlane that referenced this pull request Nov 12, 2026
Moves this repository's askalf/ci pins (its reusable workflow calls,
their redline-ref inputs and any action pins) to
askalf/ci@bbc8b21, the main commit that
last changed the review, the fix or an action (main 2026-11-12,
askalf/ci#209). The scripts are fetched at that same commit. The pins
are the only change.

---------

Co-authored-by: askalf-pin-bump[bot] <352119504+askalf-pin-bump[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants