fix(remote): device state: capability race, session without a device id, logout while running

[mihailt]

Stack #818 · 18/20 · base: fix/search-memory · next: codex/tool-call-profiling

Refs #697, #633.

Four bugs in the remote device's state, all on main as well. A device could be shown online while the server refused every call to it (#697's symptom), a device.json with a session but no device id made every start fail, remote --logout didn't stick while the device ran, and a status write that was never answered kept the device from publishing presence again. Capability writes now go through a queue, a session without a device id counts as not logged in, after a logout the running device saves nothing more, and status and capability writes have a time limit.

What this fixes

Problem	Fix
A device was shown online while the server refused every call to it as not broadcast-capable, until its channel rejoined again or the process restarted. #697	Capability writes go through a queue, as status writes already do, so a re-publish is no longer skipped while a withdrawal is still in flight.
A device.json with a session but no device id made every start fail with "Device not found: undefined".	A saved session without a device id is ignored.
That start exited 1 and never asked to log in, so under systemd's Restart=always it repeated on every restart.	The device goes to its normal device-code authorization.
remote --logout didn't stick while the device ran: the device wrote device.json back at its next token refresh, when it stopped, at its first save if the logout came while it was starting, or in the middle of a save; and it wrote over the device.json of a login made after the logout.	After a logout the running device keeps its session in memory and saves nothing more (it saves only while device.json still holds what it last loaded or saved); the next start asks to log in. A save's check and write and the logout's removal take turns under a lock on device.json.
After a network drop, a status write sent down a dead connection kept the device from publishing presence again until fetch gave up on it (Node's default: 300 s).	Writes to the device's row (status, capability) are aborted after 10 s and count as failed; the next attempt goes through.

Where to look

• src/remote-device/remote-channel.ts setTransportCapable(), writeTransportCapable(): capability writes queue like status writes. Before, a withdrawal that landed late (after 3 failed recreates, the channel waited only 5 s for it) overrode a re-publish that had been skipped as a duplicate. The capability write and setOnlineStatus() are each aborted after DEVICE_ROW_WRITE_TIMEOUT_MS (10 s), so nothing waits behind one that is never answered. A capability write that fails leaves the row's value unknown, so the next one is not skipped.
• src/remote-device/device.ts loadPersistedConfig(): a saved session without a device id is ignored.
• src/remote-device/device.ts writePersistedConfig(), clearPersistedConfig(): once this run finds device.json removed or replaced by someone else after loading or saving it (its text is no longer what this run last loaded or saved), it saves nothing more; the device's own removal of a revoked device's config isn't taken for a logout. The check and the write run under lockRemoteDeviceConfig() (proper-lockfile, as config-manager.ts uses for config.json), after the session is read.
• src/remote-device/device.ts removeRemoteDeviceConfig(), used by src/npm-scripts/remote.ts: remote --logout removes device.json under the same lock.
• Tests: test/test-remote-transport.js (a re-publish during a slow withdrawal; a capability or status write nothing answers), test/test-remote-device-session-without-id.js (a real device process, against the local stand-in), test/test-remote-device-logout.js (a logout while the device starts, and while it saves; a login after the logout), test/integration/remote-device-restart.js (a logout while the device runs).
• test/helpers/remote-device.js: the device-process helpers moved here unchanged, plus runLogout(), which runs the real remote --logout. test/helpers/remote-stand-in.js holdDeviceLookups(): holds a starting device between loading device.json and its first save.

How to verify

git checkout fix/remote-device-state && npx shx rm -rf dist && node test/run-all-tests.js test-remote-transport.js test-remote-device-session-without-id.js   # passes after
git checkout ee0b4f3 -- src/remote-device/device.ts src/npm-scripts/remote.ts && git checkout fix/search-memory -- src/remote-device/remote-channel.ts
npx shx rm -rf dist && node test/run-all-tests.js test-remote-transport.js test-remote-device-session-without-id.js   # both fail before
git checkout HEAD -- src/remote-device/device.ts src/npm-scripts/remote.ts src/remote-device/remote-channel.ts
git checkout 4f87de8 -- src/remote-device/device.ts src/npm-scripts/remote.ts && npx shx rm -rf dist && npm run build && node test/integration/run-all-integration-tests.js remote-device-restart.js   # the logout case fails before
git checkout HEAD -- src/remote-device/device.ts src/npm-scripts/remote.ts
git checkout 7c5a944 -- src/remote-device/device.ts src/npm-scripts/remote.ts src/remote-device/remote-channel.ts
npx shx rm -rf dist && node test/run-all-tests.js test-remote-device-logout.js test-remote-transport.js   # 4 logout cases and 2 transport cases fail before
git checkout HEAD -- src/remote-device/device.ts src/npm-scripts/remote.ts src/remote-device/remote-channel.ts
npx shx rm -rf dist && node test/run-all-tests.js test-remote-device-logout.js test-remote-transport.js   # passes after
npx shx rm -rf dist && npm run build && node test/integration/run-all-integration-tests.js remote-device-restart.js   # passes after, the logout case included
npm test   # the whole suite

Answers that change

None: tool answers and descriptions are unchanged. After a logout, the running device prints one new line in its own terminal: "🔓 Saved Remote MCP device credentials were removed (remote --logout): this run keeps its session in memory and won't save it again", or, when a login after the logout has saved its own device.json, "🔓 Saved Remote MCP device credentials were replaced (a login after remote --logout): this run keeps its session in memory and won't save it again". A row write that is aborted logs its failure there, as other failed writes do ("Failed to update device status: TimeoutError: …"). If another process holds the device.json lock for longer than about 10 s (a stuck process; a save holds it for milliseconds), remote --logout ends with exit 1: "❌ Failed to remove saved Remote MCP credentials: Lock file is already being held".

Commits and test results

Commit	What it does
70ff698	A capability re-publish is not skipped behind a slow withdrawal.
ee0b4f3	The device-process test helpers move to test/helpers (no behavior change).
4f87de8	A saved session without a device id asks to log in.
7c5a944	A logout while the device runs sticks.
4a27ddc	A logout while the device starts sticks.
ece6506	A logout can't land between the device's check and its save (a lock on device.json).
aa01807	A device-row write nothing answers can't hold back presence (status and capability writes aborted after 10 s).
9cd90e5	A login after the logout is not written over by the device that was logged out.

• Full suites at the top of the stack, on main (c774c3b): Windows 11 / Node 24.18: unit 168/168, integration 4/4, repros 19/19. macOS 26.6.2 / Node 24.15: unit 168/168, integration 4/4, repros 19/19. Checks skipped for the platform, missing rights or a missing tool: 7 on Windows, 10 on macOS.
• Each fix's test up to 7c5a944 fails on the commit before and passes on its own commit, on Windows 11 / Node 24.18 and macOS 26.6.2 / Node 24.15.
• remote-device-restart.js at 7c5a944: Windows passes, the logout case included and the SIGTERM case skipped (58.5 s); macOS 6/6 cases (81.0 s).
• At 4f87de8 and 7c5a944, 10 of the 12 test-remote-*.js files they had before main gained fix(remote): stop device auth polling on denied/expired codes #783's test-remote-device-auth-poll.js were run, and pass on both OSes.
• Windows 11 / Node 24.18, clean dist/: each of 4a27ddc, ece6506 and aa01807 has its tests fail with the product code before it and pass on it. At aa01807 the 13 test-remote-*.js files it had before main gained fix(remote): stop device auth polling on denied/expired codes #783's test pass 13/13, and remote-device-restart.js passes, logout case included (5 cases, the SIGTERM case skipped as on Windows before; 49.1 s). The real Supabase client aborts a PATCH that is never answered at the set bound (checked with the bound at 300 ms: both writes return failed after 315 ms). On macOS 26.6.2, with the product code from 7c5a944, the 3 logout and 2 transport cases fail; at aa01807 the same 13 test-remote-*.js files pass 13/13 and remote-device-restart.js passes 6/6.
• 9cd90e5 on Windows 11 / Node 24.18, clean dist/: the new logout case fails on the commit before and passes at it, and remote-device-restart.js passes, logout case included (the SIGTERM case skipped, 43.5 s). With the product code from 7c5a944, 4 logout cases and 2 transport cases fail. In the full suites at the top, all 14 test-remote-*.js files pass on Windows and macOS (macOS: logout 4/4).
• Known and not changed:
  • It is unknown how a device.json without a device id arises (older builds, a hand edit, or a server answer).
  • A remote --logout from an older version doesn't take the lock, so it can still land in the middle of a running device's save.
  • A status or capability write aborted after 10 s may still reach the server and land after a later one, as a fetch that fails at 300 s can on main; ordering them needs a row version on the server's device row.
  • The heartbeat's last_seen write has no bound of its own either. Presence and the write queues don't wait for it; only the next heartbeat does.

Stack #818: #781 makes the tests run on Windows and macOS; #770–#768 fix what that exposed; #773–#779 fix the issues listed in each; #780 fixes the remote device's state; #794 reports targeted Broadcast receipts.

🤖 Generated with Claude Code

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: a19bc887-dd82-4dbc-b9eb-0f6f631e376c

📥 Commits

Reviewing files that changed from the base of the PR and between dce47bd and 704e467.

📒 Files selected for processing (2)

• src/remote-device/device.ts
• test/test-remote-device-logout.js

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 3 remain after this review.

---

📝 Walkthrough

Walkthrough

The device now coordinates configuration saves with logout removal and ignores persisted sessions without a device ID. RemoteChannel serializes capability writes and applies timeouts to device-row writes. Tests cover logout races, session loading, and write timeouts.

Changes

Persisted session handling

Layer / File(s)	Summary
Configuration lifecycle and logout src/remote-device/device.ts, src/npm-scripts/remote.ts	Configuration removal and writes now use a shared lock. The device tracks config presence and skips recreating a file after detecting removal or replacement. The CLI logout path uses the removal helper.
Session and logout coverage test/test-remote-device-session-without-id.js, test/test-remote-device-logout.js, test/helpers/remote-device.js, test/integration/remote-device-restart.js	Tests cover sessions without device IDs and logout during startup, locked saves, session reads, and relogin. Shared helpers support process control and the restart integration test.

Remote device-row writes

Layer / File(s)	Summary
Serialize and bound device-row writes src/remote-device/remote-channel.ts	Capability writes are serialized and deduplicated against queued or confirmed values. Capability and status writes use a 10-second abort signal. Failed capability writes clear the confirmed-value cache.
Write timeout and ordering tests test/test-remote-transport.js, test/test-remote-channel-reconnect.js, test/test-remote-device-readiness.js, test/test-remote-device-ready-requires-channel.js, test/helpers/remote-stand-in.js	Regression tests cover delayed and unanswered writes during Presence republication. Test clients support abort-signal chaining, and the remote stand-in can hold device lookups.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant RemoteChannel
  participant DeviceRowAPI
  participant Presence
  RemoteChannel->>DeviceRowAPI: Queue capability write with abort signal
  DeviceRowAPI-->>RemoteChannel: Confirm write or return error
  RemoteChannel->>Presence: Continue publication after queued write
  Presence->>RemoteChannel: Request capability update

Loading

Suggested reviewers: edgarsskore

Merge Risk: 🔵 Low · up to 704e4

The change makes logout stick while a device is running and keeps device-row writes from blocking presence publishing. One reported limitation remains: a timed-out status or capability write can still land late and briefly reverse newer state. Owners should be aware of it, but it does not block merge.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 704e4

The changes strengthen normal logout and credential-save coordination without establishing broader command authority. Remaining uncertainty concerns delayed remote updates and exceptional credential-cleanup paths, with effects concentrated on the affected device.

Retained concerns

• Low · reliability · inferred: The new client deadlines do not establish ordering of committed remote dispatch state. After an aborted request releases the queue, an older capability or status mutation may commit after a newer recovery update. Serialization and cache invalidation help normal execution but do not fence late commits. The resulting device-state drift is inferred; server-side cancellation or ordering guarantees remain unverified.

Security review details

Security Blast Radius

• inferred — The inspected mutations address the active device row and its local credential file. Delivered commands can reach desktop tools with the device process's authority, but the changed paths do not establish increased command privileges or cross-tenant reachability. Server-side enforcement and maximum service-wide exposure remain outside the verified scope.

Trust Boundaries and Controls

• observed — The inspected broadcast path uses a private per-user channel. A doorbell must match the current device ID and causes a device-scoped database claim or read-back; desktop execution receives the retrieved call row rather than tool arguments supplied directly by the doorbell.

Resilience and Maintainability Implications

• observed — The new credential lock logs compromise rather than terminating the operation. Consequently, the normal lock-held ownership guarantee is not demonstrated after lock loss; this is a recovery limitation, not evidence of a newly enabled attacker path.

Hardening Proposals

• proposed — If the server cannot guarantee that aborted mutations will not commit late, consider server-enforced mutation generations or conditional updates so older device-state writes cannot overwrite newer recovery state.
• proposed — Consider extending shared locking and file-identity checks to revoked-device cleanup, and refusing credential mutation after lock compromise, to make exceptional recovery paths follow the same ownership invariant as normal save and logout.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 57.14% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 21 functions across 12 files.	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.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main remote-device fixes: capability races, sessions without a device ID, and logout while the device is running.

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

---

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.}