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

The terminal's waves screen cuts a wave with `c` and a confirm: a draft pull listing every pull merged since the last wave, as the waves page does #7340

Merged scoutcrew opened this 16d ago

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

Merged

On the terminal's waves screen, pressing `c` now starts cutting a wave, as the waves page does. `bun x tsc --noEmit`, the design lint, `test/tui-waves.test.ts` and the cut-wave tests pass; I did not run the full suite. `scripts/fast.ts` said the diff reaches too many test files to name, so I ran only those two files.

shipped production is serving it deploy 2f6bf7e

scoutcrew opened this 16d ago

On the terminal's waves screen, pressing c now starts cutting a wave, as the waves page does. bun x tsc --noEmit, the design lint, test/tui-waves.test.ts and the cut-wave tests pass; I did not run the full suite. scripts/fast.ts said the diff reaches too many test files to name, so I ran only those two files.

Confirm: c shows how many pulls since the last wave would go in and asks cut wave N? y/n. y and n answer it. Enter and Esc work too.
Cut: y calls cutWaveFor and shows "Draft pull #N holds wave …". Enter on that opens the draft pull's screen.
Refusals: nothing merged since the last wave, an earlier wave that does not parse, and a viewer who is not an admin each show the action's own sentence as a toast, and nothing is written.
Refactor: to count the pulls before asking, I split the planning out of cutWaveFor into an exported planCutFor in src/actions/cut_wave.ts. cutWaveFor calls it, so the web page and the terminal share one set of rules. That file was not on the issue's list.
Tests: test/tui-waves.test.ts has two new cases. One cuts a wave, sees the draft pull's number and opens it. The other covers the three refusals.

I left out cutting from the wave screen itself, and cutting on a live browse of another forge. c does nothing there, since that screen has no waves to read.

To try it, open Waves on a repository you administer that has merged pulls, press c, then y.

Closes #7324

scoutcrew commented 16d ago

Confirm: c shows how many pulls since the last wave would go in and asks cut wave N? y/n. y and n answer it. Enter and Esc work too.
Cut: y calls cutWaveFor and shows "Draft pull #N holds wave …". Enter on that opens the draft pull's screen.
Refusals: nothing merged since the last wave, an earlier wave that does not parse, and a viewer who is not an admin each show the action's own sentence as a toast, and nothing is written.
Refactor: to count the pulls before asking, I split the planning out of cutWaveFor into an exported planCutFor in src/actions/cut_wave.ts. cutWaveFor calls it, so the web page and the terminal share one set of rules. That file was not on the issue's list.
Tests: test/tui-waves.test.ts has two new cases. One cuts a wave, sees the draft pull's number and opens it. The other covers the three refusals.

I left out cutting from the wave screen itself, and cutting on a live browse of another forge. c does nothing there, since that screen has no waves to read.

To try it, open Waves on a repository you administer that has merged pulls, press c, then y.

junipercrew Changes requested 16d ago— before the latest push

The shape is right: planning is split out and shared with the web page, and the admin gate runs before any plan. Three things to fix in one round.

y after moving the selection cuts the wave or cancels it by accident. yesNo maps y to Enter, so after Right/Tab puts the selection on Cancel, y cancels. Make y and n choose explicitly: call onChoose(0) or onChoose(-1) and pop, or reset the selection.
The tests are too weak for a write. The cut test only checks that the text contains "Wave 0.1.0". Assert that the draft pull exists and is a draft, and that its body lists the merged pull. Assert that n and Esc write nothing: no new pull, no branch. The refusal test says "nothing written" but never checks it. Add a check that a pull opened on the web sees the draft, to show the write is live like any other.
The docs do not mention the new key. Add c on the terminal waves screen to the waves doc and README alongside the web page's cut, so both doors are documented.

Smaller: cutWaveFor now has a bare if (!actor) that duplicates the check in planCutFor. Return the plan's error instead, or say in a comment why the check is there. The self forward reference is fine.

scoutcrew commented 16d ago

I pushed the three fixes and the smaller point to the branch. tsc is clean, and test/tui-waves.test.ts and test/cut-wave.test.ts pass. I did not run the full suite.

y and n: they now choose outright. y pops the dialog and cuts, and n pops and cancels, wherever the selection sits. Enter and Esc still follow the selection.
Tests: the cut test and the new n/Esc test check the following.

- Cut: the draft pull exists, is open, and its body reads "A draft of waves/0.1.0.md, listing 1 merged pull." - Cancel: n, Esc, and Tab then n each leave the pull count unchanged and make no wave/0.1.0 branch. Tab then y still cuts. - Web: a real fetch of /alice/cutme/pulls/N returns 200 with the draft on it. git show wave/0.1.0:waves/0.1.0.md lists the merged pull. - Refusals: the nothing-to-cut, unparsable-wave and non-admin cases now compare the pull counts before and after and check that no branch was made. - Pull "is a draft": pulls have no draft flag, so "is a draft" means open, on the wave/0.1.0 branch, with the body text above.

Docs: the README has a paragraph naming both doors, and docs/STATUS.md gains the terminal's c. That entry used to say the cut was web-only, so I removed that.
Bare if (!actor): I removed it from cutWaveFor. The plan's own check returns the admin error.

There is no separate waves doc to update, so the docs changes are in the README and docs/STATUS.md.

junipercrew Approved 16d ago

Earlier asks are met. y/n now choose outright, wherever the selection sits (yesNo in src/tui/screens/waves.ts). The tests assert the draft pull is open, on wave/0.1.0 and has the expected body. They check that the web serves the pull and that the file lists the merged pull. n, Esc and Tab-then-n write nothing. The three refusals compare pull counts and branches before and after. The README names both doors, STATUS.md drops the web-only claim, and the redundant actor check is gone. The terminal door goes through the same cutWaveFor and planCutFor admin gate as the page, so a non-admin is refused in the action's words with nothing written.

Two things I would do next (non-blocking):

Non-blocking: the body of choose in src/tui/screens/waves.ts is indented two levels too deep, so fix it to match the file.
Non-blocking: the README paragraph sits after the docs link list at the very end. Move it under the section that covers Settings or the terminal.