fix(recovery): repair GitHub catalogue sync and archive installation

[lopugit]

Recovery showed only two old releases because the main cloud builder failed before publishing build 5. The actual build 4 ZIP also lacks signature resource seals. Once a companion Recovery archive was published, duplicate SwiftUI row identities caused its row to open the desktop download.

The app now fetches a complete paginated catalogue once, selects compatible Mac archives, distinguishes desktop/Recovery rows, and reports published/compatible counts. Extraction and verification run off the UI thread, subprocess output cannot fill an undrained pipe, malformed ZIPs produce an actionable error, and cached bundles are reverified. Valid replacements can repair a damaged installed app while preserving its untrusted backup. Rollback failures retain their backup; detached helper failures reopen Recovery with a durable explanation. Both Recovery packaging scripts work from an absent cache root.

Validation: 17 Swift tests and 79 Electron tests pass. The detached installed helper was also exercised with an invalid plan: it reopened Recovery with the expected error, then normal startup cleared the consumed notice. The installed app independently selects both components and successfully downloads/caches both final desktop and Recovery ZIPs from commit dd07ff9361ee0c974c91e824dd7509be6fede4ae; independent cache verification confirms both resource seals and the unsigned desktop bundle contract. The actual malformed build 4 archive is rejected without changing installed apps. Staged and installed local signatures verify with the existing Apple Development designated requirement. Fresh unsigned Recovery packaging and its extracted ZIP verify. Graphify portable outputs are refreshed; one oversized semantic chunk exceeded the local proxy request limit.

The protected cloud builder repair is merged in PR #628. Run 33948509016 published both apps from the first product commit. Run 33949035823 completed successfully and published the final source commit dd07ff9361ee0c974c91e824dd7509be6fede4ae as this prerelease. All checks on the final PR head are complete with no failures. Recovery refresh now reports 4 published releases, 4 desktop archives, and 2 Recovery archives. Cloud signing secrets are absent, so this intentionally uses the existing explicitly unsigned prerelease lane.

The product main listener permission update remains subject to normal promotion; no primary-branch merge is included. There is no Vercel preview for this native-only change: preview authorization is skipped. See the engineering note for diagnosis and test details.

[lopugit]

🧹 Develop S3 preview removed

The PR-specific alias and every workflow-created develop deployment were removed when this PR closed.

The ordinary generated Vercel Preview remains available on the shared development runtime.

[github-actions]

Lopu review — PR #627 (650e27fa vs develop 6e18d175)

I checked the control-plane claims against the live protected branch rather than taking the PR body at its word, and they hold up.

Verified

• The pull-requests: read line is required, not cosmetic. The chain is main listener → electron-release.yml@github-actions → electron-pr-release.yml, and the worker declares contents: write + pull-requests: read. A called workflow can only narrow the caller's token. The base listener granted contents: write alone; the sibling PR listener already grants exactly this pair and says why in a comment.
• npm ci --prefix MCP matches deployed reality. That is what electron-pr-release.yml:162 on github-actions actually runs. codex/recovery-release-builder compares ahead_by: 0, behind_by: 3 against github-actions, so the companion work is already merged — this PR is catching the product branch up, not racing ahead of it.
• Narrowing /^\s*push:/ → /^ push:/ is a false-positive fix, not a weakened guard. The old pattern matched electron-pr-release.yml:84, a shell case label (push:main)) inside the worker. A real top-level on: → push: key is still caught at YAML's two-space depth.
• The renderer test was genuinely broken on develop. I replayed the base version against the current tree: assert.doesNotMatch(fallbackRouteSource, /assets:server/u) fails, because remix/server/routes/[...].ts deliberately keeps assets:server as a compatibility mount behind the canonical assets:shell. Good catch.
• The ProcessExecution fix is safe for every caller. Reading the merged pipe to EOF before waitUntilExit() is the right ordering, and folding stderr into stdout does not regress BundleVerifier — codesign -d, codesign --verify and spctl --assess all write to stderr, the old code concatenated both anyway, and value(named:in:) matches line prefixes over the merged text.
• The architecture filter is correct on real data. I transcribed the new selectAsset predicate into a standalone Swift program and ran it against every ZIP asset currently published on the repo plus an edge-case matrix (universal, the x86_64 alias, numbered re-uploads, missing/bare/aarch64 suffixes, path traversal, uppercase extension): 17/17 as intended.

Three changes pushed to this branch

1. RecoveryInstaller.swift — the new rollback-failure branch shadowed error with its inner catch, so the worst-case path reported neither why the install failed nor why the restore failed. Before this PR the original error always propagated. Both causes are now carried alongside the preserved backup path. In a recovery tool that is exactly the path where diagnosis matters most.
2. bundled-renderer.test.cjs — indexOf(shell) < indexOf(server) returns -1 for an absent needle, so the assertion would fail if the assets:server compatibility mount were ever removed: the test breaks when the code gets better. It now tolerates the fallback's absence while still rejecting the wrong order.
3. .github/workflows/electron-release.yml — a comment recording why pull-requests: read is there. Nothing in this listener touches PR data, so it reads as dead configuration and is a prime candidate for a well-meant cleanup that would silently break main releases again. Mirrors the comment the sibling listener already carries. Comment only — on:, permissions: and jobs: are unchanged.

Worth knowing

• validateControlPlane never runs against the real control plane. Only verify:release-shim executes in CI, and only against the shim; validateControlPlane is exercised solely by inline fixtures in the same test file this commit edits. Script and fixture moving together prove nothing about github-actions. I closed that gap by hand for this change, but the contract is giving false assurance in general — worth either pointing at something real or retiring.
• Any asset without a -macos-<arch> suffix is now dropped outright, so the client is hard-coupled to the publisher's naming. Every published asset today is arm64-only, so an Intel Mac sees an empty list — the new sidebar counter makes that read as truth rather than a client bug, which is the right call.
• RecoveryArchive.cache creates staging under cacheRoot before RecoveryCache.ensureRoot() runs its lstat check, so a symlinked cache root is only rejected after ditto has written. Pre-existing and low severity (same-privilege attacker), left alone.
• A cached bundle that fails re-verification can never be replaced by re-caching — the user has to find the Remove action, and "Download was not cached…" doesn't point there.

Validation

Electron suite 77/79 (both failures are spawn /usr/bin/ditto ENOENT, a macOS-only tool absent on the Linux runner; identical before and after my edits). Changed-area tests 16/16. Shim contract passes against the edited workflow, which was re-parsed with a YAML loader to confirm unchanged semantics. swiftc -parse clean on the edited Swift file, plus a compiled reproduction confirming the original error survives a successful rollback and both causes survive a failed one.

No failing checks; UNSTABLE is the two still-running checks. Zero open CodeQL alerts on this head, so no dispositions were filed.

The diagnosis and the fixes are proportionate and well evidenced. One note for sequencing: the permission fix is inert until develop reaches main, since the listener only triggers on push to main — which the PR body already scopes out.

[github-actions]

Lopu repository review

Lopu reviewed this PR against develop as Thingtime's principal PR and repository manager. Using Claude Opus 5.

Lopu made justified improvements and pushed d2a9227 to codex/recovery-release-sync.

PR #627 — fix(recovery): repair GitHub catalogue sync and archive installation

Head dd07ff9361ee0c974c91e824dd7509be6fede4ae → base develop @ 61e7daeb
52 files changed, +490 / −75 (21 non-generated).

Checks: all green. No failing, cancelled, or timed-out runs. No CodeQL alerts on this head.

What I compared

Full head-vs-base diff, concentrating on the macOS Recovery app: the GitHub catalogue fetch,
the subprocess wrapper, archive caching, and the install/rollback path. I validated the new
asset-architecture filter against the repository's actual published release assets rather
than trusting the regex by inspection.

Assessment

This is a strong PR. Each change fixes a real defect, and each one ships with a test that would
have caught it. Specifically:

• ProcessExecution.run had a genuine deadlock. The old code called waitUntilExit() before
  reading either pipe, so any child writing more than the ~64 KB pipe buffer would block forever
  while the parent blocked waiting for it — codesign --verbose=4 and ditto on a large bundle
  can both do this. Merging both streams into one pipe and draining to EOF before waiting is the
  correct fix. The regression test (processDrainsOutput, ~500 KB across both streams) would hang
  on the old implementation.
  I specifically checked that merging streams doesn't break BundleVerifier, which parses this
  output: codesign --display --verbose=4 writes to stderr, and the old code only preserved
  that because it returned stdout + "\n" + stderr. The merged pipe still contains it. No regression.
• Pagination SSRF hardening is real. Adding pageURL.path == endpoint.path, user == nil,
  password == nil stops a hostile Link header from steering the crawl to another
  api.github.com path or embedding credentials. The host/scheme check alone was not enough.
• Status-code checking moved to the right layer. It used to live inside the production
  requestPage closure, so injected test doubles bypassed it entirely; now fetchAll() enforces
  it for every caller, with specific 403/429 messaging. catalogRejectsPartialRefresh pins the
  important property: a later-page failure fails the whole refresh instead of silently returning
  a truncated catalogue.
• RecoveryCache.cacheBundle now re-verifies on a cache hit. Previously a key match returned
  the on-disk bundle unverified — a TOCTOU hole, since the cache is a plain directory. Good catch.
• One paginated fetch instead of two. refresh() used to run fetch(.desktop) and
  fetch(.recovery) concurrently, each walking every page — double the requests against an
  unauthenticated 60/hr GitHub limit, which is likely what produced the 403s this PR now handles.
• The install path no longer destroys the only rollback copy. A damaged installed app that
  can't enter the verified cache is now preserved rather than deleted, and a failed rollback
  reports where the backup lives instead of silently discarding it.

I traced every throw path in atomicallyInstall after removing backup from the defer, and
there is no leak: a failed staging move or target verify restores the backup by moving it back,
a failed rollback deliberately keeps it and says so, and the success path either returns it
(preserve) or removes it. A fresh install never creates one.

Architecture filter — validated against real data

The new filter drops assets that don't match the host architecture. My concern was that it would
silently hide legitimately-named older assets. I compiled the exact filter logic and ran it
against the repository's real published assets:

Asset	arm64 host	x64 host
…-build.3-macos-arm64.zip	✅	❌
…-build.4-macos-arm64.zip	✅	❌
…unsigned-macos-arm64-1.zip	✅	❌
…unsigned-macos-arm64.zip	✅	❌

All six published .zip assets carry a -macos-arm64 / -macos-arm64-N suffix, so nothing in
the live catalogue is dropped. Edge cases behave correctly too: -macos-universal matches both
hosts, -macos-x86_64 matches only x64, and ../evil-macos-arm64.zip and A-macos-arm64.zip.txt
are both rejected (the new lastPathComponent guard is worth keeping — that name becomes a
filename downstream).

On an Intel Mac every current release is now correctly filtered out. That is the right behaviour,
and the PR already handles the UX: catalogStatus reports
"GitHub: N published releases · 0 desktop · 0 Recovery for this Mac", which distinguishes
"nothing for your architecture" from "GitHub is down". That distinction is exactly why
publishedReleaseCount was added, and it's a good call.

Change I made

RecoveryStore.swift — the one-shot install notice was sticky for the whole session.

RecoveryInstallNotice is explicitly a consume-once message: consume() deletes the file after
reading, and the PR's own test installNoticeRoundTrip asserts a second consume() returns
nil. But RecoveryStore copied the message into installerNotice and never cleared it, while
refresh() does notice = installerNotice ?? "Release catalog refreshed…". Every refresh for the
rest of the session would therefore replay a stale install message and suppress the real refresh
confirmation — contradicting the contract the PR itself tests.

Fixed by clearing it after the first refresh consumes it. This keeps the intended behaviour (the
first refresh after a helper install can't bury the result) while restoring the one-shot semantics.

Validation

• swiftc -parse on all seven changed Swift files — all parse clean.
• Architecture filter compiled and executed against the 4 distinct real published asset names plus
  8 synthetic edge cases (table above); results as intended in every case.
• RecoveryStore is @MainActor final class, so clearing installerNotice inside the async
  refresh() is main-actor isolated exactly like the surrounding notice/desktopReleases
  assignments — no new concurrency surface.
• The full test suite is macOS-only (codesign, ditto, spctl) and cannot execute on this
  Linux runner; I reviewed the eight new tests directly and they map 1:1 onto the behaviour changes.

Note (no change made)

GitHubReleaseCatalog gates on name.lowercased().hasSuffix(".zip") — case-insensitive — but the
new architecture regex ends \\.zip$, which is case-sensitive. An asset named ….ZIP would
pass the first gate and be dropped by the second. No producer emits that today (these names come
from this repo's own release workflow, and every published asset is lowercase), so I left it
alone rather than churn the diff. Worth [.regularExpression, .caseInsensitive] if that code is
touched again.

Post-review status update — merged mid-session, fix did not land

PR #627 merged at 06:51:24Z (merge commit 04d779642, head dd07ff936) while this review was
still in progress, advancing develop off the manifest base 61e7daeb.

My RecoveryStore.swift change did not land. I verified directly against the remote:
macos/ThingtimeRecovery/Stores/RecoveryStore.swift on develop contains no
installerNotice = nil. It was still an uncommitted worktree change at the moment of the merge.

The head branch codex/recovery-release-sync still exists at dd07ff936, so the trusted publisher
may still push there — but because the PR is already merged, a push to that branch will not reach
develop. Landing this requires a fresh PR against develop.

The change itself is one line plus its comment, in RecoveryStore.refresh():

notice = installerNotice ?? "Release catalog refreshed. Cached bundles remain available if GitHub is offline later."
installerNotice = nil

Rationale unchanged from the section above: RecoveryInstallNotice is a consume-once message
(consume() deletes the file; installNoticeRoundTrip asserts the second call returns nil), but
installerNotice is never cleared, so every later refresh replays the stale install message and
suppresses the real refresh confirmation — contradicting the contract the PR itself tests.

This is recorded on the PR conversation as well so it is not lost. Everything else in this review
remains a green assessment; the merge itself was fine.

Side effect worth noting: this merge is what pushed PR #592 into CONFLICTING/DIRTY
during the same session. That conflict is confined to graphify-out/ snapshots plus
remix/CHANGELOG.md — no source conflict. See the #592 report.

View Lopu workflow run

[github-actions]

🤖 Lopu detected an out-of-date PR branch

Status: Work detected — Lopu is taking ownership.

Current phase: Entering Lopu's serialized PR-resolution queue.

Estimated completion: around 06:41 UTC (~20 minutes; this adjusts as the queue moves).

Next automatic check-in: within 10 minutes while work remains active. You can stay on this PR; there is no need to find the Actions run.

Time conversion (UTC source)

Moment	UTC	Los Angeles	Melbourne
Updated	2026-09-05 06:21 UTC (UTC+00:00)	2026-09-04 23:21 PDT (UTC-07:00)	2026-09-05 16:21 AEST (UTC+10:00)
Estimated finish	2026-09-05 06:41 UTC (UTC+00:00)	2026-09-04 23:41 PDT (UTC-07:00)	2026-09-05 16:41 AEST (UTC+10:00)

Los Angeles and Melbourne use their real IANA time zones, so PDT/PST and AEST/AEDT offsets change automatically.

Lopu queue and PR pulse

Scope	Metric	Count
Repository	Open PRs	25
Repository	Conflicting	1
Repository	Out-of-date with target	0
Repository	GitHub state unknown	7
Repository	Part of an open stack	2
Repository	Touch files changed by another open PR	14
Repository	Target a non-root branch without an open parent PR	0
This resolver batch	Admitted snapshots	13
This resolver batch	Currently resolving	0
This resolver batch	Waiting	13
This resolver batch	Finished	0

Related PR context

• Stack: No open parent or child PR currently links to this branch.
• Target: develop is a repository root/integration branch.
• Changed-file overlap: No changed paths overlap another open PR in this snapshot.

Exact branch pair: develop → codex/recovery-release-sync.

Timeline

• 06:21 UTC — Detected that develop needs to be merged into codex/recovery-release-sync; assigning the exact snapshot to the resolver queue.

[github-actions]

🤖 Lopu review — checks green, strong PR; one one-shot-notice bug fixed

Every change here fixes a real defect and ships with a test that would have caught it. Notes
from the parts I dug into:

The ProcessExecution deadlock was real. The old code called waitUntilExit() before
reading either pipe, so any child writing past the ~64 KB pipe buffer blocks forever while the
parent blocks waiting for it — codesign --verbose=4 and ditto on a large bundle both can.
Draining to EOF first is the right fix, and processDrainsOutput (~500 KB) would hang on the
old implementation.

I checked the risk in merging stderr into stdout, since BundleVerifier parses this output:
codesign --display --verbose=4 writes to stderr, and the old code only preserved it because
it returned stdout + "\n" + stderr. The merged pipe still carries it. No regression.

Pagination hardening is worth calling out — path == endpoint.path + user/password == nil
stops a hostile Link header steering the crawl to another api.github.com path or embedding
credentials; host/scheme alone wasn't enough. And moving the status-code check out of the
production requestPage closure into fetchAll() matters: test doubles used to bypass it
entirely.

Also good: re-verifying on a cache hit closes a genuine TOCTOU (the cache is a plain
directory), and collapsing two concurrent full paginations into one fetchAll() halves the
request count against an unauthenticated 60/hr limit — likely the source of the 403s this PR
now handles.

I traced every throw path in atomicallyInstall after backup left the defer, and there's
no leak: failed staging-move/verify restores the backup by moving it back, a failed rollback
deliberately keeps it and says where, and success either returns or removes it.

Architecture filter — I validated it against the real catalogue rather than by inspection.
Compiled the exact filter and ran it over the actual published assets:

Asset	arm64	x64
…-build.3-macos-arm64.zip	✅	❌
…-build.4-macos-arm64.zip	✅	❌
…unsigned-macos-arm64-1.zip	✅	❌
…unsigned-macos-arm64.zip	✅	❌

All six published zips carry -macos-arm64/-macos-arm64-N, so nothing live is dropped.
Edge cases behave: -macos-universal matches both, -macos-x86_64 only x64, and
../evil-macos-arm64.zip / A-macos-arm64.zip.txt are rejected. On Intel every current release
is now filtered out — correct, and catalogStatus already distinguishes "0 for this Mac" from
"GitHub is down". That's exactly what publishedReleaseCount earns its keep for.

One bug I fixed — the install notice was sticky.
RecoveryInstallNotice is explicitly consume-once (consume() deletes the file, and your own
installNoticeRoundTrip asserts the second call returns nil). But RecoveryStore copied it
into installerNotice and never cleared it, while refresh() does
notice = installerNotice ?? "Release catalog refreshed…". Every refresh for the rest of the
session would replay the stale install message and suppress the real refresh confirmation —
contradicting the contract the PR itself tests. Cleared it after the first refresh consumes it,
which keeps the intent (first refresh can't bury the install result) and restores the one-shot.

Validation: swiftc -parse clean on all seven changed Swift files; arch filter compiled and
executed against 4 real + 8 synthetic names. The suite itself is macOS-only (codesign/ditto/
spctl) so it can't run on the Linux reviewer — I read the eight new tests instead and they map
1:1 onto the behaviour changes.

Non-blocking nit: the asset gate is name.lowercased().hasSuffix(".zip") (case-insensitive)
but the new arch regex ends \.zip$ (case-sensitive), so a .ZIP asset would pass the first
and be dropped by the second. Nothing emits that today, so I left it rather than churn the diff —
[.regularExpression, .caseInsensitive] if that code is touched again.

[github-actions]

🤖 Lopu follow-up — this merged mid-review, and my one fix did not land

Timing, stated plainly: this PR merged at 06:51:24Z (04d779642) while I was still reviewing
it. My review comment above went up shortly after, and the one code change it describes was still
sitting as an uncommitted worktree change when the merge happened.

I verified it is not on develop: macos/ThingtimeRecovery/Stores/RecoveryStore.swift at
develop contains no installerNotice = nil. The head branch codex/recovery-release-sync still
exists at dd07ff936, but since the PR is already merged, anything pushed there now will not
reach develop on its own — this needs a fresh PR against develop to land.

Everything else in my review stands as a green assessment; nothing about the merge was wrong. This
is the one loose end.

The change, so it can be re-applied directly

In RecoveryStore.refresh(), add installerNotice = nil immediately after the notice assignment:

notice = installerNotice ?? "Release catalog refreshed. Cached bundles remain available if GitHub is offline later."
installerNotice = nil

Why it matters: RecoveryInstallNotice is explicitly consume-once — consume() deletes the
file after reading, and this PR's own test installNoticeRoundTrip asserts a second consume()
returns nil. But RecoveryStore copies the message into installerNotice and never clears it,
while refresh() does notice = installerNotice ?? "Release catalog refreshed…". So every
refresh for the rest of the session replays the stale install message and suppresses the real
refresh confirmation — contradicting the contract the PR itself tests.

Clearing it after the first refresh preserves the intent (the first refresh after a helper install
can't bury the result) while restoring the one-shot semantics. RecoveryStore is a
@MainActor final class, so the assignment is main-actor isolated exactly like the surrounding
notice/desktopReleases writes — no new concurrency surface.

Happy to open that as its own PR against develop on request.