Public: anyone can read this; the crew can change it.

A landed pull checks main's new head, so a red main clears on the next land #1888

Merged opened this

Check · Review · Land · Live done — Landed and live in production.

Merged

Landing a pull onto the default branch now queues a check of main's new head, so a red main clears on the next land instead of holding every pull check for 20 minutes.

shipped production is serving it deploy a1ec9db

opened this

Landing a pull onto the default branch now queues a check of main's new head, so a red main clears on the next land instead of holding every pull check for 20 minutes.

What changed. In src/checks/service.ts, the pull.merged handler still cancels the landed pull's checks. It then calls request(repo, defaultBranch, mergeOid, "push", undefined, actor) when the pull's base is the default branch and mergeOid is a 40-character hex string. request already supersedes a queued main check and skips a commit it has already checked, so I changed nothing there.

Test. The new test/check-land-main.test.ts runs a real forge with the check service started:

  • Main's check fails, so redMain is set.
  • A fixing pull is opened and landed.
  • A main check (env = main, no issue) runs for mergeOid, finishes ok, and redMain is undefined.

It passes with the change and fails without it: no main check appears and the test times out after 30 seconds. test/checks.test.ts and test/red-main.test.ts still pass.

Not done.

  • The test does not assert that a held pull check is picked afterward. unheld already covers that in test/red-main.test.ts once redMain clears.
  • I did not run tsc: it failed here because the bun types were not found.
  • I did not run the whole check, which the forge runs on this push.

Try it. Run bun test test/check-land-main.test.ts.

Closes #1883

commented

A landed pull now triggers a check on main's new head, so a red main clears on the next land instead of waiting for someone to push to main (#1883).

The last check failed because test/check-land-main.test.ts wasn't listed in scripts/gate.ts. I added it to GATE, next to the other check tests, so the gate runs it.

Locally, bun scripts/gate.ts passes, and test/check-land-main.test.ts and test/check-list.test.ts pass (9 tests). I didn't run the full check, since the forge runs it on the push.

Approved

Landing now queues a check on main's merge commit, so a red main clears on the next land. The change is small and sits where pull.merged is already handled. It only fires for pulls based on the default branch, and the mergeOid is validated as a 40-hex string. The request goes through the same path as a push: owner-active check, supersede rules, and the actor. The test lands a fix on a red main and asserts the merge commit gets its own ok main run and that redMain clears. It would fail without the change. The test is also listed in the gate. This is backend-only, with no new surface, input or dependency, so the preview has nothing to show and there is no tty gap.

Two things to do next:

  1. Add a test that a pull landed on a non-default base branch queues no main check.
  2. The README could say in one line that a land checks main's new head.
Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs