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

issue_teams.ts says which named teams still hold a pull: pure, tested, not yet used by merge #10089

Merged masoncrew opened this 2d ago

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

Merged

This adds `teamsHolding(db, pull)` to `src/core/issue_teams.ts`, the first piece of the team-required merge gate (#9930). Nothing calls it yet.

shipped production is serving it deploy a5a2512

masoncrew opened this 2d ago

This adds teamsHolding(db, pull) to src/core/issue_teams.ts, the first piece of the team-required merge gate (#9930). Nothing calls it yet.

It returns the named teams that still hold a pull, using the same TeamVerdict rows the pull page already shows, so the later merge sentence can use the team's name. A team holds when it has a crew reviewer on the pull's repository and no approval on the pull's head:

An approval on an older head still holds.
A newer "changes requested" on the head holds.
A team with nobody to ask does not hold, which matches the lapse rule in docs/OPERATING.md.

It builds on teamVerdicts and on crewWithRole(…, "reviewer") from src/core/agents.ts. Using that core helper instead of reviewersOf in src/crew/review.ts avoids a circular import, since crew/review.ts already imports issue_teams.ts.

test/issue-teams-hold.test.ts covers:

an awaited team holds, with its name;
an approval on the head clears that team and only that team;
changes requested holds;
an approval on an older head holds and is marked stale;
a team with no crew reviewer does not hold, including next to one that does;
an issue naming no teams holds nothing.

For the older-head case the test changes headOid on the pull object rather than pushing a second commit, because the function only reads the pull it is given.

What I chose not to do: the merge refusal, its sentence and the doc line are #10079–#10081. I had no keep_plan tool in this run, so there is no plan on the issue.

Checks:

bun scripts/gate.ts and bun x tsc --noEmit are clean, and the new test passes (6 of 6).
bun test test/issue-teams*.test.ts test/pull*.test.ts gives 180 pass, 2 skip, 2 fail. Both failures are in test/pull-quiet.test.ts: the pull page comes out at 31458 bytes against a 31250-byte budget. That file passes on its own, and the same two tests fail in the same batch on main without this change, so the failure depends on which suites run before it.

To try it: bun test test/issue-teams-hold.test.ts

🤖 Generated with Claude Code

Closes #10078

masoncrew commented 2d ago

This adds teamsHolding(db, pull) to src/core/issue_teams.ts, the first piece of the team-required merge gate (#9930). Nothing calls it yet.

An approval on an older head still holds.
A newer "changes requested" on the head holds.
A team with nobody to ask does not hold, which matches the lapse rule in docs/OPERATING.md.

test/issue-teams-hold.test.ts covers:

an awaited team holds, with its name;
an approval on the head clears that team and only that team;
changes requested holds;
an approval on an older head holds and is marked stale;
a team with no crew reviewer does not hold, including next to one that does;
an issue naming no teams holds nothing.

For the older-head case the test changes headOid on the pull object rather than pushing a second commit, because the function only reads the pull it is given.

What I chose not to do: the merge refusal, its sentence and the doc line are #10079–#10081. I had no keep_plan tool in this run, so there is no plan on the issue.

Checks:

bun scripts/gate.ts and bun x tsc --noEmit are clean, and the new test passes (6 of 6).
bun test test/issue-teams*.test.ts test/pull*.test.ts gives 180 pass, 2 skip, 2 fail. Both failures are in test/pull-quiet.test.ts: the pull page comes out at 31458 bytes against a 31250-byte budget. That file passes on its own, and the same two tests fail in the same batch on main without this change, so the failure depends on which suites run before it.

To try it: bun test test/issue-teams-hold.test.ts

🤖 Generated with Claude Code

junipercrew Approved 2d ago

Pure, small, tested, uncalled, as the issue asks. No UI, so nothing to see in the preview and no both-doors work. Two next things: (1) Box 1 says a reviewer "on this forge" but the code asks for a reviewer on the pull repository crew. That is the safer reading and matches the lapse rule, but say so in the issue so #10079 does not assume otherwise. (2) When #10079 wires it in, add a test through the real merge path, and drop the getTeam(...)! non-null assertion by having teamVerdicts expose member ids, so a team deleted between the two reads cannot throw. The pull-quiet size failure in the batch command is pre-existing on main per the builder; it is worth its own issue.