fix(ui): make Important a toggle, like Delete and Toggle unread

The `flag` action only ever added the `flagged` tag, so pressing Ctrl+I on a
thread or message that was already important re-applied a tag it already had.
Re-applying a tag changes nothing and repaints nothing, so the key read as
dead, and removing `flagged` meant opening the tag dialog.

It now reads the current state and picks a direction, exactly as `delete` and
`toggle_unread` beside it do. One direction is chosen for the whole selection:
it unmarks only when every selected row is already important, so a single
keystroke cannot leave a selection in two states.

The direction comes from everySelectedRowHasTag(), never a hand-rolled loop.
Two separate bugs went into that helper on 2026-08-16 (items 88 and 105), and
a copy of the then-current `delete` loop would have inherited both: resolving
a reply's row number against the top-level list, and asking a reply's THREAD
where the write is message-scoped, which makes a toggle one-way.

The reply test needs THREE different states to mean anything: the first thread
in the list unflagged, the reply's own thread flagged, and the reply itself
unflagged. With the reply left in its thread's state, the mutation putting
item 105's bug back stayed green, measured. The fixture helper defaults
replyTags to the thread's, so a test that does not pass them explicitly
asserts nothing about scope.

Backlog item 98.

1 files changed, 129 insertions, 0 deletions

diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp
index 07cc56b..b188ef4 100644
--- a/tests/test_mainwindow.cpp
+++ b/tests/test_mainwindow.cpp
@@ -276,6 +276,9 @@ private slots:
     void deleteOnAMixedSelectionDeletesRatherThanSplittingIt();
     void deleteOnAReplyReadsItsOwnThreadNotTheFirstInTheList();
     void toggleUnreadOnAReplyReadsItsOwnThreadNotTheFirstInTheList();
+    void importantOnAnAlreadyImportantThreadRemovesTheTag();
+    void importantOnAPlainThreadStillAddsTheTag();
+    void importantOnAReplyReadsItsOwnStateNotItsThreads();
     void editTagsOnAReplyCountsItsOwnThreadNotTheFirstInTheList();
     void markCurrentThreadReadResolvesTheThreadThroughTheIndex();
     void deletingAReplyRepaintsThatReplyRow();
@@ -4798,6 +4801,132 @@ void TestMainWindow::toggleUnreadOnAReplyReadsItsOwnThreadNotTheFirstInTheList()
                             .arg(window.undoTextForTesting())));
 }
 
+void TestMainWindow::importantOnAnAlreadyImportantThreadRemovesTheTag()
+{
+    // Item 98. `flag` was a one-way add, so pressing it on a thread that is
+    // already important re-sent a tag the thread had: a no-op write, and a
+    // no-op repaints nothing, so the key read as dead. Its two neighbours,
+    // Delete and Toggle unread, had been toggles for a long time.
+    const Config config;
+    MainWindow window(config);
+
+    auto *model = window.findChild<ThreadListModel *>();
+    QVERIFY(model);
+    auto *view = window.findChild<QTreeView *>();
+    QVERIFY(view);
+    auto *action = window.findChild<QAction *>(QStringLiteral("flag"));
+    QVERIFY(action);
+
+    model->appendBatch({ makeThread(QStringLiteral("t1"),
+                                    QStringList{ QStringLiteral("inbox"),
+                                                 QStringLiteral("flagged") }) });
+    QApplication::processEvents();
+
+    view->setCurrentIndex(model->index(0, 0));
+    view->selectionModel()->select(model->index(0, 0),
+                                   QItemSelectionModel::ClearAndSelect
+                                       | QItemSelectionModel::Rows);
+    QApplication::processEvents();
+
+    action->trigger();
+
+    QCOMPARE(window.undoDepthForTesting(), 1);
+    QVERIFY2(window.undoTextForTesting().contains(
+                 QStringLiteral("Unmark important")),
+             qPrintable(QStringLiteral(
+                            "Important on an already-important thread did not "
+                            "remove the tag: %1. A one-way add is a no-op the "
+                            "user cannot see.")
+                            .arg(window.undoTextForTesting())));
+}
+
+void TestMainWindow::importantOnAPlainThreadStillAddsTheTag()
+{
+    // The other direction, so a mutation inverting the test above cannot pass.
+    const Config config;
+    MainWindow window(config);
+
+    auto *model = window.findChild<ThreadListModel *>();
+    QVERIFY(model);
+    auto *view = window.findChild<QTreeView *>();
+    QVERIFY(view);
+    auto *action = window.findChild<QAction *>(QStringLiteral("flag"));
+    QVERIFY(action);
+
+    model->appendBatch({ makeThread(QStringLiteral("t1"),
+                                    QStringList{ QStringLiteral("inbox") }) });
+    QApplication::processEvents();
+
+    view->setCurrentIndex(model->index(0, 0));
+    view->selectionModel()->select(model->index(0, 0),
+                                   QItemSelectionModel::ClearAndSelect
+                                       | QItemSelectionModel::Rows);
+    QApplication::processEvents();
+
+    action->trigger();
+
+    QCOMPARE(window.undoDepthForTesting(), 1);
+    QVERIFY2(window.undoTextForTesting().contains(
+                 QStringLiteral("Mark important"))
+                 && !window.undoTextForTesting().contains(
+                     QStringLiteral("Unmark")),
+             qPrintable(QStringLiteral(
+                            "Important on a plain thread did not add the tag: "
+                            "%1")
+                            .arg(window.undoTextForTesting())));
+}
+
+void TestMainWindow::importantOnAReplyReadsItsOwnStateNotItsThreads()
+{
+    // The trap items 88 and 105 each fixed once, which is why item 98 says to
+    // call everySelectedRowHasTag() rather than copy the then-current Delete
+    // loop.
+    //
+    // THREE states, all different, which is what the test needs to distinguish
+    // the two wrong answers from the right one. t1 (the first thread in the
+    // list) is unflagged, t2 (the reply's own thread) is flagged, and the
+    // REPLY is unflagged. Reading t1 by row number answers "not flagged" and
+    // reading the reply's THREAD answers "flagged", so only a read of the
+    // message itself gives "not flagged" for the right reason.
+    //
+    // Leaving the reply's tags defaulted to its thread's is the trap: a reply
+    // in the same state as its thread answers identically whichever of the two
+    // the code reads, and the mutation putting item 105's bug back stays green.
+    // Measured: it did.
+    const Config config;
+    MainWindow window(config);
+
+    auto *model = window.findChild<ThreadListModel *>();
+    QVERIFY(model);
+    auto *view = window.findChild<QTreeView *>();
+    QVERIFY(view);
+    auto *action = window.findChild<QAction *>(QStringLiteral("flag"));
+    QVERIFY(action);
+
+    const QModelIndex reply = expandSecondThreadAndSelectItsReply(
+        view, model, {}, { QStringLiteral("flagged") },
+        QStringList{});
+    QVERIFY2(reply.isValid(),
+             "the fixture did not produce a reply row at row 0, so this test "
+             "would assert nothing");
+
+    action->trigger();
+
+    QVERIFY2(window.pendingMessageIdsForTesting().contains(
+                 QStringLiteral("m1@example.org")),
+             "Important on a reply did not act on that reply");
+    QCOMPARE(window.undoDepthForTesting(), 1);
+    QVERIFY2(window.undoTextForTesting().contains(
+                 QStringLiteral("Mark important"))
+                 && !window.undoTextForTesting().contains(
+                     QStringLiteral("Unmark")),
+             qPrintable(QStringLiteral(
+                            "Important on an unflagged reply chose the wrong "
+                            "direction: %1. Its own THREAD is flagged, so "
+                            "reading the thread gives Unmark.")
+                            .arg(window.undoTextForTesting())));
+}
+
 void TestMainWindow::editTagsOnAReplyCountsItsOwnThreadNotTheFirstInTheList()
 {
     // The tag dialog is modal, so what is tested is the count it is BUILT