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

The pull page's merge box and the terminal's m say which team a pull waits on, and OPERATING.md stops saying it is not required #10141

Merged opened this

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

Merged

When a team named on the issue holds a pull, the pull page and the terminal now both say which team and refuse to merge, using the same `teamsHolding` check that `mergePull` uses to refuse with `team_review_required` (#10080).

shipped production is serving it deploy a5a2512

opened this

When a team named on the issue holds a pull, the pull page and the terminal now both say which team and refuse to merge, using the same teamsHolding check that mergePull uses to refuse with team_review_required (#10080).

  • Web (src/web/pages/pull.ts): the merge box gets a team step after the review check and before the checks check, the same order mergePull uses. It reads "Waiting on Design's review.", explains that nothing merges until the team approves the current head, and shows the Merge button disabled.
  • Terminal (src/tui/screens/pull.ts): mergeState takes an optional sixth argument, teamsHeld, and returns a new team_required state. The Merge box shows a team badge with "Waiting on Design's review; it cannot merge until it approves this head". Pressing m shows "Waiting on Design's review. Nothing was merged." and does not merge.
  • docs/OPERATING.md (cross-team work): the "Not done" sentence is gone. It now says the gate holds, that an approval of an older head does not count, what the API, the page and the terminal each say, and that a team with no reviewer here (or the owner's break-glass) does not hold the merge.

Three tests are added to test/team-merge-gate.test.ts: the Files page names the team with Merge disabled and stops naming it once the team approves; m in the terminal shows the sentence and leaves the pull unmerged; and mergeState orders the review, team and checks steps correctly. All three fail without the source change. bun scripts/gate.ts, bun x tsc --noEmit and bun scripts/design-lint.ts pass. scripts/fast.ts named 1565 test files, well over its 5-minute budget, so I ran the 41 test files that cover the pull screen, the merge box and the team gate: 340 tests, all passing. The full suite runs in the forge's check.

The keep_plan tool the issue asks for was not available in this session, so no plan was kept on the issue.

To try it: name a team with a crew reviewer on an issue, open a pull whose body says Closes #N, then look at /<owner>/<repo>/pulls/<n>/files or press m on the pull in the terminal.

🤖 Generated with Claude Code

Closes #10081

commented

When a team named on the issue holds a pull, the pull page and the terminal now both say which team and refuse to merge, using the same teamsHolding check that mergePull uses to refuse with team_review_required (#10080).

  • Web (src/web/pages/pull.ts): the merge box gets a team step after the review check and before the checks check, the same order mergePull uses. It reads "Waiting on Design's review.", explains that nothing merges until the team approves the current head, and shows the Merge button disabled.
  • Terminal (src/tui/screens/pull.ts): mergeState takes an optional sixth argument, teamsHeld, and returns a new team_required state. The Merge box shows a team badge with "Waiting on Design's review; it cannot merge until it approves this head". Pressing m shows "Waiting on Design's review. Nothing was merged." and does not merge.
  • docs/OPERATING.md (cross-team work): the "Not done" sentence is gone. It now says the gate holds, that an approval of an older head does not count, what the API, the page and the terminal each say, and that a team with no reviewer here (or the owner's break-glass) does not hold the merge.

Three tests are added to test/team-merge-gate.test.ts: the Files page names the team with Merge disabled and stops naming it once the team approves; m in the terminal shows the sentence and leaves the pull unmerged; and mergeState orders the review, team and checks steps correctly. All three fail without the source change. bun scripts/gate.ts, bun x tsc --noEmit and bun scripts/design-lint.ts pass. scripts/fast.ts named 1565 test files, well over its 5-minute budget, so I ran the 41 test files that cover the pull screen, the merge box and the team gate: 340 tests, all passing. The full suite runs in the forge's check.

The keep_plan tool the issue asks for was not available in this session, so no plan was kept on the issue.

To try it: name a team with a crew reviewer on an issue, open a pull whose body says Closes #N, then look at /<owner>/<repo>/pulls/<n>/files or press m on the pull in the terminal.

🤖 Generated with Claude Code

commented

When a team named on the issue holds a pull, the pull page's merge box and the terminal's m now say which team and refuse to merge. They use the same teamsHolding check that mergePull uses to refuse with team_review_required (#10080).

  • Web (src/web/pages/pull.ts): the merge box gets a team step between the review step and the checks step, the order mergePull uses. It reads "Waiting on Design's review.", explains that nothing merges until the team approves the current head, and shows Merge disabled.
  • Terminal (src/tui/screens/pull.ts): mergeState takes an optional sixth argument, teamsHeld, and returns a new team_required state. The Merge box shows a team badge. Pressing m shows "Waiting on Design's review. Nothing was merged." and does not merge.
  • docs/OPERATING.md (cross-team work): the "Not done" sentence is gone. It now says the gate holds and that an approval of an older head does not count. It says what the API, the page and the terminal each show, and that a team with no reviewer on the repository doesn't hold the merge, and neither does the owner's override.

Three tests are added to test/team-merge-gate.test.ts:

  • The Files page names the team with Merge disabled, and stops once the team approves.
  • m in the terminal shows the sentence and leaves the pull unmerged.
  • mergeState checks reviews first, then teams, then checks.

All three fail without the source change.

The first push's check failed on the gate rule "say how to try it on the preview": no commit message had a Try: line. Force pushes are refused, so a second, empty commit adds that line. The rule (tryItProblems) returns no problems for it locally. The deploy itself had passed.

bun scripts/gate.ts, bun x tsc --noEmit and bun scripts/design-lint.ts pass. scripts/fast.ts named 1565 test files for this diff, far over its 5-minute budget. Instead I ran the 41 files that cover the pull screen, the merge box and the team gate: 340 tests, all passing. The full suite runs in the forge's check.

The keep_plan tool the issue asks for wasn't available in this session, so no plan was kept on the issue.

To try it on the preview: on the issue that /preview-admin/sample/pulls/3 closes, name a team that has a crew reviewer (Details → Teams). Then open /preview-admin/sample/pulls/3/files, or press m on that pull in the terminal.

🤖 Generated with Claude Code

Approved

Approved. The merge box (one call site, the Files step) uses the existing box/btn components, disabled Merge works without JavaScript, the order review → team → checks matches mergePull, and the terminal m shows the same sentence without merging. Tests cover page, terminal and ordering. Next: (1) the main pull page does not repeat the merge box, but check the conversation tab shows the hold sentence if it ever gains one; (2) the nested ternary in drawMergeBox in src/tui/screens/pull.ts is now four deep and mis-indented, pull it into a small function.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs