feat: unified WS event bus, v2Hosts data model, real diff stats#3224
Merged
Conversation
- Add unified /events WebSocket endpoint on host-service replacing per-workspace filesystem WS connections. Carries git:changed (auto) and fs:events (on-demand) over a single connection per host. - Add v2_hosts, v2_clients, v2_users_hosts tables replacing v2_devices, v2_device_presence, v2_users_devices. Workspaces now reference hostId instead of deviceId. Hosts identified by machineId (null = cloud). - Wire real git diff stats into left sidebar chips using event bus to trigger refetches instead of polling. - Update tRPC routers (ensureV2Host, ensureV2Client, workspace create), Electric SQL sync, and all renderer references.
- Route diff stats per-workspace to correct host (local vs remote proxy URL) - Set GIT_OPTIONAL_LOCKS=0 on all host-service git operations to prevent index.lock contention with user git commands - Fix Changes tab header stats not updating when switching filter dropdown - Fix diff stats pill padding (w-fit + justify-self-end) - Restore LuFolderGit2 icon for local workspaces - Update WorkspaceHostTarget kind from "device" to "host" - Make v2_hosts.machineId NOT NULL (every host is a machine) - Bump PR polling to 10s (host-service + client)
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughReplaces device-centric model with host/client entities, migrates DB (devices → hosts/clients/users_hosts), adds an EventBus and GitWatcher for fs/git events, removes legacy filesystem WebSocket APIs, and updates API, collections, trpc, workspace-client, and desktop UI/hooks to use hostId and the new event-bus plumbing. Changes
Sequence Diagram(s)mermaid UI->>Client: getEventBus(hostUrl).on("git:changed", workspaceId, cb) Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Contributor
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
Greptile SummaryThis PR introduces a unified Key changes:
Issues found:
Confidence Score: 3/5Not safe to merge without fixing the cleanup effect bug and reviewing the destructive migration The unified event bus architecture is well-structured, the data model refactor is clean, and the client reconnection logic is solid. However, two issues require attention before merging: the [workspaceHosts.map] dependency bug in useDashboardDiffStats causes permanent stale state accumulation (a real correctness defect in a new feature), and the DELETE FROM v2_workspaces in the migration irreversibly destroys all workspace data. The debounce gap is a performance concern that adds unnecessary RPC load. Fixing the cleanup bug and documenting/reconsidering the migration deletion would bring this to a 4. useDashboardDiffStats.ts (cleanup effect logic bug) and 0031_v2_hosts_and_clients.sql (destructive DELETE statement) Important Files Changed
Sequence DiagramsequenceDiagram
participant C as Desktop Client
participant EB as EventBus (/events WS)
participant GW as GitWatcher
participant FS as FilesystemManager
participant GT as git.getStatus (tRPC)
C->>EB: WebSocket connect /events
EB->>GW: start()
GW->>GW: rescan() every 30s, fs.watch(.git recursive)
Note over GW,EB: On any .git file change (no debounce)
GW-->>EB: onChanged(workspaceId)
EB-->>C: {type:"git:changed", workspaceId}
C->>GT: git.getStatus.query({workspaceId})
GT-->>C: {againstBase, staged, unstaged}
C->>C: update sidebar diffStats chip
C->>EB: {type:"fs:watch", workspaceId}
EB->>FS: watchPath(rootPath, recursive)
loop fs events
FS-->>EB: FsWatchEvent[]
EB-->>C: {type:"fs:events", workspaceId, events}
end
C->>EB: {type:"fs:unwatch", workspaceId}
EB->>FS: iterator.return() — stop stream
Reviews (1): Last reviewed commit: "fix: multi-host routing, GIT_OPTIONAL_LO..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+127
to
+128
| }, [workspaceHosts.map]); | ||
|
|
There was a problem hiding this comment.
workspaceHosts.map is a stable method reference
workspaceHosts.map resolves to Array.prototype.map, which is always the exact same function reference regardless of the array's contents. React sees an unchanged dependency, so this effect only runs on mount/unmount and never reruns when workspaces are added or removed. Stale workspaceId entries will accumulate indefinitely in statsMap.
The fix is to use workspaceHosts (which is memoized upstream via useMemo) or the pre-computed _workspaceHostsKey string already computed at lines 57–59 for exactly this purpose but never wired up:
Suggested change
| }, [workspaceHosts.map]); | |
| }, [workspaceHosts]); |
| @@ -0,0 +1,63 @@ | |||
| DELETE FROM "v2_workspaces";--> statement-breakpoint | |||
There was a problem hiding this comment.
DELETE FROM "v2_workspaces" — irreversible data loss on every migration run
This statement unconditionally deletes every row in v2_workspaces before adding the NOT NULL host_id column. While it sidesteps a constraint failure (no existing rows need a host_id), it permanently destroys all workspace data in every environment the migration executes against — staging, developer databases, and production if reached.
If the v2 workspace table genuinely contains only throwaway data at this stage, that intent should be documented with a comment in the migration. Otherwise, a safer approach is to add host_id as nullable first, backfill, then tighten to NOT NULL in a follow-up:
-- Safer alternative: make host_id nullable initially
ALTER TABLE "v2_workspaces" ADD COLUMN "host_id" uuid;
-- Then backfill host_id for existing rows, then:
ALTER TABLE "v2_workspaces" ALTER COLUMN "host_id" SET NOT NULL;
Comment on lines
+126
to
+131
|
|
||
| try { | ||
| const watcher = watch(gitDir, { recursive: true }, () => { | ||
| this.emit(workspaceId); | ||
| }); | ||
|
|
There was a problem hiding this comment.
.git watcher — burst of RPC calls per git operation
watch(gitDir, { recursive: true }, ...) fires once per individual file changed inside .git. A single git commit touches COMMIT_EDITMSG, index, HEAD, and refs/... in rapid succession, emitting multiple git:changed events to all connected clients. Each emission causes fetchDiffStats in the client to make a full git.getStatus RPC call, so one commit can trigger 4–6 concurrent status queries.
Consider debouncing per workspace (e.g. 200 ms) so a burst collapses into a single notification:
private readonly debounceTimers = new Map<string, ReturnType<typeof setTimeout>>();
private emit(workspaceId: string): void {
const existing = this.debounceTimers.get(workspaceId);
if (existing) clearTimeout(existing);
this.debounceTimers.set(workspaceId, setTimeout(() => {
this.debounceTimers.delete(workspaceId);
for (const listener of this.listeners) listener(workspaceId);
}, 200));
}Also ensure close() iterates over debounceTimers and clears any pending timers.
Comment on lines
+57
to
+62
| const _workspaceHostsKey = workspaceHosts | ||
| .map((wh) => `${wh.workspaceId}:${wh.hostUrl}`) | ||
| .join(","); | ||
|
|
||
| // Keep a ref so event handlers can access current mapping | ||
| const workspaceHostsRef = useRef(workspaceHosts); |
There was a problem hiding this comment.
_workspaceHostsKey is computed but never used
This variable serializes the workspace-host list into a stable string, which is exactly the type of primitive value that works well as a useEffect dependency. It appears it was intended to be used in the cleanup effect at line 127 (in place of the incorrect workspaceHosts.map), but was never wired up. Using it as the dependency there would fix the logic bug above. Either wire it up or remove it to avoid confusion.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
10 issues found across 49 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/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardDiffStats/useDashboardDiffStats.ts">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardDiffStats/useDashboardDiffStats.ts:49">
P2: Avoid empty `catch` blocks here; swallowing errors makes diff-stat failures invisible during runtime/debugging.</violation>
<violation number="2" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardDiffStats/useDashboardDiffStats.ts:57">
P3: `_workspaceHostsKey` is computed but never referenced. It was likely intended as the stable dependency for the cleanup effect at line 128 (which currently uses the broken `workspaceHosts.map`). Either wire it up as the effect dependency or remove it to avoid dead code.</violation>
<violation number="3" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardDiffStats/useDashboardDiffStats.ts:127">
P1: The cleanup effect depends on `workspaceHosts.map` instead of workspace host data, so stale stats may remain when the workspace list changes.</violation>
</file>
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/useChangesTab.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/useChangesTab.tsx:281">
P2: For the "all" filter, deduplicating with `map.set(path, f)` drops additions/deletions from earlier entries of the same file path, so stats can undercount when a file exists in multiple status groups.</violation>
</file>
<file name="packages/db/drizzle/0031_v2_hosts_and_clients.sql">
<violation number="1" location="packages/db/drizzle/0031_v2_hosts_and_clients.sql:1">
P1: `DELETE FROM "v2_workspaces"` unconditionally deletes every row before adding the `NOT NULL` `host_id` column. This permanently destroys all workspace data in every environment the migration runs against (dev, staging, production). If the table genuinely contains only throwaway data at this stage, add a comment documenting that intent. Otherwise, add `host_id` as nullable first, backfill existing rows, then tighten to `NOT NULL` in a follow-up migration.</violation>
</file>
<file name="packages/host-service/src/events/git-watcher.ts">
<violation number="1" location="packages/host-service/src/events/git-watcher.ts:81">
P2: Do not silently swallow rescan query failures; log the error context so git-watcher outages are diagnosable.
(Based on your team's feedback about handling async errors explicitly and avoiding silent catch blocks.) [FEEDBACK_USED]</violation>
<violation number="2" location="packages/host-service/src/events/git-watcher.ts:117">
P1: Absolute git-dir detection is not cross-platform; Windows absolute paths are treated as relative and produce invalid watch paths.</violation>
<violation number="3" location="packages/host-service/src/events/git-watcher.ts:128">
P2: `watch(gitDir, { recursive: true })` fires once per individual file changed inside `.git`. A single `git commit` touches `COMMIT_EDITMSG`, `index`, `HEAD`, and `refs/...` in rapid succession, emitting multiple `git:changed` events. Each emission triggers a full `git.getStatus` RPC call on every connected client, so one commit can cause 4–6 concurrent status queries.
Add a per-workspace debounce (e.g. 200ms) in `emit()` so a burst collapses into a single notification.</violation>
</file>
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardSidebarData/useDashboardSidebarData.ts">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardSidebarData/useDashboardSidebarData.ts:275">
P2: Remote workspaces are fetched/subscribed for diff stats but then always forced to `null`, causing unnecessary host traffic and missing remote diff chips.</violation>
</file>
<file name="packages/host-service/src/events/event-bus.ts">
<violation number="1" location="packages/host-service/src/events/event-bus.ts:115">
P1: A throwing `send` for one client will abort the loop and prevent remaining clients from receiving the `git:changed` event. Wrap each `sendMessage` in a try-catch so a single broken connection doesn't block the broadcast.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| } | ||
| } | ||
|
|
||
| private broadcast(message: ServerMessage): void { |
Contributor
There was a problem hiding this comment.
P1: A throwing send for one client will abort the loop and prevent remaining clients from receiving the git:changed event. Wrap each sendMessage in a try-catch so a single broken connection doesn't block the broadcast.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/host-service/src/events/event-bus.ts, line 115:
<comment>A throwing `send` for one client will abort the loop and prevent remaining clients from receiving the `git:changed` event. Wrap each `sendMessage` in a try-catch so a single broken connection doesn't block the broadcast.</comment>
<file context>
@@ -0,0 +1,252 @@
+ }
+ }
+
+ private broadcast(message: ServerMessage): void {
+ for (const socket of this.clients.keys()) {
+ sendMessage(socket, message);
</file context>
| }) | ||
| .from(workspaces) | ||
| .all(); | ||
| } catch { |
Contributor
There was a problem hiding this comment.
P2: Do not silently swallow rescan query failures; log the error context so git-watcher outages are diagnosable.
(Based on your team's feedback about handling async errors explicitly and avoiding silent catch blocks.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/host-service/src/events/git-watcher.ts, line 81:
<comment>Do not silently swallow rescan query failures; log the error context so git-watcher outages are diagnosable.
(Based on your team's feedback about handling async errors explicitly and avoiding silent catch blocks.) </comment>
<file context>
@@ -0,0 +1,148 @@
+ })
+ .from(workspaces)
+ .all();
+ } catch {
+ return;
+ }
</file context>
- Add useWorkspaceEvent(type, workspaceId, callback) hook that resolves workspace → host → event bus connection automatically - Simplify useDiffStats to a per-workspace hook called from each DashboardSidebarWorkspaceItem instead of batch-fetching in sidebar data - Remove diff stats logic from useDashboardSidebarData (no more workspaceHosts mapping, diffStatsByWorkspaceId, or diffStats on type) - Pass diffStats as prop to ExpandedWorkspaceRow and HoverCardContent
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/db/src/schema/schema.ts (1)
452-468:⚠️ Potential issue | 🟠 MajorInclude
typein thev2_clientsuniqueness key.Line 465 currently makes every client for the same user/machine collapse into one row. That means a valid
desktop+webcombination on the same host will conflict or overwritetypeduring upserts.Suggested constraint change
- unique("v2_clients_org_user_machine_unique").on( + unique("v2_clients_org_user_machine_type_unique").on( table.organizationId, table.userId, table.machineId, + table.type, ),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/db/src/schema/schema.ts` around lines 452 - 468, The unique constraint v2_clients_org_user_machine_unique currently only covers organizationId, userId and machineId which forces different client types (field type in the v2_clients schema) to collide; update the uniqueness definition in the schema.ts table builder to include table.type (and rename the constraint to reflect type, e.g., v2_clients_org_user_machine_type_unique) so rows are unique by organizationId, userId, machineId and type instead of collapsing different client types into one row.
🧹 Nitpick comments (3)
packages/host-service/src/runtime/pull-requests/pull-requests.ts (1)
25-25: Polling interval reduced to 10s - verify rate limit impact.The refresh interval is now 3x more frequent (10s vs 30s). While the code has in-flight deduplication and per-project cooldowns, consider whether this could approach GitHub's rate limits (5000 req/hour authenticated) for organizations with many active projects and workspaces.
The existing
nextProjectRefreshAtthrottling helps, but worth monitoring in production.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/host-service/src/runtime/pull-requests/pull-requests.ts` at line 25, PROJECT_REFRESH_INTERVAL_MS was reduced from 30_000 to 10_000 which increases polling frequency and may hit GitHub rate limits; revert to a safer default or make the interval configurable and add safeguards: restore PROJECT_REFRESH_INTERVAL_MS to 30_000 (or read from env e.g. PROJECT_REFRESH_INTERVAL_MS_DEFAULT), ensure existing in-flight deduplication and nextProjectRefreshAt per-project cooldown logic (refer to nextProjectRefreshAt and the in-flight request tracking) remain intact, and add exponential backoff/short-circuiting and a runtime metric/log when polls are dropped due to cooldown so you can monitor rate-limit pressure in production.apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/useChangesTab.tsx (1)
270-288: Consider consolidating filter/dedupe computation to a single source.
filteredFiles/totals are computed here, but similar"all"dedupe logic is repeated later forfileList. Keeping one shared derivation would reduce drift risk between header stats and rendered rows.Refactor sketch
-const filteredFiles = useMemo(() => { +const { filteredFiles, totalChanges, totalAdditions, totalDeletions } = useMemo(() => { if (!status.data) return []; if (filter.kind === "uncommitted") { - return [...status.data.staged, ...status.data.unstaged]; + const files = [...status.data.staged, ...status.data.unstaged]; + return { + filteredFiles: files, + totalChanges: files.length, + totalAdditions: files.reduce((sum, f) => sum + f.additions, 0), + totalDeletions: files.reduce((sum, f) => sum + f.deletions, 0), + }; } ... - return Array.from(map.values()); + const files = Array.from(map.values()); + return { + filteredFiles: files, + totalChanges: files.length, + totalAdditions: files.reduce((sum, f) => sum + f.additions, 0), + totalDeletions: files.reduce((sum, f) => sum + f.deletions, 0), + }; }, [status.data, filter.kind, commitFiles.data?.files]); - -const totalChanges = filteredFiles.length; -const totalAdditions = filteredFiles.reduce((sum, f) => sum + f.additions, 0); -const totalDeletions = filteredFiles.reduce((sum, f) => sum + f.deletions, 0);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/useChangesTab.tsx around lines 270 - 288, filteredFiles and the header totals duplicate the "all" dedupe logic that is later repeated for fileList; consolidate by extracting a single source-of-truth (e.g., compute a deduplicated fileList inside the same useMemo or a shared helper used by both) and derive totalChanges/totalAdditions/totalDeletions from that unified file list instead of re-running dedupe logic elsewhere; update references to use filteredFiles (or the new shared fileList) and ensure the useMemo dependency array (status.data, filter.kind, commitFiles.data?.files) covers all inputs so header stats and rendered rows stay consistent.packages/host-service/src/events/git-watcher.ts (1)
128-130: Debounce fs.watch notifications before emittinggit:changed.Each watcher notification triggers an emit. Git operations like checkout or rebase mutate multiple files under
.git, causingfs.watchto fire repeatedly for a single logical change. This creates redundant event bursts that can overload status refreshes. Add a debounce to coalesce rapid notifications per workspace.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/host-service/src/events/git-watcher.ts` around lines 128 - 130, The current fs.watch callback (created by watch(...) assigning to watcher) calls this.emit(workspaceId) for every notification; change it to debounce emissions per workspace by maintaining a map of timers keyed by workspaceId, so the watch callback clears any existing timer for that workspace and sets a new short timeout (e.g., 200–500ms) to call this.emit(workspaceId) once when the burst subsides; reference the watcher/watch(...) callback and this.emit(workspaceId) when making the change and ensure timers are cleared when the watcher is disposed.
🤖 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/_dashboard/components/DashboardSidebar/hooks/useDashboardDiffStats/useDashboardDiffStats.ts`:
- Around line 56-127: The three useEffect hooks inside useDashboardDiffStats are
using unstable dependencies (workspaceHosts or workspaceHosts.map) so they don't
reliably rerun; replace those with the stable _workspaceHostsKey: for the
initial fetch effect and the git:changed subscription effect change their
dependency arrays from [fetchDiffStats, workspaceHosts] to [fetchDiffStats,
_workspaceHostsKey], and for the cleanup effect change its dependency from
[workspaceHosts.map] to [_workspaceHostsKey] while keeping setStatsMap as
before; this ensures fetchDiffStats, getEventBus subscription code, and the
stale-entry cleanup run whenever the workspace-host list actually changes.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardSidebarData/useDashboardSidebarData.ts`:
- Around line 145-161: The current useDashboardSidebarData hook builds
workspaceHosts (useMemo) and subscribes remote hosts via useDashboardDiffStats
but then ignores those fetched diff stats for workspaces of type
"remote-device"; update the rendering branch that handles remote-device
workspaces (where the code currently discards stats) to consume and display the
corresponding diff stats from useDashboardDiffStats for each workspaceHost entry
instead of throwing them away, or alternatively stop including remote hosts in
workspaceHosts; locate the workspaceHosts useMemo and the place that
renders/handles workspace.type === "remote-device" and either (A) map the
matching WorkspaceHostInfo.hostUrl or workspaceId to the diff stats returned by
useDashboardDiffStats and render them, or (B) remove remote hosts from
workspaceHosts to avoid unnecessary subscriptions.
In `@packages/db/drizzle/0031_v2_hosts_and_clients.sql`:
- Line 1: The migration currently contains a destructive DELETE FROM
"v2_workspaces" which will drop all workspace data; remove that DELETE and
instead implement a safe backfill: in 0031_v2_hosts_and_clients.sql
create/insert a default or derived host row into the new hosts table (e.g.,
INSERT INTO hosts (...) VALUES (...)), capture its id, UPDATE v2_workspaces SET
host_id = <default_host_id> for existing rows, and only after verifying backfill
add the NOT NULL constraint on host_id (or add it in a separate migration step);
ensure the script preserves existing workspace rows and documents the
default-host fallback for production.
In `@packages/host-service/src/events/event-bus.ts`:
- Around line 121-127: startFsWatch currently treats any existing
state.fsSubscriptions.get(workspaceId) entry as an active watch, but the
streaming task can exit without calling dispose() or removing that entry,
preventing future fs:watch calls; modify startFsWatch (and the corresponding
streaming teardown code around the stream task at lines ~175-200) to detect
non-active/closed subscriptions and to always call dispose() and remove the
workspaceId from state.fsSubscriptions when the stream ends or errors (use a
finally / stream 'close'/'error' handler to ensure cleanup), so that after a
stream exit the entry is cleared and a new fs:watch can be established on the
same socket.
In `@packages/trpc/src/router/device/device.ts`:
- Around line 34-49: The upsert on v2Hosts using
dbWs.insert(...).onConflictDoUpdate currently always proceeds to create an {
role: "owner" } link for the caller (see the owner insert block around lines
59-75), which lets any caller claim ownership when a machineId already exists;
fix by only granting owner when the host was actually inserted (not on conflict)
or by rejecting the operation unless the caller is already linked: implement
this by detecting whether the upsert created a new row (use
RETURNING/affectedRows from the insert or perform an initial SELECT to check
existence) and only run the owner insert when the host was newly created,
otherwise check v2HostUsers for an existing link for userId and fail the
mutation if none exists. Ensure you update the logic around the dbWs.insert(...)
.onConflictDoUpdate and the subsequent owner-insert code to follow this
conditional flow.
In `@packages/trpc/src/router/v2-workspace/v2-workspace.ts`:
- Around line 35-50: getScopedHost only checks host belongs to organization but
doesn't verify the requesting user owns or is allowed to use that host; update
getScopedHost (and the other places noted around the v2 workspace handlers) to
additionally join or query v2_users_hosts to assert a row exists where userId
equals ctx.session.user.id and hostId equals the requested hostId before
returning the host; specifically modify the requireOrgScopedResource call (or
the DB query used inside it) so it filters for both v2Hosts.organizationId and
an existence check on v2_users_hosts (userId = ctx.session.user.id, hostId =
hostId) and return the same BAD_REQUEST error if no such mapping exists,
ensuring the checks are applied in the other referenced handlers that accept
hostId as well.
In `@packages/workspace-client/src/lib/eventBus.ts`:
- Around line 98-105: The reconnect logic currently closes over the
creation-time getWsToken causing stale tokens; modify ConnectionState to include
a mutable getWsToken field and update connect (and any reconnect handlers used
in the connect/reconnect flow such as the code paths referenced around lines
122-150 and 154-173) to read the token via state.getWsToken instead of the
original closed-over getter; also update callers that create or reuse
ConnectionState to assign the latest getter into state.getWsToken so all
subsequent reconnects use the refreshed token source.
- Around line 44-50: buildEventBusUrl currently uses new URL("/events", hostUrl)
which resets the pathname to root and breaks proxy routing for remote hosts;
change it to append a relative path (e.g., "events" instead of "/events") or
otherwise join to the existing hostUrl.pathname so the resulting origin+path
becomes .../api/v2-hosts/{id}/events rather than /events; keep the protocol
selection and token query logic (function buildEventBusUrl and its wsToken
handling) the same while ensuring the path is appended relative to hostUrl.
---
Outside diff comments:
In `@packages/db/src/schema/schema.ts`:
- Around line 452-468: The unique constraint v2_clients_org_user_machine_unique
currently only covers organizationId, userId and machineId which forces
different client types (field type in the v2_clients schema) to collide; update
the uniqueness definition in the schema.ts table builder to include table.type
(and rename the constraint to reflect type, e.g.,
v2_clients_org_user_machine_type_unique) so rows are unique by organizationId,
userId, machineId and type instead of collapsing different client types into one
row.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/useChangesTab.tsx:
- Around line 270-288: filteredFiles and the header totals duplicate the "all"
dedupe logic that is later repeated for fileList; consolidate by extracting a
single source-of-truth (e.g., compute a deduplicated fileList inside the same
useMemo or a shared helper used by both) and derive
totalChanges/totalAdditions/totalDeletions from that unified file list instead
of re-running dedupe logic elsewhere; update references to use filteredFiles (or
the new shared fileList) and ensure the useMemo dependency array (status.data,
filter.kind, commitFiles.data?.files) covers all inputs so header stats and
rendered rows stay consistent.
In `@packages/host-service/src/events/git-watcher.ts`:
- Around line 128-130: The current fs.watch callback (created by watch(...)
assigning to watcher) calls this.emit(workspaceId) for every notification;
change it to debounce emissions per workspace by maintaining a map of timers
keyed by workspaceId, so the watch callback clears any existing timer for that
workspace and sets a new short timeout (e.g., 200–500ms) to call
this.emit(workspaceId) once when the burst subsides; reference the
watcher/watch(...) callback and this.emit(workspaceId) when making the change
and ensure timers are cleared when the watcher is disposed.
In `@packages/host-service/src/runtime/pull-requests/pull-requests.ts`:
- Line 25: PROJECT_REFRESH_INTERVAL_MS was reduced from 30_000 to 10_000 which
increases polling frequency and may hit GitHub rate limits; revert to a safer
default or make the interval configurable and add safeguards: restore
PROJECT_REFRESH_INTERVAL_MS to 30_000 (or read from env e.g.
PROJECT_REFRESH_INTERVAL_MS_DEFAULT), ensure existing in-flight deduplication
and nextProjectRefreshAt per-project cooldown logic (refer to
nextProjectRefreshAt and the in-flight request tracking) remain intact, and add
exponential backoff/short-circuiting and a runtime metric/log when polls are
dropped due to cooldown so you can monitor rate-limit pressure in production.
🪄 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: b8da2691-c849-41a4-abd3-07b65ce13e75
📒 Files selected for processing (49)
apps/api/src/app/api/electric/[...path]/utils.tsapps/desktop/src/renderer/lib/v2-workspace-host.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/DashboardSidebarWorkspaceItem.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/components/DashboardSidebarExpandedWorkspaceRow/DashboardSidebarExpandedWorkspaceRow.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/components/DashboardSidebarWorkspaceDiffStats/DashboardSidebarWorkspaceDiffStats.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/components/DashboardSidebarWorkspaceHoverCardContent/DashboardSidebarWorkspaceHoverCardContent.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/components/DashboardSidebarWorkspaceIcon/DashboardSidebarWorkspaceIcon.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/utils/getWorkspaceRowMocks.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardDiffStats/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardDiffStats/useDashboardDiffStats.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardSidebarData/useDashboardSidebarData.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/types.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/V2WorkspaceOpenInButton/V2WorkspaceOpenInButton.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/useChangesTab.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/layout.tsxapps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/components/DevicePicker/DevicePicker.tsxapps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/components/DevicePicker/hooks/useWorkspaceHostOptions/index.tsapps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/components/DevicePicker/hooks/useWorkspaceHostOptions/useWorkspaceHostOptions.tsapps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/collections.tsapps/electric-proxy/src/where.tspackages/db/drizzle/0031_v2_hosts_and_clients.sqlpackages/db/drizzle/meta/0031_snapshot.jsonpackages/db/drizzle/meta/_journal.jsonpackages/db/src/schema/enums.tspackages/db/src/schema/relations.tspackages/db/src/schema/schema.tspackages/host-service/package.jsonpackages/host-service/src/app.tspackages/host-service/src/events/event-bus.tspackages/host-service/src/events/git-watcher.tspackages/host-service/src/events/index.tspackages/host-service/src/events/types.tspackages/host-service/src/filesystem/events.tspackages/host-service/src/filesystem/index.tspackages/host-service/src/index.tspackages/host-service/src/runtime/git/git.tspackages/host-service/src/runtime/pull-requests/pull-requests.tspackages/host-service/src/trpc/router/workspace/workspace.tspackages/trpc/src/router/device/device.tspackages/trpc/src/router/v2-workspace/v2-workspace.tspackages/workspace-client/src/hooks/useEventBus/index.tspackages/workspace-client/src/hooks/useEventBus/useEventBus.tspackages/workspace-client/src/hooks/useGitChangeEvents/index.tspackages/workspace-client/src/hooks/useGitChangeEvents/useGitChangeEvents.tspackages/workspace-client/src/index.tspackages/workspace-client/src/lib/eventBus.tspackages/workspace-client/src/lib/workspaceFsEventRegistry.tspackages/workspace-client/src/providers/WorkspaceClientProvider/WorkspaceClientProvider.tsxpackages/workspace-client/src/providers/WorkspaceClientProvider/index.ts
💤 Files with no reviewable changes (5)
- packages/workspace-client/src/providers/WorkspaceClientProvider/index.ts
- packages/host-service/src/filesystem/index.ts
- packages/workspace-client/src/providers/WorkspaceClientProvider/WorkspaceClientProvider.tsx
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/utils/getWorkspaceRowMocks.ts
- packages/host-service/src/filesystem/events.ts
Comment on lines
+56
to
+127
| // Stable serialization key for the workspace-host list | ||
| const _workspaceHostsKey = workspaceHosts | ||
| .map((wh) => `${wh.workspaceId}:${wh.hostUrl}`) | ||
| .join(","); | ||
|
|
||
| // Keep a ref so event handlers can access current mapping | ||
| const workspaceHostsRef = useRef(workspaceHosts); | ||
| workspaceHostsRef.current = workspaceHosts; | ||
|
|
||
| // Fetch initial data for all workspaces | ||
| useEffect(() => { | ||
| for (const { workspaceId, hostUrl } of workspaceHosts) { | ||
| void fetchDiffStats(workspaceId, hostUrl); | ||
| } | ||
| }, [fetchDiffStats, workspaceHosts]); | ||
|
|
||
| // Subscribe to git:changed events per unique host | ||
| useEffect(() => { | ||
| if (workspaceHosts.length === 0) return; | ||
|
|
||
| // Group workspaces by hostUrl | ||
| const byHost = new Map<string, Set<string>>(); | ||
| for (const { workspaceId, hostUrl } of workspaceHosts) { | ||
| let set = byHost.get(hostUrl); | ||
| if (!set) { | ||
| set = new Set(); | ||
| byHost.set(hostUrl, set); | ||
| } | ||
| set.add(workspaceId); | ||
| } | ||
|
|
||
| const cleanups: Array<() => void> = []; | ||
|
|
||
| for (const [hostUrl, workspaceIds] of byHost) { | ||
| const bus = getEventBus(hostUrl, () => getHostServiceWsToken(hostUrl)); | ||
|
|
||
| const removeListener = bus.on( | ||
| "git:changed", | ||
| "*", | ||
| (changedWorkspaceId) => { | ||
| if (workspaceIds.has(changedWorkspaceId)) { | ||
| void fetchDiffStats(changedWorkspaceId, hostUrl); | ||
| } | ||
| }, | ||
| ); | ||
|
|
||
| const release = bus.retain(); | ||
| cleanups.push(removeListener, release); | ||
| } | ||
|
|
||
| return () => { | ||
| for (const cleanup of cleanups) { | ||
| cleanup(); | ||
| } | ||
| }; | ||
| }, [fetchDiffStats, workspaceHosts]); | ||
|
|
||
| // Clean up stale entries when workspace list changes | ||
| useEffect(() => { | ||
| const idSet = new Set(workspaceHosts.map((wh) => wh.workspaceId)); | ||
| setStatsMap((prev) => { | ||
| let changed = false; | ||
| const next = new Map(prev); | ||
| for (const key of next.keys()) { | ||
| if (!idSet.has(key)) { | ||
| next.delete(key); | ||
| changed = true; | ||
| } | ||
| } | ||
| return changed ? next : prev; | ||
| }); | ||
| }, [workspaceHosts.map]); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd apps/desktop && find . -name "useDashboardDiffStats.ts" -type fRepository: superset-sh/superset
Length of output: 192
🏁 Script executed:
cat -n "src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardDiffStats/useDashboardDiffStats.ts" | sed -n '50,130p'Repository: superset-sh/superset
Length of output: 222
🏁 Script executed:
cat -n "./src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardDiffStats/useDashboardDiffStats.ts" | sed -n '50,130p'Repository: superset-sh/superset
Length of output: 224
🏁 Script executed:
find . -name "useDashboardDiffStats.ts" -type f 2>/dev/nullRepository: superset-sh/superset
Length of output: 205
🏁 Script executed:
cat -n "./apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardDiffStats/useDashboardDiffStats.ts" | sed -n '50,130p'Repository: superset-sh/superset
Length of output: 2659
Use the stable workspace-host key in the cleanup effect dependency array.
The computed _workspaceHostsKey is unused. The cleanup effect (line 114–127) depends on workspaceHosts.map (a method reference that never changes), so it never reruns when the workspace list changes. This leaves stale stats entries behind and prevents cleanup when workspaces are removed. Replace [workspaceHosts.map] with [_workspaceHostsKey] on line 127, and do the same for the other two effects to stabilize their dependencies against parent re-renders.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardDiffStats/useDashboardDiffStats.ts`
around lines 56 - 127, The three useEffect hooks inside useDashboardDiffStats
are using unstable dependencies (workspaceHosts or workspaceHosts.map) so they
don't reliably rerun; replace those with the stable _workspaceHostsKey: for the
initial fetch effect and the git:changed subscription effect change their
dependency arrays from [fetchDiffStats, workspaceHosts] to [fetchDiffStats,
_workspaceHostsKey], and for the cleanup effect change its dependency from
[workspaceHosts.map] to [_workspaceHostsKey] while keeping setStatsMap as
before; this ensures fetchDiffStats, getEventBus subscription code, and the
stale-entry cleanup run whenever the workspace-host list actually changes.
| @@ -0,0 +1,63 @@ | |||
| DELETE FROM "v2_workspaces";--> statement-breakpoint | |||
Contributor
There was a problem hiding this comment.
Data loss: Migration deletes all workspaces.
DELETE FROM "v2_workspaces" will permanently remove all existing workspace data. This is extremely destructive for production environments.
If this is intentional for a breaking schema change, ensure:
- This migration is only applied during an early/beta phase where data loss is acceptable
- A data migration strategy exists for production deployments
- This is clearly documented in release notes
If not intentional, consider a migration path that preserves existing data by:
- Creating a default/placeholder host for existing workspaces
- Backfilling
host_idbefore making it NOT NULL
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/db/drizzle/0031_v2_hosts_and_clients.sql` at line 1, The migration
currently contains a destructive DELETE FROM "v2_workspaces" which will drop all
workspace data; remove that DELETE and instead implement a safe backfill: in
0031_v2_hosts_and_clients.sql create/insert a default or derived host row into
the new hosts table (e.g., INSERT INTO hosts (...) VALUES (...)), capture its
id, UPDATE v2_workspaces SET host_id = <default_host_id> for existing rows, and
only after verifying backfill add the NOT NULL constraint on host_id (or add it
in a separate migration step); ensure the script preserves existing workspace
rows and documents the default-host fallback for production.
Comment on lines
+121
to
+127
| private startFsWatch( | ||
| socket: WsSocket, | ||
| state: ClientState, | ||
| workspaceId: string, | ||
| ): void { | ||
| // Already watching this workspace for this client | ||
| if (state.fsSubscriptions.has(workspaceId)) return; |
Contributor
There was a problem hiding this comment.
Clear broken FS subscriptions when the stream exits.
Line 127 treats any existing entry as an active watch, but Lines 175-200 can exit the streaming task without calling dispose() or removing that entry. After the first stream error/end, the client cannot successfully fs:watch that workspace again on the same socket.
Suggested fix
void (async () => {
try {
while (!disposed && iterator) {
const next = await iterator.next();
if (disposed || next.done) return;
@@
} catch (error) {
if (disposed) return;
console.error("[event-bus] fs stream failed:", {
workspaceId,
error,
});
sendMessage(socket, {
type: "error",
message:
error instanceof Error
? error.message
: "Filesystem event stream failed",
});
+ } finally {
+ dispose();
+ const current = state.fsSubscriptions.get(workspaceId);
+ if (current?.dispose === dispose) {
+ state.fsSubscriptions.delete(workspaceId);
+ }
}
})();Also applies to: 161-201
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/host-service/src/events/event-bus.ts` around lines 121 - 127,
startFsWatch currently treats any existing
state.fsSubscriptions.get(workspaceId) entry as an active watch, but the
streaming task can exit without calling dispose() or removing that entry,
preventing future fs:watch calls; modify startFsWatch (and the corresponding
streaming teardown code around the stream task at lines ~175-200) to detect
non-active/closed subscriptions and to always call dispose() and remove the
workspaceId from state.fsSubscriptions when the stream ends or errors (use a
finally / stream 'close'/'error' handler to ensure cleanup), so that after a
stream exit the entry is cleared and a new fs:watch can be established on the
same socket.
Comment on lines
+34
to
49
| const [host] = await dbWs | ||
| .insert(v2Hosts) | ||
| .values({ | ||
| organizationId, | ||
| clientId: input.clientId, | ||
| machineId: input.machineId, | ||
| name: input.name, | ||
| type: "host", | ||
| lastSeenAt: now, | ||
| createdByUserId: userId, | ||
| }) | ||
| .onConflictDoUpdate({ | ||
| target: [v2Devices.organizationId, v2Devices.clientId], | ||
| target: [v2Hosts.organizationId, v2Hosts.machineId], | ||
| set: { | ||
| name: input.name, | ||
| type: "host", | ||
| lastSeenAt: now, | ||
| }, | ||
| }) |
Contributor
There was a problem hiding this comment.
Don't grant owner on every machineId conflict.
Line 44 upserts solely on (organizationId, machineId), and Lines 59-73 then unconditionally insert { role: "owner" } for the caller. That lets any org member who can call this mutation with an existing machineId attach themselves to someone else’s host and rename it via Line 46. Only create the owner link on insert, or reject unless the caller is already linked to that host.
Also applies to: 59-75
🤖 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 34 - 49, The upsert
on v2Hosts using dbWs.insert(...).onConflictDoUpdate currently always proceeds
to create an { role: "owner" } link for the caller (see the owner insert block
around lines 59-75), which lets any caller claim ownership when a machineId
already exists; fix by only granting owner when the host was actually inserted
(not on conflict) or by rejecting the operation unless the caller is already
linked: implement this by detecting whether the upsert created a new row (use
RETURNING/affectedRows from the insert or perform an initial SELECT to check
existence) and only run the owner insert when the host was newly created,
otherwise check v2HostUsers for an existing link for userId and fail the
mutation if none exists. Ensure you update the logic around the dbWs.insert(...)
.onConflictDoUpdate and the subsequent owner-insert code to follow this
conditional flow.
Comment on lines
+35
to
50
| async function getScopedHost(organizationId: string, hostId: string) { | ||
| return requireOrgScopedResource( | ||
| () => | ||
| dbWs.query.v2Devices.findFirst({ | ||
| dbWs.query.v2Hosts.findFirst({ | ||
| columns: { | ||
| id: true, | ||
| organizationId: true, | ||
| }, | ||
| where: eq(v2Devices.id, deviceId), | ||
| where: eq(v2Hosts.id, hostId), | ||
| }), | ||
| { | ||
| code: "BAD_REQUEST", | ||
| message: "Device not found in this organization", | ||
| message: "Host not found in this organization", | ||
| organizationId, | ||
| }, | ||
| ); |
Contributor
There was a problem hiding this comment.
Enforce per-user host access before accepting hostId.
getScopedHost() only proves that the host belongs to the org. A forged request can still create/update a workspace against another user's host unless this also verifies a matching v2_users_hosts row for ctx.session.user.id.
Also applies to: 112-123, 151-153
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/trpc/src/router/v2-workspace/v2-workspace.ts` around lines 35 - 50,
getScopedHost only checks host belongs to organization but doesn't verify the
requesting user owns or is allowed to use that host; update getScopedHost (and
the other places noted around the v2 workspace handlers) to additionally join or
query v2_users_hosts to assert a row exists where userId equals
ctx.session.user.id and hostId equals the requested hostId before returning the
host; specifically modify the requireOrgScopedResource call (or the DB query
used inside it) so it filters for both v2Hosts.organizationId and an existence
check on v2_users_hosts (userId = ctx.session.user.id, hostId = hostId) and
return the same BAD_REQUEST error if no such mapping exists, ensuring the checks
are applied in the other referenced handlers that accept hostId as well.
Comment on lines
+44
to
+50
| function buildEventBusUrl(hostUrl: string, wsToken: string | null): string { | ||
| const url = new URL("/events", hostUrl); | ||
| url.protocol = url.protocol === "https:" ? "wss:" : "ws:"; | ||
| if (wsToken) { | ||
| url.searchParams.set("token", wsToken); | ||
| } | ||
| return url.toString(); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
How does the WHATWG URL constructor resolve new URL("events", "https://example.com/api/v2-hosts/123/trpc")` versus new URL("/events", "https://example.com/api/v2-hosts/123/trpc")?`
💡 Result:
-
new URL("events", "https://example.com/api/v2-hosts/123/trpc").href- Result:
https://example.com/api/v2-hosts/123/events - Why:
"events"is a path-relative URL string (it does not start with/) so it’s resolved relative to the base URL’s current directory (i.e., up to the last/). Since the base path is/api/v2-hosts/123/trpc(no trailing slash), the current directory is/api/v2-hosts/123/, and"events"is appended there. [1] [2]
- Result:
-
new URL("/events", "https://example.com/api/v2-hosts/123/trpc").href- Result:
https://example.com/events - Why:
"/events"is a path-absolute URL string (starts with/), which is resolved from the origin root, ignoring the base URL’s path. [1] [2]
- Result:
Sources:
[1] WHATWG URL Standard (definitions of path-absolute vs path-relative URL strings) (url.spec.whatwg.org)
[2] MDN: Resolving relative references to a URL (current-directory vs root-relative behavior) (developer.mozilla.org)
Citations:
- 1: https://url.spec.whatwg.org/?a=b
- 2: https://developer.mozilla.org/en-US/docs/Web/API/URL_API/Resolving_relative_references
🏁 Script executed:
# Find where buildEventBusUrl is called
rg "buildEventBusUrl" --type ts --type tsx -B 3 -A 3
# Find where hostUrl parameter comes from
rg "hostUrl" packages/workspace-client/src/lib/eventBus.ts -B 2 -A 2Repository: superset-sh/superset
Length of output: 2033
🏁 Script executed:
# Check the entire eventBus.ts file to understand context
cat packages/workspace-client/src/lib/eventBus.tsRepository: superset-sh/superset
Length of output: 6384
🏁 Script executed:
# Search for how hostUrl is constructed/obtained in the workspace-client
fd eventBus packages/workspace-client/src -type f | xargs rg "getOrCreateConnection|buildEventBusUrl" -lRepository: superset-sh/superset
Length of output: 297
🏁 Script executed:
# Find all calls to getEventBus
rg "getEventBus" --type ts -B 2 -A 2Repository: superset-sh/superset
Length of output: 5641
🏁 Script executed:
# Search for backend routing or endpoint definitions for /events
fd "events" packages --type f | head -20Repository: superset-sh/superset
Length of output: 277
🏁 Script executed:
# Look for any host configuration or hostUrl construction
rg "hostUrl\s*=" --type ts -B 1 -A 1 | head -40Repository: superset-sh/superset
Length of output: 1873
🏁 Script executed:
# Find how hostUrl is constructed/used in workspace-client provider
rg "hostUrl" packages/workspace-client/src/providers -B 2 -A 2 --type tsRepository: superset-sh/superset
Length of output: 6914
🏁 Script executed:
# Look for backend routing or event endpoint definitions
fd -e ts -e tsx | xargs rg "/events" | grep -v node_modules | head -20Repository: superset-sh/superset
Length of output: 1692
🏁 Script executed:
# Search for event endpoint in backend code (likely Python/Flask)
fd -e py | xargs rg "/events" | grep -v node_modules | head -20Repository: superset-sh/superset
Length of output: 2040
🏁 Script executed:
# Check if there's proxy configuration or host resolution logic
rg "resolveCreateWorkspaceHostUrl|getHostServiceWsToken" --type ts -A 5 | head -40Repository: superset-sh/superset
Length of output: 5849
🏁 Script executed:
# Find getRemoteHostUrl implementation
rg "getRemoteHostUrl|resolveCreateWorkspaceHostUrl" --type ts -B 1 -A 10 | head -60Repository: superset-sh/superset
Length of output: 7071
🏁 Script executed:
# Check the host-service app setup to understand the full routing
rg "app\.use|registerEventBusRoute" packages/host-service/src/app.ts -B 2 -A 2Repository: superset-sh/superset
Length of output: 698
🏁 Script executed:
# Find where host-service is mounted/proxied in the application
fd -e ts -e tsx | xargs rg "host-service|host.*service" | grep -E "(proxy|mount|route)" | head -20Repository: superset-sh/superset
Length of output: 3896
Use a relative URL path for the /events WebSocket endpoint to preserve proxy routing.
Line 45 uses new URL("/events", hostUrl), which resets the pathname to the origin root. Since hostUrl for remote hosts is ${API_URL}/api/v2-hosts/{id}/trpc and the host-service's /events endpoint is registered at the app root (same level as /trpc), the absolute path /events routes the WebSocket connection to https://api.example.com/events instead of the correct https://api.example.com/api/v2-hosts/{id}/events. This causes event subscriptions to fail for remote/cloud hosts.
Suggested fix
- const url = new URL("/events", hostUrl);
+ const url = new URL("events", hostUrl);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/workspace-client/src/lib/eventBus.ts` around lines 44 - 50,
buildEventBusUrl currently uses new URL("/events", hostUrl) which resets the
pathname to root and breaks proxy routing for remote hosts; change it to append
a relative path (e.g., "events" instead of "/events") or otherwise join to the
existing hostUrl.pathname so the resulting origin+path becomes
.../api/v2-hosts/{id}/events rather than /events; keep the protocol selection
and token query logic (function buildEventBusUrl and its wsToken handling) the
same while ensuring the path is appended relative to hostUrl.
Comment on lines
+98
to
+105
| function connect( | ||
| state: ConnectionState, | ||
| hostUrl: string, | ||
| getWsToken: () => string | null, | ||
| ): void { | ||
| if (state.disposed) return; | ||
|
|
||
| const wsUrl = buildEventBusUrl(hostUrl, getWsToken()); |
Contributor
There was a problem hiding this comment.
Refresh the token source when reusing a shared connection.
Lines 158-160 reuse the existing ConnectionState by hostUrl only, while Lines 98-150 keep reconnect logic closed over the creation-time getWsToken. After a token rotation or session change, reconnects will still use the stale getter and can come back with expired or wrong credentials. Store the latest getter in ConnectionState and have reconnects read from that shared state.
Also applies to: 122-150, 154-173
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/workspace-client/src/lib/eventBus.ts` around lines 98 - 105, The
reconnect logic currently closes over the creation-time getWsToken causing stale
tokens; modify ConnectionState to include a mutable getWsToken field and update
connect (and any reconnect handlers used in the connect/reconnect flow such as
the code paths referenced around lines 122-150 and 154-173) to read the token
via state.getWsToken instead of the original closed-over getter; also update
callers that create or reuse ConnectionState to assign the latest getter into
state.getWsToken so all subsequent reconnects use the refreshed token source.
Delete getWorkspaceRowMocks entirely — no more fake data in the sidebar. workspaceStatus will be null until wired up with real data.
Move all host-service-related hooks to renderer/hooks/host-service/ following repo structure guidelines (one hook per file, folder per hook, matching names). These will eventually move to a shared package. - Split useWorkspaceEvent.ts into useWorkspaceEvent/ and useWorkspaceHostUrl/ - Rename useDashboardDiffStats/ → useDiffStats/ (match hook name) - Move useFileTree/ and useFileDocument/ from v2-workspace route - Update all consumer import paths
Contributor
There was a problem hiding this comment.
2 issues found across 5 files (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/host-service/src/events/git-watcher.ts">
<violation number="1" location="packages/host-service/src/events/git-watcher.ts:79">
P2: Guard debounced notifications so a pending timer does not emit events for a workspace that has already been removed/unwatched.</violation>
</file>
<file name="apps/desktop/src/renderer/hooks/host-service/useDiffStats/useDiffStats.ts">
<violation number="1" location="apps/desktop/src/renderer/hooks/host-service/useDiffStats/useDiffStats.ts:31">
P2: Path deduplication currently overwrites staged/unstaged entries, which can lose diff counts for partially staged files.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Comment on lines
+79
to
+81
| for (const listener of this.listeners) { | ||
| listener(workspaceId); | ||
| } |
Contributor
There was a problem hiding this comment.
P2: Guard debounced notifications so a pending timer does not emit events for a workspace that has already been removed/unwatched.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/host-service/src/events/git-watcher.ts, line 79:
<comment>Guard debounced notifications so a pending timer does not emit events for a workspace that has already been removed/unwatched.</comment>
<file context>
@@ -54,16 +59,28 @@ export class GitWatcher {
+ workspaceId,
+ setTimeout(() => {
+ this.debounceTimers.delete(workspaceId);
+ for (const listener of this.listeners) {
+ listener(workspaceId);
+ }
</file context>
Suggested change
| for (const listener of this.listeners) { | |
| listener(workspaceId); | |
| } | |
| if (this.closed || !this.watched.has(workspaceId)) return; | |
| for (const listener of this.listeners) { | |
| listener(workspaceId); | |
| } |
- Add 300ms per-workspace debounce to GitWatcher — a single git operation no longer fires dozens of events - Remove dead v2DeviceType and v2UsersDeviceRole enums from schema - Make v2_hosts.machineId nullable (cloud hosts have no machine) - Deduplicate files by path in useDiffStats to prevent double-counting when a file appears in both againstBase and staged/unstaged - Remove getConnectionKey no-op function from event bus client
saddlepaddle
force-pushed
the
saddlepaddle/nickel-bladder
branch
from
d0bc94e to
1aeccdc
Compare
MocA-Love
pushed a commit
to MocA-Love/superset
that referenced
this pull request
Apr 10, 2026
…rset-sh#3224) * WIP - checkpoint * feat: unified WS event bus, v2Hosts data model, real diff stats - Add unified /events WebSocket endpoint on host-service replacing per-workspace filesystem WS connections. Carries git:changed (auto) and fs:events (on-demand) over a single connection per host. - Add v2_hosts, v2_clients, v2_users_hosts tables replacing v2_devices, v2_device_presence, v2_users_devices. Workspaces now reference hostId instead of deviceId. Hosts identified by machineId (null = cloud). - Wire real git diff stats into left sidebar chips using event bus to trigger refetches instead of polling. - Update tRPC routers (ensureV2Host, ensureV2Client, workspace create), Electric SQL sync, and all renderer references. * fix: multi-host routing, GIT_OPTIONAL_LOCKS, UI polish - Route diff stats per-workspace to correct host (local vs remote proxy URL) - Set GIT_OPTIONAL_LOCKS=0 on all host-service git operations to prevent index.lock contention with user git commands - Fix Changes tab header stats not updating when switching filter dropdown - Fix diff stats pill padding (w-fit + justify-self-end) - Restore LuFolderGit2 icon for local workspaces - Update WorkspaceHostTarget kind from "device" to "host" - Make v2_hosts.machineId NOT NULL (every host is a machine) - Bump PR polling to 10s (host-service + client) * refactor: per-workspace diff stats via useWorkspaceEvent hook - Add useWorkspaceEvent(type, workspaceId, callback) hook that resolves workspace → host → event bus connection automatically - Simplify useDiffStats to a per-workspace hook called from each DashboardSidebarWorkspaceItem instead of batch-fetching in sidebar data - Remove diff stats logic from useDashboardSidebarData (no more workspaceHosts mapping, diffStatsByWorkspaceId, or diffStats on type) - Pass diffStats as prop to ExpandedWorkspaceRow and HoverCardContent * chore: remove workspace status mock data Delete getWorkspaceRowMocks entirely — no more fake data in the sidebar. workspaceStatus will be null until wired up with real data. * refactor: delete FS event adapter layer, event-driven Changes tab - Extend useWorkspaceEvent to support "fs:events" (with watchFs/unwatchFs) - Move useFileTree + useFileDocument from workspace-client to desktop app, swap useWorkspaceFsEvents → useWorkspaceEvent internally - Update FilesTab to use useWorkspaceEvent directly for search invalidation - Switch useChangesTab from 3s polling to git:changed event bus invalidation - Delete workspaceFsEventRegistry, useWorkspaceFsEventBridge, useWorkspaceFsEvents from workspace-client entirely * fix: re-export FS types from workspace-fs/client instead of /host Renderer code shouldn't import from @superset/workspace-fs/host. Re-export FsEntry, FsEntryKind, FsWatchEvent from the browser-safe /client entry point. * fix: host URL resolution race, editor width, watchFs ref counting - Remove electron IPC dependency from useWorkspaceHostUrl — resolve local vs remote by checking if org has a running host-service in the services map instead of comparing machineId - Fix CodeRenderer not taking full pane width (add w-full + min-w-0) - Fix watchFs/unwatchFs lacking ref counting in event bus client (multiple subscribers could prematurely unwatch) * fix: ref-count watchFs/unwatchFs in event bus client Multiple subscribers to fs:events for the same workspace now correctly share a single watch. Only the first subscriber sends fs:watch, only the last unsubscriber sends fs:unwatch. * refactor: co-locate host-service hooks in renderer/hooks/host-service Move all host-service-related hooks to renderer/hooks/host-service/ following repo structure guidelines (one hook per file, folder per hook, matching names). These will eventually move to a shared package. - Split useWorkspaceEvent.ts into useWorkspaceEvent/ and useWorkspaceHostUrl/ - Rename useDashboardDiffStats/ → useDiffStats/ (match hook name) - Move useFileTree/ and useFileDocument/ from v2-workspace route - Update all consumer import paths * fix: debounce git watcher, remove stale enums, fix double-counting - Add 300ms per-workspace debounce to GitWatcher — a single git operation no longer fires dozens of events - Remove dead v2DeviceType and v2UsersDeviceRole enums from schema - Make v2_hosts.machineId nullable (cloud hosts have no machine) - Deduplicate files by path in useDiffStats to prevent double-counting when a file appears in both againstBase and staged/unstaged - Remove getConnectionKey no-op function from event bus client
MocA-Love
added a commit
to MocA-Love/superset
that referenced
this pull request
Apr 10, 2026
After cherry-picking upstream superset-sh#3250 (relay) which renamed host-service-manager → host-service-coordinator: 1. main/index.ts: alias import as getHostServiceManager for minimal diff with fork's quit lifecycle code 2. tray/index.ts: alias import, simplify tray menu to use new coordinator API (getProcessStatus instead of getServiceInfo, remove degraded/restarting statuses, remove restart button that now requires SpawnConfig) 3. FilesPane.tsx: migrate useWorkspaceFsEventBridge/useWorkspaceFsEvents to useWorkspaceEvent (from upstream superset-sh#3224 event bus) 4. WorkspaceFilePreviewContent.tsx: useFileDocument import path 5. WorkspaceFilesTreeItem.tsx: FileTreeNode import path 6. settings-state.ts: fix duplicate "project" entry, add "security" 7. settings/index.ts: fix missing closure in preventAgentSleep mutation
14 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
/eventsWebSocket endpoint on host-service replacing per-workspace filesystem WS connections. Carriesgit:changed(auto-pushed for all workspaces) andfs:events(on-demand per client request). Client-side connection manager with reconnection and ref-counting.v2Devices/v2DevicePresence/v2UsersDeviceswithv2Hosts(machineId NOT NULL, lastSeenAt),v2Clients(machineId, userId, type: desktop/mobile/web),v2UsersHosts(owner/member).v2Workspaces.deviceId→hostId.+N / -Nfromgit.getStatus, triggered bygit:changedevents instead of polling. Multi-host routing ready (local → localhost, remote → proxy URL pattern).Test plan
git diff --shortstat/eventsWS connection per hostgit checkout/git commitin terminal doesn't hit index.lock errorsbun run lint:fix && bun run typecheckpassesSummary by cubic
Unifies host-service events behind a single
/eventsWebSocket and completes thev2Hosts/v2Clientsmigration, delivering live diff stats and an event‑driven Changes tab. Reduces connections, improves multi‑host routing, and avoids git lock conflicts.New Features
/eventsWS per host emitsgit:changed(auto) andfs:events(opt‑in); one shared connection with reconnection and ref‑counting.useDiffStatsanduseWorkspaceEvent; dedupes by path to avoid double‑counting; Changes tab invalidates status/commits ongit:changed(no polling).v2Hosts,v2Clients,v2UsersHosts; workspaces now usehostId.ensureV2Host(machineId)and newensureV2Client.Bug Fixes
api/v2-hosts/:hostId/trpc).watchFs/unwatchFs; 300ms per‑workspace debounce in GitWatcher; removed legacy FS event bridge inworkspace-client.GIT_OPTIONAL_LOCKS=0to avoid index.lock contention.LuFolderGit2, editor width); cleanup of stale DB enums; re‑exported FS types from@superset/workspace-fs/client.Written for commit 1aeccdc. Summary will update on new commits.
Summary by CodeRabbit
New Features
Refactor
Performance