[PATCH b4 00/27] Stop the editor branch guard from eating review replies
Christian Brauner <[email protected]> Fri, 31 Jul 2026 11:20:59 +0200
| Newsgroups | org.kernel.linux.tools |
|---|---|
| Message-ID | <20260731-work-b4-editor-branch-guard-v1-0-de68a7c8e4cb@kernel.org> |
Reported from a live "b4 review tui" session. The reply editor sat open across a branch switch made in the same worktree from another terminal, and quitting the editor dumped the reply into /tmp and took the whole TUI down with a RuntimeError. Digging into that turned up four separate problems, one patch each. 1) The branch guard in edit_in_editor() was written for "b4 prep --edit-cover", which stores the edited text in the tracking commit of whatever branch HEAD points at. It lives in the shared helper and reads HEAD unconditionally, so it fires for callers that write to an explicit ref too. The review TUI stores replies with save_tracking_ref() on the review branch and reads the patches by SHA, so HEAD is not involved anywhere in the flow. For those callers the guard only destroys work. It is opt-in now and only the three b4 prep callers take it. 2) The guard read HEAD, and the editor scratch file was created, in whatever tree the process happened to be sitting in rather than the one the edit belongs to. For the review TUI those are the same today, so that patch removes an assumption rather than fixing a bug -- but it is exactly the assumption the rest of that app is written to avoid, and naming the tree moves the core.editor lookup to it as well. edit_in_editor() takes that tree as an argument now. 3) An exception from the editor inside "with app.suspend()" unwinds out of the key handler and tears the app down, losing every other unsaved change in the session. Four of the eight TUI call sites had no handler at all and the rest had grown their own. They share one helper now that notifies and returns None. 4) The review TUI records the branch to restore when it starts and checks it back out when it exits, even when the user is the one who moved HEAD. That can happen from another terminal sharing the worktree. Both restore paths confirm the checkout was b4's own first. The patches before those are follow-up fixes to review-tracker code that went in last round. Outgoing mail is marked read on every send path and not just on review replies, the messages database is closed and configured like the tracking one next to it, archiving a series reports a write failure instead of raising it into the send path that called it, and the publish check for a queued thank-you runs in the repository the commit was applied in rather than in the process cwd. The last three carry the same reasoning onto the review send path, which had been left with the shape the thank-you path just lost. Bookkeeping there gets its own error handler, a tracking write that does not land is reported instead of dropped and the tracking database is closed when archiving fails. Signed-off-by: Christian Brauner (Amutable) <[email protected]> --- Christian Brauner (27): review-tui: mark all outgoing mail as read, not just review replies tests: cover the shared outgoing-seen helper review: close the messages database when auto-marking fails review: use the same busy timeout for both review databases review: drop the unused return value from set_flags_bulk() review: don't let archiving a series raise tests: cover an unwritable series archive review-tui: keep post-send bookkeeping out of the send error path tests: cover the thank-you send's post-send bookkeeping review-tui: say when a take didn't complete tests: cover the unaccepted take in the thank-and-archive chain review-tui: use the shared helper to delete a review branch ty: check reachability in the repository the commit landed in tests: cover the publish check using the repository it is given edit_in_editor: make the branch guard opt-in tests: cover the opt-in branch guard in edit_in_editor edit_in_editor: work in the tree the caller names tests: cover edit_in_editor working in the caller's tree tui: route editor launches through one non-fatal helper tests: cover an editor failure leaving the review TUI standing review-tui: only put back a branch b4 checked out itself tests: cover the review TUI's branch-restore guard tests: pin the default branch in the queue-delivery fixture ty: an unknown remote tip is undetermined, not unpublished review-tui: keep post-send bookkeeping out of the review send error path tests: cover the review send's post-send bookkeeping review: close the tracking database when archiving fails src/b4/__init__.py | 76 ++++++++++++----- src/b4/bugs/_tui.py | 37 +++------ src/b4/ez.py | 14 +++- src/b4/review/_review.py | 80 +++++++++++------- src/b4/review/messages.py | 13 ++- src/b4/review_tui/_common.py | 26 ++++++ src/b4/review_tui/_entry.py | 10 ++- src/b4/review_tui/_lite_app.py | 25 +++--- src/b4/review_tui/_review_app.py | 122 ++++++++++++++++------------ src/b4/review_tui/_tracking_app.py | 79 +++++++++--------- src/b4/tui/__init__.py | 3 + src/b4/tui/_common.py | 27 +++++++ src/b4/ty.py | 48 ++++++++--- src/tests/test___init__.py | 118 +++++++++++++++++++++++++++ src/tests/test_ez.py | 16 +++- src/tests/test_messages.py | 87 ++++++++++---------- src/tests/test_review.py | 21 +++++ src/tests/test_tui_review.py | 162 ++++++++++++++++++++++++++++++++++++- src/tests/test_tui_tracking.py | 85 ++++++++++++++++++- src/tests/test_ty.py | 66 ++++++++++++--- 20 files changed, 850 insertions(+), 265 deletions(-) --- base-commit: af86560d1c2fb7476e2a9d33925a6eaf0292100a change-id: 20260731-work-b4-editor-branch-guard-ab9435cf9a50