Bound the duplicate-app Spotlight scan and take it off the main actor

[malpern]

Summary

Opening Settings → Advanced could freeze KeyPath behind a spinner indefinitely. Observed on a real install: the debug log stopped mid-validation and a single mdfind kMDItemFSName == 'KeyPath.app'c child ran for 70s+; killing it unblocked the app instantly (the same query run by hand returned in 0.18s).

Two defects compounded:

1. HelperMaintenance.detectDuplicateAppCopies() ran mdfind with an unbounded waitUntilExit(), bypassing SubprocessRunner and its timeout.
2. Every caller invoked it synchronously from main-actor UI code (AdvancedSettingsTabView .task, SettingsView.copyPublishedStatus(), both wizard pages' .onAppear), so a slow Spotlight query froze the whole window.

Changes

• detectDuplicateAppCopies() is async and runs mdfind through SubprocessRunner with a 2s timeout. Timeout, failure, non-zero exit, or empty output fall back to the canonical install locations, and every fallback path now logs.
• The canonical fallback returns only locations that exist (it previously returned all three hardcoded paths when none existed, which read as phantom duplicates).
• Settings/Advanced/wizard callers await it. The wizard pages use .task so a dismissed page cancels its scan (and SubprocessRunner kills the mdfind). copyPublishedStatus() stays synchronous — its validation observer contract (SettingsStatusValidationLintTests) is unchanged — and folds the scan in from a follow-up task, throttled to once a minute.
• /Sparkle_generate_appcast/ staging copies are excluded from the duplicate report.
• Bug note: docs/bugs/2026-09-21-duplicate-app-scan-mdfind-hang.md.

Installer/repair matrix: no row affected — duplicate detection is diagnostic only (it logs a warning); no repair decision depends on it.

Test plan

• swift build (Xcode 27 pin)
• New: HelperMaintenanceTests.testDetectDuplicateAppCopiesUsesBoundedSpotlightScan, testDetectDuplicateAppCopiesFallsBackWhenSpotlightTimesOut
• New: SubprocessRunnerTests.testRealRunnerTimeoutTerminatesLongRunningProcess — real runner vs /bin/sleep 10 with a 0.5s budget (returns in ~0.52s)
• Targeted: HelperMaintenanceTests|InstallerEngineEndToEndTests|SettingsStatusValidationLintTests|SubprocessRunnerTests — 47 passed
• Full gate (./Scripts/test-full.sh): KeyPathTests 4292 tests, 0 failures. KeyPathSnapshotTests: 3 failures, pre-existing on master and unrelated — EasyViewSnapshotTests.testHomeRowTiming{Slider,PerFinger,FastTyping} render 1300×586 vs a 1300×638 reference; the references date from 2026-06-07 (Prefer privileged helper in debug app builds #803) and HomeRowTimingSection changed layout on 2026-07-25 (Make home row timing mode-aware #1222). This PR does not touch that view.

Review gate

./Scripts/review-gate.sh ran locally (exit 0). Dispositions:

1. Phantom duplicates from the canonical fallback — fixed.
2. .onAppear + Task leaks the scan past view teardown — fixed (.task).
3. Rescan on every validation publish — fixed (60s throttle).
4. Real timeout path untested — fixed (real-runner test above) + assertion on the Sparkle exclusion.
5. Banner warm-up window — documented at refreshDuplicateAppCopies.
6. Silent non-zero/empty fallbacks — fixed (logged).
7. WizardSignatureHealthCheck.isRunningAdHoc() runs codesign with waitUntilExit() — same anti-pattern, out of scope; worth a follow-up.
8. @MainActor protocol satisfied by a nonisolated async method — builds clean under Swift 6 language mode (-swift-version 6), no new warnings.

🤖 Generated with Claude Code

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-21T14:35:33.337115Z	b7bd0ff	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[claude]

One test-coverage concern in this diff:

Tests/KeyPathTests/Managers/HelperMaintenanceTests.swift — testDetectDuplicateAppCopiesFallsBackWhenSpotlightTimesOut can pass vacuously.

let copies = await HelperMaintenance.shared.detectDuplicateAppCopies()
XCTAssertTrue(copies.allSatisfy { $0.hasSuffix("/KeyPath.app") })
XCTAssertTrue(copies.allSatisfy { FileManager.default.fileExists(atPath: $0) })

canonicalAppCandidates() was changed to return only the canonical paths that actually exist on disk (return candidates, no longer candidates.isEmpty ? defaults : candidates). So in a CI/test environment where none of the hardcoded canonical locations (/Applications/KeyPath.app, etc.) are present, the timeout fallback returns [], and both allSatisfy checks on an empty array trivially succeed — the test would pass without ever verifying that the fallback path actually produces a canonical, existing location. That's the exact regression this PR is trying to lock in (docs/bugs/2026-09-21-duplicate-app-scan-mdfind-hang.md).

Suggest adding XCTAssertFalse(copies.isEmpty, ...) (or asserting the count matches whatever canonical paths are known to exist in the test environment) so the test actually fails if canonicalAppCandidates() degrades to empty, rather than silently passing either way.

Everything else in the diff (moving detectDuplicateAppCopies() to async + SubprocessRunner with a bounded timeout, the Sparkle_generate_appcast exclusion, switching wizard pages from .onAppear to .task for cancellation) looks correct and well-covered.