A pull's fast check tests the merge of the pull into its base, as the full tier does: 46 parked pulls fail head-only #1318

A pull's fast check now runs on the pull as it would land: its head merged into the base's tip. The 46 parked pulls going green is not confirmed; that needs a re-run once this is live.

test/check-tiers.test.ts passes (28 tests). I could not run test/checks.test.ts or the API tests here, because the sandbox refuses the unix-socket listen; they fail the same way without my change. tsc could not run either, since the bun types are not installed here, so the new code is untested against the type check. The new endpoint and the page button (pullChecksSubmit) have no test of their own. A test covers the shared askCheck call behind them.

• Merge checkout: runTier in src/checks/service.ts builds the merge commit (new src/checks/merged.ts, same recipe as landing.ts). It reads forge.json and package.json from that merge and exports it as the checkout. The log says merge of 50ca42e into main@d34639c. A pull forked before the base learned --tier/--diff is judged with the base's script.
• Fast set: FORGE_CHECK_BASE is now the base's tip, so the diff is baseTip..merge. Main's own drift no longer widens it.
• Conflicts: a conflicted merge is a red check with the summary does not merge into main: src/a.ts, test/b.test.ts. judgeFailedCheck in src/crew/dispatch.ts classifies that as cause=change without asking the decider, so it never reads as environment and the builder is handed the pull to rebase.
• Check again: POST /api/v1/repos/:owner/:name/pulls/:number/checks (scope rw, writers only) queues a check at the current head and answers 202 {run}. The pull page has a "Check again" button for writers on open pulls. Both go through src/checks/again.ts. A check already active at that head is returned instead of queued twice. A head that declares no check, or that waits for approval, answers 409.
• Tests: test/check-tiers.test.ts now covers four cases:

- A pull whose head lacks a script that main added is green once merged. - A conflicting pull is red with its paths in the summary. - A main that moved after the fork does not widen the diff. - A writer can ask for a check again at the current head.

I edited the existing fork-point assertion in that file, because FORGE_CHECK_BASE is deliberately the base tip now, not the fork point.

Left out:

• No dedicated web or API test for the new endpoint and button, as noted above.
• I did not re-run the 46 parked pulls.
• I added no dependencies.

To try it, push a pull whose branch forked before main gained a file the fast command needs. Its first check log opens with the merge of … into main@… line and passes. A writer can then press "Check again" on the pull page.

Closes #1316

Reviewed as the person and carried upstream as trevato-apps/forge PR #35 (your commit as is, plus one test of mine: the base-tier test built its heads to conflict with main on forge.json, which your merge check now rightly refuses, so main declares the tier first). On upstream main: tsc clean, 262 tests across the eight check/app suites pass. This pull stays open until the release is live and the parked pulls have been re-run; the converge brings it back here.

I made no change in response to this feedback, and nothing new is pushed. The branch has nothing to fix: my commit is already on it, and origin/main has no commits that this branch lacks.

The test you adjusted upstream, where the base-tier test's heads conflicted with main on forge.json, is not on this branch. The tests I added don't have that problem. In test/check-tiers.test.ts, main declares forge.json first and the pulls then change other files. They passed here before I pushed.

I agree with your change. The merge check is meant to refuse a head that conflicts with main on forge.json, so a test that depended on that conflict being tolerated was wrong, and main declaring the tier first is the right fix.

Still open from my side: the before/after times for the 46 parked pulls, which can only be measured once this is live.

The new tests are in and pushed. bun test test/check-tiers.test.ts passes (21 tests) and tsc --noEmit is clean. test/ledger.test.ts fails here, but it fails the same on main: this sandbox blocks unix-socket listen (EPERM).

Added to the pull

• Both routes: the tests call the router in-process with forgeHttp(app).handle, using tokens for the API and session cookies for the page.

- API: a reader's rw token gets 403 and no run is queued. A guest gets 401. A writer gets 202 with {run}. A closed pull gets 409 and queues nothing. - Page: a reader gets 403 and queues nothing. A writer gets a 303 back to the pull. - Gate check: I did not delete the canWrite lines to confirm the new tests fail without them.

• The button: "Check again" is on the pull's /files tab, not the studio change page at /pulls/N. The test asserts the form is rendered for a writer and absent for a reader, a guest, and a writer once the pull is closed.
• The conflict shortcut: a test with a decider configured shows judgeFailedCheck returns cause=change for a conflicted run without calling the decider. A failure that only quotes the phrase still goes to the decider.

Change to the code

• Prefix match: judgeFailedCheck now matches startsWith("check failed: does not merge into") instead of includes. That is the exact prefix checks/service.ts writes, so a summary that merely quotes the phrase is no longer treated as a conflict. I used the full prefix because the summary starts with check failed: , so matching startsWith(NO_MERGE) alone would never have matched.

Nothing is left out from the review.

🤖 Generated with Claude Code

Pull #1318 now merges cleanly into main; I merged origin/main into the branch to fix the conflict in src/checks/service.ts. I didn't wait for the forge's check, so the pull isn't yet confirmed green.

The conflict was in how the fast tier picks the commit to diff against. main had started computing a fork point with mergeBase. This pull already checks out the merge of the pull into the base's tip, so it diffs against that tip directly. I kept the pull's version and dropped main's fork-point code. Because the checked-out tree already contains the base's tip, mergeBase would return that same commit.

Checks run:

• The type check passed.
• test/check-tiers.test.ts passes.
• The full bun test run here had 524 failures and 1 error, so I did not use it. In the subset I reran, test/checks.test.ts failed because the sandbox refuses to open a unix socket (EPERM on listen). I haven't looked at the other failures, so I don't know whether they share that cause.

I did not run test/checks.test.ts outside the sandbox, so its tests have not passed here.

I merged origin/main into the branch and resolved the two conflicts, in src/crew/dispatch.ts and test/check-tiers.test.ts, by taking main's version of both files. I ran only test/check-tiers.test.ts (17 pass) and pushed; I did not run the type check or the full suite.

• No net change: the merged tree is now identical to origin/main, so the pull adds nothing beyond what main already has. The "fast check tests the pull merged into its base" change had already landed on main.
• What got dropped: the branch's "asking again over HTTP" test block is gone with main's version of the file. It isn't anywhere else in test/, so that coverage is no longer in the repo.

The forge's check should now find the pull merges cleanly into main.

The full gate on land was red and named no test; the pull is open again.

The full gate on land was red and named no test; the pull is open again.

The fast-check-tests-the-merge change was already on main, so this pull now only adds the HTTP-gate tests Juniper found missing. I merged current main into the branch, which went in clean. Beyond that, the branch adds one test file and one line in the gate list.

What changed

• test/pull-check-again.test.ts covers POST /api/v1/repos/:owner/:name/pulls/:number/checks:

- A guest gets 401. - A reader gets 403. - A writer gets 202 with {run}, and the run's reason is asked. - A closed pull gets 409.

• The same file checks that "Check again" appears to a writer on an open pull, and not to a reader or on a closed pull. The button lives on the pull's /files tab, where the merge box is, so the test fetches that tab.
• scripts/gate.ts lists the new test in GATE. test/check-list.test.ts goes red for a test file in neither list, which may be what made the earlier full gate red without naming a test. I haven't seen that log, so this is a guess.
• bun test passes for the new file, check-tiers, and check-list. tsc --noEmit is clean.

Left out

• I did not confirm that the 46 parked pulls go green. That needs a re-run once the release is live (Juniper's second point).
• I did not run the full gate here.

To try it: bun test test/pull-check-again.test.ts

🤖 Generated with Claude Code

The full gate on land was red and named no test; the pull is open again.