Skip to content

fix(desktop): decouple machineId from host-service connection#3876

Merged
saddlepaddle merged 1 commit intomainfrom
debug-superset-remote-wor
Apr 29, 2026
Merged

fix(desktop): decouple machineId from host-service connection#3876
saddlepaddle merged 1 commit intomainfrom
debug-superset-remote-wor

Conversation

@saddlepaddle
Copy link
Copy Markdown
Collaborator

@saddlepaddle saddlepaddle commented Apr 29, 2026

Summary

  • machineId was sourced from hostServiceCoordinator.getConnection, which returns null whenever no host-service instance is running. After macOS sleep killed the detached host-service child, the renderer received machineId: null, every v2 workspace failed the workspace.hostId === machineId check, and all workspaces fell through to the relay branch — so data didn't load until the app was restarted.
  • machineId is a deterministic per-device value (getHostId()), so it shouldn't be coupled to host-service liveness in the first place. Added a dedicated device.getMachineId tRPC query and gate LocalHostServiceProvider render on it, making machineId non-nullable everywhere downstream.
  • This only fixes the misclassification. Reviving the host-service on wake (powerMonitor.on('resume', ...)) is being handled in a separate change.

Test plan

  • Open a v2 workspace, close laptop for >30 min, reopen. Workspace data should still classify as local (isLocal === true); behavior should match the pre-sleep state for any host-service that survived. If the host-service died, the workspace should show WorkspaceNotFoundState rather than silently routing through the relay.
  • Cold app start: provider waits one IPC roundtrip for device.getMachineId before rendering children — verify no perceptible delay.
  • bun run typecheck and bun run lint pass.

Summary by cubic

Fixes local workspace misclassification after macOS sleep by decoupling the machine ID from host-service liveness and sourcing it directly from the device. Host-service auto-revive on wake will be handled separately.

  • Bug Fixes
    • Added device tRPC router with device.getMachineId using getHostId() from @superset/shared/host-info.
    • Updated LocalHostServiceProvider to fetch the machine ID once, make it non-nullable, and stop reading it from hostServiceCoordinator.getConnection.
    • Updated project location query to use the required machineId directly.

Written for commit 3a74bc3. Summary will update on new commits. Review in cubic

Summary by CodeRabbit

  • Bug Fixes
    • Fixed machine identification issues affecting project location and host management configuration.

Previously, machineId was sourced from `hostServiceCoordinator.getConnection`,
which returns null whenever no host-service instance is running. After macOS
sleep killed the detached host-service child, every v2 workspace failed the
`workspace.hostId === machineId` check and got routed through the relay as
if it were remote, causing data not to load until app restart.

machineId is a deterministic per-device value (`getHostId()`), so expose it
via a dedicated `device.getMachineId` tRPC query and gate provider render
on it so the context value is non-nullable.

Note: this only fixes the misclassification. Reviving the host-service on
wake (e.g. via `powerMonitor`) is a separate change.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 29, 2026

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

A new TRPC device router endpoint (getMachineId) is introduced to retrieve the machine identifier from the host system. The LocalHostServiceProvider is refactored to fetch the machine ID through this endpoint instead of deriving it from the active connection, ensuring consistent availability of a non-null machine ID value.

Changes

Cohort / File(s) Summary
New Device Router
apps/desktop/src/lib/trpc/routers/device.ts, apps/desktop/src/lib/trpc/routers/index.ts
Introduces createDeviceRouter() factory with a getMachineId query that calls getHostId() and returns it as machineId. Registers the device router in the app-level TRPC router composition.
LocalHostServiceProvider Updates
apps/desktop/src/renderer/routes/_authenticated/providers/LocalHostServiceProvider/LocalHostServiceProvider.tsx
Refactors provider to fetch machineId via electronTrpc.device.getMachineId with infinite cache freshness instead of sourcing from activeConnection.machineId. Changes context type to enforce non-null machineId, blocks rendering until data is available, and updates dependency tracking.
Query Filter Logic
apps/desktop/src/renderer/routes/_authenticated/settings/v2-project/$projectId/components/V2ProjectSettings/components/ProjectLocationSection/ProjectLocationSection.tsx
Updates host query filter from eq(hosts.machineId, machineId ?? "") to eq(hosts.machineId, machineId), removing the fallback to empty string matching behavior now that machineId is guaranteed non-null.

Sequence Diagram

sequenceDiagram
    participant Client as LocalHostServiceProvider
    participant TRPC as Device Router<br/>(TRPC Endpoint)
    participant HostInfo as Host Info Service<br/>(getHostId)
    
    Client->>TRPC: electronTrpc.device.getMachineId()
    activate TRPC
    TRPC->>HostInfo: getHostId()
    activate HostInfo
    HostInfo-->>TRPC: hostId (string)
    deactivate HostInfo
    TRPC-->>Client: { machineId: string }
    deactivate TRPC
    Client->>Client: Cache result with infinite freshness
    Client->>Client: Render context with non-null machineId
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A router now gathers the host's true name,
No longer guessing from connection's refrain,
The provider awaits, then holds it with pride—
A machine ID fresh, with nowhere to hide!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.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
Title check ✅ Passed The title accurately summarizes the main change: decoupling machineId from the host-service connection, which is the core objective of this PR.
Description check ✅ Passed The description includes a comprehensive summary of the problem, solution, and test plan. However, it is missing the required template sections like 'Type of Change', 'Related Issues', and 'Testing' checklist.
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.

✏️ 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 debug-superset-remote-wor

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
Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.

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

@saddlepaddle saddlepaddle merged commit 71f8eed into main Apr 29, 2026
6 of 7 checks passed
@github-actions
Copy link
Copy Markdown
Contributor

🧹 Preview Cleanup Complete

The following preview resources have been cleaned up:

  • ⚠️ Neon database branch

Thank you for your contribution! 🎉

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