Repository navigation
fix(ios): connection, deep-link and privacy fixes from the app review - #278
Merged
Merged
Conversation
Problem: Five connection paths left the client in a wrong state. - A silent push that arrived while the app was on screen switched it to background mode. Streaming stopped and the visible terminal froze until the next inactive/active cycle. - PAIR_REJECT and a Mac identity mismatch disabled reconnects but left the handshake loop sending HELLO and PAIR_REQUEST, which carries the pairing code, once a second to that peer. After 12 s the handshake timeout replaced the Re-pair error with a retryable one. - When a socket closed mid-send, the approval cards showed "Sending" indefinitely, because the fenced completion never resolved them. - A Keychain read that failed while the device was still locked (a push wake before the first unlock) looked like a missing key. The client minted a throwaway identity and ran unpaired for the whole process. - Every connect attempt re-wrote the unchanged pairing to the Keychain, and push-wake telemetry forwarded the whole APNs payload to the Mac. Decision: - Only a push that reaches an inactive app enters background mode (RemoteConnectionStartPolicy.pushWakeEntersBackground). - A broken pairing now stops the handshake and closes the socket before the failure is reported. - ApprovalCoordinator.resetInFlight returns the answers it releases, and the client moves those cards back to "Queued". - KeychainStore.read distinguishes found, not found and unavailable. The device key is minted only when it is truly missing, and the client reloads the key and pairing on the next foreground or push wake. - Save the pairing only when it changes, log Keychain write failures, and send only request_id and prompt_id in push-wake telemetry. Rationale: Each decision is a small pure policy in a file the host-less test bundle already compiles, so the behaviour is tested without the app host. RemoteClient keeps only the side effects. Validation: xcodebuild test, Chau7RemoteApp scheme, iPhone 17 Pro simulator: 130 tests, 0 failures. New tests cover push-wake gating, released in-flight answers and the device-key decision for found, missing, corrupt and locked reads. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Problem: The Live Activity's Approve and Deny buttons opened chau7remote://approve?request_id=… and the app answered the request at once, with no confirmation. That skipped the hold-to-confirm gate on destructive commands, offered Approve even when lock-screen details were redacted, and let any app or web page that knew a request id answer it. Queued link actions never expired, so an approve could fire minutes later once /pending listed the id. Separately: - After 120 s without an update an activity went .stale, and the next update requested a second card instead of updating the first. - An explicit Disconnect or a pairing change left the card on screen until the 8-hour cap. - A notification tap on a cold launch was posted before the root view subscribed, so it never opened Approvals. Decision: - RemoteActivityURLAction gets a review case. The review, approve and deny hosts all map to it, and it only navigates. The widget shows one Review button. - RemoteClient.approvalsNavigation records the request. The root view switches tab, and ApprovalsView selects the card (waiting for it if /pending has not delivered it yet). Notification taps use the same path, which removes the .openApprovals post and the unused approvalNotificationResponse bridge. - Queued tab navigation keeps only the newest link. - Live Activities treat .stale as updatable, snapshot existing activities before any request, and end every activity except the kept one. disconnect(discardUserIntent:) ends the activity. - Drop the iOS 16.1 availability checks, which are dead at the iOS 18 deployment target. Rationale: The product owner chose confirming in the app over one-tap answers with a destructive-only gate: one extra tap keeps every decision behind the UI the in-app card already enforces. Validation: xcodebuild test, Chau7RemoteApp scheme, iPhone 17 Pro simulator: 133 tests, 0 failures. swift test --filter RemotePureUnitTests: 20 tests, 0 failures. New tests cover legacy link mapping, the activity-state policy and the navigation targets. Not exercised on a device: the widget rendering and the lock-screen tap. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Problem: - The Re-pair sheet JSON-encoded the stored pairing, including the relay secret and pairing code, into a visible text editor, where screenshots and screen recordings could capture it. - PairingPayloadValidator skipped the expiry check when expires_at could not be parsed, so a malformed code passed. - The diagnostics log, which can hold typed terminal input, was included in device backups, and every export left a copy in the temporary folder. The export filename formatter used the user's calendar. - The QR scanner skipped stopRunning while a start was still in flight, and its start and stop ran on a concurrent queue, so the camera could stay on after the sheet closed. Decision: - Drop the Re-pair prefill. - Move PairingPayloadValidator next to RemotePairingStore, where the test bundle can reach it. It now rejects an unreadable expiry and takes an injectable now. - Mark diagnostics.jsonl as excluded from backup after every full write. - Route both exports through DiagnosticsExportFile. It empties one export folder first, writes with complete file protection, and uses a POSIX Gregorian timestamp. - Give the scanner one serial session queue. It always enqueues the stop and only starts while the view is visible. Rationale: Each fix is local to the type that owns the data, and none changes what the user sees beyond the empty Re-pair editor. Validation: xcodebuild test, Chau7RemoteApp scheme, iPhone 17 Pro simulator: 136 tests, 0 failures. New tests cover fresh, expired and unreadable-expiry pairing payloads. Not exercised on a device: the camera lifecycle. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
schiste
marked this pull request as ready for review
October 23, 2026 15:43
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…au7Core CI now installs SwiftFormat 0.64.0, whose new rules flag these two files on every Chau7Core change. The edits only merge conditions; behaviour is unchanged (swift test: 2384 tests, 0 failures). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…de-review-optimization-im # Conflicts: # CHANGELOG.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Correctness and security fixes in the iOS companion app, found during a full review of
apps/chau7-ios. There are three commits, each independently tested.Changes
Connection and approval state (
fix(ios): keep connection and approval state honest across failures)request_idandprompt_id.Deep links and Live Activities (
fix(ios): open approvals from links instead of answering them)chau7remote://approve|denyused to answer the request with no confirmation. That skipped the hold-to-confirm gate on destructive commands, and any app or web page could trigger it. Links now only open the matching approval card through a newreviewaction, and the legacy hosts map to it. The widget shows a single Review button..openApprovalspost had no observer yet.Privacy hardening (
fix(ios): protect pairing secrets and diagnostics on the device)diagnostics.jsonl, which can hold typed input, is excluded from backups. Exports reuse one temporary folder that is emptied before each export, and use a POSIX timestamp.Risks
Test plan
xcodebuild test, Chau7RemoteApp scheme, iPhone 17 Pro simulator: 136 tests, 0 failures (baseline on main: 125).swift test --package-path apps/chau7-ios/Chau7Core --filter RemotePureUnitTests: 20 tests, 0 failures.🤖 Generated with Claude Code