fix(desktop): tray shows correct org name for each host-service#3629
Conversation
host.info was calling organization.getActiveFromJwt, which resolves to organizationIds[0] on the JWT regardless of which org the host-service is configured for. Users in multiple orgs saw the same (first) membership name for every Host Service tray entry. Adds organization.getByIdFromJwt and has host.info use ctx.organizationId so each per-org host-service reports its own org.
📝 WalkthroughWalkthroughThe changes refactor organization lookup in the tRPC host service to accept an explicit Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 fixes a bug where the macOS tray submenu showed the same (first) organization name for every Host Service entry when a user belonged to multiple orgs. The root cause was Changes:
Confidence Score: 5/5Safe to merge — the fix is surgically correct, the new endpoint applies proper dual-layer auth (JWT + live DB), and no regressions are introduced. The bug is clearly identified and fixed at the source. The new getByIdFromJwt procedure follows the exact same security pattern as getActiveFromJwt (JWT array check + DB membership verification), and the cache now correctly scopes its single entry to the process's configured org ID. The only open items are non-blocking style suggestions (single-entry cache documentation and a potential query consolidation), neither of which affects correctness or security. No files require special attention.
|
| Filename | Overview |
|---|---|
| packages/host-service/src/trpc/router/host/host.ts | Passes ctx.organizationId to getOrganization and includes it in the cache key; removes the stale type cast on ctx.api. Single-entry cache is correct given per-process org assumption but could be made more explicit. |
| packages/trpc/src/router/organization/organization.ts | Adds getByIdFromJwt jwtProcedure with correct dual-layer validation (JWT organizationIds array + live DB membership check). Two sequential DB queries could be one join but is otherwise correct and safe. |
Sequence Diagram
sequenceDiagram
participant Tray as macOS Tray
participant HS as Host Service (per org)
participant API as TRPC API
participant JWT as JWT Claims
participant DB as Database
Tray->>HS: host.info query
HS->>HS: getOrganization(api, ctx.organizationId)
alt Cache hit (id matches & not expired)
HS-->>Tray: cached { id, name, slug }
else Cache miss
HS->>API: organization.getByIdFromJwt({ id: organizationId })
API->>JWT: check organizationIds.includes(id)
alt id not in JWT
API-->>HS: null → PRECONDITION_FAILED
else id in JWT
API->>DB: members WHERE userId & organizationId
alt No membership row
API-->>HS: null → PRECONDITION_FAILED
else Membership exists
API->>DB: organizations WHERE id
API-->>HS: { id, name, slug }
HS->>HS: update cache (orgId-keyed check)
HS-->>Tray: { hostId, hostName, organization, … }
end
end
end
Comments Outside Diff (1)
-
packages/host-service/src/trpc/router/host/host.ts, line 10-13 (link)Single-entry cache may miss in multi-org scenarios
cachedOrganizationis a module-level singleton that holds exactly one entry. The newid === organizationIdguard correctly prevents serving the wrong org's data, but it means whenever a request arrives for an org that isn't the currently-cached one, the cache is completely bypassed and a fresh API call is made.Per the PR description the host-service is "per-process configured", so in practice only one
organizationIdever flows through a given process and the single-entry cache is effectively correct. However, if that assumption ever relaxes (e.g. a single process handles multiple org connections), every org but the last cached one will incur a round-trip on every call.Consider either:
- A
Map<string, { data; cachedAt }>keyed byorganizationIdso each org gets its own TTL slot, or - A comment at the cache declaration noting the single-org-per-process assumption so future maintainers don't inadvertently break it.
Prompt To Fix With AI
This is a comment left during a code review. Path: packages/host-service/src/trpc/router/host/host.ts Line: 10-13 Comment: **Single-entry cache may miss in multi-org scenarios** `cachedOrganization` is a module-level singleton that holds exactly one entry. The new `id === organizationId` guard correctly prevents serving the wrong org's data, but it means whenever a request arrives for an org that isn't the currently-cached one, the cache is completely bypassed and a fresh API call is made. Per the PR description the host-service is "per-process configured", so in practice only one `organizationId` ever flows through a given process and the single-entry cache is effectively correct. However, if that assumption ever relaxes (e.g. a single process handles multiple org connections), every org but the last cached one will incur a round-trip on every call. Consider either: - A `Map<string, { data; cachedAt }>` keyed by `organizationId` so each org gets its own TTL slot, or - A comment at the cache declaration noting the single-org-per-process assumption so future maintainers don't inadvertently break it. How can I resolve this? If you propose a fix, please make it concise.
- A
Prompt To Fix All With AI
This is a comment left during a code review.
Path: packages/host-service/src/trpc/router/host/host.ts
Line: 10-13
Comment:
**Single-entry cache may miss in multi-org scenarios**
`cachedOrganization` is a module-level singleton that holds exactly one entry. The new `id === organizationId` guard correctly prevents serving the wrong org's data, but it means whenever a request arrives for an org that isn't the currently-cached one, the cache is completely bypassed and a fresh API call is made.
Per the PR description the host-service is "per-process configured", so in practice only one `organizationId` ever flows through a given process and the single-entry cache is effectively correct. However, if that assumption ever relaxes (e.g. a single process handles multiple org connections), every org but the last cached one will incur a round-trip on every call.
Consider either:
- A `Map<string, { data; cachedAt }>` keyed by `organizationId` so each org gets its own TTL slot, or
- A comment at the cache declaration noting the single-org-per-process assumption so future maintainers don't inadvertently break it.
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/organization/organization.ts
Line: 95-113
Comment:
**Two sequential DB round-trips can be collapsed into one**
`getByIdFromJwt` first queries `members` to verify membership exists, then queries `organizations` to fetch the org row. These are two separate database round-trips even though both conditions must be satisfied. A single join query would retrieve the org data while simultaneously confirming membership:
```ts
const result = await db
.select({ id: organizations.id, name: organizations.name, slug: organizations.slug })
.from(members)
.innerJoin(organizations, eq(organizations.id, members.organizationId))
.where(
and(
eq(members.userId, ctx.userId),
eq(members.organizationId, input.id),
),
)
.limit(1);
return result[0] ?? null;
```
This mirrors the pattern used in `getActive` on the session-based path and halves the DB round-trips for every tray refresh.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "fix(desktop): tray shows correct org nam..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| getByIdFromJwt: jwtProcedure | ||
| .input(z.object({ id: z.string() })) | ||
| .query(async ({ ctx, input }) => { | ||
| if (!ctx.organizationIds.includes(input.id)) return null; | ||
|
|
||
| const membership = await db.query.members.findFirst({ | ||
| where: and( | ||
| eq(members.userId, ctx.userId), | ||
| eq(members.organizationId, input.id), | ||
| ), | ||
| }); | ||
| if (!membership) return null; | ||
|
|
||
| const org = await db.query.organizations.findFirst({ | ||
| where: eq(organizations.id, input.id), | ||
| columns: { id: true, name: true, slug: true }, | ||
| }); | ||
| return org ?? null; | ||
| }), |
There was a problem hiding this comment.
getByIdFromJwt first queries members to verify membership exists, then queries organizations to fetch the org row. These are two separate database round-trips even though both conditions must be satisfied. A single join query would retrieve the org data while simultaneously confirming membership:
const result = await db
.select({ id: organizations.id, name: organizations.name, slug: organizations.slug })
.from(members)
.innerJoin(organizations, eq(organizations.id, members.organizationId))
.where(
and(
eq(members.userId, ctx.userId),
eq(members.organizationId, input.id),
),
)
.limit(1);
return result[0] ?? null;This mirrors the pattern used in getActive on the session-based path and halves the DB round-trips for every tray refresh.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/trpc/src/router/organization/organization.ts
Line: 95-113
Comment:
**Two sequential DB round-trips can be collapsed into one**
`getByIdFromJwt` first queries `members` to verify membership exists, then queries `organizations` to fetch the org row. These are two separate database round-trips even though both conditions must be satisfied. A single join query would retrieve the org data while simultaneously confirming membership:
```ts
const result = await db
.select({ id: organizations.id, name: organizations.name, slug: organizations.slug })
.from(members)
.innerJoin(organizations, eq(organizations.id, members.organizationId))
.where(
and(
eq(members.userId, ctx.userId),
eq(members.organizationId, input.id),
),
)
.limit(1);
return result[0] ?? null;
```
This mirrors the pattern used in `getActive` on the session-based path and halves the DB round-trips for every tray refresh.
How can I resolve this? If you propose a fix, please make it concise.![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/host-service/src/trpc/router/host/host.ts (1)
15-39: Cache scope is per-process/per-user — fine under current assumptions, worth a brief note.The module-level
cachedOrganizationis effectively a 1-entry cache keyed byorganizationId. That matches the PR intent because each host-service process has a fixedctx.organizationId(fromconfig.organizationIdinapp.ts) and a single user's JWT. Two caveats worth being aware of:
- The cache is not keyed by the caller's identity. If this process is ever reached by more than one JWT/user (e.g., future multi-tenant reuse of a host-service process), the first user's org record would be served to subsequent users. Not exploitable today given the one-user-per-host-service model, but brittle if that assumption changes.
- If membership for this org is revoked mid-session, stale data will still be returned until the 1h TTL expires (or the process restarts). The PR description mentions the "remove user from org B → reopen tray" flow — that works for a fresh process but not within an already-warm cache window.
Neither is blocking; consider at minimum a short comment documenting the single-user-per-process assumption, or incorporate
ctx.userId/JWT hash into the cache key if you want to harden against (1).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/host-service/src/trpc/router/host/host.ts` around lines 15 - 39, The module-level cachedOrganization in getOrganization is a 1-entry cache keyed only by organizationId and can serve the wrong org if this process ever handles multiple JWTs; either document the single-user-per-process assumption with a brief comment near cachedOrganization/ORGANIZATION_CACHE_TTL_MS, or harden the cache by including the caller identity in the key (e.g., store cachedOrganization.key = `${organizationId}:${callerIdOrJwtHash}` and compare that when validating the cache inside getOrganization) so the cache check uses both organizationId and the caller identity.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/host-service/src/trpc/router/host/host.ts`:
- Around line 15-39: The module-level cachedOrganization in getOrganization is a
1-entry cache keyed only by organizationId and can serve the wrong org if this
process ever handles multiple JWTs; either document the single-user-per-process
assumption with a brief comment near
cachedOrganization/ORGANIZATION_CACHE_TTL_MS, or harden the cache by including
the caller identity in the key (e.g., store cachedOrganization.key =
`${organizationId}:${callerIdOrJwtHash}` and compare that when validating the
cache inside getOrganization) so the cache check uses both organizationId and
the caller identity.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c9bc13d7-96ed-42f9-8cd1-e62cf8e05fe2
📒 Files selected for processing (2)
packages/host-service/src/trpc/router/host/host.tspackages/trpc/src/router/organization/organization.ts
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
1 participant
Summary
host.infowas callingorganization.getActiveFromJwt, which resolves toorganizationIds[0]on the JWT regardless of which org the host-service is configured for. Users in multiple orgs saw the same (first) membership name for every Host Service entry in the tray submenu.organization.getByIdFromJwt({ id })that verifies the requested id is in the JWT'sorganizationIdsand that the membership row still exists, then returns{ id, name, slug }.host.infonow passesctx.organizationId(the per-process configured org) so each instance reports its own org. Cache key now includes orgId.Test plan
Summary by cubic
Fixes the tray showing the wrong organization name for users in multiple orgs. Each Host Service entry now displays its own org, and stale JWTs no longer return a misleading name.
organization.getByIdFromJwt({ id })to validate the org against the JWT and active membership, returning{ id, name, slug }.host.infoto usectx.organizationIdand fetch via the new method; cache now keys by orgId.Written for commit 837f8ec. Summary will update on new commits.
Summary by CodeRabbit