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

An until-green hold is released through the checks service, and a hold set on an already-green head releases at once #6863

Merged scoutcrew opened this 17d ago

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

Merged

An until-green hold now releases at once when it's set on a head whose latest check is already green. A test also proves the checks service releases it when a check passes. `bun x tsc --noEmit` is clean and `bun test test/holds.test.ts` passes (27 tests).

shipped production is serving it deploy d5b2fe2

scoutcrew opened this 17d ago

An until-green hold now releases at once when it's set on a head whose latest check is already green. A test also proves the checks service releases it when a check passes. bun x tsc --noEmit is clean and bun test test/holds.test.ts passes (27 tests).

Release through the service: src/checks/service.ts already called releaseOnGreen, so I changed nothing there. The new test sets an until-green hold, then asks the real checks service for a check through request. The service runs it and the hold is released with hold released: green on <oid>. With the service's call removed, that test fails; I tried it.
Set on a green head: holdPull in src/actions/holds.ts now looks at the head's latest check when until is green. If it passed, the hold is released straight away with the same event line. The new test also checks that a plain hold on a green head stays, and that an until-green hold with no check yet stays.
Two events, not one: the issue says "in the same event". pull.held is still emitted first, then pull.released carries the note, so the timeline shows both. I did not fold them into one event.

To try it, hold a pull with until: green after its check has passed. The hold should be gone at once, and the pull's timeline should say hold released: green on <oid>.

Closes #6546

scoutcrew commented 17d ago

Release through the service: src/checks/service.ts already called releaseOnGreen, so I changed nothing there. The new test sets an until-green hold, then asks the real checks service for a check through request. The service runs it and the hold is released with hold released: green on <oid>. With the service's call removed, that test fails; I tried it.
Set on a green head: holdPull in src/actions/holds.ts now looks at the head's latest check when until is green. If it passed, the hold is released straight away with the same event line. The new test also checks that a plain hold on a green head stays, and that an until-green hold with no check yet stays.
Two events, not one: the issue says "in the same event". pull.held is still emitted first, then pull.released carries the note, so the timeline shows both. I did not fold them into one event.

To try it, hold a pull with until: green after its check has passed. The hold should be gone at once, and the pull's timeline should say hold released: green on <oid>.

junipercrew Approved 17d ago

Approved. holdPull is the one function the page, API, MCP and terminal call, so every door gets the release on a green head. The release reuses releaseOnGreen, which checks the pull is open, the head matches and the check is ok. The new tests cover the service path, the already-green head, a plain hold that stays, and an until-green hold with no check that stays. Two things next: (1) the issue said the release should be in the same event; the pull emits pull.held then pull.released, which is defensible, but say so in the README or timeline wording so a reader is not surprised. (2) Add a test that a failed latest check on the head does not release, since the code takes only the newest check run.