[brennanb2025]

Summary

Fixes the "Remove does not delete" half of #8791: removing (or renaming) a host committed to disk, but the home screen could immediately repaint the pre-write list, so the removed host's card came back.

Root cause: loadHosts() de-duplicates concurrent calls by handing back the in-flight pass. That de-dupe defeats the store's own await hostListMutation invariant — a pass that started before a write still holds the pre-write snapshot, and a load issued after the write commits (exactly the shape of handleRemove in mobile/app/index.tsx) was served that stale snapshot. Now every durable host write drops the shared pass (mobile/src/transport/host-list-load-sharing.ts), so the next load reads fresh; concurrent loads with no write between them still share one Keychain pass.

Note on scope: the freeze half of #8791 ("app frozen after tapping Remove") needs no code change — the host action sheet already sets closeBeforePress on Edit host + Remove since #8536. This PR extracts that sheet into mobile/src/host-list-action-sheet-actions.ts so the deferral is pinned by tests and can't silently regress.

What the user sees before and after

Before

Removing or renaming a host writes the change, but the home screen can reload an older snapshot. A removed host card then reappears, making it look like removal failed.

After

The next host-list refresh reads the newly saved state, so removed hosts stay gone and renamed hosts retain their new names.

Simulator validation

Manual iOS Simulator A/B on Lane A device PR 8647 QA iPhone: true merge-base 5d88c1093fd8a75c4e1c3a86c34bf4f704a9d6e8 versus exact head fed029f1f6da37c0a45912afff3a228a18303347.

Before — parent resurrects the removed card

Durable storage was already orca:hosts=[], but the post-remove refresh reused the parked pre-write result and repainted Host 1 as disconnected.

After — exact head keeps the card removed

The same parked old pass resolved with the former host id, while the post-write refresh read orca:hosts=[]; the card stayed gone beyond 15 seconds.

Testing

• pnpm lint
• pnpm typecheck
• pnpm test
• pnpm build (not run: mobile-only change, mobile/ has no build script; Mobile Checks CI covers it)
• Added or updated high-quality tests that would catch regressions, or explained why tests were not needed

Run in mobile/ on node 24. Baseline at parent commit: 351 files / 2587 tests → with this change: 353 files / 2601 tests (+14). Non-vacuity proven both ways: the new host-store tests park a load mid-Keychain-read, commit a removal/rename underneath it, then assert a load issued after the write never sees the pre-write snapshot — they fail on the parent commit and pass here; the shared-pass test asserts de-dupe still works when no write intervenes.

AI Review Report

Reviewed the stale-snapshot race end to end: writer paths (mutateStoredHosts, persistHost relay-upgrade + overlay-removal branches, removeHost overlay cleanup) all drop the shared pass after their durable commit; the late-settle case (a dropped pass resolving after its replacement started) is handled by only retiring the pass still on offer, with a test pinning it. Checked that the action-sheet extraction is behavior-preserving (same labels, ordering, live-state gating, closeBeforePress placement). Cross-platform: this is React Native (iOS/Android) code only — no desktop shortcuts, labels, paths, shell, or Electron surface touched; no platform-conditional logic involved.

Security Audit

No new input handling, command execution, path handling, auth, or IPC. The change only affects when an in-memory promise cache is invalidated; credential reads still go through the existing SecureStore path and token cache, and write serialization via hostListMutation is unchanged. No new dependencies.

Notes

Residual (pre-existing, not addressed here): MobileEndpointSupervisor persists via plain saveHost (mobile/src/transport/mobile-endpoint-lifecycle.ts:92), which appends a host that was removed mid-flight instead of throwing like saveExistingHostRelayUpgrade does — a background relay upgrade racing a removal can still resurrect the host through that path.

[coderabbitai]

📝 Walkthrough

Walkthrough

The change extracts host action-sheet construction into getHostListActionSheetActions and updates the home-screen host modal to use it. It adds tests for connection-aware actions, callbacks, dismissal behavior, and navigation wiring. Host loading now uses shared in-flight state with revision-based invalidation. The host store invalidates that state after host and overlay changes and guards token-cache updates against stale reads. Additional tests cover concurrent loads and stale snapshots.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly identifies the mobile stale-snapshot fix that is the primary change in the pull request.
Description check	✅ Passed	The description covers the required summary, validation, tests, AI review, security audit, and notes, with a clear reason for not running the build.

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[greptile-apps]

Greptile Summary

This PR fixes the "removed host reappears" half of issue #8791 by invalidating the shared Keychain-read promise whenever a durable host write commits, and guarding the token cache with a revision counter to prevent stale tokens from surviving a write. It also extracts the home-screen host action-sheet items into a separate function (getHostListActionSheetActions) so that the closeBeforePress deferral on Edit and Remove is pinned by tests and cannot silently regress.

• host-list-load-sharing.ts (new): Splits the in-flight-promise deduplication into its own module; dropSharedHostListLoad() atomically clears the shared pass and increments a revision counter after every durable write, ensuring the next loadHosts() always reads fresh storage. A finally guard prevents a late-settling dropped pass from evicting a replacement pass that has already started.
• host-store.ts: Calls dropSharedHostListLoad() after each async write step in mutateStoredHosts, persistHost, and removeHost; the token cache is protected with a revision check so a pre-write Keychain read cannot poison it after a write completes.
• host-list-action-sheet-actions.ts (new) + index.tsx: Extracts the action-sheet builder; comprehensive tests now assert that Edit and Remove carry closeBeforePress: true and that their callbacks are wired correctly.

Confidence Score: 5/5

Safe to merge — the fix is targeted, well-tested, and the core invariant (a post-write load never reuses a pre-write promise) is proven by tests that fail on the parent commit and pass here.

All writer paths drop the shared pass after their durable commit, the late-settle guard preserves any replacement pass already in flight, and the revision counter closes the token-cache poisoning window. The action-sheet extraction is behavior-preserving with no logic changes. No regressions were identified.

Files Needing Attention: No files require special attention.

Important Files Changed

Filename	Overview
mobile/src/transport/host-list-load-sharing.ts	New module extracting the shared-load pass into its own unit; the finally guard correctly avoids retiring a replacement pass when a dropped load settles late, and the revision counter prevents stale tokens from entering the cache.
mobile/src/transport/host-store.ts	Integrates hostListLoads.dropSharedHostListLoad() after each durable write step and guards token-cache updates with a revision check; the multi-drop pattern in persistHost is intentional (each async write step earns its own invalidation).
mobile/src/transport/host-list-load-sharing.test.ts	Three new tests covering concurrent sharing, post-write freshness, and late-settle-of-dropped-pass; all three cases were explicitly mentioned as required non-vacuity checks in the PR description.
mobile/src/transport/host-store.test.ts	Five new integration tests that park a load mid-Keychain-read, commit a write underneath it, then assert post-write loads never see the pre-write snapshot; also covers token-cache poisoning and pass-sharing preservation.
mobile/src/host-list-action-sheet-actions.ts	Extracts action-sheet item construction; closeBeforePress: true placement and onDismiss call sites match the original inline code exactly, preserving behavior.
mobile/src/host-list-action-sheet-actions.test.ts	Comprehensive tests pin closeBeforePress on Edit and Remove, label logic, callback wiring, and null-host guard; well-structured and covers the critical iOS-freeze regression path.
mobile/app/index.tsx	Replaces the inline IIFE that built action items with a call to getHostListActionSheetActions; removes now-unused icon imports and a now-unneeded eslint-disable comment.
mobile/src/home-host-edit-navigation-source.test.ts	Source-text test that asserts the cold-navigator-safe navigateToMobileHostEdit call is wired through the new onEdit prop; an intentional change-detector to prevent silent regression.

Sequence Diagram

sequenceDiagram
    participant UI as Home Screen
    participant HS as host-store
    participant LLS as host-list-load-sharing
    participant KC as Keychain / AsyncStorage

    Note over UI,KC: Pre-write: concurrent mounts share one pass
    UI->>HS: loadHosts() [call A]
    HS->>HS: await hostListMutation (resolved)
    HS->>LLS: shareHostListLoad(doLoadHosts)
    LLS->>KC: doLoadHosts() starts Keychain reads
    LLS-->>HS: inflight promise (new)

    UI->>HS: loadHosts() [call B, concurrent]
    HS->>LLS: shareHostListLoad(doLoadHosts)
    LLS-->>HS: inflight promise (same, shared)

    Note over UI,KC: Write commits
    UI->>HS: removeHost(hostId)
    HS->>KC: AsyncStorage.setItem (mutateStoredHosts)
    HS->>LLS: "dropSharedHostListLoad() inflight=null, revision++"

    Note over UI,KC: Post-write load must NOT reuse pre-write pass
    UI->>HS: loadHosts() [call C, after write]
    HS->>HS: await hostListMutation (write settled)
    HS->>LLS: shareHostListLoad(doLoadHosts)
    LLS->>KC: doLoadHosts() fresh Keychain reads
    LLS-->>HS: new inflight promise (fresh)

    Note over LLS,KC: Pre-write pass settles late
    KC-->>LLS: stale snapshot resolved
    LLS->>LLS: "finally guard: inflight != started, skip"
    LLS->>LLS: revision guard: skip tokenCache.set

Loading

_{Reviews (4): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile}

[brennanb2025]

Re the two Greptile P2 observations — both are accurate readings, and both are deliberate consequences of the module invariant stated on dropSharedHostListLoad: every durable host write drops the shared pass, with no exceptions.

1. Drop inside removeHost's overlay-removal try: redundant today, agreed — loadMobileRelayHostOverlayState filters overlays by the host-id set already committed to AsyncStorage, so the earlier drop in mutateStoredHosts suffices. But removeMobileRelayHostOverlay is a durable write, and omitting the drop there would couple this call site's correctness to doLoadHosts' current overlay-filtering behavior (the same overlay writes in persistHost do affect hosts still on the list). The uniform rule is cheap and survives refactors; special-casing it is the misleading option.

2. updateLastConnected evicting the pass on every connection event: intended. lastConnected is durable data served by loadHosts(), so a load issued after that write should read fresh — same invariant as remove/rename. The drop itself does no work; the only cost is a mount burst racing a connection event paying one extra Keychain pass, and sharing within any burst that starts after the drop is unaffected.

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

mobile/src/transport/host-store.ts (1)

128-143: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Capture the revision once per load pass.

Because readRevision is captured inside the host loop, an old doLoadHosts() snapshot can reach a host after a write has already advanced the revision, capture the new revision, and cache a pre-write token. A later load can then reuse that stale tokenCache entry. Capture the revision at the start of doLoadHosts() and compare every token-cache update against that pass-level revision.

Suggested fix

 async function doLoadHosts(): Promise<HostProfile[]> {
+  const loadRevision = hostListLoads.getHostListLoadRevision()
   const raw = await AsyncStorage.getItem(STORAGE_KEY)
   ...
     let token = tokenCache.get(stored.id)
     if (!token) {
-      const readRevision = hostListLoads.getHostListLoadRevision()
       ...
-      if (readRevision === hostListLoads.getHostListLoadRevision()) {
+      if (loadRevision === hostListLoads.getHostListLoadRevision()) {
         tokenCache.set(stored.id, token)
       }

Add a regression test where a multi-host load is paused before a later token read, a host mutation invalidates the pass, and the late read must not repopulate tokenCache.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 9ec8c671-ce5e-413d-b6d7-f85f1f99afc7

📥 Commits

Reviewing files that changed from the base of the PR and between 421c8a4 and cb45a3d.

📒 Files selected for processing (4)

• mobile/app/index.tsx
• mobile/src/transport/host-list-load-sharing.ts
• mobile/src/transport/host-store.test.ts
• mobile/src/transport/host-store.ts

💤 Files with no reviewable changes (1)

• mobile/app/index.tsx

🚧 Files skipped from review as they are similar to previous changes (1)

• mobile/src/transport/host-store.test.ts