Skip to content

Remove device heartbeat polling to reduce Vercel costs#2904

Merged
saddlepaddle merged 4 commits into
mainfrom
saddlepaddle/likeable-leptoceratops
Mar 28, 2026
Merged

Remove device heartbeat polling to reduce Vercel costs#2904
saddlepaddle merged 4 commits into
mainfrom
saddlepaddle/likeable-leptoceratops

Conversation

@saddlepaddle
Copy link
Copy Markdown
Collaborator

@saddlepaddle saddlepaddle commented Mar 26, 2026

Summary

  • Replaces 30s heartbeat polling with a single device registration on app startup (~2,880 → 1 request/device/day)
  • Removes device.listOnlineDevices tRPC procedure (was unused)
  • Removes Devices settings page, MCP online-check gate, and devicePresence Electric collection
  • MCP command routing still works — offline devices simply time out after 30s

Test plan

  • bun run typecheck — 22/22 pass
  • bun run lint — clean
  • bun test — all failures pre-existing
  • Verify MCP list_devices and executeOnDevice work with registered device
  • Verify desktop app registers device on startup
  • Verify Devices settings page is gone from sidebar

Summary by cubic

Replaced 30s device heartbeats with org‑scoped, one‑time device registration on app startup to cut ~2,880 requests per device per day and lower Vercel costs. MCP routing is unchanged; offline devices time out, commands only run on the current user’s devices, and the old device.heartbeat endpoint remains as a deprecated alias for older clients.

  • Refactors

    • Renamed device.heartbeat to device.registerDevice (single upsert; re-registers on org switch) and kept device.heartbeat as a deprecated alias.
    • Removed heartbeat intervals from desktop/mobile presence hooks.
    • Deleted unused device.listOnlineDevices tRPC and the Devices settings page/sidebar entry.
    • Dropped devicePresence and v2DevicePresence Electric collections; removed online indicators in DevicePicker and presence joins in host options.
    • MCP tools: list_devices returns registered devices only; executeOnDevice verifies org and ownership, relying on timeout for offline cases.
  • Migration

    • Update callsites from device.heartbeat to device.registerDevice (older clients will keep working via the deprecated alias).
    • Remove links to the Devices settings page.
    • MCP consumers: list_devices no longer includes isOnline.

Written for commit 76ddd91. Summary will update on new commits.

Summary by CodeRabbit

  • Features Removed

    • Devices settings page, its sidebar navigation entry, and related settings section removed.
    • Online/offline badges and status text removed from device lists, pickers, and settings UI.
  • Changes

    • Device presence now performs a one-time registration instead of periodic heartbeats.
    • Device listings and related tools no longer compute or expose an "isOnline" flag; they show registered devices only.

Replace 30s heartbeat polling with a single device registration on app
startup. This eliminates ~2,880 requests/device/day to Vercel while
keeping MCP command routing and ownership checks intact.

- Remove heartbeat interval from desktop and mobile, register once on mount
- Rename `device.heartbeat` to `device.registerDevice` (single-fire)
- Remove `device.listOnlineDevices` (unused)
- Remove online-check gate from MCP `executeOnDevice` (timeout handles offline)
- Remove Devices settings page and sidebar entry
- Remove `devicePresence` Electric collection (no longer needed client-side)
- Simplify `list_devices` MCP tool (no online/offline status)
- Remove online indicator from DevicePicker UI
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 26, 2026

📝 Walkthrough

Walkthrough

Replaced periodic device heartbeat polling with a one-time registerDevice mutation; removed device presence collections, online-status computation/fields and UI indicators, deleted the Devices settings page/route, and updated API/tools to stop filtering or returning isOnline.

Changes

Cohort / File(s) Summary
Device Presence Hooks
apps/desktop/src/renderer/.../useDevicePresence/useDevicePresence.ts, apps/mobile/hooks/useDevicePresence/useDevicePresence.ts
Removed interval-based heartbeat and sendHeartbeat; added one-time registerDevice call guarded by registeredScopeRef/registeredRef with retry-on-error. Deleted heartbeat constants and unused React imports.
Device Picker UI & Host Options
apps/desktop/src/renderer/.../DevicePicker/DevicePicker.tsx, apps/desktop/src/renderer/.../useWorkspaceHostOptions/useWorkspaceHostOptions.ts
Removed online/offline status indicator and isOnline property; eliminated presence joins and lastSeenAt from queries; simplified device option mapping.
Settings: Removal of Devices Section
apps/desktop/src/renderer/.../SettingsSidebar/GeneralSettings.tsx, apps/desktop/src/renderer/stores/settings-state.ts, apps/desktop/src/renderer/.../settings/layout.tsx
Deleted "/settings/devices" route literal, removed "devices" from SettingsSection type, SECTION_ORDER, and sidebar SECTION_GROUPS.
Devices Settings Component & Route
apps/desktop/src/renderer/.../settings/devices/components/DevicesSettings/DevicesSettings.tsx, apps/desktop/src/renderer/.../DevicesSettings/index.ts, apps/desktop/src/renderer/.../settings/devices/page.tsx
Removed DevicesSettings component, its barrel re-export, and the devices route file.
Collections Provider
apps/desktop/src/renderer/.../providers/CollectionsProvider/collections.ts
Removed v2DevicePresence and devicePresence from OrgCollections and stopped creating/returning those Electric collections.
tRPC: Device Router
packages/trpc/src/router/device/device.ts
Removed listOnlineDevices and OFFLINE_THRESHOLD_MS; changed heartbeat to a non-returning upsert response ({ success: true }); added registerDevice mutation returning { device, timestamp }.
MCP Tools & Utils
packages/mcp/src/tools/devices/list-devices/list-devices.ts, packages/mcp/src/tools/utils/utils.ts, packages/mcp/src/tools/utils/index.ts
list_devices no longer accepts includeOffline, always queries registered devices (no recency filter), and omits isOnline from output; removed DEVICE_ONLINE_THRESHOLD_MS export/usage and adjusted error text and handler signatures.

Sequence Diagram

sequenceDiagram
    participant App as Application
    participant Hook as useDevicePresence Hook
    participant Client as API Client
    participant Server as tRPC Server
    participant DB as Database

    rect rgba(100,200,150,0.5)
    Note over App,DB: Old Flow — Periodic Heartbeat
    App->>Hook: mount with deviceInfo & org
    Hook->>Hook: start setInterval (heartbeat)
    loop every N ms
        Hook->>Client: heartbeat.mutate()
        Client->>Server: heartbeat mutation
        Server->>DB: update lastSeenAt
        DB-->>Server: ack
        Server-->>Client: success
    end
    end

    rect rgba(150,180,220,0.5)
    Note over App,DB: New Flow — One-Time Registration
    App->>Hook: mount with deviceInfo & org
    Hook->>Hook: check registeredScopeRef
    alt not registered
        Hook->>Client: registerDevice.mutate()
        Client->>Server: registerDevice mutation
        Server->>DB: insert/update device presence
        DB-->>Server: success
        Server-->>Client: {device, timestamp}
        Client-->>Hook: onSuccess -> set registeredScopeRef
    else already registered
        Hook->>Hook: skip registration
    end
    end
Loading

Estimated Code Review Effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 I hopped in once, then paused my feet,
No ticking heart, no rhythmic beat.
Devices slimmed from lists and screens,
One gentle register now keeps things clean.
A quiet burrow, tidy and neat. 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title directly summarizes the main change: replacing heartbeat polling with device registration to reduce costs, which is the core objective.
Description check ✅ Passed The description includes all required template sections (Summary, Related Issues, Type of Change, Testing, Screenshots, Additional Notes) with substantive content covering the change rationale, test plan, and migration guidance.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch saddlepaddle/likeable-leptoceratops

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

Comment @coderabbitai help to get the list of available commands and usage tips.

No consumers remain after removing the presence join from
useWorkspaceHostOptions. Presence will be reimplemented via WSS.
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 26, 2026

🧹 Preview Cleanup Complete

The following preview resources have been cleaned up:

  • ✅ Neon database branch
  • ✅ Electric Fly.io app

Thank you for your contribution! 🎉

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 15 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/mobile/hooks/useDevicePresence/useDevicePresence.ts">

<violation number="1" location="apps/mobile/hooks/useDevicePresence/useDevicePresence.ts:38">
P1: The global `registeredRef` guard prevents re-registering when `activeOrganizationId` changes, so device presence can stay linked to the wrong organization.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread apps/mobile/hooks/useDevicePresence/useDevicePresence.ts Outdated
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

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

⚠️ Outside diff range comments (1)
packages/mcp/src/tools/utils/utils.ts (1)

46-80: ⚠️ Potential issue | 🟠 Major

Scope the presence lookup to ctx.userId.

This query still assumes deviceId is unique within an organization, but registerDevice upserts devicePresence by (userId, deviceId). If two users register the same machine in one org, limit(1) can grab the other user's row first and reject a command for the caller's own device.

Suggested fix
 	.where(
 		and(
 			eq(devicePresence.deviceId, deviceId),
 			eq(devicePresence.organizationId, ctx.organizationId),
+			eq(devicePresence.userId, ctx.userId),
 		),
 	)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/mcp/src/tools/utils/utils.ts` around lines 46 - 80, The
devicePresence lookup in the utils function is not scoped to the caller and can
return another user's row because registerDevice upserts by (userId, deviceId);
update the db.select() .where(...) used to fetch devicePresence to include
eq(devicePresence.userId, ctx.userId) so the query only returns the row for the
current user (ensuring the subsequent device and ownership checks are correct) —
adjust the where clause around devicePresence/deviceId and organizationId to
also require the matching userId from ctx.userId.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/components/AgentHooks/hooks/useDevicePresence/useDevicePresence.ts`:
- Around line 13-30: The current registration guard (registeredRef in the
useEffect) prevents re-registering when the active organization or user changes
for the session; change the guard to be scoped to the current org/user (for
example track lastRegisteredScopeRef = useRef<string | null> storing a
concatenated key like `${userId}:${orgId}`) and in the useEffect compare the
current scope key (derived from deviceInfo,
session?.session?.activeOrganizationId and session?.session?.userId) to
lastRegisteredScopeRef.current; only skip registration when they match, set
lastRegisteredScopeRef.current when registration succeeds, and reset it to null
on failure (catch) so future org/user changes will trigger a new registerDevice
call via apiTrpcClient.device.registerDevice.

In `@apps/mobile/hooks/useDevicePresence/useDevicePresence.ts`:
- Around line 29-53: The one-shot guard (registeredRef) prevents re-registering
when the user/org changes; replace the boolean guard with a ref that stores the
last registered scope (e.g., lastRegisteredScopeRef) and include the current
scope (activeOrganizationId and optionally session?.session?.userId) in the
check inside the useEffect; only skip registration when
lastRegisteredScopeRef.current strictly equals the current scope, and on
successful apiClient.device.registerDevice.mutate set
lastRegisteredScopeRef.current to the current scope, while on catch clear it
(set to null/undefined) so future org/account switches will trigger upsert;
ensure this uses the same useEffect([deviceId, activeOrganizationId,
session?.session?.userId]) and referenced symbols (registeredRef ->
lastRegisteredScopeRef, deviceId, activeOrganizationId,
apiClient.device.registerDevice.mutate).

---

Outside diff comments:
In `@packages/mcp/src/tools/utils/utils.ts`:
- Around line 46-80: The devicePresence lookup in the utils function is not
scoped to the caller and can return another user's row because registerDevice
upserts by (userId, deviceId); update the db.select() .where(...) used to fetch
devicePresence to include eq(devicePresence.userId, ctx.userId) so the query
only returns the row for the current user (ensuring the subsequent device and
ownership checks are correct) — adjust the where clause around
devicePresence/deviceId and organizationId to also require the matching userId
from ctx.userId.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4bba1502-ab44-4988-b024-33cf79e6fca4

📥 Commits

Reviewing files that changed from the base of the PR and between a5625e8 and aeadf70.

📒 Files selected for processing (15)
  • apps/desktop/src/renderer/routes/_authenticated/components/AgentHooks/hooks/useDevicePresence/useDevicePresence.ts
  • apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/components/DevicePicker/DevicePicker.tsx
  • apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/components/DevicePicker/hooks/useWorkspaceHostOptions/useWorkspaceHostOptions.ts
  • apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/collections.ts
  • apps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/GeneralSettings.tsx
  • apps/desktop/src/renderer/routes/_authenticated/settings/devices/components/DevicesSettings/DevicesSettings.tsx
  • apps/desktop/src/renderer/routes/_authenticated/settings/devices/components/DevicesSettings/index.ts
  • apps/desktop/src/renderer/routes/_authenticated/settings/devices/page.tsx
  • apps/desktop/src/renderer/routes/_authenticated/settings/layout.tsx
  • apps/desktop/src/renderer/stores/settings-state.ts
  • apps/mobile/hooks/useDevicePresence/useDevicePresence.ts
  • packages/mcp/src/tools/devices/list-devices/list-devices.ts
  • packages/mcp/src/tools/utils/index.ts
  • packages/mcp/src/tools/utils/utils.ts
  • packages/trpc/src/router/device/device.ts
💤 Files with no reviewable changes (7)
  • apps/desktop/src/renderer/routes/_authenticated/settings/devices/components/DevicesSettings/index.ts
  • apps/desktop/src/renderer/routes/_authenticated/settings/devices/page.tsx
  • apps/desktop/src/renderer/routes/_authenticated/settings/layout.tsx
  • apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/collections.ts
  • apps/desktop/src/renderer/stores/settings-state.ts
  • apps/desktop/src/renderer/routes/_authenticated/settings/devices/components/DevicesSettings/DevicesSettings.tsx
  • apps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/GeneralSettings.tsx

Comment on lines 13 to 30
const registeredRef = useRef(false);

const sendHeartbeat = useCallback(async () => {
useEffect(() => {
if (!deviceInfo || !session?.session?.activeOrganizationId) return;
if (registeredRef.current) return;
registeredRef.current = true;

try {
await apiTrpcClient.device.heartbeat.mutate({
apiTrpcClient.device.registerDevice
.mutate({
deviceId: deviceInfo.deviceId,
deviceName: deviceInfo.deviceName,
deviceType: "desktop",
})
.catch(() => {
// Registration can fail when offline — will retry on next app launch
registeredRef.current = false;
});
} catch {
// Heartbeat can fail when offline - ignore
}
}, [deviceInfo, session?.session?.activeOrganizationId]);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Scope the desktop registration guard to the current user/org.

This has the same lifetime bug as the mobile hook: after the first successful call, later org/account changes are ignored for the rest of the session. That leaves the device_presence row in the old scope until the app is restarted, which breaks MCP routing in the new org.

Suggested fix
-	const registeredRef = useRef(false);
+	const registeredScopeRef = useRef<string | null>(null);
...
 	useEffect(() => {
-		if (!deviceInfo || !session?.session?.activeOrganizationId) return;
-		if (registeredRef.current) return;
-		registeredRef.current = true;
+		const registrationScope =
+			session?.user?.id && session?.session?.activeOrganizationId
+				? `${session.user.id}:${session.session.activeOrganizationId}`
+				: null;
+		if (!deviceInfo || !registrationScope) return;
+		if (registeredScopeRef.current === registrationScope) return;
+		registeredScopeRef.current = registrationScope;

 		apiTrpcClient.device.registerDevice
 			.mutate({
 				deviceId: deviceInfo.deviceId,
 				deviceName: deviceInfo.deviceName,
 				deviceType: "desktop",
 			})
 			.catch(() => {
-				registeredRef.current = false;
+				registeredScopeRef.current = null;
 			});
-	}, [deviceInfo, session?.session?.activeOrganizationId]);
+	}, [deviceInfo, session?.session?.activeOrganizationId, session?.user?.id]);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const registeredRef = useRef(false);
const sendHeartbeat = useCallback(async () => {
useEffect(() => {
if (!deviceInfo || !session?.session?.activeOrganizationId) return;
if (registeredRef.current) return;
registeredRef.current = true;
try {
await apiTrpcClient.device.heartbeat.mutate({
apiTrpcClient.device.registerDevice
.mutate({
deviceId: deviceInfo.deviceId,
deviceName: deviceInfo.deviceName,
deviceType: "desktop",
})
.catch(() => {
// Registration can fail when offline — will retry on next app launch
registeredRef.current = false;
});
} catch {
// Heartbeat can fail when offline - ignore
}
}, [deviceInfo, session?.session?.activeOrganizationId]);
const registeredScopeRef = useRef<string | null>(null);
useEffect(() => {
const registrationScope =
session?.user?.id && session?.session?.activeOrganizationId
? `${session.user.id}:${session.session.activeOrganizationId}`
: null;
if (!deviceInfo || !registrationScope) return;
if (registeredScopeRef.current === registrationScope) return;
registeredScopeRef.current = registrationScope;
apiTrpcClient.device.registerDevice
.mutate({
deviceId: deviceInfo.deviceId,
deviceName: deviceInfo.deviceName,
deviceType: "desktop",
})
.catch(() => {
// Registration can fail when offline — will retry on next app launch
registeredScopeRef.current = null;
});
}, [deviceInfo, session?.session?.activeOrganizationId, session?.user?.id]);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/desktop/src/renderer/routes/_authenticated/components/AgentHooks/hooks/useDevicePresence/useDevicePresence.ts`
around lines 13 - 30, The current registration guard (registeredRef in the
useEffect) prevents re-registering when the active organization or user changes
for the session; change the guard to be scoped to the current org/user (for
example track lastRegisteredScopeRef = useRef<string | null> storing a
concatenated key like `${userId}:${orgId}`) and in the useEffect compare the
current scope key (derived from deviceInfo,
session?.session?.activeOrganizationId and session?.session?.userId) to
lastRegisteredScopeRef.current; only skip registration when they match, set
lastRegisteredScopeRef.current when registration succeeds, and reset it to null
on failure (catch) so future org/user changes will trigger a new registerDevice
call via apiTrpcClient.device.registerDevice.

Comment thread apps/mobile/hooks/useDevicePresence/useDevicePresence.ts Outdated
- Replace boolean registeredRef with org-scoped ref so switching orgs
  triggers re-registration on both desktop and mobile
- Scope executeOnDevice ownership query to ctx.userId to prevent
  returning another user's device row
Existing desktop/mobile clients still call device.heartbeat on a 30s
interval. Keep it around as a deprecated alias so they don't error out
until users update.
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 1 file (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/trpc/src/router/device/device.ts">

<violation number="1" location="packages/trpc/src/router/device/device.ts:92">
P2: `heartbeat` duplicates `registerDevice`'s upsert logic. Extract a shared helper so both endpoints stay behaviorally consistent.

(Based on your team's feedback about avoiding duplicated business logic in multiple places.) [FEEDBACK_USED]</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

* @deprecated Kept for backwards compat with shipped desktop/mobile clients
* that still call heartbeat on a 30s interval. Same logic as registerDevice.
*/
heartbeat: protectedProcedure
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot Mar 28, 2026

Choose a reason for hiding this comment

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

P2: heartbeat duplicates registerDevice's upsert logic. Extract a shared helper so both endpoints stay behaviorally consistent.

(Based on your team's feedback about avoiding duplicated business logic in multiple places.)

View Feedback

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/trpc/src/router/device/device.ts, line 92:

<comment>`heartbeat` duplicates `registerDevice`'s upsert logic. Extract a shared helper so both endpoints stay behaviorally consistent.

(Based on your team's feedback about avoiding duplicated business logic in multiple places.) </comment>

<file context>
@@ -85,6 +85,54 @@ export const deviceRouter = {
+	 * @deprecated Kept for backwards compat with shipped desktop/mobile clients
+	 * that still call heartbeat on a 30s interval. Same logic as registerDevice.
+	 */
+	heartbeat: protectedProcedure
+		.input(
+			z.object({
</file context>
Fix with Cubic

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Caution

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

⚠️ Outside diff range comments (1)
packages/trpc/src/router/device/device.ts (1)

88-133: ⚠️ Potential issue | 🟠 Major

Keep heartbeat API-compatible, not just route-compatible.

Line 133 changes the deprecated alias to return { success: true }. Since this endpoint is being kept specifically for shipped clients, changing its response contract can still break released callers that read the old payload even if the mutation itself succeeds. Preserve the legacy response shape here, or have both mutations share one helper while adapting heartbeat back to the old contract.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/trpc/src/router/device/device.ts` around lines 88 - 133, The
heartbeat mutation (protectedProcedure.heartbeat) currently returns { success:
true } which breaks legacy clients that depend on the original response shape;
change heartbeat to preserve the legacy response contract (match the original
registerDevice/old heartbeat payload) — either by returning the same object
structure as registerDevice or by extracting the shared DB insert/upsert logic
into a helper (e.g., upsertDevicePresence) and calling that helper from both
registerDevice and heartbeat, then map the heartbeat's return value to the
legacy shape expected by shipped clients.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@packages/trpc/src/router/device/device.ts`:
- Around line 88-133: The heartbeat mutation (protectedProcedure.heartbeat)
currently returns { success: true } which breaks legacy clients that depend on
the original response shape; change heartbeat to preserve the legacy response
contract (match the original registerDevice/old heartbeat payload) — either by
returning the same object structure as registerDevice or by extracting the
shared DB insert/upsert logic into a helper (e.g., upsertDevicePresence) and
calling that helper from both registerDevice and heartbeat, then map the
heartbeat's return value to the legacy shape expected by shipped clients.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8de5f61b-03a7-4708-8a3f-85f928d6656c

📥 Commits

Reviewing files that changed from the base of the PR and between 9a62030 and 76ddd91.

📒 Files selected for processing (1)
  • packages/trpc/src/router/device/device.ts

@saddlepaddle saddlepaddle merged commit 6837adc into main Mar 28, 2026
15 checks passed
rogalio added a commit to rogalio/superset that referenced this pull request Apr 4, 2026
Since PR superset-sh#2904 removed the 30-second heartbeat to cut Vercel costs,
devices register only once at startup. The MCP `list_devices` tool
still checks a 60-second online window, so every device appears
offline after one minute and MCP commands can no longer reach it.

- Add a 5-minute periodic re-registration in useDevicePresence
  (288 calls/device/day vs the old 2,880 — 90% cost reduction kept)
- Bump the online window from 60s to 10 minutes (2x the heartbeat
  interval to tolerate network hiccups)
- Update tests and documentation to match

Closes superset-sh#3091
saddlepaddle added a commit that referenced this pull request Apr 9, 2026
PR #2904 deliberately removed device heartbeat polling (desktop calls
device.registerDevice once at startup) because executeOnDevice already
handles unreachable devices via its command-poll timeout. PR #2988
reintroduced an isOnline field computed from a 60s lastSeenAt window
without restoring heartbeats, so every device looked offline ~60s after
app start — surfacing in the Slack integration agent as "status: offline".

Revert the isOnline reintroduction (keep #2988's schema hardening), drop
includeOffline from the input, update the Slack run-agent to stop
rendering the removed status, and refresh the docs + stubbed CLI flag.
Also fix a pre-existing mock-module bleed in list-devices.test.ts that
was stripping executeOnDevice from start-agent-session.test.ts.
saddlepaddle added a commit that referenced this pull request Apr 9, 2026
PR #2904 deliberately removed device heartbeat polling (desktop calls
device.registerDevice once at startup) because executeOnDevice already
handles unreachable devices via its command-poll timeout. PR #2988
reintroduced an isOnline field computed from a 60s lastSeenAt window
without restoring heartbeats, so every device looked offline ~60s after
app start — surfacing in the Slack integration agent as "status: offline".

Revert the isOnline reintroduction (keep #2988's schema hardening), drop
includeOffline from the input, update the Slack run-agent to stop
rendering the removed status, and refresh the docs + stubbed CLI flag.
Also fix a pre-existing mock-module bleed in list-devices.test.ts that
was stripping executeOnDevice from start-agent-session.test.ts.
MocA-Love pushed a commit to MocA-Love/superset that referenced this pull request Apr 10, 2026
…et-sh#3299)

PR superset-sh#2904 deliberately removed device heartbeat polling (desktop calls
device.registerDevice once at startup) because executeOnDevice already
handles unreachable devices via its command-poll timeout. PR superset-sh#2988
reintroduced an isOnline field computed from a 60s lastSeenAt window
without restoring heartbeats, so every device looked offline ~60s after
app start — surfacing in the Slack integration agent as "status: offline".

Revert the isOnline reintroduction (keep superset-sh#2988's schema hardening), drop
includeOffline from the input, update the Slack run-agent to stop
rendering the removed status, and refresh the docs + stubbed CLI flag.
Also fix a pre-existing mock-module bleed in list-devices.test.ts that
was stripping executeOnDevice from start-agent-session.test.ts.
saddlepaddle added a commit that referenced this pull request May 13, 2026
The renderer caller was removed in #2904 (2026-03-28) and the
MINIMUM_DESKTOP_VERSION floor (1.5.0, 2026-04-11) blocks any client
that predates the caller removal from using the app. The endpoint
was kept "for backwards compat with shipped clients", but the only
clients still polling are stuck behind the UpdateRequiredPage gate
and their heartbeat traffic has no downstream consumer — heartbeat
only updates devicePresence.lastSeenAt, which is purely cosmetic
(MCP list-devices ORDER BY).

Removing the route turns those stale-build calls into a silent tRPC
NOT_FOUND. registerDevice stays — MCP's executeOnDevice still uses
the row for ownership verification.

Trims ~10% of /api log volume on Vercel.
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