fix(tags): let a tag be removed even when its name breaks the rules

validateTagName() ran on the removal list as well as the addition list, so
a tag whose name contains a space could be seen on a message and never
deleted: the one dialog that could clear it refused the only text that
names it, and it did so with a modal warning, so the dialog would not even
close.

Whether a tag SHOULD exist is a separate question from whether the user may
delete one that already does, and the answer to the second is always yes.
Validation now runs on additions only, which is where the rule earns its
keep: it stops a troublesome name being created.

Reached by a real Maildir folder named "Inbox/SlackBuilds users", whose
origin tag carried the space through. Only the TYPED route was ever
blocked; unchecking the tag in the list appends to the removal list after
validation has run and worked throughout. The test asserts both routes for
that reason, and asserts that ADDING a spaced tag is still refused, since
the fix must not weaken the rule it narrows.

The natural mutation for this test hangs rather than fails: restoring the
validation raises a modal warning with nothing to dismiss it. The test
avoids calling accept() on the add-rejection case and asks the validator
directly, and the mutation check drops the tag silently instead.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

1 files changed, 19 insertions, 10 deletions

diff --git a/src/tagdialog.cpp b/src/tagdialog.cpp
index 1fb3f18..fb2c629 100644
--- a/src/tagdialog.cpp
+++ b/src/tagdialog.cpp
@@ -268,16 +268,25 @@ void TagDialog::accept()
     QStringList add = splitTags(m_addEdit->text());
     QStringList remove = splitTags(m_removeEdit->text());
 
-    // Validate before applying anything: a partial change is worse than none,
-    // since the user cannot tell which half landed.
-    for (const QStringList &list : { add, remove }) {
-        for (const QString &tag : list) {
-            const TagNameProblem problem = validateTagName(tag);
-            if (problem != TagNameProblem::Ok) {
-                QMessageBox::warning(this, tr("Invalid tag"),
-                                     tagNameProblemText(problem, tag));
-                return;   // Stay open, with the text still there to fix.
-            }
+    // Validate what is being ADDED. A partial change is worse than none, since
+    // the user cannot tell which half landed, so this runs before anything is
+    // applied.
+    //
+    // REMOVAL is deliberately not validated. The rules here exist to stop a
+    // troublesome tag being CREATED; a tag that already exists is a fact, and
+    // refusing to remove it because it breaks a rule leaves the user with a
+    // tag they can see, cannot type, and cannot get rid of. That happened with
+    // `deleted-from:Inbox/SlackBuilds users`: an origin tag naming a Maildir
+    // folder whose name contains a space, rejected by the space rule, so the
+    // one dialog that could have cleared it refused the only text that names
+    // it. Whether such a tag SHOULD exist is a separate question from whether
+    // the user may delete it, and the answer to the second is always yes.
+    for (const QString &tag : add) {
+        const TagNameProblem problem = validateTagName(tag);
+        if (problem != TagNameProblem::Ok) {
+            QMessageBox::warning(this, tr("Invalid tag"),
+                                 tagNameProblemText(problem, tag));
+            return;   // Stay open, with the text still there to fix.
         }
     }