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 v2 00/44] Stop the editor branch guard from eating review replies
Date: Fri, 31 Jul 2026 23:58:41 +0200 [thread overview]
Message-ID: <20260731-work-b4-editor-branch-guard-v2-0-243fd19d322d@kernel.org> (raw)
Reported from a live "b4 review tui" session: a reply editor left
open across a branch switch dumped the reply into /tmp and took the
TUI down with a RuntimeError. Digging into that turned up four editor
problems, one patch each:
- The branch guard in edit_in_editor() exists for b4 prep, which
writes to the branch HEAD is on, but it fired for callers that
write to an explicit ref. It is opt-in now.
- edit_in_editor() worked in the process cwd rather than the tree the
edit belongs to. Callers name the tree now, "b4 ty -g" included.
- An editor exception inside app.suspend() tore the whole session
down. The call sites share one non-fatal helper.
- The TUI checked its start branch back out even when the user was
the one who moved HEAD. Both restore paths only undo b4's own
checkout.
The patches before those are follow-up fixes to last round's review
tracker work: outgoing mail is marked read on every send path, the
review databases get consistent closing and timeouts, archiving
reports failures instead of raising into the send path, post-send
bookkeeping is kept out of the send error handlers, --email-dry-run
no longer records sends that never happened, a take whose record
could not be written is told apart from one that never ran, and the
queued-thanks publish check runs in the repository the commit landed
in and warns when the remote's tips are entirely absent from it.
The last group puts the worktree back wherever the TUI moved it.
Restores record a position rather than a branch name so a detached
HEAD works too, the tracking loop restores however it ends,
create_review_branch() cleans up on every failure, the SystemExit
that b4 helpers exit with is caught where only Exception was, and
deleting the branch HEAD is on lands the user back where the session
started.
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
Changes in v2:
- Put the worktree back when a revision upgrade moves HEAD onto the
review branch.
- Restore the branch however run_tracking_tui() ends, not only after a
review pass.
- Catch the SystemExit that create_review_branch() and load_tracking()
exit with, in the upgrade error path, the series checkout and the
status sync.
- Clean up after create_review_branch() when the patch range cannot be
read.
- Land HEAD back where the session started when abandoning or archiving
deletes the branch it is on.
- Record the restore point as a position rather than a branch, so every
restore works from a detached HEAD.
- Stop --email-dry-run from recording a send that never happened.
- Tell a take whose patches landed but whose record could not be
written apart from one that never ran.
- Close the tracking database when the status sync fails.
- Edit the "b4 ty -g" review in the tree it was pointed at.
- Trim the commit messages and the cover letter.
- Link to v1: https://patch.msgid.link/20260731-work-b4-editor-branch-guard-v1-0-de68a7c8e4cb@kernel.org
---
Christian Brauner (44):
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 which way a take did not reach 'accepted'
tests: cover the take statuses 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
ty: edit the thank-you review in the tree it was pointed at
tests: cover the ty review editing in the named 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
review-tui: put HEAD back where it was when it was not on a branch
tests: cover the restore from a detached HEAD
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
review: clean up a review branch that cannot be finished
tests: cover create_review_branch cleaning up a half-built branch
review-tui: put the branch back after a revision upgrade
tests: cover the revision upgrade's branch handling
review-tui: restore the original branch however the tracking loop ends
review-tui: do not let a failed tracking load skip the branch restore
tests: cover the tracking TUI restoring the branch on the way out
review-tui: close the tracking database when the status sync fails
tests: cover the status sync closing its database
review-tui: catch the exit a failed checkout reports itself with
tests: cover a failed checkout leaving the tracking list standing
review-tui: put the user back when the branch they are on is deleted
tests: cover the branch delete leaving the worktree on a branch
src/b4/__init__.py | 93 ++++-
src/b4/bugs/_tui.py | 37 +-
src/b4/ez.py | 14 +-
src/b4/review/_review.py | 114 +++---
src/b4/review/messages.py | 13 +-
src/b4/review_tui/_common.py | 26 ++
src/b4/review_tui/_entry.py | 181 +++++----
src/b4/review_tui/_lite_app.py | 25 +-
src/b4/review_tui/_review_app.py | 137 ++++---
src/b4/review_tui/_tracking_app.py | 321 +++++++++------
src/b4/tui/__init__.py | 3 +
src/b4/tui/_common.py | 27 ++
src/b4/ty.py | 52 ++-
src/tests/test___init__.py | 148 +++++++
src/tests/test_ez.py | 16 +-
src/tests/test_messages.py | 87 ++--
src/tests/test_review.py | 113 ++++++
src/tests/test_tui_review.py | 238 ++++++++++-
src/tests/test_tui_tracking.py | 804 ++++++++++++++++++++++++++++++++++++-
src/tests/test_ty.py | 111 ++++-
20 files changed, 2115 insertions(+), 445 deletions(-)
---
base-commit: af86560d1c2fb7476e2a9d33925a6eaf0292100a
change-id: 20260731-work-b4-editor-branch-guard-ab9435cf9a50
next reply other threads:[~2026-07-31 21:58 UTC|newest]
Thread overview: 45+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-31 21:58 Christian Brauner [this message]
2026-07-31 21:58 ` [PATCH b4 v2 01/44] review-tui: mark all outgoing mail as read, not just review replies Christian Brauner
2026-07-31 21:58 ` [PATCH b4 v2 02/44] tests: cover the shared outgoing-seen helper Christian Brauner
2026-07-31 21:58 ` [PATCH b4 v2 03/44] review: close the messages database when auto-marking fails Christian Brauner
2026-07-31 21:58 ` [PATCH b4 v2 04/44] review: use the same busy timeout for both review databases Christian Brauner
2026-07-31 21:58 ` [PATCH b4 v2 05/44] review: drop the unused return value from set_flags_bulk() Christian Brauner
2026-07-31 21:58 ` [PATCH b4 v2 06/44] review: don't let archiving a series raise Christian Brauner
2026-07-31 21:58 ` [PATCH b4 v2 07/44] tests: cover an unwritable series archive Christian Brauner
2026-07-31 21:58 ` [PATCH b4 v2 08/44] review-tui: keep post-send bookkeeping out of the send error path Christian Brauner
2026-07-31 21:58 ` [PATCH b4 v2 09/44] tests: cover the thank-you send's post-send bookkeeping Christian Brauner
2026-07-31 21:58 ` [PATCH b4 v2 10/44] review-tui: say which way a take did not reach 'accepted' Christian Brauner
2026-07-31 21:58 ` [PATCH b4 v2 11/44] tests: cover the take statuses in the thank-and-archive chain Christian Brauner
2026-07-31 21:58 ` [PATCH b4 v2 12/44] review-tui: use the shared helper to delete a review branch Christian Brauner
2026-07-31 21:58 ` [PATCH b4 v2 13/44] ty: check reachability in the repository the commit landed in Christian Brauner
2026-07-31 21:58 ` [PATCH b4 v2 14/44] tests: cover the publish check using the repository it is given Christian Brauner
2026-07-31 21:58 ` [PATCH b4 v2 15/44] edit_in_editor: make the branch guard opt-in Christian Brauner
2026-07-31 21:58 ` [PATCH b4 v2 16/44] tests: cover the opt-in branch guard in edit_in_editor Christian Brauner
2026-07-31 21:58 ` [PATCH b4 v2 17/44] edit_in_editor: work in the tree the caller names Christian Brauner
2026-07-31 21:58 ` [PATCH b4 v2 18/44] tests: cover edit_in_editor working in the caller's tree Christian Brauner
2026-07-31 21:59 ` [PATCH b4 v2 19/44] ty: edit the thank-you review in the tree it was pointed at Christian Brauner
2026-07-31 21:59 ` [PATCH b4 v2 20/44] tests: cover the ty review editing in the named tree Christian Brauner
2026-07-31 21:59 ` [PATCH b4 v2 21/44] tui: route editor launches through one non-fatal helper Christian Brauner
2026-07-31 21:59 ` [PATCH b4 v2 22/44] tests: cover an editor failure leaving the review TUI standing Christian Brauner
2026-07-31 21:59 ` [PATCH b4 v2 23/44] review-tui: only put back a branch b4 checked out itself Christian Brauner
2026-07-31 21:59 ` [PATCH b4 v2 24/44] tests: cover the review TUI's branch-restore guard Christian Brauner
2026-07-31 21:59 ` [PATCH b4 v2 25/44] review-tui: put HEAD back where it was when it was not on a branch Christian Brauner
2026-07-31 21:59 ` [PATCH b4 v2 26/44] tests: cover the restore from a detached HEAD Christian Brauner
2026-07-31 21:59 ` [PATCH b4 v2 27/44] tests: pin the default branch in the queue-delivery fixture Christian Brauner
2026-07-31 21:59 ` [PATCH b4 v2 28/44] ty: an unknown remote tip is undetermined, not unpublished Christian Brauner
2026-07-31 21:59 ` [PATCH b4 v2 29/44] review-tui: keep post-send bookkeeping out of the review send error path Christian Brauner
2026-07-31 21:59 ` [PATCH b4 v2 30/44] tests: cover the review send's post-send bookkeeping Christian Brauner
2026-07-31 21:59 ` [PATCH b4 v2 31/44] review: close the tracking database when archiving fails Christian Brauner
2026-07-31 21:59 ` [PATCH b4 v2 32/44] review: clean up a review branch that cannot be finished Christian Brauner
2026-07-31 21:59 ` [PATCH b4 v2 33/44] tests: cover create_review_branch cleaning up a half-built branch Christian Brauner
2026-07-31 21:59 ` [PATCH b4 v2 34/44] review-tui: put the branch back after a revision upgrade Christian Brauner
2026-07-31 21:59 ` [PATCH b4 v2 35/44] tests: cover the revision upgrade's branch handling Christian Brauner
2026-07-31 21:59 ` [PATCH b4 v2 36/44] review-tui: restore the original branch however the tracking loop ends Christian Brauner
2026-07-31 21:59 ` [PATCH b4 v2 37/44] review-tui: do not let a failed tracking load skip the branch restore Christian Brauner
2026-07-31 21:59 ` [PATCH b4 v2 38/44] tests: cover the tracking TUI restoring the branch on the way out Christian Brauner
2026-07-31 21:59 ` [PATCH b4 v2 39/44] review-tui: close the tracking database when the status sync fails Christian Brauner
2026-07-31 21:59 ` [PATCH b4 v2 40/44] tests: cover the status sync closing its database Christian Brauner
2026-07-31 21:59 ` [PATCH b4 v2 41/44] review-tui: catch the exit a failed checkout reports itself with Christian Brauner
2026-07-31 21:59 ` [PATCH b4 v2 42/44] tests: cover a failed checkout leaving the tracking list standing Christian Brauner
2026-07-31 21:59 ` [PATCH b4 v2 43/44] review-tui: put the user back when the branch they are on is deleted Christian Brauner
2026-07-31 21:59 ` [PATCH b4 v2 44/44] tests: cover the branch delete leaving the worktree on a branch 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-v2-0-243fd19d322d@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