Repository navigation
test: validate retarget reporting lifecycle - #1162
piotr-iohk wants to merge 7 commits into
Conversation
talosmachina
left a comment
There was a problem hiding this comment.
No findings. Moves retarget validation from re-running the test workflows on the PR head to a dispatcher. It creates pending e2e-status and Run Integration Tests checks on the PR head, waits until it has a merge commit verified against the new base, dispatches both suites from master pinned to that commit, and lets each suite report back only to the check it owns. Reviewed 0e589f1, full tier (CI workflow files). Its tree is identical to #1161 at f7bc57c, so this also covers the three commits after 53fa8ef, the head I reviewed on #1161.
Static review: talos-pr-run refused to run retarget-tests.test.cjs here because it could not confirm the author's permission, so the node tests were not run by me. This head is [skip ci] and has no runs yet.
What I checked, and 7 candidates I ruled out
Read in full: retarget-tests.cjs, e2e-retarget.yml, e2e-tests.yml and integration-tests.yml at the head, and the triggers of every other workflow.
Ruleset: master requires Run Integration Tests, Run Tests, SwiftFormat and e2e-status, all from integration 15368 (GitHub Actions). Check runs that github-script creates with the job token are attributed to that app, so the dispatcher's checks can satisfy the rule. unit-tests.yml and lint.yml have no branches filter, so Run Tests and SwiftFormat already exist on a stacked head. The two suites this PR routes are the only required ones a retarget leaves missing.
Ruled out
- Retarget steps leak into normal runs: on
pull_requesttheinputscontext is null, andnull != ''is false in Actions expressions, because both coerce to 0. Everyinputs.retarget_run != ''step and bothreport-retarget-resultjobs skip, andcheckoutgetsref: '', the default. - Stale merge candidate after a base change: the dispatcher accepts
merge_commit_shaonly when parent 0 ispr.base.shaand parent 1 is the head. The reporter re-checks parent 1 against the head it reports. - Push during a retarget run: the new
integration-tests-<pr>ande2e-tests-<pr>groups let thesynchronizerun cancel the dispatched one. Itsalways()reporter sees the head moved and leaves the old head's check alone. The new head gets checks from its ownpull_requestrun. - Re-running a completed child run: a completed check cannot reopen. The
startphase dispatches a freshe2e-retargetand fails the job, and thecompletephase returns early on a completed check, so the old conclusion is never overwritten.actions: writeis granted on every job that can take that path. - Older generation overwrites a newer check: the reporter matches
external_idretarget:<run>-<attempt>:<name>, the check name and the head before updating, and it only ever updates the check id it was handed. - New job-level
permissionsbreak the build:mastersets none, so the jobs ran on the default token, which never includesid-token.namespace-stack.sh(nsc) and theswift-packagesaction work without it today, andGITHUB_TOKENkeepscontentsandpackagesread. - Code from a same-repo PR runs with
checks: write: theeditedpath is gated to the head repo and a non-draft PR. The manual path rejects a fork head in the script. Both are reachable only by someone with write already, as with the dispatcher onmaster.
Not proven, so not posted: the dispatcher polls for a verified merge commit for about 20 s (10 x 2 s). A slower mergeability computation fails both checks, and recovery then goes through the manual dispatch. I have no run that shows GitHub taking longer than that.
Merge confidence: 4/5, a change to two required checks whose unit tests I could not run and that has no live run on this head yet.
Temporary validation fixture for #1161. It uses the production reporter at f7bc57c plus a probe-only helper adapter to select proposed workflow definitions before merge. This PR will be closed after validation.
Description
Design
N/A — no UI changes.
Preview
N/A
QA Notes
Setup
Temporary parent-to-master retarget, using workflow definitions on the probe branch through the fixture adapter.
Journeys
N/A — no user-visible behaviour change.
Manual Tests
Automated Checks
Production reporter: 25 regression tests and actionlint pass, recorded in #1161.
Evidence
Native pull_request helper 40742671425 dispatched E2E 40742705214 and integration 40742708445. The PR required-check list showed queued e2e-status 123008197780 and Run Integration Tests 123008202069, with no skipped required copies.
Isolated closed-check probe 40742192584 requested fresh helper 40742226481, which created new checks 123006673796 and 123006677249 and dispatched both suites. The original closed failure was preserved. That earlier skip-CI fixture supplied API-level evidence; the subsequent native retarget above supplied PR-display evidence.
This description update exercises metadata-only behavior before cleanup.
Gaps
This fixture verifies routing/reporting, not wallet-test success. Probe runs will be cancelled and this PR closed after validation.
Models used
gpt-6.1-sol(reasoning:high)gpt-6.1-sol(reasoning:high)gpt-6.1-sol(reasoning:high)