diff options
Diffstat (limited to 'assets/hooks')
| -rwxr-xr-x | assets/hooks/post-new | 68 | ||||
| -rwxr-xr-x | assets/hooks/qtmaildirconf.py | 35 | ||||
| -rwxr-xr-x | assets/hooks/test_post_new.py | 93 | ||||
| -rwxr-xr-x | assets/hooks/test_qtmaildirconf.py | 70 |
4 files changed, 193 insertions, 73 deletions
diff --git a/assets/hooks/post-new b/assets/hooks/post-new index fca31a1..cad566c 100755 --- a/assets/hooks/post-new +++ b/assets/hooks/post-new @@ -78,19 +78,19 @@ def _quote(value): return '"' + value.replace("\\", "\\\\").replace('"', '\\"') + '"' -def sent_only(query, folders): +def not_arrival_only(query, folders): """The ids matching `query` whose files are ALL inside `folders`. None on failure, so the caller leaves `tag:new` in place rather than stripping on a half-read answer. - **Every file has to be in a sent folder, not merely one of them.** notmuch - deduplicates by Message-ID, so mail the user sends to another of their own - accounts is ONE message with two files: the sender's Sent copy and the - recipient's Inbox copy. The old query matched on a path and the tag applied - to the message, so matching the Sent copy stripped `inbox` from the copy - that had genuinely arrived, and the mail was missing from the account that - received it (item 166). + **Every file has to be in a non-arrival folder, not merely one of them.** + notmuch deduplicates by Message-ID, so mail the user sends to another of + their own accounts is ONE message with two files: the sender's Sent copy + and the recipient's Inbox copy. The old query matched on a path and the tag + applied to the message, so matching the Sent copy stripped `inbox` from the + copy that had genuinely arrived, and the mail was missing from the account + that received it (item 166). This cannot be expressed as a query, which is why it is a loop. Measured against a two-file message: `not path:"Inbox/**"` does NOT exclude it, and @@ -101,7 +101,7 @@ def sent_only(query, folders): """ root = mail_root() if root is None: - log("no mail root; leaving sent mail alone") + log("no mail root; leaving non-arrival mail alone") return None prefixes = [root / folder for folder in folders] @@ -110,10 +110,11 @@ def sent_only(query, folders): if matched is None: return None - # ponytail: one `notmuch search` per matched message. N is the sent mail - # in tag:new, so an ordinary sync is a handful and a first-run reindex is - # the whole corpus. Batch by parsing --format=json once if that ever - # matters; it does not at this size, and the loop is the readable form. + # ponytail: one `notmuch search` per matched message. N is the + # non-arrival mail in tag:new, so an ordinary sync is a handful and a + # first-run reindex is the whole corpus. Batch by parsing --format=json + # once if that ever matters; it does not at this size, and the loop is the + # readable form. ids = [] for message_id in matched: paths = search(f"id:{_quote(message_id)}", "files") @@ -176,20 +177,21 @@ def search(query, output): return values -def strip_inbox_from_sent(run): - """Take `inbox` off mail the user SENT, and nothing else. +def strip_inbox_from_non_arrivals(run): + """Take `inbox` off mail that did not ARRIVE, and nothing else. `notmuch new` applies new.tags to every file it indexes, and it cannot - tell an arrival from the copy qtmaildir files into a sent folder after a - send. The result is sent mail carrying `inbox`, which puts it in an inbox - view it never arrived in and in any hand-typed `tag:inbox` search. + tell an arrival from a copy this system filed itself: sent mail, a saved + draft, or mail a provider's filter dropped into a spam folder. The result + is such mail carrying `inbox`, which puts it in an inbox view it never + arrived in and in any hand-typed `tag:inbox` search. This is NOT a relaxation of PROTECTED_REMOVALS below, and the difference is the whole reason it can run unattended. That guard is about a RULE removing `inbox` from mail whose provenance the hook cannot judge. Here - the provenance is the file's own path: a message inside a configured sent - folder is one this system sent, and `inbox` was never true of it. Nothing - the user could act on is being hidden. + the provenance is the file's own path: a message whose files are ALL + inside a configured sent, drafts or spam folder never arrived, and `inbox` + was never true of it. Nothing the user could act on is being hidden. Only `inbox`. `unread` is untouched, because maildir.synchronize_flags is true and removing it rewrites Maildir filenames, which reaches the server @@ -198,17 +200,17 @@ def strip_inbox_from_sent(run): Scoped to tag:new like every rule, so a sync never rewrites tags across the whole corpus. Mail already indexed keeps whatever it has. """ - folders = qtmaildirconf.sent_folders() + folders = qtmaildirconf.not_arrival_folders() if not folders: - # No config, or no account keeping sent mail locally. Nothing to - # protect, and this must NOT fall through to an empty query: notmuch - # reads that as "match everything", which would strip `inbox` from - # every newly indexed message on the system. + # No config, or no account keeping non-arrival folders locally. + # Nothing to protect, and this must NOT fall through to an empty + # query: notmuch reads that as "match everything", which would strip + # `inbox` from every newly indexed message on the system. return True - query = f"{SCOPE} and ({qtmaildirconf.sent_query(folders)})" + query = f"{SCOPE} and ({qtmaildirconf.not_arrival_query(folders)})" - ids = sent_only(query, folders) + ids = not_arrival_only(query, folders) if ids is None: return False @@ -220,7 +222,7 @@ def strip_inbox_from_sent(run): # "applied" whether it stripped four messages or none, and item 164 is # exactly the case where that distinction is the whole question: a draft # that kept `inbox` on a pass whose log claimed the carve-out had run. - log(f"sent-folder carve-out applied over {len(folders)} folder(s), " + log(f"non-arrival carve-out applied over {len(folders)} folder(s), " f"{len(ids)} message(s)") return True @@ -296,11 +298,11 @@ def main(): applied += 1 # AFTER the rules and BEFORE the marker is consumed. After, so a rule can - # still see its own sent mail with `inbox` on it and match the way it - # always did; before, because the marker is what scopes this to newly + # still see its own non-arrival mail with `inbox` on it and match the way + # it always did; before, because the marker is what scopes this to newly # indexed mail and consuming it first would leave nothing to match. - if not strip_inbox_from_sent(run_tag): - log("sent-folder carve-out failed; leaving tag:new in place") + if not strip_inbox_from_non_arrivals(run_tag): + log("non-arrival carve-out failed; leaving tag:new in place") return 1 # Only after every rule succeeded. A failure part way through leaves the diff --git a/assets/hooks/qtmaildirconf.py b/assets/hooks/qtmaildirconf.py index e709a28..882c94a 100755 --- a/assets/hooks/qtmaildirconf.py +++ b/assets/hooks/qtmaildirconf.py @@ -16,9 +16,9 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. """Reads the account layout out of qtmaildir.conf, for the post-new hook. -Only the sent folders are read, and only so the hook can tell mail the user -SENT from mail that arrived. Everything else in that file belongs to the -application. +Only the non-arrival folders are read, and only so the hook can tell mail +this system filed itself (sent, drafts, spam) from mail that genuinely +arrived. Everything else in that file belongs to the application. Stdlib only: this is imported by a notmuch hook that runs on every sync. @@ -52,7 +52,7 @@ def _accounts(path): A file that will not parse yields NO accounts rather than raising. The caller is a hook running after `notmuch new` has already indexed the mail: failing the sync over a malformed application config is worse than - not protecting sent mail for one cycle, and the hook logs the miss. + not protecting non-arrival mail for one cycle, and the hook logs the miss. """ parser = configparser.ConfigParser( # QSettings writes `;` comments, and `#` appears inside values (a @@ -77,20 +77,27 @@ def _accounts(path): # Folders mail does not ARRIVE in: this system put the message there itself. +# notmuch's `new.tags` applies `inbox` to every file it indexes, so a file in +# any of these folders would otherwise appear in the tag:inbox Inbox view. # -# Trash is deliberately absent. qtmaildir's own Delete leaves `inbox` on a -# trashed message so Restore can put it back where it came from, and stripping -# it here would fight that. -NOT_ARRIVALS = ("sent", "drafts") - - -def sent_folders(path=None): +# Trash is deliberately absent. This is scope, not a claim it can never +# happen: measured 0 trashed files carried `inbox`, and the reported defect is +# spam (item 202). qtmaildir's own Delete no longer leaves `inbox` on a +# trashed message either: it strips `unread` and `inbox` (item 168), and +# Restore re-adds `inbox` from the `moved-from:` origin tag once the message +# returns to its origin folder. A file moved into the trash folder by another +# client could still pick up `inbox` from `new.tags`; that is unmeasured and +# out of scope for now, so it is not in this list. +NOT_ARRIVALS = ("sent", "drafts", "spam") + + +def not_arrival_folders(path=None): """Every folder mail does not arrive in, relative to the mail root. An account contributes nothing unless it names a maildir: a bare `Sent` would match every account's folder of that name at once. Each of the keys in NOT_ARRIVALS is optional on its own, since an account may keep no sent - mail or no drafts locally. + mail, no drafts or no spam folder locally. """ if path is None: path = default_path() @@ -107,12 +114,12 @@ def sent_folders(path=None): return folders -def sent_query(folders): +def not_arrival_query(folders): """A notmuch query matching everything inside the given folders. Empty for an empty list, and the caller MUST check: an empty query means "match everything" to notmuch, so handing this straight to a tag command - would treat the whole corpus as sent mail. + would treat the whole corpus as non-arrival mail. `path:` is hierarchical, so `<folder>/**` covers `cur/` and `new/` and any nesting a provider invents underneath. diff --git a/assets/hooks/test_post_new.py b/assets/hooks/test_post_new.py index 07728a6..074bf24 100755 --- a/assets/hooks/test_post_new.py +++ b/assets/hooks/test_post_new.py @@ -212,7 +212,7 @@ def test_a_protected_removal_is_skipped_whole_and_the_run_continues(): assert count(env, "tag:new") == 0 -def setup_accounts(tmp, sent_config=True, split_index=False): +def setup_accounts(tmp, sent_config=True, split_index=False, with_spam=False): """A maildir laid out as qtmaildir configures it: two accounts, each with an Inbox and a Sent folder, one message in each. @@ -220,6 +220,9 @@ def setup_accounts(tmp, sent_config=True, split_index=False): thing that cares where a file sits. The folder names are the awkward ones deliberately: a bracketed, spaced provider folder is what the real config carries, and a flat `Sent` is what the other half carries. + + `with_spam` adds each account's spam folder and its `spam` key, for the + test that a non-arrival folder added later is picked up the same way. """ root = Path(tmp) / "Mail" folders = { @@ -231,6 +234,10 @@ def setup_accounts(tmp, sent_config=True, split_index=False): "acct-two/Inbox", "acct-two/[Provider]/Posta inviata"): for part in ("new", "cur", "tmp"): (root / sub / part).mkdir(parents=True) + if with_spam: + for sub in ("acct-one/Spam", "acct-two/[Provider]/Spam"): + for part in ("new", "cur", "tmp"): + (root / sub / part).mkdir(parents=True) make_message(root / "acct-one/Inbox", "arrived-one", "friend@example.org", "an arrival") @@ -269,8 +276,10 @@ def setup_accounts(tmp, sent_config=True, split_index=False): conf.parent.mkdir(parents=True, exist_ok=True) conf.write_text( "[account.one]\nmaildir = acct-one\nsent = Sent\n" - "[account.two]\nmaildir = acct-two\n" - "sent = [Provider]/Posta inviata\n") + + ("spam = Spam\n" if with_spam else "") + + "[account.two]\nmaildir = acct-two\n" + "sent = [Provider]/Posta inviata\n" + + ("spam = [Provider]/Spam\n" if with_spam else "")) subprocess.run(["notmuch", "new"], env=env, capture_output=True, check=True) @@ -346,6 +355,84 @@ def test_a_message_that_was_sent_AND_received_keeps_inbox(): assert count(env, "tag:inbox and id:sent-one@example.org") == 0 +def test_spam_mail_does_not_keep_the_inbox_tag(): + """Spam is not an arrival either. A provider's filter files it into a spam + folder, notmuch's new.tags still applies `inbox`, and without the carve-out + it shows up in the Inbox view (item 202).""" + with tempfile.TemporaryDirectory() as tmp: + env = setup_accounts(tmp, with_spam=True) + root = Path(tmp) / "Mail" + make_message(root / "acct-one/Spam", "spam-one", + "spammer@example.net", "cheap stuff") + subprocess.run(["notmuch", "new"], env=env, capture_output=True, + check=True) + + write_rules(env, []) + assert count(env, 'tag:inbox and path:"acct-one/Spam/**"') == 1 + + result = subprocess.run([str(HOOK)], env=env, capture_output=True, + text=True) + assert result.returncode == 0, result.stderr + assert count(env, 'tag:inbox and path:"acct-one/Spam/**"') == 0 + # The arrival is untouched, which fails if the spam prefix is wrong. + assert count(env, 'tag:inbox and path:"acct-one/Inbox/**"') == 1 + + +def test_a_message_that_is_spam_AND_arrived_keeps_inbox(): + """The all-files rule, for spam. notmuch deduplicates by Message-ID, so a + message with one file in a spam folder and one in an inbox DID arrive and + must keep `inbox` (item 202, same constraint as item 166 for sent).""" + with tempfile.TemporaryDirectory() as tmp: + env = setup_accounts(tmp, with_spam=True) + root = Path(tmp) / "Mail" + make_message(root / "acct-one/Spam", "spam-and-inbox", + "spammer@example.net", "came twice") + make_message(root / "acct-one/Inbox", "spam-and-inbox", + "spammer@example.net", "came twice") + subprocess.run(["notmuch", "new"], env=env, capture_output=True, + check=True) + assert count(env, "id:spam-and-inbox@example.org") == 1 + assert files(env, "id:spam-and-inbox@example.org") == 2 + + write_rules(env, []) + result = subprocess.run([str(HOOK)], env=env, capture_output=True, + text=True) + assert result.returncode == 0, result.stderr + assert count(env, "tag:inbox and id:spam-and-inbox@example.org") == 1 + + +def test_an_account_without_a_spam_key_contributes_nothing(): + """`spam` is per account. One account names its spam folder; the other + does not. Mail in the unconfigured account's Spam folder is an arrival by + the only rule the hook has and keeps `inbox`, which is the empty-path + guard: a missing key must not fall back to the account maildir, matching + everything under it.""" + with tempfile.TemporaryDirectory() as tmp: + env = setup_accounts(tmp, with_spam=True) + root = Path(tmp) / "Mail" + make_message(root / "acct-one/Spam", "one-spam", + "spammer@example.net", "spam for one") + make_message(root / "acct-two/[Provider]/Spam", "two-spam", + "spammer@example.net", "spam for two") + subprocess.run(["notmuch", "new"], env=env, capture_output=True, + check=True) + + # Rewrite the config so only account one carries a spam key. + conf = Path(env["XDG_CONFIG_HOME"]) / "qtmaildir" / "qtmaildir.conf" + conf.write_text( + "[account.one]\nmaildir = acct-one\nsent = Sent\nspam = Spam\n" + "[account.two]\nmaildir = acct-two\n" + "sent = [Provider]/Posta inviata\n") + + write_rules(env, []) + result = subprocess.run([str(HOOK)], env=env, capture_output=True, + text=True) + assert result.returncode == 0, result.stderr + assert count(env, 'tag:inbox and path:"acct-one/Spam/**"') == 0 + assert count( + env, 'tag:inbox and path:"acct-two/[Provider]/Spam/**"') == 1 + + def test_the_carve_out_works_with_the_index_split_from_the_mail(): """notmuch can hold the Xapian index outside the Maildir, and this user's does. `database.path` is then the index directory, so a carve-out reading diff --git a/assets/hooks/test_qtmaildirconf.py b/assets/hooks/test_qtmaildirconf.py index c8aa78d..89565d9 100755 --- a/assets/hooks/test_qtmaildirconf.py +++ b/assets/hooks/test_qtmaildirconf.py @@ -15,7 +15,7 @@ # along with this program; if not, write to the Free Software # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. """Unit checks for the qtmaildir.conf reader the post-new hook uses to find -the sent folders. +the non-arrival folders. The file is written by QSettings, not by configparser, so the cases that matter are the ones where the two disagree: a section name carrying a dot, a @@ -37,37 +37,51 @@ def write_config(tmp, text): return path -def test_sent_folders_are_read_per_account(): +def test_not_arrival_folders_are_read_per_account(): with tempfile.TemporaryDirectory() as tmp: path = write_config(tmp, "[account.work]\n" "maildir = work\n" "sent = Sent\n" "trash = Trash\n") - assert qtmaildirconf.sent_folders(path) == ["work/Sent"] + assert qtmaildirconf.not_arrival_folders(path) == ["work/Sent"] -def test_drafts_are_excluded_alongside_sent(): - """A draft never arrived either, so it must not carry `inbox`. Both keys - feed one list: the hook asks a single question, "is this a folder mail - arrives in", and sent and drafts answer it the same way. +def test_drafts_and_spam_are_excluded_alongside_sent(): + """Neither a draft nor spam ever arrived, so neither must carry `inbox`. + All three keys feed one list: the hook asks a single question, "is this a + folder mail arrives in", and sent, drafts and spam answer it the same way. - Trash is deliberately NOT here. qtmaildir's own Delete leaves `inbox` on - a trashed message so Restore can put it back where it came from, and - stripping it here would fight that. + Trash is deliberately NOT here: the reported defect is spam and 0 trashed + files carried `inbox` (item 202), and qtmaildir's own Delete now strips + `unread` and `inbox` (item 168), so its own moves do not leave the tag + behind. """ with tempfile.TemporaryDirectory() as tmp: path = write_config(tmp, "[account.work]\n" "maildir = work\n" "sent = Sent\n" "drafts = Drafts\n" + "spam = Spam\n" "trash = Trash\n") - assert qtmaildirconf.sent_folders(path) == ["work/Sent", "work/Drafts"] + assert qtmaildirconf.not_arrival_folders(path) == [ + "work/Sent", "work/Drafts", "work/Spam"] + + +def test_spam_may_be_the_only_non_arrival_folder(): + """An account need not keep sent mail or drafts locally, and Gmail's + spam folder is one the reader must still pick up on its own.""" + with tempfile.TemporaryDirectory() as tmp: + path = write_config(tmp, "[account.g]\n" + "maildir = gmail\n" + "spam = [Gmail]/Spam\n") + assert qtmaildirconf.not_arrival_folders(path) == [ + "gmail/[Gmail]/Spam"] def test_an_account_with_only_drafts_still_contributes(): with tempfile.TemporaryDirectory() as tmp: path = write_config(tmp, "[account.a]\nmaildir = a\ndrafts = Drafts\n") - assert qtmaildirconf.sent_folders(path) == ["a/Drafts"] + assert qtmaildirconf.not_arrival_folders(path) == ["a/Drafts"] def test_an_account_section_may_carry_a_dot(): @@ -78,7 +92,7 @@ def test_an_account_section_may_carry_a_dot(): path = write_config(tmp, "[account.provider.name]\n" "maildir = provider-name\n" "sent = Sent\n") - assert qtmaildirconf.sent_folders(path) == ["provider-name/Sent"] + assert qtmaildirconf.not_arrival_folders(path) == ["provider-name/Sent"] def test_a_folder_may_contain_spaces_and_brackets(): @@ -88,7 +102,7 @@ def test_a_folder_may_contain_spaces_and_brackets(): path = write_config(tmp, "[account.g]\n" "maildir = gmail\n" "sent = [Gmail]/Posta inviata\n") - assert qtmaildirconf.sent_folders(path) == [ + assert qtmaildirconf.not_arrival_folders(path) == [ "gmail/[Gmail]/Posta inviata"] @@ -99,13 +113,23 @@ def test_an_account_without_a_sent_key_contributes_nothing(): with tempfile.TemporaryDirectory() as tmp: path = write_config(tmp, "[account.a]\nmaildir = a\ntrash = Trash\n" "[account.b]\nmaildir = b\nsent = Sent\n") - assert qtmaildirconf.sent_folders(path) == ["b/Sent"] + assert qtmaildirconf.not_arrival_folders(path) == ["b/Sent"] def test_an_empty_sent_value_contributes_nothing(): with tempfile.TemporaryDirectory() as tmp: path = write_config(tmp, "[account.a]\nmaildir = a\nsent =\n") - assert qtmaildirconf.sent_folders(path) == [] + assert qtmaildirconf.not_arrival_folders(path) == [] + + +def test_an_account_without_a_spam_key_contributes_nothing(): + """`spam` is optional like the others. A missing key must not append the + bare maildir, and an EMPTY value must not either: either would become an + empty-path prefix matching the whole account.""" + with tempfile.TemporaryDirectory() as tmp: + path = write_config(tmp, "[account.a]\nmaildir = a\nsent = Sent\n" + "[account.b]\nmaildir = b\nspam =\n") + assert qtmaildirconf.not_arrival_folders(path) == ["a/Sent"] def test_an_account_without_a_maildir_contributes_nothing(): @@ -113,7 +137,7 @@ def test_an_account_without_a_maildir_contributes_nothing(): and a bare `Sent` would match every account's sent folder at once.""" with tempfile.TemporaryDirectory() as tmp: path = write_config(tmp, "[account.a]\nsent = Sent\n") - assert qtmaildirconf.sent_folders(path) == [] + assert qtmaildirconf.not_arrival_folders(path) == [] def test_comments_and_other_sections_are_ignored(): @@ -127,14 +151,14 @@ def test_comments_and_other_sections_are_ignored(): "; another comment\n" "maildir = a\n" "sent = Sent\n") - assert qtmaildirconf.sent_folders(path) == ["a/Sent"] + assert qtmaildirconf.not_arrival_folders(path) == ["a/Sent"] def test_a_missing_file_yields_no_folders(): """The hook must run on a system with no qtmaildir config at all: it then protects nothing, rather than failing the sync.""" with tempfile.TemporaryDirectory() as tmp: - assert qtmaildirconf.sent_folders(Path(tmp) / "absent.conf") == [] + assert qtmaildirconf.not_arrival_folders(Path(tmp) / "absent.conf") == [] def test_an_unreadable_file_yields_no_folders(): @@ -143,25 +167,25 @@ def test_an_unreadable_file_yields_no_folders(): mail for one cycle.""" with tempfile.TemporaryDirectory() as tmp: path = write_config(tmp, "this is not an ini file\n[[[\n") - assert qtmaildirconf.sent_folders(path) == [] + assert qtmaildirconf.not_arrival_folders(path) == [] def test_the_query_scopes_every_folder(): folders = ["a/Sent", "g/[Gmail]/Posta inviata"] - query = qtmaildirconf.sent_query(folders) + query = qtmaildirconf.not_arrival_query(folders) assert query == ('path:"a/Sent/**" or path:"g/[Gmail]/Posta inviata/**"') def test_the_query_is_empty_when_no_folder_is_configured(): """An empty query means "match everything" to notmuch, so the caller must be able to tell "nothing to protect" from "protect the world".""" - assert qtmaildirconf.sent_query([]) == "" + assert qtmaildirconf.not_arrival_query([]) == "" def test_a_folder_containing_a_quote_cannot_break_out_of_the_query(): """The folder name reaches a notmuch query as a quoted string. A stray double quote would end the term and let the rest be read as syntax.""" - query = qtmaildirconf.sent_query(['a/He said "hi"']) + query = qtmaildirconf.not_arrival_query(['a/He said "hi"']) assert query.count('"') % 2 == 0 assert "\\\"" in query or '""' in query |