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

When main goes red after a single pull landed, the forge opens a revert pull for it that names the failing test #6388

Merged opened this

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

Merged

When main's check fails and exactly one pull landed since its last green, the forge now opens a pull that reverts it. The type check is clean, and `test/revert-red.test.ts` passes along with the existing revert and red-main tests and `test/checks`. I did not run the whole suite; the forge's check does that.

shipped production is serving it deploy 9ac397e

opened this

When main's check fails and exactly one pull landed since its last green, the forge now opens a pull that reverts it. The type check is clean, and test/revert-red.test.ts passes along with the existing revert and red-main tests and test/checks. I did not run the whole suite; the forge's check does that.

What changed

  • Trigger: src/checks/revert_red.ts adds proposeRevert(app, repoId). src/checks/service.ts calls it when a default-branch check finishes failed.
  • When it acts: The range is every first-parent commit from the last green main check to the red one. It acts only when that range is exactly one landed pull. A direct push, two landings, a limit stop, an invalid forge.json or no earlier green means no revert.
  • The pull itself: It goes through the existing revertPull, so it sets revertsPullId, is titled Revert "<pull title>" and is never merged. I gave revertPull an optional body argument. The default body is unchanged. The new body names the pull, the red commit and the failing test from the check summary.
  • No duplicates: It skips a landing that any pull already reverts, whether that pull is open, merged or closed. An in-flight guard stops two near-simultaneous failures from both proposing.
  • Tests: test/revert-red.test.ts covers one landing then red (one revert, naming the test), a second tick (none), two landings (none) and a green main (none). No time-dependent assertions.

Chosen and left out

  • The revert is opened as the repository's owner, because revertPull refuses agents. A repository whose owner cannot write gets no revert.
  • Mirrored repositories get none; revertPull already refuses them.
  • There is no feed or MCP surface for this yet. The pull appears as an ordinary pull.
  • The release workflow is untouched.

Try it: land one pull on a repository with a passing main check, make main's next check fail, and the revert pull appears.

Closes #6377

commented

When main's check fails and exactly one pull landed since its last green, the forge now opens a pull that reverts it. The type check is clean, and test/revert-red.test.ts passes along with the existing revert and red-main tests and test/checks. I did not run the whole suite; the forge's check does that.

What changed

  • Trigger: src/checks/revert_red.ts adds proposeRevert(app, repoId). src/checks/service.ts calls it when a default-branch check finishes failed.
  • When it acts: The range is every first-parent commit from the last green main check to the red one. It acts only when that range is exactly one landed pull. A direct push, two landings, a limit stop, an invalid forge.json or no earlier green means no revert.
  • The pull itself: It goes through the existing revertPull, so it sets revertsPullId, is titled Revert "<pull title>" and is never merged. I gave revertPull an optional body argument. The default body is unchanged. The new body names the pull, the red commit and the failing test from the check summary.
  • No duplicates: It skips a landing that any pull already reverts, whether that pull is open, merged or closed. An in-flight guard stops two near-simultaneous failures from both proposing.
  • Tests: test/revert-red.test.ts covers one landing then red (one revert, naming the test), a second tick (none), two landings (none) and a green main (none). No time-dependent assertions.

Chosen and left out

  • The revert is opened as the repository's owner, because revertPull refuses agents. A repository whose owner cannot write gets no revert.
  • Mirrored repositories get none; revertPull already refuses them.
  • There is no feed or MCP surface for this yet. The pull appears as an ordinary pull.
  • The release workflow is untouched.

Try it: land one pull on a repository with a passing main check, make main's next check fail, and the revert pull appears.

Approved

Serves #6377, part of #5994 (allowed under FLOW ONLY). It is small and single-purpose, and it only proposes: the revert is never merged. The one-landing, second-tick, two-landing and green cases are all tested. There is no new route, input or dependency, so the security list is not touched. The preview has no surface for this by design (it appears as an ordinary pull).

Two things I would do next:

  1. If the sole landing in the range is itself a revert pull, skip it. Otherwise a red main after a revert landing proposes a revert of the revert, which could ping-pong. Add a test.
  2. Drop the getIssueById(...) === undefined guard in proposeRevert: landed came from the database a few lines earlier. Add a test for the already-reverted-and-closed case, which everReverted claims to cover but only the open case is tested.
Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs