Skip to content

feat(mobile): keep machine connections in a Zustand store and cache the last status per machine - #1551

Merged
janicduplessis merged 5 commits into
mainfrom
feat/1543-mobile-status-store
Sep 26, 2026
Merged

janicduplessis merged 5 commits into
mainfrom
feat/1543-mobile-status-store

Conversation

@janicduplessis

Copy link
Copy Markdown
Collaborator

Description

The phone app kept every machine connection in one React context, so every status push re-rendered every consumer, even a push that changed nothing. Nothing survived a restart, so a cold launch showed a spinner, or "No machine connected" when the machine was down.

This adds a native module (react-native-mmkv on react-native-nitro-modules), so it needs a new TestFlight build. The runtime fingerprint changes; an OTA update cannot carry it.

Solution

Zustand store for the machine pool (src/lib/machine-store.ts). Each machine has a link (connection, state, home, disconnectedAt), a snapshot (status), and usage, in separate records so a usage poll does not touch status consumers. The MacLink lifecycle components are unchanged apart from writing to the store.

  • shareStatus matches environments by path and keeps the identity of every part equal to the previous push. A push equal to the current status makes no set at all, the common case since perf(status): refresh only log facts on log appends and report activity to the minute #1540.
  • The store derives home's HomeItem[], reusing an item while its env and names are unchanged.
  • WorkspaceRow and DeviceGridTile are memoized and select their own machine state; workspace detail reads its workspace with useWorkspace(macId, path).

MMKV cache, one key per machine (status:<macId> in src/lib/status-cache.ts), without Zustand's persist. - Written at most every 5 s, and at once when the connection drops or the app leaves the foreground.

  • Read synchronously when the store is created; cached rows show dimmed with "Last seen" (the existing offline dimming) until the first live push.
  • Bounded: an entry over 1 MB is not stored, and a successful pairing read removes entries of machines no longer paired, so Forget clears the key.

Where this differs from the agreed design:

  • The entry stores the machine's name, so cached rows render before SecureStore returns the pairing list.
  • seenAt is when the status was last known current (write time while connected, or the drop time), not when it was received; with rare pushes, receive time would make an open connection look stale.
  • disconnectedAt is not seeded from the cache, so a cold launch does not flash an Offline attention item while connecting. The attention strip and recents ignore a still-cached status, as before (no status until the first push).

AGP 9 patches (remove once upstream ships). React Native 0.88 uses AGP 9.2.1, which registers the Kotlin plugin itself; react-native-mmkv@4.3.2 and react-native-nitro-modules@0.37.1 apply org.jetbrains.kotlin.android unconditionally, which fails Gradle configuration. The pnpm patches guard that line like the open upstream fixes, margelo/react-native-mmkv#1090 and margelo/nitro#1579. Both packages are pinned exactly for patchedDependencies. patches/ is exempted in the release QA matrix, like apps/mobile.

Test plan

Re-renders per status push. I added uncommitted per-component render counters and counted renders in the 120 ms after each push. The run used the iOS simulator against pnpm run mock-server (15 workspaces, default Live filter, 4 rows). For the "one workspace changed" case, a local mock tweak changed one workspace's errorsSinceMarker on every push.

Case main this PR
Home, identical push 31 0
Home, one workspace changed 32 9
Workspace detail (home mounted below), identical push 46 0
Detail, the viewed workspace changed 47 18
Detail, another workspace changed not measured (main re-renders the detail on every push) 10

A push that lands with a usage poll adds 3 (machine chip and attention strip).

Unit tests:

  • status-share.test.ts: an identical push returns the previous status; one changed environment keeps the others' identity; path matching survives reorder and insert; items are reused.
  • status-cache.test.ts: round trip, removal of unreadable entries, the size bound, keepOnly.
  • machine-store.test.ts: hydration before the pairings load, no set on an identical push, the debounced write and its seenAt, the immediate write on a drop, no write-back of a status that is still cached, and pruning, including no pruning after a failed pairing read.

Devices (dev client built with this branch; mock server on its own port, paired with dev:pair --mock):

  • iOS simulator (iPhone 18 Pro, iOS 27.0):
    • With the mock stopped and the app terminated and relaunched, the first app frame after the splash shows the cached rows, dimmed with "Last seen <1m ago", and the chip shows Offline.
    • Starting the mock replaces them with live, undimmed rows.
    • Forget clears the key: getAllKeys() on the MMKV instance is [] after the next reload.
    • Home, workspace detail, device grid, logs and machines view look as before.
  • Android emulator (AGP 9 build with the patches):
    • The cold launch with the mock stopped shows the cached, dimmed rows, and reconnecting replaces them.
    • Workspace detail and the device viewer work.
    • I did not repeat the Forget check on Android; the path is JS-only.
  • Not a regression: Devices grid tiles stay black against the mock server; main does the same after a full reload.

Fixes #1543

@janicduplessis janicduplessis left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fresh review of #1551 (issue #1543). Local tsc --noEmit, jest (142 passed), oxfmt --check and expo lint pass in apps/mobile. Required CI is red, and both failures come from this PR.

Bugs

  1. Windows CI: pnpm cannot parse the new patches. In test (windows), setup-stim fails with ERR_PNPM_INVALID_PATCH ... react-native-nitro-modules@0.37.1.patch ... invalid char in unquoted filename. The repo has no .gitattributes, so the Windows checkout converts patches/*.patch to CRLF, and pnpm reads the \r as part of the file name. Fix: add a .gitattributes with patches/*.patch text eol=lf (or -text).

  2. test/e2e/release-prep.e2e.js fails on Linux (node 22 and node 24). The failing test is "a bump rewrites all six versions and leaves the lockfile alone", with Failed to read patch file /tmp/stim-release-prep-*/patches/react-native-mmkv@4.3.2.patch. makeWorkspace() (around lines 29-43) copies pnpm-workspace.yaml and pnpm-lock.yaml into a temp root, and both now reference patches/, but the directory is never copied. Fix: add cpSync(join(REPO, 'patches'), join(root, 'patches'), { recursive: true }) to makeWorkspace. The real scripts/release-prep.mjs runs in the repo root, so a real release is not affected, but CI stays red until the fixture copies the directory.

Risks

  1. "Last seen" can describe old cached data as fresh (apps/mobile/src/hooks/mac-connection.tsx, useMachinePresence; apps/mobile/src/lib/attention.ts:51). lastSeenAt is link.disconnectedAt ?? cachedSeenAt. Take a machine whose cache is hours old. On cold launch it connects (wasOpen becomes true) and then drops before its first status push. disconnectedAt becomes now while the rows still show the cached status. The rows then read "Last seen <1m ago", activity and build chips freeze at the drop time, and the attention item says the same. write() skips this snapshot because cachedSeenAt !== null, so the disk entry stays correct and only the display is wrong. The window is small because the server pushes status on subscribe. Fix: prefer the cache time while the snapshot is still cached, i.e. lastSeenAt: cachedSeenAt ?? link?.disconnectedAt ?? null, and in attention mac.seenAt ?? mac.disconnectedAt.

  2. Cold launch can briefly show machines the filters hide (apps/mobile/src/screens/home.tsx, macIds from macs ?? []). Until the pairings load, macIds is []. filterWorkspaces then drops every filters.macs entry, so the cached rows and tiles of machines excluded by the machine filter show until listMacs resolves. Before this PR the list was empty in that window, so this is new. Fix: while macs === null, build macIds from the snapshot ids (the keys workspaces is built from).

  3. The patches release-QA exemption covers the whole directory (scripts/release-qa-matrix.data.mjs). The rule exempts all of patches/ as "patches of Expo mobile app dependencies". A later pnpm patch to a dependency of a published package would also be exempt without anyone noticing. Suggestion: reword the exemption so reviewers check it when a patch targets a published package, or keep it and note that constraint in the PR.

Nits

  1. apps/mobile/src/lib/attention.ts:20: seenAt?: number | null is optional while disconnectedAt is required. Making it required keeps callers from silently leaving it out.
  2. apps/mobile/src/lib/status-cache.ts: the doc comment /** The subset of react-native-mmkv's MMKV the cache uses. */ restates the interface. Under the comment policy it can go.
  3. apps/mobile/src/lib/status-share.test.ts, "treats an added or removed field as a change": the array assertion share([1, 2], [1, 2, 3])).toEqual([1, 2, 3]) checks only the value, so it passes whether or not identity is handled. .not.toBe(prev) on a named prev would test the identity contract.

Checked, no issue found

  • The share/shareStatus merge: it keys environments by path, handles added and removed keys through hasOwn, and returns prev for a deep-equal push. receiveStatus makes no set for an equal live push, and a cached-to-live transition keeps the same workspaces array.
  • The selectors return stable references: raw record entries, IDLE_LINK, or useShallow over primitives. None of them builds a new object on every call.
  • DeviceGridTile's comparator can skip tile.device because the device comes from item.env.
  • onOpen and onAspect are stable callbacks.
  • The write, debounce and prune paths behave correctly. On forget, setMacs prunes the snapshot before MacLink unmounts, so removeLink finds nothing to write back.
  • Recents and the attention strip ignore a cached status. Read-only scope and the settings screens are unchanged.
  • The AGP 9 guards match the open upstream PRs margelo/react-native-mmkv#1090 and margelo/nitro#1579, and the YAML comment that links them is allowed by the comment policy.

@janicduplessis janicduplessis left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review of d2f188d. It resolves all 8 earlier findings, but one new format failure keeps CI red.

Still open

  1. test (node 22) and test (node 24) fail at "Format check (oxfmt)" in scripts/release-qa-matrix.data.mjs:330. The new exempt string is too long for one line; oxfmt breaks it after the key:
        exempt:
          'pnpm patches of Expo mobile app dependencies; a patch of a published package dependency needs its own rule',
    Run pnpm run format (or oxfmt scripts/release-qa-matrix.data.mjs) and push. Those two jobs stop at this step, so their unit and e2e steps did not run on this commit.

Verified fixed

  • .gitattributes patches/*.patch text eol=lf: the blobs are already LF, so a fresh Windows checkout keeps them LF. test (windows) was still pending when I checked.
  • The release-prep fixture now copies patches/.
  • lastSeenAt now prefers the cached time, in both useMachinePresence (cachedSeenAt ?? disconnectedAt ?? null) and attention (seenAt ?? disconnectedAt).
    • Once a live push lands, cachedSeenAt is null and the drop time is used again. The usual path, live then dropped, is therefore unchanged.
    • A link that opens and drops before its first push now shows the cache time.
  • Home macIds comes from the items' machine ids while the pairings load. The machine filter now applies to cached rows at cold launch.
    • A cached machine with no environments has no items. Its id is left out, but it also has no rows to filter.
    • If the only selected machine has no rows, the filter shows every machine. That is the same as main's behavior for a selected machine that is not paired, so it is not a regression.
    • After the pairings load, macIds comes from macs as before. The items dependency only costs a recompute per push while the pairings are loading.
  • seenAt is required, the restating comment is gone, and the share array test now checks identity in both directions.

In apps/mobile, tsc --noEmit and jest pass (142 tests). The root pnpm run format:check fails only on the file above. With that line reformatted I have nothing else open.

@janicduplessis
janicduplessis marked this pull request as ready for review September 26, 2026 17:17
@janicduplessis
janicduplessis merged commit 49df817 into main Sep 26, 2026
9 checks passed
@janicduplessis
janicduplessis deleted the feat/1543-mobile-status-store branch September 26, 2026 17:17
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.

mobile: move the machine pool to a Zustand store and cache the last status per machine

1 participant