[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