Skip to content

fix(ios): connection, deep-link and privacy fixes from the app review - #278

Merged
schiste merged 6 commits into
mainfrom
agent/ios-app-full-code-review-optimization-im
Oct 23, 2026
Merged

schiste merged 6 commits into
mainfrom
agent/ios-app-full-code-review-optimization-im

Conversation

@schiste

@schiste schiste commented Oct 23, 2026 •

Copy link
Copy Markdown
Member

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)

  • A silent push that arrives while the app is active no longer switches the client to approvals-only streaming, so the visible terminal stays live.
  • PAIR_REJECT or a Mac identity mismatch now stops the HELLO/PAIR_REQUEST loop and closes the socket. Before, the loop kept sending the pairing code, and the later handshake timeout replaced the Re-pair prompt with Retry.
  • Approval cards whose socket closed mid-send go back to "Queued" instead of staying on "Sending".
  • A locked Keychain at launch (a push wake before the first unlock) is no longer treated as a missing key. The device key is minted only when it is truly absent, and the key and pairing reload on the next foreground or push wake.
  • The pairing is saved only when it changes, Keychain write failures are logged, and push-wake telemetry sends only request_id and prompt_id.

Deep links and Live Activities (fix(ios): open approvals from links instead of answering them)

  • chau7remote://approve|deny used 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 new review action, and the legacy hosts map to it. The widget shows a single Review button.
  • Notification taps take the same path, which fixes the cold-launch race where the .openApprovals post had no observer yet.
  • A stale Live Activity is updated in place instead of getting a duplicate. Disconnect and pairing changes end it. Dead iOS 16.1 availability checks are removed (the deployment target is iOS 18).

Privacy hardening (fix(ios): protect pairing secrets and diagnostics on the device)

  • The Re-pair sheet no longer pre-fills the stored pairing, which contains the relay secret, into a visible editor.
  • A pairing code whose expiry cannot be parsed is rejected. Before, the check was skipped and the code passed.
  • 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.
  • The QR scanner starts and stops on one serial queue and always stops on disappear, so the camera no longer stays on.

Risks

  • Answering from the lock screen now takes one extra tap. This was a deliberate product choice.
  • Not exercised on a device: widget rendering, the lock-screen tap, the camera lifecycle, and the locked-Keychain launch. The pure decisions behind each are unit-tested.

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

schiste and others added 3 commits October 23, 2026 15:44
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 schiste changed the title fix(ios): keep connection and approval state honest across failures fix(ios): connection, deep-link and privacy fixes from the app review Oct 23, 2026
@schiste
schiste marked this pull request as ready for review October 23, 2026 15:43
schiste and others added 3 commits October 23, 2026 17:52
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
@schiste
schiste merged commit 2e54834 into main Oct 23, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant