Add hosts settings page for managing host access#3768
Conversation
Adds a Hosts entry to the Organization group in settings, mirroring the existing Projects two-pane layout: sub-sidebar lists the org's hosts, detail page shows members with owner-only add/remove/role-change. New v2Host tRPC router handles the membership mutations with owner-role enforcement.
|
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:
📝 WalkthroughWalkthroughAdds a Hosts settings section: sidebar listing and index/detail routes, host detail UI for membership management (members table, add/remove/set-role), client optimistic collection sync to server, and new server-side v2Host tRPC mutations for membership operations. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as HostSettings UI
participant Hook as useOptimisticCollectionActions
participant Coll as v2UsersHosts Collection
participant tRPC as v2Host Router
participant Server as Backend
participant DB as Database
User->>UI: Click "Add Member"
UI->>Hook: v2UsersHosts.addMember({hostId,userId,organizationId,role?})
Hook->>Coll: insert({id,hostId,userId,role,createdAt,updatedAt}) -- optimistic
Coll->>UI: render optimistic member
Coll->>tRPC: v2Host.addMember({id,hostId,userId,role})
tRPC->>Server: validate caller & org membership
Server->>DB: check existing membership / insert
alt membership exists
Server-->>tRPC: CONFLICT
tRPC-->>Coll: error (resolve tx)
Coll-->>UI: revert + show error
else inserted
DB-->>Server: inserted row + txid
Server-->>tRPC: {row,txid}
tRPC-->>Coll: {row,txid}
Coll->>Coll: resolve Electric tx with txid
Coll->>UI: show success toast
end
UI-->>User: member confirmed
sequenceDiagram
participant User
participant UI as HostSettings UI
participant Hook as useOptimisticCollectionActions
participant Coll as v2UsersHosts Collection
participant tRPC as v2Host Router
participant Server as Backend
participant DB as Database
User->>UI: Change member role
UI->>Hook: v2UsersHosts.setMemberRole(rowId, role)
Hook->>Coll: update(rowId,{role}) -- optimistic
Coll->>UI: show optimistic role change
Coll->>tRPC: v2Host.setMemberRole({hostId,userId,role})
tRPC->>Server: validate ownership & constraints
Server->>DB: check membership and owner counts
alt invalid demotion/removal
Server-->>tRPC: BAD_REQUEST
tRPC-->>Coll: error
Coll-->>UI: revert + show error
else updated
DB-->>Server: success + txid
Server-->>tRPC: {success:true,txid}
tRPC-->>Coll: {success:true,txid}
Coll->>Coll: resolve Electric tx with txid
Coll->>UI: show success toast
end
UI-->>User: role change confirmed
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
Greptile SummaryThis PR adds a Hosts settings page (sub-sidebar, detail view, member management) alongside a matching All four findings are P2: the last-owner guards in Confidence Score: 5/5Safe to merge — all findings are P2 style/hardening suggestions with no blocking correctness issues on the happy path. All four findings are P2: the TOCTOU race in the last-owner guards requires concurrent mutation requests to manifest (unlikely in a settings UI), the silent no-op in setMemberRole has no user-visible impact in normal flows, the loading flash is a minor UX glitch, and the unscoped orgUsers fetch doesn't expose wrong data due to downstream filters. No P0/P1 defects were identified. packages/trpc/src/router/v2-host/v2-host.ts — TOCTOU in removeMember/setMemberRole and silent no-op in setMemberRole worth addressing in a follow-up.
|
| Filename | Overview |
|---|---|
| packages/trpc/src/router/v2-host/v2-host.ts | New tRPC router with addMember/removeMember/setMemberRole; owner-guard and safety rails are logically correct but the last-owner checks are non-atomic (TOCTOU), and setMemberRole silently succeeds when the target user isn't on the host. |
| apps/desktop/src/renderer/routes/_authenticated/settings/hosts/$hostId/components/HostSettings/HostSettings.tsx | New host-members detail page; owner-only controls are correctly gated both on client (disabled state) and server; orgUsers query lacks an org-scope filter, and orgMembers depends on host?.organizationId which starts undefined. |
| apps/desktop/src/renderer/routes/_authenticated/settings/hosts/page.tsx | New index page that auto-redirects to the first (online-first) host; briefly renders "No hosts yet." on initial load before the live query hydrates. |
| apps/desktop/src/renderer/routes/_authenticated/settings/projects/page.tsx | Adds auto-redirect to first project; isEmpty check fires while both queries default to [] on first render, causing a transient "No projects yet." flash. |
| apps/desktop/src/renderer/routes/_authenticated/settings/hosts/components/HostsSettingsSidebar/HostsSettingsSidebar.tsx | New sidebar with live filter, online/offline grouping, and active-item highlighting; mirrors ProjectsSettingsSidebar pattern cleanly. |
| apps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/GeneralSettings.tsx | Adds Projects and Hosts entries to the Organization group; fixes active-route detection with fuzzy matching; removes now-redundant ProjectsSettings component. |
| apps/desktop/src/renderer/routes/_authenticated/settings/utils/settings-search/settings-search.ts | Adds three new HOST_* search items under the "hosts" section with sensible keywords; no issues found. |
| apps/desktop/src/renderer/routes/_authenticated/settings/layout.tsx | Registers "hosts" in SECTION_ORDER, getSectionFromPath, getPathFromSection, and the skip-navigation guard; looks correct. |
| apps/desktop/src/renderer/routes/_authenticated/settings/projects/components/ProjectsSettingsSidebar/ProjectsSettingsSidebar.tsx | Adds a live name filter to the existing sidebar; noMatches guard correctly excludes the filter-is-empty case. |
Sequence Diagram
sequenceDiagram
participant UI as HostSettings (UI)
participant tRPC as v2Host tRPC Router
participant DB as Database
UI->>tRPC: addMember({ hostId, userId, role })
tRPC->>DB: requireHostOwner (check v2Hosts + v2UsersHosts)
tRPC->>DB: requireOrgMember (check members table)
tRPC->>DB: INSERT v2UsersHosts (onConflictDoNothing)
tRPC-->>UI: { success: true }
UI->>tRPC: removeMember({ hostId, userId })
tRPC->>DB: requireHostOwner
tRPC->>DB: SELECT role from v2UsersHosts (target)
tRPC->>DB: countOwners (if target is owner)
Note over tRPC,DB: TOCTOU - count and delete are not atomic
tRPC->>DB: DELETE from v2UsersHosts
tRPC-->>UI: { success: true }
UI->>tRPC: setMemberRole({ hostId, userId, role })
tRPC->>DB: requireHostOwner
tRPC->>DB: COUNT owners WHERE userId != target (if role=member)
Note over tRPC,DB: TOCTOU - count and update are not atomic
tRPC->>DB: UPDATE v2UsersHosts SET role
Note over tRPC,DB: Silent no-op if userId not on host
tRPC-->>UI: { success: true }
Prompt To Fix All With AI
This is a comment left during a code review.
Path: packages/trpc/src/router/v2-host/v2-host.ts
Line: 103-118
Comment:
**Non-atomic last-owner guard (TOCTOU)**
`countOwners` is read and then the `delete` (or role `update`) runs as separate queries. Two concurrent requests arriving simultaneously — e.g., two owners each removing the other — can both pass the `countOwners > 1` check before either delete commits, leaving the host with zero owners. Using a database transaction (or a CTE-based conditional delete) would close this gap.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: packages/trpc/src/router/v2-host/v2-host.ts
Line: 189-197
Comment:
**`setMemberRole` silently succeeds for non-existent memberships**
If `input.userId` has no entry in `v2UsersHosts` (i.e., the user isn't on the host), the `UPDATE` matches zero rows but the procedure still returns `{ success: true }`. Callers get no indication that nothing changed. A quick existence check before the update (or checking `rowsAffected`) would surface this.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/settings/hosts/page.tsx
Line: 54-62
Comment:
**False "No hosts yet" flash on initial load**
`hosts` defaults to `[]` while the live query is still hydrating, so `hosts.length === 0` is `true` on the very first render and the "No hosts yet." message briefly appears before real data arrives. The same pattern applies to `ProjectsIndexPage`. Adding an explicit loading/initializing state before trusting an empty result would prevent this flash.
The same issue appears in `apps/desktop/src/renderer/routes/_authenticated/settings/projects/page.tsx` where `isEmpty = v2Projects.length === 0 && groups.length === 0` evaluates `true` on first render before either query resolves.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/settings/hosts/$hostId/components/HostSettings/HostSettings.tsx
Line: 78-85
Comment:
**`orgUsers` query has no organization scope**
The `orgUsers` live query fetches from `collections.users` without any `organizationId` filter, pulling all users in the local cache. While the downstream `candidates` list is correctly scoped through `orgMembers`, the `userMap` built from all users could resolve names/emails for users outside the current org if they appear in `hostUserRows`. Adding a filter analogous to the one in the `orgMembers` query would be safer and more efficient.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "add hosts settings page for managing hos..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| }); | ||
|
|
||
| return { success: true }; | ||
| }), | ||
|
|
||
| removeMember: protectedProcedure | ||
| .input( | ||
| z.object({ | ||
| hostId: z.string().uuid(), | ||
| userId: z.string().uuid(), | ||
| }), | ||
| ) | ||
| .mutation(async ({ ctx, input }) => { | ||
| const organizationId = requireActiveOrgId(ctx); | ||
| await requireHostOwner(ctx.session.user.id, input.hostId, organizationId); | ||
|
|
There was a problem hiding this comment.
countOwners is read and then the delete (or role update) runs as separate queries. Two concurrent requests arriving simultaneously — e.g., two owners each removing the other — can both pass the countOwners > 1 check before either delete commits, leaving the host with zero owners. Using a database transaction (or a CTE-based conditional delete) would close this gap.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/trpc/src/router/v2-host/v2-host.ts
Line: 103-118
Comment:
**Non-atomic last-owner guard (TOCTOU)**
`countOwners` is read and then the `delete` (or role `update`) runs as separate queries. Two concurrent requests arriving simultaneously — e.g., two owners each removing the other — can both pass the `countOwners > 1` check before either delete commits, leaving the host with zero owners. Using a database transaction (or a CTE-based conditional delete) would close this gap.
How can I resolve this? If you propose a fix, please make it concise.| ); | ||
| if (ownerCount.length === 0) { | ||
| throw new TRPCError({ | ||
| code: "BAD_REQUEST", | ||
| message: "A host must have at least one owner.", | ||
| }); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
setMemberRole silently succeeds for non-existent memberships
If input.userId has no entry in v2UsersHosts (i.e., the user isn't on the host), the UPDATE matches zero rows but the procedure still returns { success: true }. Callers get no indication that nothing changed. A quick existence check before the update (or checking rowsAffected) would surface this.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/trpc/src/router/v2-host/v2-host.ts
Line: 189-197
Comment:
**`setMemberRole` silently succeeds for non-existent memberships**
If `input.userId` has no entry in `v2UsersHosts` (i.e., the user isn't on the host), the `UPDATE` matches zero rows but the procedure still returns `{ success: true }`. Callers get no indication that nothing changed. A quick existence check before the update (or checking `rowsAffected`) would surface this.
How can I resolve this? If you propose a fix, please make it concise.| if (hosts.length === 0) { | ||
| return ( | ||
| <div className="flex items-center justify-center h-full p-6 text-sm text-muted-foreground"> | ||
| No hosts yet. | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| return null; |
There was a problem hiding this comment.
hosts defaults to [] while the live query is still hydrating, so hosts.length === 0 is true on the very first render and the "No hosts yet." message briefly appears before real data arrives. The same pattern applies to ProjectsIndexPage. Adding an explicit loading/initializing state before trusting an empty result would prevent this flash.
The same issue appears in apps/desktop/src/renderer/routes/_authenticated/settings/projects/page.tsx where isEmpty = v2Projects.length === 0 && groups.length === 0 evaluates true on first render before either query resolves.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/settings/hosts/page.tsx
Line: 54-62
Comment:
**False "No hosts yet" flash on initial load**
`hosts` defaults to `[]` while the live query is still hydrating, so `hosts.length === 0` is `true` on the very first render and the "No hosts yet." message briefly appears before real data arrives. The same pattern applies to `ProjectsIndexPage`. Adding an explicit loading/initializing state before trusting an empty result would prevent this flash.
The same issue appears in `apps/desktop/src/renderer/routes/_authenticated/settings/projects/page.tsx` where `isEmpty = v2Projects.length === 0 && groups.length === 0` evaluates `true` on first render before either query resolves.
How can I resolve this? If you propose a fix, please make it concise.| q.from({ users: collections.users }).select(({ users }) => ({ | ||
| id: users.id, | ||
| name: users.name, | ||
| email: users.email, | ||
| })), | ||
| [collections], | ||
| ); | ||
|
|
There was a problem hiding this comment.
orgUsers query has no organization scope
The orgUsers live query fetches from collections.users without any organizationId filter, pulling all users in the local cache. While the downstream candidates list is correctly scoped through orgMembers, the userMap built from all users could resolve names/emails for users outside the current org if they appear in hostUserRows. Adding a filter analogous to the one in the orgMembers query would be safer and more efficient.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/settings/hosts/$hostId/components/HostSettings/HostSettings.tsx
Line: 78-85
Comment:
**`orgUsers` query has no organization scope**
The `orgUsers` live query fetches from `collections.users` without any `organizationId` filter, pulling all users in the local cache. While the downstream `candidates` list is correctly scoped through `orgMembers`, the `userMap` built from all users could resolve names/emails for users outside the current org if they appear in `hostUserRows`. Adding a filter analogous to the one in the `orgMembers` query would be safer and more efficient.
How can I resolve this? If you propose a fix, please make it concise.
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (5)
apps/desktop/src/renderer/routes/_authenticated/settings/projects/components/ProjectsSettingsSidebar/ProjectsSettingsSidebar.tsx (1)
81-83: Optional: alignnoMatchescheck with the trimmed filter.
matches()uses the trimmed/lowercased value, butnoMatcheschecksfilter !== "". The current behavior is correct (whitespace-only input keepsv2Rows/v1Rowspopulated, sonoMatchesstays false), but using the trimmed value here makes intent clearer and avoids relying on that indirect interaction.♻️ Suggested tweak
- const isEmpty = totalUnfiltered === 0; - const noMatches = - !isEmpty && v2Rows.length === 0 && v1Rows.length === 0 && filter !== ""; + const trimmedFilter = filter.trim(); + const isEmpty = totalUnfiltered === 0; + const noMatches = + !isEmpty && + v2Rows.length === 0 && + v1Rows.length === 0 && + trimmedFilter !== "";And update the “No matches” message body to use
trimmedFilter(or keepfilterif you prefer to echo back the user's literal input).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/settings/projects/components/ProjectsSettingsSidebar/ProjectsSettingsSidebar.tsx` around lines 81 - 83, Update the noMatches computation to use the same trimmed/lowercased input that matches() uses: replace the filter check in the noMatches expression to compare against trimmedFilter (or its normalized form) instead of raw filter so whitespace-only input is handled consistently; also update the “No matches” message rendering to display trimmedFilter (or keep raw filter if you intentionally want to echo literal input) so the displayed text matches the value used in the noMatches logic.apps/desktop/src/renderer/routes/_authenticated/settings/hosts/$hostId/components/HostSettings/HostSettings.tsx (1)
162-174: Consider a confirmation step before removing a host member.
handleRemoveMemberimmediately calls the mutation on click. For a destructive action that revokes a teammate's access to a host, a confirm dialog (or undo toast) reduces the impact of accidental clicks. The current self-remove guard (disabled={busy || isSelf}) doesn't help against fat-fingering another member.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/settings/hosts/`$hostId/components/HostSettings/HostSettings.tsx around lines 162 - 174, Add a user confirmation step before performing the destructive remove action: in handleRemoveMember, prompt the user (using the app's modal/confirm utility or window.confirm) and only call apiTrpcClient.v2Host.removeMember.mutate({ hostId, userId }) if the user confirms; setBusy(true) after the confirmation (and reset in finally), and optionally replace or augment the confirmation with an undo-capable toast if available so the mutation can be reversed within a short window. Ensure you update any UI callers of handleRemoveMember to preserve the same signature and maintain the existing isSelf/busy guards.apps/desktop/src/renderer/routes/_authenticated/settings/hosts/layout.tsx (1)
9-9: Replace theascast with direct destructuring for better type safety.TanStack Router's
useParams()API returns a union type of all possible params from parent routes whenstrict: falseis used. Instead of casting, directly destructure the params to get simpler, type-checked access:♻️ Suggested approach
- const params = useParams({ strict: false }) as { hostId?: string }; + const { hostId } = useParams({ strict: false });This lets TypeScript infer the available params from your route structure while keeping the code cleaner and more maintainable.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/settings/hosts/layout.tsx` at line 9, Replace the unsafe type cast on the useParams call by destructuring the returned params directly: remove "const params = useParams({ strict: false }) as { hostId?: string }" and instead call useParams({ strict: false }) and extract hostId via const { hostId } = useParams({ strict: false }); this lets TypeScript infer the union of possible params rather than forcing an incorrect type, so update any subsequent uses of params.hostId to use hostId directly and adjust null/undefined checks accordingly.packages/trpc/src/router/v2-host/v2-host.ts (2)
89-103: ClarifyaddMembersemantics when the user is already a host member.
onConflictDoNothingmeans that if the target user already has any role on this host, the call is silently a no-op — including when the caller passed an explicitroleintending to change it. That's defensible (callers should usesetMemberRoleto change roles), but currently indistinguishable from a successful add. Consider either:
- Documenting this in a comment so future readers / API consumers understand
addMemberwill not update an existing role, or- Returning a discriminator (e.g.
{ success: true, added: boolean }) so the UI can react (toast "already a member" vs "added").🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/trpc/src/router/v2-host/v2-host.ts` around lines 89 - 103, The addMember DB insert uses onConflictDoNothing (v2UsersHosts.insert(...).onConflictDoNothing) which silently no-ops when the user already has any role, making it indistinguishable from a successful add; update the addMember handler to return a discriminator like { success: true, added: boolean } instead of a blind success so callers can tell if a new membership was created or it already existed—implement this by detecting whether the insert affected any rows (or by doing a lightweight existence check against v2UsersHosts before/after the insert) and keep the current behavior that role changes must go through setMemberRole, or alternatively add a clear inline comment next to the onConflictDoNothing call documenting that addMember will not update an existing role.
65-73: Use SQLcount()instead of materializing rows for better efficiency.
countOwnersfetches all owner rows just to read.length. Thecount()aggregate is more efficient—it performs the count at the database level without transferring row data.However, the proposed diff has implementation errors. The codebase pattern (seen in
packages/auth/src/server.ts) uses{ count: count() }as the property name and accesses the first result array element. Correct it to:-import { and, eq, ne } from "drizzle-orm"; +import { and, count, eq, ne } from "drizzle-orm"; @@ async function countOwners(hostId: string) { - const owners = await db - .select({ id: v2UsersHosts.id }) - .from(v2UsersHosts) - .where( - and(eq(v2UsersHosts.hostId, hostId), eq(v2UsersHosts.role, "owner")), - ); - return owners.length; + const result = await db + .select({ count: count() }) + .from(v2UsersHosts) + .where( + and(eq(v2UsersHosts.hostId, hostId), eq(v2UsersHosts.role, "owner")), + ); + return result[0]?.count ?? 0; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/trpc/src/router/v2-host/v2-host.ts` around lines 65 - 73, countOwners currently materializes rows to compute length; change it to perform a DB-side count by selecting { count: count() } from v2UsersHosts with the same where clause (use db.select({ count: count() }).from(v2UsersHosts).where(and(eq(v2UsersHosts.hostId, hostId), eq(v2UsersHosts.role, "owner")))), then read the numeric count from the first result (e.g., result[0].count) and return it as a number. Ensure you import/use the same count() helper as the codebase pattern and keep the function name countOwners and conditions unchanged.
🤖 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/settings/hosts/`$hostId/components/HostSettings/HostSettings.tsx:
- Around line 150-186: The three handlers (handleAddMember, handleRemoveMember,
handleSetRole) share a single busy flag which can be flipped false by a
later-resolving request; replace the boolean with a pending counter or
per-action pending map and derive busy from it. Concretely: introduce a
pendingCount state/ref (or a Record<string, number> for per-action keys),
increment it before the try in each handler and decrement it in finally
(guarding against negative), and setBusy(pendingCount > 0) or compute the
disabled state from the map (e.g., Object.values(map).some(v => v > 0)); update
the handlers to use increment/decrement instead of setBusy(true)/setBusy(false)
so the UI only becomes enabled when all in-flight mutations complete.
- Around line 303-341: The AddMemberDropdown component (function
AddMemberDropdown with props candidates: CandidateRow[], disabled, onPick) must
be extracted into its own foldered component per convention: create
HostSettings/AddMemberDropdown/AddMemberDropdown.tsx containing the component,
export it via HostSettings/AddMemberDropdown/index.ts, and update this file to
import AddMemberDropdown instead of declaring it inline; ensure the new file
preserves the same props shape (CandidateRow type import) and behavior (returns
Button when candidates.length === 0, DropdownMenu otherwise) and update any
local references to use the imported component.
- Around line 103-119: The code currently force-casts row.role to "owner" |
"member" inside the members useMemo, which can hide invalid DB values; update
the members mapping (the useMemo that consumes hostUserRows and userMap) to
validate row.role at runtime (e.g., check row.role === "owner" || row.role ===
"member") and then set role to that validated value, otherwise fall back to a
safe default like "member" (or skip the row if preferred) instead of using a
blind cast; keep the rest of the mapping (usersHostsId, userId, name, email)
unchanged and preserve the existing sort logic.
In
`@apps/desktop/src/renderer/routes/_authenticated/settings/hosts/components/HostsSettingsSidebar/HostsSettingsSidebar.tsx`:
- Around line 70-76: The filter input inside HostsSettingsSidebar (the <input>
with placeholder "Filter hosts...") lacks an accessible name; update the element
in HostsSettingsSidebar.tsx to provide one by adding an aria-label (e.g.,
aria-label="Filter hosts") or by adding a visually hidden <label> linked via id
to the input, ensuring the onChange/value behavior remains unchanged so screen
readers announce the control correctly.
- Around line 115-155: Extract the Section and HostLink components into their
own component folders and barrel exports: create
HostsSettingsSidebar/Section/Section.tsx exporting the Section component and
HostsSettingsSidebar/Section/index.ts exporting it as default (preserving its
props: title: string | null, children: React.ReactNode), and create
HostsSettingsSidebar/HostLink/HostLink.tsx plus index.ts for HostLink
(preserving props: row: HostRow, isActive: boolean and its use of cn, Link, and
HostRow type). In the original HostsSettingsSidebar file, replace the inline
definitions with imports from the new barrels and ensure any used
utilities/types (cn, Link, HostRow) are imported where needed; run type checks
and update relative import paths accordingly.
In `@apps/desktop/src/renderer/routes/_authenticated/settings/hosts/page.tsx`:
- Around line 44-52: The effect currently calls navigate whenever firstHostId
changes, causing unwanted redirects; modify the effect (useEffect) to only call
navigate when firstHostId is truthy AND different from the currently
selectedHostId (compare to selectedHostId) and only once on initial mount by
tracking a ref (e.g., hasRedirected via useRef<boolean>) so you skip redirects
after the first successful redirect; implement the check order: if
(!firstHostId) return; if (selectedHostId && firstHostId === selectedHostId)
return; if (hasRedirected.current) return; perform navigate({ to:
"/settings/hosts/$hostId", params: { hostId: firstHostId }, replace: true }) and
set hasRedirected.current = true.
In `@apps/desktop/src/renderer/routes/_authenticated/settings/projects/page.tsx`:
- Around line 24-39: The empty-state flash happens because
electronTrpc.workspaces.getAllGrouped.useQuery and the useLiveQuery for
collections.v2Projects both default to [] so isEmpty is true on first render;
update the component to wait for the queries to settle before showing "No
projects yet." by checking the loading/ready flags (e.g., useQuery's
isLoading/isFetched or status) for
electronTrpc.workspaces.getAllGrouped.useQuery and the loading state exposed by
useLiveQuery for v2Projects (confirm whether it's isLoading/isPending/status)
and only evaluate isEmpty or render the empty message once at least one of those
sources reports settled/fetched; reference the variables
electronTrpc.workspaces.getAllGrouped.useQuery, v2Projects (from useLiveQuery)
and the isEmpty check to locate and gate the empty-state rendering.
In `@packages/trpc/src/router/v2-host/v2-host.ts`:
- Around line 179-206: In setMemberRole: prevent the TOCTOU by wrapping the
owner-count check and the subsequent update in a single transaction (use
db.transaction or the project's transaction helper) and perform the count with
row locking (SELECT ... FOR UPDATE) on v2UsersHosts like removeMember does, then
only proceed to update; also verify the target membership exists before
returning success — after locking, query the specific (hostId, userId) row and
if missing return/throw a NOT_FOUND (or error) instead of silently succeeding;
refer to symbols setMemberRole, v2UsersHosts, db, and the removeMember
implementation for the exact transaction/locking pattern.
- Around line 138-155: The owner-count check in v2-host.ts (the call to
countOwners and the subsequent db.delete on v2UsersHosts) is non-atomic and
vulnerable to TOCTOU; wrap the read-and-delete in a single database transaction
(use db.transaction or your DB client's transaction helper) and perform the
owner lookup with a row lock (SELECT ... FOR UPDATE or your ORM's
forUpdate/select for update) inside that transaction, re-check owner count there
and only then run the delete of v2UsersHosts; ensure the functions/queries
involved (countOwners logic or its equivalent select, and the delete against
v2UsersHosts) are executed inside the same transaction scope so concurrent
removeMember calls cannot both pass the check.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/settings/hosts/`$hostId/components/HostSettings/HostSettings.tsx:
- Around line 162-174: Add a user confirmation step before performing the
destructive remove action: in handleRemoveMember, prompt the user (using the
app's modal/confirm utility or window.confirm) and only call
apiTrpcClient.v2Host.removeMember.mutate({ hostId, userId }) if the user
confirms; setBusy(true) after the confirmation (and reset in finally), and
optionally replace or augment the confirmation with an undo-capable toast if
available so the mutation can be reversed within a short window. Ensure you
update any UI callers of handleRemoveMember to preserve the same signature and
maintain the existing isSelf/busy guards.
In `@apps/desktop/src/renderer/routes/_authenticated/settings/hosts/layout.tsx`:
- Line 9: Replace the unsafe type cast on the useParams call by destructuring
the returned params directly: remove "const params = useParams({ strict: false
}) as { hostId?: string }" and instead call useParams({ strict: false }) and
extract hostId via const { hostId } = useParams({ strict: false }); this lets
TypeScript infer the union of possible params rather than forcing an incorrect
type, so update any subsequent uses of params.hostId to use hostId directly and
adjust null/undefined checks accordingly.
In
`@apps/desktop/src/renderer/routes/_authenticated/settings/projects/components/ProjectsSettingsSidebar/ProjectsSettingsSidebar.tsx`:
- Around line 81-83: Update the noMatches computation to use the same
trimmed/lowercased input that matches() uses: replace the filter check in the
noMatches expression to compare against trimmedFilter (or its normalized form)
instead of raw filter so whitespace-only input is handled consistently; also
update the “No matches” message rendering to display trimmedFilter (or keep raw
filter if you intentionally want to echo literal input) so the displayed text
matches the value used in the noMatches logic.
In `@packages/trpc/src/router/v2-host/v2-host.ts`:
- Around line 89-103: The addMember DB insert uses onConflictDoNothing
(v2UsersHosts.insert(...).onConflictDoNothing) which silently no-ops when the
user already has any role, making it indistinguishable from a successful add;
update the addMember handler to return a discriminator like { success: true,
added: boolean } instead of a blind success so callers can tell if a new
membership was created or it already existed—implement this by detecting whether
the insert affected any rows (or by doing a lightweight existence check against
v2UsersHosts before/after the insert) and keep the current behavior that role
changes must go through setMemberRole, or alternatively add a clear inline
comment next to the onConflictDoNothing call documenting that addMember will not
update an existing role.
- Around line 65-73: countOwners currently materializes rows to compute length;
change it to perform a DB-side count by selecting { count: count() } from
v2UsersHosts with the same where clause (use db.select({ count: count()
}).from(v2UsersHosts).where(and(eq(v2UsersHosts.hostId, hostId),
eq(v2UsersHosts.role, "owner")))), then read the numeric count from the first
result (e.g., result[0].count) and return it as a number. Ensure you import/use
the same count() helper as the codebase pattern and keep the function name
countOwners and conditions unchanged.
🪄 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: 2181d09d-1c22-41e1-b51c-e569811b3502
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (18)
apps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/GeneralSettings.tsxapps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/ProjectsSettings.tsxapps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/SettingsSidebar.tsxapps/desktop/src/renderer/routes/_authenticated/settings/hosts/$hostId/components/HostSettings/HostSettings.tsxapps/desktop/src/renderer/routes/_authenticated/settings/hosts/$hostId/components/HostSettings/index.tsapps/desktop/src/renderer/routes/_authenticated/settings/hosts/$hostId/page.tsxapps/desktop/src/renderer/routes/_authenticated/settings/hosts/components/HostsSettingsSidebar/HostsSettingsSidebar.tsxapps/desktop/src/renderer/routes/_authenticated/settings/hosts/components/HostsSettingsSidebar/index.tsapps/desktop/src/renderer/routes/_authenticated/settings/hosts/layout.tsxapps/desktop/src/renderer/routes/_authenticated/settings/hosts/page.tsxapps/desktop/src/renderer/routes/_authenticated/settings/layout.tsxapps/desktop/src/renderer/routes/_authenticated/settings/projects/components/ProjectsSettingsSidebar/ProjectsSettingsSidebar.tsxapps/desktop/src/renderer/routes/_authenticated/settings/projects/page.tsxapps/desktop/src/renderer/routes/_authenticated/settings/utils/settings-search/settings-search.tsapps/desktop/src/renderer/stores/settings-state.tspackages/trpc/src/root.tspackages/trpc/src/router/v2-host/index.tspackages/trpc/src/router/v2-host/v2-host.ts
💤 Files with no reviewable changes (1)
- apps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/ProjectsSettings.tsx
| const members: MemberRow[] = useMemo(() => { | ||
| return hostUserRows | ||
| .map((row) => { | ||
| const u = userMap.get(row.userId); | ||
| return { | ||
| usersHostsId: row.id, | ||
| userId: row.userId, | ||
| role: row.role as "owner" | "member", | ||
| name: u?.name ?? "Unknown user", | ||
| email: u?.email ?? "", | ||
| }; | ||
| }) | ||
| .sort((a, b) => { | ||
| if (a.role !== b.role) return a.role === "owner" ? -1 : 1; | ||
| return a.name.localeCompare(b.name); | ||
| }); | ||
| }, [hostUserRows, userMap]); |
There was a problem hiding this comment.
Narrow the role cast or validate it.
row.role as "owner" | "member" silently trusts the DB value. If the schema ever grows a third role (or returns null), this cast hides the bug and isOwner checks downstream will be wrong. A small runtime check (row.role === "owner" || row.role === "member") and falling back to "member" (or skipping the row) would be safer.
♻️ Suggested fix
- return hostUserRows
- .map((row) => {
- const u = userMap.get(row.userId);
- return {
- usersHostsId: row.id,
- userId: row.userId,
- role: row.role as "owner" | "member",
- name: u?.name ?? "Unknown user",
- email: u?.email ?? "",
- };
- })
+ return hostUserRows
+ .map((row): MemberRow => {
+ const u = userMap.get(row.userId);
+ const role: "owner" | "member" =
+ row.role === "owner" ? "owner" : "member";
+ return {
+ usersHostsId: row.id,
+ userId: row.userId,
+ role,
+ name: u?.name ?? "Unknown user",
+ email: u?.email ?? "",
+ };
+ })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/routes/_authenticated/settings/hosts/`$hostId/components/HostSettings/HostSettings.tsx
around lines 103 - 119, The code currently force-casts row.role to "owner" |
"member" inside the members useMemo, which can hide invalid DB values; update
the members mapping (the useMemo that consumes hostUserRows and userMap) to
validate row.role at runtime (e.g., check row.role === "owner" || row.role ===
"member") and then set role to that validated value, otherwise fall back to a
safe default like "member" (or skip the row if preferred) instead of using a
blind cast; keep the rest of the mapping (usersHostsId, userId, name, email)
unchanged and preserve the existing sort logic.
| function AddMemberDropdown({ | ||
| candidates, | ||
| disabled, | ||
| onPick, | ||
| }: { | ||
| candidates: CandidateRow[]; | ||
| disabled: boolean; | ||
| onPick: (userId: string) => void; | ||
| }) { | ||
| if (candidates.length === 0) { | ||
| return ( | ||
| <Button size="sm" variant="outline" disabled> | ||
| <HiOutlinePlus className="h-4 w-4 mr-1" /> | ||
| Add member | ||
| </Button> | ||
| ); | ||
| } | ||
|
|
||
| return ( | ||
| <DropdownMenu> | ||
| <DropdownMenuTrigger asChild> | ||
| <Button size="sm" variant="outline" disabled={disabled}> | ||
| <HiOutlinePlus className="h-4 w-4 mr-1" /> | ||
| Add member | ||
| </Button> | ||
| </DropdownMenuTrigger> | ||
| <DropdownMenuContent align="end" className="w-64"> | ||
| {candidates.map((c) => ( | ||
| <DropdownMenuItem key={c.userId} onSelect={() => onPick(c.userId)}> | ||
| <div className="flex flex-col"> | ||
| <span className="text-sm">{c.name}</span> | ||
| <span className="text-xs text-muted-foreground">{c.email}</span> | ||
| </div> | ||
| </DropdownMenuItem> | ||
| ))} | ||
| </DropdownMenuContent> | ||
| </DropdownMenu> | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Split AddMemberDropdown into its own file.
Per repo convention, each component lives in its own ComponentName/ComponentName.tsx folder. Move AddMemberDropdown to a sibling component file (e.g. HostSettings/AddMemberDropdown/AddMemberDropdown.tsx) and import it here.
As per coding guidelines: "Do not create multi-component files; use one component per file." and "Use one folder per component with structure: ComponentName/ComponentName.tsx + index.ts for barrel export."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/routes/_authenticated/settings/hosts/`$hostId/components/HostSettings/HostSettings.tsx
around lines 303 - 341, The AddMemberDropdown component (function
AddMemberDropdown with props candidates: CandidateRow[], disabled, onPick) must
be extracted into its own foldered component per convention: create
HostSettings/AddMemberDropdown/AddMemberDropdown.tsx containing the component,
export it via HostSettings/AddMemberDropdown/index.ts, and update this file to
import AddMemberDropdown instead of declaring it inline; ensure the new file
preserves the same props shape (CandidateRow type import) and behavior (returns
Button when candidates.length === 0, DropdownMenu otherwise) and update any
local references to use the imported component.
| function Section({ | ||
| title, | ||
| children, | ||
| }: { | ||
| title: string | null; | ||
| children: React.ReactNode; | ||
| }) { | ||
| return ( | ||
| <div> | ||
| {title && ( | ||
| <h2 className="text-xs font-semibold text-muted-foreground uppercase tracking-wide px-2 mb-2"> | ||
| {title} | ||
| </h2> | ||
| )} | ||
| <nav className="flex flex-col gap-0.5">{children}</nav> | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| function HostLink({ row, isActive }: { row: HostRow; isActive: boolean }) { | ||
| return ( | ||
| <Link | ||
| to="/settings/hosts/$hostId" | ||
| params={{ hostId: row.id }} | ||
| className={cn( | ||
| "flex items-center gap-2 px-2 py-1.5 text-sm rounded-md transition-colors", | ||
| isActive | ||
| ? "bg-accent text-accent-foreground" | ||
| : "text-muted-foreground hover:bg-accent/50 hover:text-foreground", | ||
| )} | ||
| > | ||
| <span | ||
| className={cn( | ||
| "h-1.5 w-1.5 rounded-full shrink-0", | ||
| row.isOnline ? "bg-emerald-500" : "bg-muted-foreground/40", | ||
| )} | ||
| /> | ||
| <span className="truncate flex-1">{row.name}</span> | ||
| </Link> | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Split Section and HostLink into their own files.
This file defines three components (HostsSettingsSidebar, Section, HostLink). Per repository convention, each component should live in its own ComponentName/ComponentName.tsx folder with an index.ts barrel, so these helpers should be extracted to siblings (e.g. HostsSettingsSidebar/Section/Section.tsx and HostsSettingsSidebar/HostLink/HostLink.tsx) and imported here.
As per coding guidelines: "Do not create multi-component files; use one component per file." and "Use one folder per component with structure: ComponentName/ComponentName.tsx + index.ts for barrel export."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/routes/_authenticated/settings/hosts/components/HostsSettingsSidebar/HostsSettingsSidebar.tsx`
around lines 115 - 155, Extract the Section and HostLink components into their
own component folders and barrel exports: create
HostsSettingsSidebar/Section/Section.tsx exporting the Section component and
HostsSettingsSidebar/Section/index.ts exporting it as default (preserving its
props: title: string | null, children: React.ReactNode), and create
HostsSettingsSidebar/HostLink/HostLink.tsx plus index.ts for HostLink
(preserving props: row: HostRow, isActive: boolean and its use of cn, Link, and
HostRow type). In the original HostsSettingsSidebar file, replace the inline
definitions with imports from the new barrels and ensure any used
utilities/types (cn, Link, HostRow) are imported where needed; run type checks
and update relative import paths accordingly.
| useEffect(() => { | ||
| if (firstHostId) { | ||
| navigate({ | ||
| to: "/settings/hosts/$hostId", | ||
| params: { hostId: firstHostId }, | ||
| replace: true, | ||
| }); | ||
| } | ||
| }, [firstHostId, navigate]); |
There was a problem hiding this comment.
Effect runs navigate even when firstHostId doesn't actually change.
useEffect reruns whenever firstHostId flips between null and an id (e.g. when the live query refreshes while user is on a child route). When user is already on /settings/hosts/$hostId, this index isn't mounted, so this is mostly fine. But if the live query briefly returns [] then repopulates after this index re-mounts, you can get a redirect loop to the "first" host even if the user manually navigated to a different one prior. Consider also guarding navigation against the case where firstHostId === selectedHostId already, or only redirecting once on initial mount via a ref.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/renderer/routes/_authenticated/settings/hosts/page.tsx`
around lines 44 - 52, The effect currently calls navigate whenever firstHostId
changes, causing unwanted redirects; modify the effect (useEffect) to only call
navigate when firstHostId is truthy AND different from the currently
selectedHostId (compare to selectedHostId) and only once on initial mount by
tracking a ref (e.g., hasRedirected via useRef<boolean>) so you skip redirects
after the first successful redirect; implement the check order: if
(!firstHostId) return; if (selectedHostId && firstHostId === selectedHostId)
return; if (hasRedirected.current) return; perform navigate({ to:
"/settings/hosts/$hostId", params: { hostId: firstHostId }, replace: true }) and
set hasRedirected.current = true.
| const { data: groups = [] } = | ||
| electronTrpc.workspaces.getAllGrouped.useQuery(); | ||
|
|
||
| const { data: v2Projects = [] } = useLiveQuery( | ||
| (q) => | ||
| q | ||
| .from({ projects: collections.v2Projects }) | ||
| .where(({ projects }) => | ||
| eq(projects.organizationId, activeOrganizationId ?? ""), | ||
| ) | ||
| .select(({ projects }) => ({ | ||
| id: projects.id, | ||
| name: projects.name, | ||
| })), | ||
| [collections, activeOrganizationId], | ||
| ); |
There was a problem hiding this comment.
Minor: avoid “No projects yet.” flash while queries load.
Both useQuery and useLiveQuery default to [] before they settle, so on the first render isEmpty is true and users with projects briefly see "No projects yet." before the redirect fires. Gate the empty state on a loaded/settled signal (or simply render null until at least one source has resolved).
♻️ Suggested fix
- const { data: groups = [] } =
- electronTrpc.workspaces.getAllGrouped.useQuery();
+ const { data: groups = [], isPending: groupsPending } =
+ electronTrpc.workspaces.getAllGrouped.useQuery();
- const { data: v2Projects = [] } = useLiveQuery(
+ const { data: v2Projects = [], isLoading: v2Loading } = useLiveQuery(
(q) =>
q
.from({ projects: collections.v2Projects })
.where(({ projects }) =>
eq(projects.organizationId, activeOrganizationId ?? ""),
)
.select(({ projects }) => ({
id: projects.id,
name: projects.name,
})),
[collections, activeOrganizationId],
);
@@
- const isEmpty = v2Projects.length === 0 && groups.length === 0;
- if (isEmpty) {
+ const isLoading = groupsPending || v2Loading;
+ const isEmpty =
+ !isLoading && v2Projects.length === 0 && groups.length === 0;
+ if (isEmpty) {
return (
<div className="flex items-center justify-center h-full p-6 text-sm text-muted-foreground">
No projects yet.
</div>
);
}Verify the exact loading-state field exposed by useLiveQuery from @tanstack/react-db (e.g., isLoading, status, isPending).
#!/bin/bash
# Inspect the useLiveQuery hook surface to confirm available loading-state fields
fd -t f 'react-db' node_modules 2>/dev/null
rg -nP --type=ts -C2 '\buseLiveQuery\b' -g '!**/node_modules/**' | head -100
rg -nP --type=ts -C3 'useLiveQuery\(' -g '!**/node_modules/**' | head -80Also applies to: 68-75
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/renderer/routes/_authenticated/settings/projects/page.tsx`
around lines 24 - 39, The empty-state flash happens because
electronTrpc.workspaces.getAllGrouped.useQuery and the useLiveQuery for
collections.v2Projects both default to [] so isEmpty is true on first render;
update the component to wait for the queries to settle before showing "No
projects yet." by checking the loading/ready flags (e.g., useQuery's
isLoading/isFetched or status) for
electronTrpc.workspaces.getAllGrouped.useQuery and the loading state exposed by
useLiveQuery for v2Projects (confirm whether it's isLoading/isPending/status)
and only evaluate isEmpty or render the empty message once at least one of those
sources reports settled/fetched; reference the variables
electronTrpc.workspaces.getAllGrouped.useQuery, v2Projects (from useLiveQuery)
and the isEmpty check to locate and gate the empty-state rendering.
| if (target.role === "owner") { | ||
| const ownerCount = await countOwners(input.hostId); | ||
| if (ownerCount <= 1) { | ||
| throw new TRPCError({ | ||
| code: "BAD_REQUEST", | ||
| message: "A host must have at least one owner.", | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| await db | ||
| .delete(v2UsersHosts) | ||
| .where( | ||
| and( | ||
| eq(v2UsersHosts.hostId, input.hostId), | ||
| eq(v2UsersHosts.userId, input.userId), | ||
| ), | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
drizzle-orm transaction with select for update postgres
💡 Result:
Yes, Drizzle ORM supports PostgreSQL transactions with SELECT FOR UPDATE using the .for('update') method on select queries within a transaction callback. This locks selected rows against concurrent modifications until the transaction commits. Example for a typical read-modify-write pattern (e.g., incrementing a counter safely): await db.transaction(async (tx) => { const counter = await tx .select .from(counters) .where(eq(counters.id, 1)) .for('update'); // Locks the row(s) if (counter.length > 0) { const newCount = counter[0].count + 1; await tx .update(counters) .set({ count: newCount }) .where(eq(counters.id, 1)); } }); Key points: - Use within db.transaction(async (tx) => { ... }) - the tx instance supports all query builders. - .for('update') adds FOR UPDATE clause; options like { skipLocked: true }, { noWait: true }, or { of: tableName } (for joins) are supported. - Works with PostgreSQL drivers like node-postgres (Pool recommended over Client for concurrency). - Official docs lack dedicated section (feature request open since 2024), but API is implemented in pg-core and confirmed in GitHub issues, Discord, and API references. - Alternative: Raw SQL with tx.execute(sqlSELECT ... FOR UPDATE); for complex cases. This prevents race conditions in concurrent scenarios.
Citations:
- 1: [FEATURE]: Document
SELECT FOR UPDATEand its variants drizzle-team/drizzle-orm#2875 - 2: https://orm.drizzle.team/docs/transactions
- 3: https://www.answeroverflow.com/m/1158726168191705118
- 4: https://drizzle-team-drizzle-orm.mintlify.app/api/postgresql/query-builders
- 5: https://drizzle-team-drizzle-orm.mintlify.app/queries/select
🏁 Script executed:
# First, check if the file exists and locate it
find . -name "v2-host.ts" -type f | head -20Repository: superset-sh/superset
Length of output: 110
🏁 Script executed:
# Also search for the file using git ls-files
git ls-files | grep -i "v2-host"Repository: superset-sh/superset
Length of output: 335
🏁 Script executed:
# Read the v2-host.ts file to examine the implementation
wc -l ./packages/trpc/src/router/v2-host/v2-host.tsRepository: superset-sh/superset
Length of output: 114
🏁 Script executed:
# Read the relevant section around lines 138-155
sed -n '120,170p' ./packages/trpc/src/router/v2-host/v2-host.tsRepository: superset-sh/superset
Length of output: 1245
🏁 Script executed:
# Also search for countOwners function definition
rg -n "countOwners" ./packages/trpc/src/router/v2-host/v2-host.tsRepository: superset-sh/superset
Length of output: 172
🏁 Script executed:
# Read the countOwners function definition
sed -n '60,75p' ./packages/trpc/src/router/v2-host/v2-host.tsRepository: superset-sh/superset
Length of output: 404
🏁 Script executed:
# Read a wider range around removeMember to see if there's transaction wrapping
sed -n '90,160p' ./packages/trpc/src/router/v2-host/v2-host.tsRepository: superset-sh/superset
Length of output: 1627
TOCTOU: owner-count check is not atomic with the delete.
The owner count read at line 139 and the DELETE at lines 148–155 are separate statements with no transaction wrapping. Two concurrent removeMember calls can both observe ownerCount > 1, then both succeed, leaving the host with zero owners and defeating the safety check.
Wrap the check and delete in a single transaction with SELECT FOR UPDATE locking to ensure the invariant holds under concurrency:
🔒 Sketch of a transactional fix
- const target = await db.query.v2UsersHosts.findFirst({
- where: and(
- eq(v2UsersHosts.hostId, input.hostId),
- eq(v2UsersHosts.userId, input.userId),
- ),
- columns: { role: true },
- });
-
- if (!target) {
- return { success: true };
- }
-
- if (target.role === "owner") {
- const ownerCount = await countOwners(input.hostId);
- if (ownerCount <= 1) {
- throw new TRPCError({
- code: "BAD_REQUEST",
- message: "A host must have at least one owner.",
- });
- }
- }
-
- await db
- .delete(v2UsersHosts)
- .where(
- and(
- eq(v2UsersHosts.hostId, input.hostId),
- eq(v2UsersHosts.userId, input.userId),
- ),
- );
+ await db.transaction(async (tx) => {
+ const target = await tx.query.v2UsersHosts.findFirst({
+ where: and(
+ eq(v2UsersHosts.hostId, input.hostId),
+ eq(v2UsersHosts.userId, input.userId),
+ ),
+ columns: { role: true },
+ });
+ if (!target) return;
+
+ if (target.role === "owner") {
+ const [{ value: ownerCount } = { value: 0 }] = await tx
+ .select({ value: count() })
+ .from(v2UsersHosts)
+ .where(
+ and(
+ eq(v2UsersHosts.hostId, input.hostId),
+ eq(v2UsersHosts.role, "owner"),
+ ),
+ )
+ .for("update");
+ if (ownerCount <= 1) {
+ throw new TRPCError({
+ code: "BAD_REQUEST",
+ message: "A host must have at least one owner.",
+ });
+ }
+ }
+
+ await tx
+ .delete(v2UsersHosts)
+ .where(
+ and(
+ eq(v2UsersHosts.hostId, input.hostId),
+ eq(v2UsersHosts.userId, input.userId),
+ ),
+ );
+ });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/trpc/src/router/v2-host/v2-host.ts` around lines 138 - 155, The
owner-count check in v2-host.ts (the call to countOwners and the subsequent
db.delete on v2UsersHosts) is non-atomic and vulnerable to TOCTOU; wrap the
read-and-delete in a single database transaction (use db.transaction or your DB
client's transaction helper) and perform the owner lookup with a row lock
(SELECT ... FOR UPDATE or your ORM's forUpdate/select for update) inside that
transaction, re-check owner count there and only then run the delete of
v2UsersHosts; ensure the functions/queries involved (countOwners logic or its
equivalent select, and the delete against v2UsersHosts) are executed inside the
same transaction scope so concurrent removeMember calls cannot both pass the
check.
Move addMember/removeMember/setMemberRole to the same Electric pattern as v2Projects/v2Workspaces: procedures wrap writes in dbWs.transaction and return txid, the v2UsersHosts collection forwards mutations through onInsert/onUpdate/onDelete, and HostSettings calls into useOptimisticCollectionActions.v2UsersHosts. Failures roll the local state back and surface a toast; success is the optimistic update. Also breaks HostSettings into co-located subcomponents (HostHeader, MembersTable, MemberRow, AddMemberDropdown) per AGENTS.md.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
5 issues found across 19 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="packages/trpc/src/router/v2-host/v2-host.ts">
<violation number="1" location="packages/trpc/src/router/v2-host/v2-host.ts:89">
P2: `setMemberRole` silently returns `{ success: true }` when the target user has no membership row (zero rows updated). Unlike `removeMember`, which checks for the target first, this mutation masks invalid calls. Verify the row exists before updating, or check the affected row count and return an appropriate error.</violation>
<violation number="2" location="packages/trpc/src/router/v2-host/v2-host.ts:138">
P1: TOCTOU race condition: the owner-count check and the `DELETE` are not wrapped in a transaction. Two concurrent `removeMember` calls for different owners can both read `ownerCount > 1`, then both delete, leaving zero owners. Wrap the read + delete in `db.transaction(async (tx) => { ... })` and use `.for('update')` on the owner-count query to serialize concurrent mutations.</violation>
<violation number="3" location="packages/trpc/src/router/v2-host/v2-host.ts:179">
P1: Same TOCTOU race as `removeMember`: the owner-count query and the `UPDATE` are not in a transaction. Two concurrent role demotions can both pass the guard and leave the host with zero owners. Wrap in `db.transaction()` with a `FOR UPDATE` lock on the owner-count select.</violation>
</file>
<file name="apps/desktop/src/renderer/routes/_authenticated/settings/projects/page.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/settings/projects/page.tsx:68">
P2: False "No projects yet." flash on initial load. Both `groups` and `v2Projects` default to `[]` before their queries settle, so `isEmpty` is `true` on the first render. Gate the empty state on the loading/pending status of the queries to prevent this flash.</violation>
</file>
<file name="apps/desktop/src/renderer/routes/_authenticated/settings/hosts/page.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/settings/hosts/page.tsx:54">
P2: False "No hosts yet." flash on initial load. `hosts` defaults to `[]` while the live query hydrates, so the empty check passes on the first render before data arrives. Gate the empty state on a loading/settled signal (or return `null` until the query has resolved) to prevent the flash.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| } | ||
| }, [firstProjectId, navigate]); | ||
|
|
||
| const isEmpty = v2Projects.length === 0 && groups.length === 0; |
There was a problem hiding this comment.
P2: False "No projects yet." flash on initial load. Both groups and v2Projects default to [] before their queries settle, so isEmpty is true on the first render. Gate the empty state on the loading/pending status of the queries to prevent this flash.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/routes/_authenticated/settings/projects/page.tsx, line 68:
<comment>False "No projects yet." flash on initial load. Both `groups` and `v2Projects` default to `[]` before their queries settle, so `isEmpty` is `true` on the first render. Gate the empty state on the loading/pending status of the queries to prevent this flash.</comment>
<file context>
@@ -1,13 +1,78 @@
+ }
+ }, [firstProjectId, navigate]);
+
+ const isEmpty = v2Projects.length === 0 && groups.length === 0;
+ if (isEmpty) {
+ return (
</file context>
| } | ||
| }, [firstHostId, navigate]); | ||
|
|
||
| if (hosts.length === 0) { |
There was a problem hiding this comment.
P2: False "No hosts yet." flash on initial load. hosts defaults to [] while the live query hydrates, so the empty check passes on the first render before data arrives. Gate the empty state on a loading/settled signal (or return null until the query has resolved) to prevent the flash.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/routes/_authenticated/settings/hosts/page.tsx, line 54:
<comment>False "No hosts yet." flash on initial load. `hosts` defaults to `[]` while the live query hydrates, so the empty check passes on the first render before data arrives. Gate the empty state on a loading/settled signal (or return `null` until the query has resolved) to prevent the flash.</comment>
<file context>
@@ -0,0 +1,63 @@
+ }
+ }, [firstHostId, navigate]);
+
+ if (hosts.length === 0) {
+ return (
+ <div className="flex items-center justify-center h-full p-6 text-sm text-muted-foreground">
</file context>
Optimistic updates make changes appear instantly, but a successful remove can feel like the click did nothing. Show "Member added / removed / Role updated" once the server confirms persistence; failures still rollback and surface the existing error toast (no double-fire).
There was a problem hiding this comment.
1 issue found across 13 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/trpc/src/router/v2-host/v2-host.ts">
<violation number="1" location="packages/trpc/src/router/v2-host/v2-host.ts:81">
P1: `addMember` uses a non-atomic read-then-insert flow, so concurrent requests can bypass the pre-check and fail with an unhandled unique-constraint error instead of a deterministic conflict response.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Pull last-owner guards inside the same dbWs.transaction as the write and lock the other-owners rows with FOR UPDATE so concurrent removals or demotions serialize. Switch addMember to a single ON CONFLICT DO NOTHING insert that returns CONFLICT cleanly when the row already exists. setMemberRole now errors with NOT_FOUND if the target has no membership row instead of silently returning success. Also block removing the host's createdByUser: their host service will auto re-add them via ensureV2Host on next heartbeat, so the removal is a confusing transient. Demoting them to member is still allowed (ensureV2Host does onConflictDoNothing). aria-label on the host filter input.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
apps/desktop/src/renderer/routes/_authenticated/settings/hosts/components/HostsSettingsSidebar/HostsSettingsSidebar.tsx (1)
116-156: 🛠️ Refactor suggestion | 🟠 MajorSplit
SectionandHostLinkinto separate component files.This file still defines three components. Per repo convention, each should live in its own
ComponentName/ComponentName.tsxfolder with anindex.tsbarrel (e.g.,HostsSettingsSidebar/Section/Section.tsx,HostsSettingsSidebar/HostLink/HostLink.tsx).As per coding guidelines: "Do not create multi-component files; use one component per file." and "Use one folder per component with structure:
ComponentName/ComponentName.tsx+index.tsfor barrel export."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/settings/hosts/components/HostsSettingsSidebar/HostsSettingsSidebar.tsx` around lines 116 - 156, Split the inline components into separate component folders: create HostsSettingsSidebar/Section/Section.tsx (export default Section) and HostsSettingsSidebar/HostLink/HostLink.tsx (export default HostLink) each with a matching index.ts that re-exports the component; move the JSX and prop typings for Section and HostLink into those files (preserve props signatures: Section({ title, children }) and HostLink({ row, isActive }) where HostRow is the same type) and import any dependencies used inside (cn, Link, HostRow type) in the new files; then update the original HostsSettingsSidebar file to remove the component definitions and import Section and HostLink from their new barrel indexes. Ensure exports are default (or named consistent with callers) and update any relative import paths accordingly so the build passes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/settings/hosts/components/HostsSettingsSidebar/HostsSettingsSidebar.tsx`:
- Around line 116-156: Split the inline components into separate component
folders: create HostsSettingsSidebar/Section/Section.tsx (export default
Section) and HostsSettingsSidebar/HostLink/HostLink.tsx (export default
HostLink) each with a matching index.ts that re-exports the component; move the
JSX and prop typings for Section and HostLink into those files (preserve props
signatures: Section({ title, children }) and HostLink({ row, isActive }) where
HostRow is the same type) and import any dependencies used inside (cn, Link,
HostRow type) in the new files; then update the original HostsSettingsSidebar
file to remove the component definitions and import Section and HostLink from
their new barrel indexes. Ensure exports are default (or named consistent with
callers) and update any relative import paths accordingly so the build passes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 59bd8143-644e-4423-863b-8dfd2e6a8cfc
📒 Files selected for processing (2)
apps/desktop/src/renderer/routes/_authenticated/settings/hosts/components/HostsSettingsSidebar/HostsSettingsSidebar.tsxpackages/trpc/src/router/v2-host/v2-host.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/trpc/src/router/v2-host/v2-host.ts
The "can't remove yourself" / "can't demote yourself" guards block
valid actions ("step back from admin"). The real invariants are
owner-count (last owner can't leave) and creator (the user whose
machine runs the host service must stay owner) — both already
enforced.
Adds the missing creator-demotion guard in setMemberRole so demoting
the createdByUser to member is refused. Also drops the matching
isSelf disabling in MemberRow so the UI no longer hides actions the
server now allows.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/desktop/src/renderer/routes/_authenticated/settings/hosts/$hostId/components/HostSettings/HostSettings.tsx (2)
66-73:organizationIdfilter falls back to empty string before host loads.When
hostis still undefined on first render, this query runs witheq(m.organizationId, ""), returning zero candidates. Oncehostresolves, the query re-runs and populates correctly. Functionally fine, but if the query is observable/streamed it briefly subscribes to an always-empty filter. Consider gating the query (or memoizingcandidatesto be[]untilhostis defined) to skip the empty-string round-trip.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/settings/hosts/`$hostId/components/HostSettings/HostSettings.tsx around lines 66 - 73, The query currently runs with eq(m.organizationId, "") when host is undefined (see useLiveQuery, orgMembers, host?.organizationId, collections.members); change it to skip running until host is defined by gating the call — e.g. return an empty array or avoid calling useLiveQuery when host is falsy (or use the hook’s “enabled”/skip option if available) so the query is not subscribed with an empty-string filter and only executes once host.organizationId is present.
17-25: Consider a failure toast for membership mutations.The rejection handler is
() => {}, so persistence failures are silent for the user. The optimistic action presumably rolls back state, but the user won't get feedback that their action failed. A brieftoast.error("Failed to update members")(or similar) would close that gap.♻️ Sketch
function notifyOnPersist( tx: PersistableTransaction | null, successMessage: string, + errorMessage = "Something went wrong. Please try again.", ) { tx?.isPersisted.promise.then( () => toast.success(successMessage), - () => {}, + () => toast.error(errorMessage), ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/settings/hosts/`$hostId/components/HostSettings/HostSettings.tsx around lines 17 - 25, notifyOnPersist currently swallows persistence rejections (tx?.isPersisted.promise.then(..., () => {})), leaving users without feedback on failures; update the rejection handler in notifyOnPersist to call toast.error(...) with an appropriate message (e.g., "Failed to update members") so persistence failures surface to the user, keeping the success handler as toast.success(successMessage) and using the existing tx and successMessage parameters.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/trpc/src/router/v2-host/v2-host.ts`:
- Around line 149-167: The target row is selected without a FOR UPDATE lock
which allows two concurrent removeMember transactions to each see an owner and
both proceed; update the SELECT that fetches `target` (the query that selects
v2UsersHosts.role for the given hostId and userId) to use .for("update") so the
target row is locked in the transaction before the other-owners check, and apply
the identical change to the analogous target selection in `setMemberRole` so
concurrent role changes/ removals serialize correctly.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/settings/hosts/`$hostId/components/HostSettings/HostSettings.tsx:
- Around line 66-73: The query currently runs with eq(m.organizationId, "") when
host is undefined (see useLiveQuery, orgMembers, host?.organizationId,
collections.members); change it to skip running until host is defined by gating
the call — e.g. return an empty array or avoid calling useLiveQuery when host is
falsy (or use the hook’s “enabled”/skip option if available) so the query is not
subscribed with an empty-string filter and only executes once
host.organizationId is present.
- Around line 17-25: notifyOnPersist currently swallows persistence rejections
(tx?.isPersisted.promise.then(..., () => {})), leaving users without feedback on
failures; update the rejection handler in notifyOnPersist to call
toast.error(...) with an appropriate message (e.g., "Failed to update members")
so persistence failures surface to the user, keeping the success handler as
toast.success(successMessage) and using the existing tx and successMessage
parameters.
🪄 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: 81620c3c-813b-4cc1-b947-e57c31db27f2
📒 Files selected for processing (4)
apps/desktop/src/renderer/routes/_authenticated/settings/hosts/$hostId/components/HostSettings/HostSettings.tsxapps/desktop/src/renderer/routes/_authenticated/settings/hosts/$hostId/components/HostSettings/components/MembersTable/MembersTable.tsxapps/desktop/src/renderer/routes/_authenticated/settings/hosts/$hostId/components/HostSettings/components/MembersTable/components/MemberRow/MemberRow.tsxpackages/trpc/src/router/v2-host/v2-host.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src/renderer/routes/_authenticated/settings/hosts/$hostId/components/HostSettings/components/MembersTable/components/MemberRow/MemberRow.tsx
| if (target.role === "owner") { | ||
| const otherOwners = await tx | ||
| .select({ id: v2UsersHosts.id }) | ||
| .from(v2UsersHosts) | ||
| .where( | ||
| and( | ||
| eq(v2UsersHosts.hostId, input.hostId), | ||
| eq(v2UsersHosts.role, "owner"), | ||
| ne(v2UsersHosts.userId, input.userId), | ||
| ), | ||
| ) | ||
| .for("update"); | ||
| if (otherOwners.length === 0) { | ||
| throw new TRPCError({ | ||
| code: "BAD_REQUEST", | ||
| message: "A host must have at least one owner.", | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
PostgreSQL SELECT FOR UPDATE row locking with disjoint WHERE predicates concurrent transactions
💡 Result:
In PostgreSQL, SELECT FOR UPDATE acquires row-level locks only on the specific rows that match the WHERE predicate and are returned by the query. Locks are taken progressively as rows are identified and fetched during query execution. For concurrent transactions with disjoint (non-overlapping) WHERE predicates: - No blocking occurs. Each transaction locks only its own matching rows independently. - Transaction isolation (default READ COMMITTED) ensures each sees a consistent snapshot at query start, but since predicates don't overlap, no shared rows mean no lock conflicts. - Row locks do not affect data querying (plain SELECTs); they only block writers and conflicting lockers on the same row. Example: - T1: SELECT * FROM table WHERE id = 1 FOR UPDATE; -- locks row 1 - T2: SELECT * FROM table WHERE id = 2 FOR UPDATE; -- locks row 2, no wait If predicates do overlap on a row, the second transaction waits until the first commits/rolls back, then re-evaluates the condition on the updated row (if applicable). Use SKIP LOCKED to avoid waiting (skips locked rows), or NOWAIT to error immediately. For best concurrency with non-key updates, prefer FOR NO KEY UPDATE over FOR UPDATE. To prevent deadlocks in multi-row locks, always ORDER BY a consistent key (e.g., primary key). Higher isolation like REPEATABLE READ prevents anomalies but may cause serialization failures requiring retries.
Citations:
- 1: https://www.postgresql.org/docs/current/explicit-locking.html
- 2: https://www.postgresql.org/docs/current/interactive/sql-select.html
- 3: https://stackoverflow.com/questions/67180987/postgresql-select-for-update-what-happens-with-concurrent-long-running-quer
- 4: https://www.postgresql.org/docs/15/explicit-locking.html
🏁 Script executed:
# Check file exists and get lines 113-181 (removeMember) and 183-256 (setMemberRole)
wc -l packages/trpc/src/router/v2-host/v2-host.tsRepository: superset-sh/superset
Length of output: 112
🏁 Script executed:
# Read the removeMember and setMemberRole functions to verify the race condition
sed -n '113,181p' packages/trpc/src/router/v2-host/v2-host.ts | cat -nRepository: superset-sh/superset
Length of output: 2184
🏁 Script executed:
# Read setMemberRole function
sed -n '183,256p' packages/trpc/src/router/v2-host/v2-host.ts | cat -nRepository: superset-sh/superset
Length of output: 2426
🏁 Script executed:
# Check the transaction handling - look for transaction start and isolation level
sed -n '1,120p' packages/trpc/src/router/v2-host/v2-host.ts | grep -A 5 -B 5 "transaction\|readOnly"Repository: superset-sh/superset
Length of output: 464
Race condition confirmed: concurrent removal of different owners both pass the check.
With exactly two owners (e.g., A and B), simultaneous removeMember calls targeting different users can defeat the invariant:
- Tx1 (remove A): locks owner rows where
userId != A(locks B). Finds 1 other owner. Passes check. - Tx2 (remove B): locks owner rows where
userId != B(locks A). Finds 1 other owner. Passes check. - Both deletes commit → zero owners.
The issue: target is fetched without for("update"), so the two transactions don't conflict on the row being removed—only on disjoint sets of "other owners," which don't block each other.
Lock the target row in the transaction so concurrent operations serialize:
const [target] = await tx
.select({ role: v2UsersHosts.role })
.from(v2UsersHosts)
.where(and(
eq(v2UsersHosts.hostId, input.hostId),
eq(v2UsersHosts.userId, input.userId),
))
.for("update"); // ← Add this
The same fix applies to setMemberRole (line 225).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/trpc/src/router/v2-host/v2-host.ts` around lines 149 - 167, The
target row is selected without a FOR UPDATE lock which allows two concurrent
removeMember transactions to each see an owner and both proceed; update the
SELECT that fetches `target` (the query that selects v2UsersHosts.role for the
given hostId and userId) to use .for("update") so the target row is locked in
the transaction before the other-owners check, and apply the identical change to
the analogous target selection in `setMemberRole` so concurrent role changes/
removals serialize correctly.
1 participant
Summary
v2HosttRPC router (addMember,removeMember,setMemberRole) with owner-role gating and "must keep at least one owner" / "can't demote yourself" rails.ProjectsSettingsSidebarfor parity.Test plan
/settings/projects/$projectId(v1 + v2),/settings/security,/settings/account,/settings/git.🤖 Generated with Claude Code
Summary by cubic
Adds a Hosts settings page with a two-pane layout to manage per-host access, plus optimistic membership updates with server-confirmed toasts. The
v2HostAPI now performs atomic, serialized writes with clear errors.New Features
v2Host(addMember,removeMember,setMemberRole) requires active org and owner-only; atomic writes with row locks and last-owner checks; protects the creator; returnstxid; surfaces CONFLICT/NOT_FOUND; wired through optimisticv2UsersHosts.Refactors
HostSettingsvia a small notify-on-persist helper.Written for commit dc26e72. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes / Improvements