From f06c9695ab10435502303f33bb82bd84e90e0e6e Mon Sep 17 00:00:00 2001 From: "Shane B." Date: Tue, 25 Aug 2026 15:18:41 +0200 Subject: [PATCH] Stop the exporter's locking default holding back everything else **Three people have now reported #140** - every share comes back unreadable and the export fails outright - and the fix has been on `main`, verified on a real affected account, since the day it was written. It could not ship, because `main` also turns on locked-by-default, and rule 9 says an exporter that locks must not go out before an app that can open one. App 1.1.0 is waiting on unrelated work. So the gate was doing its job and the cost was invisible: a fix for a total export failure was queued behind a feature it has nothing to do with. Defaulting the wizard's lock off removes the coupling. The ordering rule was about *what a bundle is*; with locking off, this release does not change that, and the #140 fix - along with the terms dialog, the save-logs button and the rest - can go out today. **The CLI is left alone.** It has `--no-password`, its help says why, and somebody driving a terminal has read it. It is the window that needs the safe default, because that is what people download and click through without reading anything. `test_wizard_locks_by_default.py` becomes `test_wizard_bundle_locking.py`, since the name asserted the very thing that changed. Its default test now asserts *off*, with a message naming the condition for flipping it - and the module docstring says outright that this test should fail, deliberately, in the change that raises the minimum app version. The value has now moved twice with nothing noticing; this is the third time and the first with a test on both sides of it. Rule 9 gains the lesson rather than just the new state: a gate on the bundle format must not hold back fixes that do not touch the format. 560 tests pass. Co-Authored-By: Claude Opus 5 --- AGENTS.md | 23 ++++++++++---- python/exporter/wizard.py | 21 ++++++++----- ...fault.py => test_wizard_bundle_locking.py} | 31 ++++++++++++------- 3 files changed, 49 insertions(+), 26 deletions(-) rename python/test/{test_wizard_locks_by_default.py => test_wizard_bundle_locking.py} (85%) diff --git a/AGENTS.md b/AGENTS.md index cd026ad0..7ad17052 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -130,12 +130,23 @@ So releasing is two steps, in this order: **And the app's release goes out before the exporter's, whenever the exporter's changes what a bundle is.** They are separate releases with separate tags, which makes them look independent; -they are not. Exporter 1.4.0 locks bundles by default, and an app older than 1.1.0 cannot decrypt -one at all — it fails with a message about the zip rather than about a code. Publish the exporter -first and every bundle written that day is unopenable by whoever receives it, and the recipient is -the one person in that transaction who chose none of it and can fix none of it. - -Nothing enforces this — `release_version.py` checks a tag against a version, not one release +they are not. A locked bundle cannot be opened by an app older than 1.1.0 at all — it fails with a +message about the zip rather than about a code. Publish an exporter that locks by default before +that app is out, and every bundle written that day is unopenable by whoever receives it, and the +recipient is the one person in that transaction who chose none of it and can fix none of it. + +**So the wizard's lock is currently defaulted off**, in `wizard.py`'s `lock_bundle`, and +`test_wizard_bundle_locking.py` asserts that. Flip it in the same change that raises the minimum +app version, not before and not separately. + +**A gate on the format must not hold back unrelated fixes.** This nearly happened: the fix for +[#140](https://github.com/parawanderer/OpenTagViewer/issues/140) — where every share comes back +unreadable and the export fails outright — sat unreleased behind this ordering because it happened +to be on the same `main` as the locking change. Three people reported the same bug in the meantime. +If a fix does not touch what a bundle *is*, it is not what this rule is about, and shipping it +should not wait on an app release. + +Nothing enforces any of this — `release_version.py` checks a tag against a version, not one release against another — so it is a thing to remember, which is why it is written here. `scripts/release_version.py --kind exporter --tag ` enforces it, and runs in `test-release-version` diff --git a/python/exporter/wizard.py b/python/exporter/wizard.py index feaddea8..2d3221fc 100644 --- a/python/exporter/wizard.py +++ b/python/exporter/wizard.py @@ -245,16 +245,21 @@ def _build(self) -> None: self.confirm_button = ttk.Button(buttons, text="Export…", command=self._export, state="disabled") self.confirm_button.grid(row=0, column=4) - # **On by default, and the default is the whole point.** A bundle holds key material that - # cannot be revoked - the only way to withdraw an exported accessory is to unpair it - and - # it then travels through a mail account or a chat app and outlives the conversation by + # **Off until the app that can open one is released, then on.** A bundle holds key material + # that cannot be revoked - the only way to withdraw an exported accessory is to unpair it - + # and it then travels through a mail account or a chat app and outlives the conversation by # years, sitting in a backup long after everyone has forgotten it is there. Whoever most - # needs the lock is whoever would never go looking for a checkbox to turn it on. + # needs the lock is whoever would never go looking for a checkbox to turn it on, so on is + # where this belongs and where it is going. # - # The opt-out exists for one real case, the same one behind the CLI's --no-password: an - # app older than 1.1.0 cannot decrypt anything at all, so a recipient running one cannot - # open a locked bundle, and they did not choose the exporter's version. - self.lock_bundle = tk.BooleanVar(value=True) + # It is off today for one reason: **no released app can decrypt a locked bundle.** Anything + # older than 1.1.0 fails with a message about the zip rather than about a code, and the + # person who meets that failure is the recipient - who chose neither the exporter nor its + # version, and can do nothing about either. + # + # So the ordering is app first, exporter second - see AGENTS.md rule 9. Flip this to True in + # the same change that raises the minimum, not before, and not separately. + self.lock_bundle = tk.BooleanVar(value=False) ttk.Checkbutton( buttons, text="Lock with a code", diff --git a/python/test/test_wizard_locks_by_default.py b/python/test/test_wizard_bundle_locking.py similarity index 85% rename from python/test/test_wizard_locks_by_default.py rename to python/test/test_wizard_bundle_locking.py index c5dc491e..9ae92269 100644 --- a/python/test/test_wizard_locks_by_default.py +++ b/python/test/test_wizard_bundle_locking.py @@ -1,14 +1,16 @@ """ -The wizard locks the bundles it writes, and shows the code once. +The wizard can lock the bundles it writes, and shows the code once. -**This default was blocked, not missing.** ``wizard.py`` hard-coded ``password=None`` with a -comment saying so: before the Android app carried zip4j it could not decrypt anything at all, so -a locked bundle was a file nobody's installed app could open - and the people worst affected were -recipients, who did not choose the exporter's version and could not fix it from their side. +**The default is off, and it is off for a reason that will expire.** A locked bundle can only be +opened by app 1.1.0 or newer; anything older fails with a message about the zip rather than about +a code. The person who meets that failure is the recipient, who chose neither the exporter nor its +version - so until 1.1.0 is *released*, not merely built, locking by default would break exports +for the one party who can do nothing about it. -App 1.1.0 reads them. So the default flips, and these tests exist because nothing caught the old -behaviour either: no test asserted ``password=None``, so the flip would have gone unnoticed in -both directions. +**These tests assert the default in both directions on purpose.** Nothing caught the previous flip +in either direction, so the value has changed twice with no test noticing. When app 1.1.0 ships and +the default goes back on, `test_the_checkbox_starts_unticked` is the test that should fail and be +inverted - deliberately, in the same change, rather than discovered later. The code is the part with a permanent cost. It is not stored anywhere and cannot be recovered, so a bundle written without the user being shown its code is a bundle nobody can ever open. @@ -63,12 +65,17 @@ def write(window, bundle, path, *, locked: bool): class TestTheDefault: """ - A bundle holds key material that cannot be revoked, and travels through other people's - infrastructure. Whoever most needs the lock is whoever would never find a checkbox for it. + Off until an app that can open one is released - see the module docstring. + + A bundle holds key material that cannot be revoked and travels through other people's + infrastructure, so on is where this belongs eventually. It is not there yet. """ - def test_the_checkbox_starts_ticked(self, window): - assert window.lock_bundle.get() is True + def test_the_checkbox_starts_unticked(self, window): + assert window.lock_bundle.get() is False, ( + "locking by default writes bundles that no released app can open. Flip this only in" + " the change that raises the minimum app version - see AGENTS.md rule 9" + ) def test_a_bundle_is_written_with_a_code(self, window, bundle, tmp_path): write_zip, _shown, _info, _error, _closed = write(