feat(keyring): ship a patched gnome-keyring and install it from the repoclaude/hungry-franklin-09fffc

gnome-keyring 48 through 51.1 aborts when a client's first OpenSession is dispatched before the daemon has created its per-client record. Each abort drops the unlocked state and the next request pops the password dialog. On ratio that happened about three times a day from 2026-10-11, when the ChatGPT desktop app's codex app-server started bursting keyring reads. The same-day glib and keyring upgrades were a coincidence, since 50.0 reproduces it too. Upstream's fix (merge request !112) has sat unmerged since August.

packages/gnome-keyring is Arch's recipe for 1:51.1-1 built from GNOME's tarball plus one patch. It registers the client at the point of use and returns a real D-Bus error instead of completing OpenSession with a NULL body. It resolves a collection alias only after the caller's session exists. It also locks the alias table so the worker-thread filter can't read an entry the main thread is freeing, a pre-existing defect that AddressSanitizer confirmed. I chose pkgrel 1.1 so stock sorts below the rebuild and any future Arch or upstream fix replaces it on a normal upgrade. Nothing pins the package.

install_gnome_keyring_fix replaces the plain stock install, which pacman's --needed would otherwise use to downgrade the rebuild on every re-run. It installs stock only when no gnome-keyring is present, compares versions with vercmp, then stages the recipe into the source dir and builds it with makepkg as the user. The rebuild is a no-op, anything newer skips with a visible note, and a failed build leaves stock in place. I also corrected the keyring-dir comment: the default alias only picks the collection and doesn't unlock anything under autologin.

working/keyring-crash holds the probe, the guest fixture and runners, and the deployment script with backup and rollback. status.org there records the cause and results. Stock aborts on the probe's first batch. The patched daemon, its sanitizer build and the installed package pass every mode, and the upstream suite scores the same for both.

1 files changed, 209 insertions, 0 deletions

diff --git a/tests/installer-steps/test_install_gnome_keyring_fix.py b/tests/installer-steps/test_install_gnome_keyring_fix.py
new file mode 100644
index 0000000..1a4115a
--- /dev/null
+++ b/tests/installer-steps/test_install_gnome_keyring_fix.py
@@ -0,0 +1,209 @@
+"""Tests for install_gnome_keyring_fix in the archsetup installer.
+
+gnome-keyring 51.1 (and 48 through 50) aborts when a Secret Service client's
+first OpenSession is dispatched before the daemon has created its per-client
+record. Every abort loses the unlocked state, so the next request pops the
+keyring password dialog; on ratio this fired three times a day once the
+ChatGPT desktop app's codex app-server started bursting keyring reads. The
+fix is upstream merge request !112 plus an alias-lifetime correction, carried
+as a patched rebuild of Arch's own package under packages/gnome-keyring in
+this repo (pkgrel 1.1). install_gnome_keyring_fix builds and installs it with
+makepkg as the user when the stock 1:51.1-1 is what is installed.
+
+Versioning is the whole contract. The rebuild sorts above stock 1:51.1-1 and
+below any future 1:51.1-2 or 51.2, so a normal pacman upgrade replaces it the
+moment Arch or upstream ships a fix, and the step must never pin or hold the
+package. When something newer than the rebuild is already installed, the fix
+is assumed upstream and the step skips with a warning so the assumption is
+visible rather than silent. When the rebuild itself is installed, the step is
+a no-op.
+
+These tests exercise the REAL function body, extracted from the `archsetup`
+script at run time. pacman, vercmp, makepkg and sudo are replaced by
+recorders driven by environment switches; the package directory is a temp
+copy of the real packages/gnome-keyring tree.
+
+Run from repo root:
+    python3 -m unittest tests.installer-steps.test_install_gnome_keyring_fix
+"""
+
+import os
+import re
+import shutil
+import subprocess
+import tempfile
+import unittest
+
+REPO_ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__), "..", ".."))
+ARCHSETUP = os.path.join(REPO_ROOT, "archsetup")
+PACKAGE_DIR = os.path.join(REPO_ROOT, "packages", "gnome-keyring")
+
+STUBS = r"""
+logfile=/dev/null
+action=""
+username=tester
+display() { echo "DISPLAY: $*"; }
+pacman_install() { echo "PACMAN_INSTALL: $1" >> "$MARKER"; }
+error_warn() { echo "WARN:$1"; return 1; }
+# pacman -Q prints the installed version, or fails when INSTALLED is empty.
+pacman() {
+    echo "PACMAN: $*" >> "$MARKER"
+    case "$1" in
+        -Q) [ -n "$INSTALLED" ] && echo "gnome-keyring $INSTALLED" ;;
+        *) return 0 ;;
+    esac
+}
+# The real vercmp is used so the ordering under test is pacman's own.
+makepkg() {
+    echo "MAKEPKG: $* (pwd=$PWD)" >> "$MARKER"
+    [ "$MAKEPKG_FAILS" = 1 ] && return 2
+    return 0
+}
+# sudo -u user cmd... runs cmd in place; the recorder notes the user.
+sudo() {
+    echo "SUDO: $*" >> "$MARKER"
+    shift 2
+    "$@"
+}
+safe_rm_rf() { rm -rf "$1"; }
+# chown needs root; the tests only care that the copy was staged.
+chown() { echo "CHOWN: $*" >> "$MARKER"; }
+"""
+
+
+def run(pkg_dir, installed, makepkg_fails=False, source_dir=None):
+    marker = tempfile.NamedTemporaryFile(prefix="keyring-fix-", delete=False)
+    marker.close()
+    source_dir = source_dir or tempfile.mkdtemp(prefix="keyring-src-")
+    script = (
+        STUBS
+        + "source <(sed -n '/^install_gnome_keyring_fix() {/,/^}/p' \"%s\")\n"
+        + 'user_archsetup_dir="%s"\n'
+        + 'source_dir="%s"\n'
+        + "install_gnome_keyring_fix\n"
+        + 'echo "RC=$?"\n'
+    ) % (ARCHSETUP, pkg_dir, source_dir)
+    env = dict(os.environ, MARKER=marker.name, INSTALLED=installed,
+               MAKEPKG_FAILS="1" if makepkg_fails else "0")
+    result = subprocess.run(["bash", "-c", script], capture_output=True,
+                            text=True, timeout=30, env=env)
+    with open(marker.name) as f:
+        calls = f.read().splitlines()
+    os.remove(marker.name)
+    return result, calls, source_dir
+
+
+class InstallGnomeKeyringFix(unittest.TestCase):
+    def setUp(self):
+        # A fake archsetup checkout holding a copy of the real package tree.
+        self.checkout = tempfile.mkdtemp(prefix="archsetup-checkout-")
+        self.addCleanup(shutil.rmtree, self.checkout, ignore_errors=True)
+        shutil.copytree(PACKAGE_DIR, os.path.join(self.checkout, "packages", "gnome-keyring"))
+        self.source_dir = tempfile.mkdtemp(prefix="keyring-src-")
+        self.addCleanup(shutil.rmtree, self.source_dir, ignore_errors=True)
+
+    def _run(self, installed, **kw):
+        return run(self.checkout, installed, source_dir=self.source_dir, **kw)
+
+    @staticmethod
+    def _makepkg_calls(calls):
+        return [c for c in calls if c.startswith("MAKEPKG:")]
+
+    # ---------------------------------------------------------- normal ----
+    def test_stock_51_1_gets_the_rebuild(self):
+        result, calls, _ = self._run("1:51.1-1")
+        self.assertEqual(len(self._makepkg_calls(calls)), 1, calls)
+        self.assertIn("RC=0", result.stdout)
+        self.assertNotIn("WARN:", result.stdout)
+
+    def test_builds_as_the_user_with_syncdeps_and_install(self):
+        _, calls, _ = self._run("1:51.1-1")
+        self.assertTrue(any(c.startswith("SUDO: -u tester makepkg") for c in calls), calls)
+        makepkg = self._makepkg_calls(calls)[0]
+        self.assertIn("--noconfirm", makepkg)
+        # -s (sync makedepends) and -i (install), spelled separately or combined.
+        short = "".join(t[1:] for t in makepkg.split() if re.fullmatch(r"-[a-zA-Z]+", t))
+        self.assertIn("s", short, makepkg)
+        self.assertIn("i", short, makepkg)
+
+    def test_builds_from_a_user_owned_copy_not_the_checkout(self):
+        # makepkg writes into its directory; the checkout must stay clean.
+        _, calls, _ = self._run("1:51.1-1")
+        makepkg = self._makepkg_calls(calls)[0]
+        self.assertIn("pwd=" + self.source_dir, makepkg)
+        self.assertNotIn("pwd=" + self.checkout, makepkg)
+        self.assertTrue(os.path.exists(os.path.join(self.source_dir, "gnome-keyring", "PKGBUILD")))
+
+    def test_older_stock_releases_also_get_the_rebuild(self):
+        # 50.0 carries the same bug; the rebuild is 51.1 so it is an upgrade.
+        _, calls, _ = self._run("1:50.0-1")
+        self.assertEqual(len(self._makepkg_calls(calls)), 1)
+
+    # -------------------------------------------------------- boundary ----
+    def test_rebuild_already_installed_is_a_noop(self):
+        result, calls, _ = self._run("1:51.1-1.1")
+        self.assertEqual(self._makepkg_calls(calls), [])
+        self.assertIn("RC=0", result.stdout)
+        self.assertNotIn("WARN:", result.stdout)
+
+    def test_newer_package_skips_visibly_without_counting_an_error(self):
+        for newer in ("1:51.1-2", "1:51.2-1", "1:52.0-1"):
+            with self.subTest(installed=newer):
+                result, calls, _ = self._run(newer)
+                self.assertEqual(self._makepkg_calls(calls), [], newer)
+                self.assertIn("newer than the rebuild", result.stdout,
+                              "a newer package means the fix is assumed upstream; say so")
+                self.assertNotIn("WARN:", result.stdout,
+                                 "a deliberate skip must not count as an installer error")
+                self.assertIn("RC=0", result.stdout)
+
+    def test_rerun_after_success_does_not_rebuild(self):
+        _, calls, _ = self._run("1:51.1-1")
+        self.assertEqual(len(self._makepkg_calls(calls)), 1)
+        _, calls, _ = self._run("1:51.1-1.1")
+        self.assertEqual(self._makepkg_calls(calls), [])
+
+    def test_missing_package_installs_stock_first_then_builds(self):
+        # With no gnome-keyring present, stock is installed as the baseline
+        # and the rebuild then upgrades it; both happen in that order.
+        result, calls, _ = self._run("")
+        self.assertEqual(len(self._makepkg_calls(calls)), 1)
+        stock = [i for i, c in enumerate(calls) if c.startswith("PACMAN_INSTALL: gnome-keyring")]
+        build = [i for i, c in enumerate(calls) if c.startswith("MAKEPKG:")]
+        self.assertEqual(len(stock), 1, calls)
+        self.assertLess(stock[0], build[0], "stock install must precede the build")
+        self.assertIn("RC=0", result.stdout)
+
+    def test_rerun_with_rebuild_installed_never_touches_stock(self):
+        # pacman's --needed only skips an equal version, so a stock install
+        # ahead of the rebuild would downgrade it on every re-run. The step
+        # must not call the stock install at all once anything is installed.
+        for installed in ("1:51.1-1.1", "1:51.1-1", "1:51.2-1"):
+            with self.subTest(installed=installed):
+                _, calls, _ = self._run(installed)
+                self.assertFalse(any(c.startswith("PACMAN_INSTALL:") for c in calls), calls)
+
+    # ----------------------------------------------------------- error ----
+    def test_failed_build_warns_and_leaves_stock_in_place(self):
+        result, calls, _ = self._run("1:51.1-1", makepkg_fails=True)
+        self.assertIn("WARN:", result.stdout)
+        self.assertNotIn("RC=0", result.stdout)
+        self.assertFalse(any(c.startswith("PACMAN: -U") for c in calls),
+                         "nothing may be installed by hand after a failed build")
+
+    def test_missing_recipe_warns_instead_of_building(self):
+        shutil.rmtree(os.path.join(self.checkout, "packages", "gnome-keyring"))
+        result, calls, _ = self._run("1:51.1-1")
+        self.assertEqual(self._makepkg_calls(calls), [])
+        self.assertIn("WARN:", result.stdout)
+
+    def test_never_pins_or_holds_the_package(self):
+        # No IgnorePkg edit and no pacman -D hold: the next upstream fix must
+        # replace the rebuild on a normal upgrade.
+        _, calls, _ = self._run("1:51.1-1")
+        self.assertFalse(any("IgnorePkg" in c for c in calls))
+        self.assertFalse(any(c.startswith("PACMAN: -D") for c in calls))
+
+
+if __name__ == "__main__":
+    unittest.main()