Skip to content

ci(e2e): dispatch through Dispatron, keep the reporting here - #1489

Draft
zoroyihan7 wants to merge 1 commit into
mainfrom
ci/e2e-canary-dispatron-trial
Draft

zoroyihan7 wants to merge 1 commit into
mainfrom
ci/e2e-canary-dispatron-trial

Conversation

@zoroyihan7

Copy link
Copy Markdown
Contributor

What

The dispatch half of .github/scripts/ci-e2e-dispatch.sh moves behind dispatron-ci, which the runner image carries. The reporting half stays here — the live commit status and the one sticky comment per PR are unchanged in behaviour.

DO NOT MERGE yet. This is the trial that proves the path on a real run.

Why this split

Dispatch is the same for every repository that wants a GPU check, and was duplicated per repository — a fix to the polling or the error vocabulary had to be made once per copy. How this repository annotates its own pull requests is nobody else's business and belongs here.

humanize_reason is gone from this file but not lost: it is the CLI's explanation output — the same sentences, sitting next to the vocabulary they describe, so the next team gets them without copying this script.

Two channels, neither of them prose

  • events file — one JSON object per poll. The live status refreshes from this, so a status refresh cannot add load to the run it is describing, nor disagree with it.
  • terminal event — the verdict, the reason, the timings.

The script no longer knows the orchestration endpoint, its phase names, its error strings, or its retry policy.

Two things it gains

The backend's run id. dispatches[-1].platform_ref was read by the old poll loop and never shown. A failed check that names only a Dispatron uid points nowhere outside Dispatron; the report now carries | backend run | claw-… on node-… |.

A stack trace that does not break the table. The old | detail | ${err} | put the platform error in a table cell, where the first newline ends the row and mangles everything after it. The sentence goes in the table now; the raw text goes under it in a folded block.

Not a line-count win

295 → ~300 lines, and it is worth saying so rather than claiming a reduction. What went away is this repository's knowledge of somebody else's HTTP API. What arrived is a declared CLI contract and a reporting layer that is all reporting.

Tested

The driver was exercised against a stub CLI over every outcome:

case exit reported as
succeeded 0 ✅ PASS
failed (multi-line platform error) 1 ❌ FAIL + folded trace
cancelled 1 🚫 CANCELLED — deliberately not a "look for a bug" red
timeout 1 ❌ FAIL (timeout)
CLI died before dispatching 1 ❌ never dispatched — distinct from a run that failed

The rendered comment was checked with a stubbed curl, including the duration maths (queue → dispatch, run time, total). The full argument list was replayed against the real CLI on the runner: every flag accepted.

How to run it

/retest will not exercise this PR — issue_comment always loads the workflow from the default branch. Use the retest label, which runs the PR's own copy.

Sequence: add e2e-canary (routes to the canary runner via #1485), then add retest.

Blast radius

None. A workflow reaches a run through the PR's own merge ref, so every other PR keeps main's script, the incumbent backend, and the eight-runner pool.

DO NOT MERGE yet -- this is the trial that proves the path on a real run.

The dispatch half of `ci-e2e-dispatch.sh` -- submit, poll, translate the platform's
error, cancel on the way out -- moves behind `dispatron-ci`, which the runner image
carries. The reporting half stays: the live commit status, and the one sticky comment
per PR. That split is the whole point. Dispatch is the same for every repository that
wants a GPU check and was duplicated per repository, so a fix to the polling or the
error vocabulary had to be made once per copy. How this repository annotates its own
pull requests is nobody else's business and belongs here.

`humanize_reason` is gone from this file, not lost: it is the CLI's `explanation`
output, the same sentences, sitting next to the vocabulary they describe so the next
team gets them without copying this script.

Two channels come back, and neither is this script reading the CLI's prose. The events
file is one JSON object per poll, which is what the live status refreshes from -- so a
status refresh cannot add load to the run it describes, nor disagree with it. The
terminal event carries the verdict, the reason and the timings.

The report gains a row it could not have had before. `dispatches[-1].platform_ref` --
the backend's own run id -- was read by the old poll loop and never shown; a failed
check that names only a Dispatron uid points nowhere outside Dispatron.

Fixed while moving it: a platform error is usually a stack trace, and the old
`| detail | ... |` put it in a table cell, where the first newline ends the row and
mangles the table. The sentence goes in the table now; the raw text goes under it in a
folded block, where it can be as long as it is.

Not a line-count win, and it is worth saying so: 295 lines to ~300. What went away is
this repository's knowledge of somebody else's HTTP API -- the endpoint, the phase
names, the error strings, the retry policy. What arrived is a declared CLI contract and
a reporting layer that is all reporting.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.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.

1 participant