From: Christian Brauner <brauner@kernel.org>
To: "Kernel.org Tools" <tools@kernel.org>
Cc: "Christian Brauner (Amutable)" <brauner@kernel.org>,
Konstantin Ryabitsev <konstantin@linuxfoundation.org>
Subject: [PATCH b4 00/27] Stop the editor branch guard from eating review replies
Date: Fri, 31 Jul 2026 11:20:59 +0200 [thread overview]
Message-ID: <20260731-work-b4-editor-branch-guard-v1-0-de68a7c8e4cb@kernel.org> (raw)
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) <brauner@kernel.org>
---
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
next reply other threads:[~2026-07-31 9:21 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-31 9:20 Christian Brauner [this message]
2026-07-31 9:21 ` [PATCH b4 01/27] review-tui: mark all outgoing mail as read, not just review replies Christian Brauner
2026-07-31 9:21 ` [PATCH b4 02/27] tests: cover the shared outgoing-seen helper Christian Brauner
2026-07-31 9:21 ` [PATCH b4 03/27] review: close the messages database when auto-marking fails Christian Brauner
2026-07-31 9:21 ` [PATCH b4 04/27] review: use the same busy timeout for both review databases Christian Brauner
2026-07-31 9:21 ` [PATCH b4 05/27] review: drop the unused return value from set_flags_bulk() Christian Brauner
2026-07-31 9:21 ` [PATCH b4 06/27] review: don't let archiving a series raise Christian Brauner
2026-07-31 9:21 ` [PATCH b4 07/27] tests: cover an unwritable series archive Christian Brauner
2026-07-31 9:21 ` [PATCH b4 08/27] review-tui: keep post-send bookkeeping out of the send error path Christian Brauner
2026-07-31 9:21 ` [PATCH b4 09/27] tests: cover the thank-you send's post-send bookkeeping Christian Brauner
2026-07-31 9:21 ` [PATCH b4 10/27] review-tui: say when a take didn't complete Christian Brauner
2026-07-31 9:21 ` [PATCH b4 11/27] tests: cover the unaccepted take in the thank-and-archive chain Christian Brauner
2026-07-31 9:21 ` [PATCH b4 12/27] review-tui: use the shared helper to delete a review branch Christian Brauner
2026-07-31 9:21 ` [PATCH b4 13/27] ty: check reachability in the repository the commit landed in Christian Brauner
2026-07-31 9:21 ` [PATCH b4 14/27] tests: cover the publish check using the repository it is given Christian Brauner
2026-07-31 9:21 ` [PATCH b4 15/27] edit_in_editor: make the branch guard opt-in Christian Brauner
2026-07-31 9:21 ` [PATCH b4 16/27] tests: cover the opt-in branch guard in edit_in_editor Christian Brauner
2026-07-31 9:21 ` [PATCH b4 17/27] edit_in_editor: work in the tree the caller names Christian Brauner
2026-07-31 9:21 ` [PATCH b4 18/27] tests: cover edit_in_editor working in the caller's tree Christian Brauner
2026-07-31 9:21 ` [PATCH b4 19/27] tui: route editor launches through one non-fatal helper Christian Brauner
2026-07-31 9:21 ` [PATCH b4 20/27] tests: cover an editor failure leaving the review TUI standing Christian Brauner
2026-07-31 9:21 ` [PATCH b4 21/27] review-tui: only put back a branch b4 checked out itself Christian Brauner
2026-07-31 9:21 ` [PATCH b4 22/27] tests: cover the review TUI's branch-restore guard Christian Brauner
2026-07-31 9:21 ` [PATCH b4 23/27] tests: pin the default branch in the queue-delivery fixture Christian Brauner
2026-07-31 9:21 ` [PATCH b4 24/27] ty: an unknown remote tip is undetermined, not unpublished Christian Brauner
2026-07-31 9:21 ` [PATCH b4 25/27] review-tui: keep post-send bookkeeping out of the review send error path Christian Brauner
2026-07-31 9:21 ` [PATCH b4 26/27] tests: cover the review send's post-send bookkeeping Christian Brauner
2026-07-31 9:21 ` [PATCH b4 27/27] review: close the tracking database when archiving fails Christian Brauner
2026-07-31 13:14 ` [PATCH b4 00/27] Stop the editor branch guard from eating review replies Christian Brauner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260731-work-b4-editor-branch-guard-v1-0-de68a7c8e4cb@kernel.org \
--to=brauner@kernel.org \
--cc=konstantin@linuxfoundation.org \
--cc=tools@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox