refactor: rename Agents to Workspaces, Aliases to Presets#456
Conversation
Breaking rework of the data model and URL structure:
- apps/api: drop `agents` and `branches` tables/modules; add `workspaces`
(scoped per-org) and `presets` (scoped per-workspace; `name`, `slug`, `model`)
- apps/api: promote traces to top-level `GET /v1/traces` with optional
`?workspace=slug`; drop the nested `/agents/:slug/branches/:slug` shape
- apps/gateway: presets-first resolver (60s LRU); preset slug can shadow a
canonical model id; BYOK two-pass provider selection (all BYOK providers
in catalog order before platform defaults); OTEL tags become
`hebo.workspace.slug` / `hebo.preset.slug`
- apps/auth: rename `teams.agentSlug` to `teams.workspaceSlug`; API keys
stamp `{ workspaceId, workspaceSlug, createdByUserId }` in metadata;
org-owner/admin-only guard on API key creation via Better Auth hook
- packages/shared-api: expose `workspaceId` / `workspaceSlug` on request
context from API key metadata
- apps/console: new `/w/:workspaceSlug/*` routes; "Presets" UI replaces the
Models/Aliases page (name + slug + model only, no routing); sidebar
becomes static "Hebo.ai" wordmark + workspace switcher; nav order is
Overview / Presets / API Keys / Traces / Settings (API Keys hidden for
org `member` role); default-workspace bootstrap creates the Better Auth
team inline via `POST /v1/workspaces`
Closes #455
Co-authored-by: Heinrich Wendel <heiwen@users.noreply.github.com>
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (11)
📝 WalkthroughWalkthroughThis PR executes a comprehensive domain model refactoring from agents/branches to workspaces/presets across the entire platform. Database schema introduces ChangesWorkspace & Preset Foundation
API Backend - Workspaces & Presets
Gateway Model Resolution
Authentication & API Keys
Console Frontend - Shell & Navigation
Console Frontend - Workspace & Preset Routes
Console Frontend - Mock & Test Support
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests (beta)
|
|
@claude a few refinements: you can test functionality with playwright-cli and the respective SKILL. |
|
Claude finished @heiwen's task in 9m 43s —— View job Refinements appliedAll 8 items addressed — pushed as
Verification
|
- Auto-create default workspace (slug: default, name: "Default Workspace") when the user has none, instead of redirecting to the create dialog. - Drop random slug postfix on workspace creation; workspaces are unique per organization. - Fold the workspaces/presets tables into the api init migration and delete the intermediate typebox_to_zod + workspaces_and_presets + rename_team_agent_slug migrations. Auth init renames agentSlug -> workspaceSlug in place. - Remove the api-key role authz plugin so any org member can create API keys; granular permissions will be revisited later. - Close the new-preset dialog and reset inputs after a successful submission; surface 409 duplicate-slug errors as a field error on slug instead of a global toast. - Auto-populate preset slug from the name as the user types (until they edit it manually). - Revalidate the shell layout's workspace list when the active slug changes so the sidebar dropdown refreshes after creating a new workspace. Co-authored-by: Heinrich Wendel <heiwen@users.noreply.github.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: db130ab493
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (workspaceId) { | ||
| const preset = await lookupPreset(prismaClient, workspaceId, modelId); |
There was a problem hiding this comment.
resolveModelAlias only attempts preset lookup inside if (workspaceId), so any request that has only a workspace slug skips presets entirely and falls through to MODEL_NOT_FOUND for preset slugs. This now affects real flows: console-generated keys pass workspaceSlug metadata (not workspaceId) and cookie-auth gateway calls also lack workspaceId, so preset slugs cannot be used from the playground or those API keys. Add a slug-based fallback (or guarantee workspaceId is always present in auth metadata) before rejecting unknown model IDs.
Useful? React with 👍 / 👎.
| ...providerHandlers, | ||
| ...modelHandlers, | ||
| ]; | ||
| let handlers = [...traceHandlers, ...workspaceHandlers, ...providerHandlers]; |
There was a problem hiding this comment.
The MSW worker no longer includes a handler for the gateway models endpoint, but the shell loader and playground still call gateway.models.get(...). In console-only development with mocks (bun run -F @hebo/console dev), this request is now unhandled, leaving shellStore.models empty and breaking model-dependent UI (preset creation/model selection and playground model entries). Re-add the removed model handlers to the worker registration.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
apps/console/app/routes/_shell.w.$workspaceSlug._index/route.mdx (1)
14-14:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winStale "agent" terminology on the overview page.
Line 14 still reads
"code your first agent with our gateway"— this wasn't updated as part of the agents → workspaces rename and is now inconsistent with the rest of the UI.📝 Suggested copy fix
-Start using our interactive playground ({kbs("mod+P")}) or code your first agent with our gateway. +Start using our interactive playground ({kbs("mod+P")}) or connect your first app with our gateway.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/console/app/routes/_shell.w`.$workspaceSlug._index/route.mdx at line 14, The overview copy still uses the old "agent" term; locate the string Start using our interactive playground ({kbs("mod+P")}) or code your first agent with our gateway in the _shell.w.$workspaceSlug._index route MDX and update "agent" to the new terminology (e.g., "workspace") so it reads something like Start using our interactive playground ({kbs("mod+P")}) or code your first workspace with our gateway; ensure braces and kbs("mod+P") remain unchanged.apps/console/app/routes/_shell.w.create/route.tsx (1)
27-32:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winNon-409 server errors fall through to
result.data!.slug, which throws aTypeErrorat runtime.Eden Treaty returns
{ data: null, error: { status, value } }for non-2xx responses rather than throwing, so thetry/catchon line 23 doesn't cover a500or403. Whenresult.error?.status !== 409, execution reachesresult.data!.slugwithresult.data === null.🔧 Proposed fix
if (result.error?.status === 409) return submission.reply({ fieldErrors: { name: [parseError(result.error.value).message] }, }); + if (result.error) + return submission.reply({ formErrors: [parseError(result.error.value).message] }); + return redirect(`/w/${result.data!.slug}`);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/console/app/routes/_shell.w.create/route.tsx` around lines 27 - 32, The current code assumes result.data is present and always redirects to redirect(`/w/${result.data!.slug}`), which throws when Eden Treaty returns an error object; modify the control flow to first check if result.error exists: if result.error?.status === 409 keep the existing fieldErrors branch, else return a submission-level error (e.g., submission.reply({ formError: [parseError(result.error.value).message] }) or similar) so non-409 errors don't fall through, and only call redirect when result.data is non-null (use result.data.slug without the non-null assertion after confirming result.data).apps/console/app/routes/_shell.w.$workspaceSlug._index/api-key.tsx (1)
12-29:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReset the generated secret when the workspace param changes.
key,error, andloadingsurvive independently ofworkspaceSlug. If this route instance is reused across workspace switches, the previous workspace's API key remains visible on the new workspace page.♻️ Suggested fix
-import { useState } from "react"; +import { useEffect, useState } from "react"; import { useParams } from "react-router"; @@ export function GenerateApiKey() { const { workspaceSlug } = useParams(); const [loading, setLoading] = useState<"idle" | "loading" | "success">("idle"); const [key, setKey] = useState<string | null>(null); const [error, setError] = useState<string | null>(null); + + useEffect(() => { + setLoading("idle"); + setKey(null); + setError(null); + }, [workspaceSlug]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/console/app/routes/_shell.w`.$workspaceSlug._index/api-key.tsx around lines 12 - 29, Reset the per-workspace UI state when the route's workspaceSlug changes: add a useEffect that watches workspaceSlug and calls setKey(null), setError(null) and setLoading("idle") so previous workspace API keys or errors aren't shown after a workspace switch; locate the state hooks (key, error, loading) and the handleGenerateAPIKey handler in the component and implement the effect there.apps/api/src/modules/traces/index.ts (1)
23-39:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winClamp trace access to the authenticated workspace.
Both handlers trust
query.workspaceas the scope selector. With workspace-scoped API keys, a caller can omit that filter or point it at another workspace in the same org and still hit these endpoints. Derive the effective workspace from auth context first, and reject mismatches instead of letting the query param widen scope.🔒 Suggested shape
- async ({ greptimeDb, organizationId, query }) => { + async ({ greptimeDb, organizationId, query, workspaceSlug: authWorkspaceSlug }) => { + if (authWorkspaceSlug && query.workspace && query.workspace !== authWorkspaceSlug) { + return status(403, "Workspace scope mismatch"); + } + + const effectiveWorkspace = authWorkspaceSlug ?? query.workspace; return status( 200, await listTraces( greptimeDb, organizationId!, - query.workspace, + effectiveWorkspace, query.from ?? DEFAULT_FROM(), query.to ?? DEFAULT_TO(), query.page!, @@ - async ({ greptimeDb, organizationId, params, query }) => { - const spans = await getSpans(greptimeDb, organizationId!, query.workspace, params.traceId); + async ({ greptimeDb, organizationId, params, query, workspaceSlug: authWorkspaceSlug }) => { + if (authWorkspaceSlug && query.workspace && query.workspace !== authWorkspaceSlug) { + return status(403, "Workspace scope mismatch"); + } + + const effectiveWorkspace = authWorkspaceSlug ?? query.workspace; + const spans = await getSpans( + greptimeDb, + organizationId!, + effectiveWorkspace, + params.traceId, + ); if (spans.length === 0) return status(404, null); return status(200, spans); },Also applies to: 49-56
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/modules/traces/index.ts` around lines 23 - 39, The handler currently uses query.workspace directly (see the async ({ greptimeDb, organizationId, query }) => block and listTraces call), so change it to derive the effective workspace from the authenticated context (e.g., the request/auth token or context-provided workspace) and use that value when calling listTraces instead of query.workspace; if query.workspace is provided and does not match the authenticated workspace, return an error (403 Forbidden) rejecting the request. Apply the same change to the other handler referenced at lines 49-56 so both use the auth-derived workspace and validate/reject mismatches rather than trusting query.workspace.apps/console/app/mocks/db.ts (1)
4-20:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd a real workspace
idto the mock schema.Right now the mock layer can't mirror the production
PresetPlain.workspace_idcontract, soapps/console/app/mocks/routes/workspaces.tshas to write the workspace slug intoworkspace_id. The real API stores the actual workspace id there, so MSW responses are drifting from production and can hide UI bugs around preset/workspace wiring.Also applies to: 22-31, 52-58
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/console/app/mocks/db.ts` around lines 4 - 20, The mock workspace schema is missing a real workspace id field, causing MSW responses to use slug as workspace_id and diverge from the production PresetPlain.workspace_id contract; update workspaceSchema to include an "id" field (string) with a realistic default (e.g., "ws-<uuid>" or "workspace-dummy") alongside existing fields so presets (the presets getter / presetSchema) can reference workspace.id; ensure any places that construct PresetPlain mock objects use workspace.id for workspace_id rather than slug and update related defaults where similar schema blocks exist (lines referenced in the review: the other workspace schema duplicates) so MSW matches production shape.apps/console/app/routes/_shell.w.$workspaceSlug.api-keys/route.tsx (1)
34-39:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAlso stamp
workspaceIdwhen creating workspace-scoped keys.This action only forwards
{ workspaceSlug }, so keys created here won't satisfy the new{ workspaceId, workspaceSlug, createdByUserId }metadata contract. That leaves them scoped by slug alone and out of sync with the auth middleware/better-auth path that already supportsworkspaceId.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/console/app/routes/_shell.w`.$workspaceSlug.api-keys/route.tsx around lines 34 - 39, The call to authService.generateApiKey only passes { workspaceSlug: params.workspaceSlug } so generated keys lack workspaceId metadata; update the options argument passed to generateApiKey in the route handler to include the workspaceId (e.g., { workspaceId: <currentWorkspaceId>, workspaceSlug: params.workspaceSlug }) so the created key meets the { workspaceId, workspaceSlug, createdByUserId } metadata contract; locate the call to authService.generateApiKey (and usage of submission.value.name / API_KEY_EXPIRATION_OPTIONS) and ensure the route supplies the correct workspaceId from the current request/context when creating workspace-scoped keys.apps/api/src/modules/workspaces/index.ts (1)
35-63:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftAvoid orphaning the Better Auth team on partial failure.
This provisions the team and membership before the Prisma insert. If
addTeamMember()orprismaClient.workspaces.create()fails, the org is left with a dangling team that no workspace row points to. Add a compensation path that tears the team back down on later failures, or otherwise make the flow idempotent around the external side effect.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/modules/workspaces/index.ts` around lines 35 - 63, Create a compensation path so a partially created Better Auth team isn't left orphaned: after calling authClient!.organization.createTeam(...) and obtaining team.id, wrap the subsequent calls to authClient!.organization.addTeamMember(...) and prismaClient.workspaces.create(...) in a try/catch; if any of those fail, call the appropriate teardown (e.g. authClient!.organization.deleteTeam({ teamId: team.id }) or remove team member then delete team) to roll back the external side effect, log any deletion errors, and rethrow the original error so failure is visible to callers; ensure you reference team.id, workspaceSlug, authClient.organization.addTeamMember, authClient.organization.createTeam and prismaClient.workspaces.create when locating the code.
🧹 Nitpick comments (5)
apps/auth/prisma/schema.prisma (1)
127-139: ⚡ Quick winMissing DB-level unique constraint for per-org workspace slug uniqueness.
The PR explicitly requires workspace slugs to be unique per organization, but the
Teamsmodel only has@@index([organizationId]). Without a@@unique([organizationId, workspaceSlug])constraint, a TOCTOU window between the application-level uniqueness check and the insert allows two concurrentPOST /workspacescalls in the same org to silently create duplicate slugs.✅ Proposed fix
@@index([organizationId]) + @@unique([organizationId, workspaceSlug]) @@map("teams")Then run
prisma migrate devto generate the corresponding migration. Based on learnings, Prisma migrations are auto-generated by the CLI — do not edit migration files directly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/auth/prisma/schema.prisma` around lines 127 - 139, The Teams model is missing a DB-level unique constraint for per-organization workspace slugs; add a composite unique constraint on organizationId and workspaceSlug (e.g. @@unique([organizationId, workspaceSlug])) inside the Teams model to enforce uniqueness at the database level and prevent TOCTOU race conditions, then run the Prisma CLI (prisma migrate dev) to generate the migration rather than editing migration files manually.apps/api/src/modules/workspaces/types.ts (1)
3-11: ⚡ Quick win
slugfield lacks format constraints — invalid slugs can be stored via direct API calls.
WorkspaceCreateSchemaaccepts an optional explicit slug. Without a regex/format constraint, values like"My Workspace"or"UPPER"pass validation and are stored verbatim, breaking URL routing. (Implicit creation vianameis safe becauseslugFromStringnormalises it, but explicit slug bypasses that.)🔧 Suggested addition
- slug: z.string(), + slug: z.string().regex(/^[a-z0-9]+(?:-[a-z0-9]+)*$/, "Slug must be lowercase alphanumeric with hyphens"),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/modules/workspaces/types.ts` around lines 3 - 11, The slug field currently accepts arbitrary strings; update WorkspacePlainSchema (and the schema used for creates, e.g., WorkspaceCreateSchema) to add a format constraint that matches the canonical slug format (the same regex/format slugFromString produces), e.g. enforce lowercase letters/numbers and hyphens only (no spaces or uppercase) using z.string().regex(...). Keep existing trim/min checks and apply the same regex to both schemas so explicit slugs are normalized/validated consistently with implicit slug generation.apps/console/app/routes/_shell.w.$workspaceSlug.presets/form.tsx (1)
93-95: ⚡ Quick winSet an explicit relative
actionon both fetcher forms.Both forms currently post to the current URL implicitly. That gets brittle as soon as this list is rendered alongside a nested child route, because create/delete can start targeting the child URL instead of the presets resource route.
As per coding guidelines, "Use
useFetcherwith explicit relativeactionin section components so submissions go to the resource route without navigating away".Also applies to: 171-172
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/console/app/routes/_shell.w`.$workspaceSlug.presets/form.tsx around lines 93 - 95, The fetcher.Form instances (the ones around the delete/create preset blocks referencing preset.slug) currently rely on implicit posting to the current URL; add an explicit relative action that targets the presets resource route (e.g., action="./presets" or the appropriate relative path to the presets action) on both fetcher.Form elements so submissions always go to the presets resource handler rather than a nested child route.apps/console/app/routes/_shell.w.$workspaceSlug.presets/route.tsx (1)
80-92: 💤 Low valueInconsistent response shape for delete intent.
The delete intent returns
{ intent, error }while create/update return{ intent, submission }. This breaks the pattern established by the coding guidelines (return{ intent, submission: submission.reply() }). If the consuming form component filterslastResultby intent and expects asubmissionfield, this could cause issues.Consider aligning the delete response with the other intents, or verify that the form component (
./form) handles this alternative shape correctly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/console/app/routes/_shell.w`.$workspaceSlug.presets/route.tsx around lines 80 - 92, The delete branch in the route handler currently returns { intent } or { intent, error }, which differs from the create/update responses; update the delete handling in the intent === "delete" branch so it always returns the same shape by including a submission field (e.g., return { intent, submission: null } on success and return { intent, submission: null, error: parseError(error).message } in the catch), changing the code around api.workspaces(...).presets(...).delete() and the catch that references parseError(error) so consumers expecting a submission field (like the form component) receive a consistent response shape.apps/console/app/mocks/routes/workspaces.ts (1)
1-116: 💤 Low valueMissing
GET /workspaces/:workspaceSlug/presets/:presetSlugendpoint.The API has a
GET /:presetSlugendpoint (per context snippet fromapps/api/src/modules/presets/index.ts:55-71) to retrieve a single preset, but this mock file doesn't implement it. If any console component fetches a single preset by slug, it will fail against the mock.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/console/app/mocks/routes/workspaces.ts` around lines 1 - 116, Add a GET handler for the single-preset route so the mock supports GET /api/v1/workspaces/:workspaceSlug/presets/:presetSlug; locate workspaceHandlers and add an http.get for "/api/v1/workspaces/:workspaceSlug/presets/:presetSlug" that finds the preset via db.presets.findFirst with where({ slug: params.presetSlug, workspace_id: params.workspaceSlug }) and returns HttpResponse.json(preset) or a 404 HttpResponse("Not found") when missing, mirroring the logic used in the existing patch/delete handlers for presets (see db.presets.findFirst and the route strings).
🤖 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/api/src/modules/presets/index.ts`:
- Around line 22-24: Queries against presets (e.g.,
prismaClient.presets.findMany) are currently filtered only by
params.workspaceSlug and must be scoped to the caller's organization to avoid
cross-tenant access; update every presets lookup/create/update/delete to include
the authenticated org id from the request context (e.g., callerOrgId from
auth/ctx) in the where clause by qualifying the workspace relation (for example:
where: { workspace: { slug: params.workspaceSlug, organizationId: callerOrgId }
} or the equivalent shape your schema uses) and apply the same change to all
other presets queries in this file (the blocks around the other ranges
mentioned) so every operation is constrained to the caller's organization.
In `@apps/api/src/modules/workspaces/index.ts`:
- Around line 105-112: The DELETE handler for "/:workspaceSlug" only
soft-deletes the workspace row (uses prismaClient.workspaces.findFirstOrThrow
and prismaClient.workspaces.softDelete) but does not remove the backing auth
team created on POST, leaving memberships and the slug reserved; modify the
handler to look up the workspace's associated auth team identifier (e.g.,
workspaces.authTeamId or the relation created by the POST / flow), call the
appropriate teardown (e.g., auth client or prisma delete for the team and its
memberships) before/while performing the workspace softDelete, and ensure errors
from tearing down the auth team are handled/logged (try/catch) so the workspace
delete remains consistent and the slug is freed for recreation.
- Around line 32-39: The code uses body.slug ?? slugFromString(body.name) which
treats an empty string as a valid slug; instead normalize and trim the incoming
slug and only use it if non-empty. Update the creation flow around workspaceSlug
(and usages like authClient!.organization.createTeam) to compute workspaceSlug
by trimming body.slug and falling back to slugFromString(body.name) when the
trimmed value is falsy (empty/whitespace), ensuring you pass a valid slug to
createTeam and related calls.
In `@apps/console/app/mocks/routes/workspaces.ts`:
- Around line 38-44: The mock DELETE handlers return 200 but should return 204
to match the real API; update the http.delete handlers for
"/api/v1/workspaces/:workspaceSlug" (where db.workspaces.delete(...) is called
and HttpResponse is constructed) and the similar "/api/v1/presets/:presetSlug"
handler to return new HttpResponse(undefined, { status: 204 }) so the mock
status matches the API's status(204) behavior.
In `@apps/console/app/routes/_shell._index/route.tsx`:
- Around line 5-19: The code in defaultWorkspaceMiddleware calls
api.workspaces.get() but ignores the returned error and proceeds to create a
workspace on transient failures; after awaiting api.workspaces.get() inspect the
returned error (the { data, error } shape) and if error is present throw it (or
throw a new Error when data is null/unexpected) so the React Router error
boundary surfaces the failure; only call api.workspaces.post(...) and redirect
to created?.slug after confirming no error and that workspaces is an empty
array.
In `@apps/console/app/routes/_shell/sidebar-nav.tsx`:
- Around line 32-38: The useEffect calling authService.getOrganization() lacks
error handling so a rejected promise leaves role undefined and hides "API Keys";
update the chain on the getOrganization call (in the useEffect around
authService.getOrganization, setRole, members) to add a .catch handler that logs
the error and sets a safe fallback for setRole (e.g., use user.role if available
or preserve prior role) so canManage isn't permanently false; ensure the promise
still returns null at the end.
In `@apps/console/app/routes/_shell/sidebar-playground.tsx`:
- Around line 24-33: The Promise.all call that invokes
api.workspaces(...).presets.get() and gateway.models.get() lacks error handling,
which can cause unhandled rejections and leave entries stale; wrap the
Promise.all(...) chain with a .catch handler (after the .then) that checks the
cancelled flag and then updates local state appropriately (e.g., call
setEntries([]) or set an error state / show a user-facing message) and logs the
error; ensure you still return null and do not swallow the error silently. Use
the existing symbols api.workspaces(...).presets.get(), gateway.models.get(),
setEntries, and cancelled to locate and implement the fix.
---
Outside diff comments:
In `@apps/api/src/modules/traces/index.ts`:
- Around line 23-39: The handler currently uses query.workspace directly (see
the async ({ greptimeDb, organizationId, query }) => block and listTraces call),
so change it to derive the effective workspace from the authenticated context
(e.g., the request/auth token or context-provided workspace) and use that value
when calling listTraces instead of query.workspace; if query.workspace is
provided and does not match the authenticated workspace, return an error (403
Forbidden) rejecting the request. Apply the same change to the other handler
referenced at lines 49-56 so both use the auth-derived workspace and
validate/reject mismatches rather than trusting query.workspace.
In `@apps/api/src/modules/workspaces/index.ts`:
- Around line 35-63: Create a compensation path so a partially created Better
Auth team isn't left orphaned: after calling
authClient!.organization.createTeam(...) and obtaining team.id, wrap the
subsequent calls to authClient!.organization.addTeamMember(...) and
prismaClient.workspaces.create(...) in a try/catch; if any of those fail, call
the appropriate teardown (e.g. authClient!.organization.deleteTeam({ teamId:
team.id }) or remove team member then delete team) to roll back the external
side effect, log any deletion errors, and rethrow the original error so failure
is visible to callers; ensure you reference team.id, workspaceSlug,
authClient.organization.addTeamMember, authClient.organization.createTeam and
prismaClient.workspaces.create when locating the code.
In `@apps/console/app/mocks/db.ts`:
- Around line 4-20: The mock workspace schema is missing a real workspace id
field, causing MSW responses to use slug as workspace_id and diverge from the
production PresetPlain.workspace_id contract; update workspaceSchema to include
an "id" field (string) with a realistic default (e.g., "ws-<uuid>" or
"workspace-dummy") alongside existing fields so presets (the presets getter /
presetSchema) can reference workspace.id; ensure any places that construct
PresetPlain mock objects use workspace.id for workspace_id rather than slug and
update related defaults where similar schema blocks exist (lines referenced in
the review: the other workspace schema duplicates) so MSW matches production
shape.
In `@apps/console/app/routes/_shell.w`.$workspaceSlug._index/api-key.tsx:
- Around line 12-29: Reset the per-workspace UI state when the route's
workspaceSlug changes: add a useEffect that watches workspaceSlug and calls
setKey(null), setError(null) and setLoading("idle") so previous workspace API
keys or errors aren't shown after a workspace switch; locate the state hooks
(key, error, loading) and the handleGenerateAPIKey handler in the component and
implement the effect there.
In `@apps/console/app/routes/_shell.w`.$workspaceSlug._index/route.mdx:
- Line 14: The overview copy still uses the old "agent" term; locate the string
Start using our interactive playground ({kbs("mod+P")}) or code your first agent
with our gateway in the _shell.w.$workspaceSlug._index route MDX and update
"agent" to the new terminology (e.g., "workspace") so it reads something like
Start using our interactive playground ({kbs("mod+P")}) or code your first
workspace with our gateway; ensure braces and kbs("mod+P") remain unchanged.
In `@apps/console/app/routes/_shell.w`.$workspaceSlug.api-keys/route.tsx:
- Around line 34-39: The call to authService.generateApiKey only passes {
workspaceSlug: params.workspaceSlug } so generated keys lack workspaceId
metadata; update the options argument passed to generateApiKey in the route
handler to include the workspaceId (e.g., { workspaceId: <currentWorkspaceId>,
workspaceSlug: params.workspaceSlug }) so the created key meets the {
workspaceId, workspaceSlug, createdByUserId } metadata contract; locate the call
to authService.generateApiKey (and usage of submission.value.name /
API_KEY_EXPIRATION_OPTIONS) and ensure the route supplies the correct
workspaceId from the current request/context when creating workspace-scoped
keys.
In `@apps/console/app/routes/_shell.w.create/route.tsx`:
- Around line 27-32: The current code assumes result.data is present and always
redirects to redirect(`/w/${result.data!.slug}`), which throws when Eden Treaty
returns an error object; modify the control flow to first check if result.error
exists: if result.error?.status === 409 keep the existing fieldErrors branch,
else return a submission-level error (e.g., submission.reply({ formError:
[parseError(result.error.value).message] }) or similar) so non-409 errors don't
fall through, and only call redirect when result.data is non-null (use
result.data.slug without the non-null assertion after confirming result.data).
---
Nitpick comments:
In `@apps/api/src/modules/workspaces/types.ts`:
- Around line 3-11: The slug field currently accepts arbitrary strings; update
WorkspacePlainSchema (and the schema used for creates, e.g.,
WorkspaceCreateSchema) to add a format constraint that matches the canonical
slug format (the same regex/format slugFromString produces), e.g. enforce
lowercase letters/numbers and hyphens only (no spaces or uppercase) using
z.string().regex(...). Keep existing trim/min checks and apply the same regex to
both schemas so explicit slugs are normalized/validated consistently with
implicit slug generation.
In `@apps/auth/prisma/schema.prisma`:
- Around line 127-139: The Teams model is missing a DB-level unique constraint
for per-organization workspace slugs; add a composite unique constraint on
organizationId and workspaceSlug (e.g. @@unique([organizationId,
workspaceSlug])) inside the Teams model to enforce uniqueness at the database
level and prevent TOCTOU race conditions, then run the Prisma CLI (prisma
migrate dev) to generate the migration rather than editing migration files
manually.
In `@apps/console/app/mocks/routes/workspaces.ts`:
- Around line 1-116: Add a GET handler for the single-preset route so the mock
supports GET /api/v1/workspaces/:workspaceSlug/presets/:presetSlug; locate
workspaceHandlers and add an http.get for
"/api/v1/workspaces/:workspaceSlug/presets/:presetSlug" that finds the preset
via db.presets.findFirst with where({ slug: params.presetSlug, workspace_id:
params.workspaceSlug }) and returns HttpResponse.json(preset) or a 404
HttpResponse("Not found") when missing, mirroring the logic used in the existing
patch/delete handlers for presets (see db.presets.findFirst and the route
strings).
In `@apps/console/app/routes/_shell.w`.$workspaceSlug.presets/form.tsx:
- Around line 93-95: The fetcher.Form instances (the ones around the
delete/create preset blocks referencing preset.slug) currently rely on implicit
posting to the current URL; add an explicit relative action that targets the
presets resource route (e.g., action="./presets" or the appropriate relative
path to the presets action) on both fetcher.Form elements so submissions always
go to the presets resource handler rather than a nested child route.
In `@apps/console/app/routes/_shell.w`.$workspaceSlug.presets/route.tsx:
- Around line 80-92: The delete branch in the route handler currently returns {
intent } or { intent, error }, which differs from the create/update responses;
update the delete handling in the intent === "delete" branch so it always
returns the same shape by including a submission field (e.g., return { intent,
submission: null } on success and return { intent, submission: null, error:
parseError(error).message } in the catch), changing the code around
api.workspaces(...).presets(...).delete() and the catch that references
parseError(error) so consumers expecting a submission field (like the form
component) receive a consistent response shape.
🪄 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: fc483b54-0fad-4dba-9295-7ce07933d378
📒 Files selected for processing (80)
apps/api/prisma/migrations/20260110030050_api_schema_init/migration.sqlapps/api/prisma/migrations/20260417014449_typebox_to_zod/migration.sqlapps/api/prisma/schema.prismaapps/api/src/db/json-types.d.tsapps/api/src/db/prisma.tsapps/api/src/index.tsapps/api/src/modules/agents/types.tsapps/api/src/modules/branches/index.tsapps/api/src/modules/branches/types.tsapps/api/src/modules/presets/index.tsapps/api/src/modules/presets/types.tsapps/api/src/modules/providers/index.tsapps/api/src/modules/providers/types.tsapps/api/src/modules/traces/index.tsapps/api/src/modules/traces/service.tsapps/api/src/modules/traces/types.tsapps/api/src/modules/workspaces/index.tsapps/api/src/modules/workspaces/types.tsapps/auth/prisma/migrations/20260119080641_auth_init/migration.sqlapps/auth/prisma/schema.prismaapps/auth/src/better-auth.tsapps/console/app/lib/auth/better-auth.tsapps/console/app/lib/auth/dummy-auth.tsapps/console/app/lib/auth/types.tsapps/console/app/mocks/browser.tsapps/console/app/mocks/db.tsapps/console/app/mocks/routes/agents.tsapps/console/app/mocks/routes/branches.tsapps/console/app/mocks/routes/models.tsapps/console/app/mocks/routes/traces.tsapps/console/app/mocks/routes/workspaces.tsapps/console/app/routes/_shell._index/route.tsxapps/console/app/routes/_shell.agent.$agentSlug.branch.$branchSlug.models/form.tsxapps/console/app/routes/_shell.agent.$agentSlug.branch.$branchSlug.models/route.tsxapps/console/app/routes/_shell.agent.$agentSlug.branch.$branchSlug.models/schema.tsapps/console/app/routes/_shell.agent.$agentSlug.branches/create.tsxapps/console/app/routes/_shell.agent.$agentSlug.branches/delete.tsxapps/console/app/routes/_shell.agent.$agentSlug.branches/route.tsxapps/console/app/routes/_shell.agent.$agentSlug.branches/table.tsxapps/console/app/routes/_shell.w.$workspaceSlug._index/api-key.tsxapps/console/app/routes/_shell.w.$workspaceSlug._index/route.mdxapps/console/app/routes/_shell.w.$workspaceSlug.api-keys/create.tsxapps/console/app/routes/_shell.w.$workspaceSlug.api-keys/revoke.tsxapps/console/app/routes/_shell.w.$workspaceSlug.api-keys/route.tsxapps/console/app/routes/_shell.w.$workspaceSlug.api-keys/table.tsxapps/console/app/routes/_shell.w.$workspaceSlug.presets/form.tsxapps/console/app/routes/_shell.w.$workspaceSlug.presets/route.tsxapps/console/app/routes/_shell.w.$workspaceSlug.presets/schema.tsapps/console/app/routes/_shell.w.$workspaceSlug.providers/clear.tsxapps/console/app/routes/_shell.w.$workspaceSlug.providers/configure.tsxapps/console/app/routes/_shell.w.$workspaceSlug.providers/list.tsxapps/console/app/routes/_shell.w.$workspaceSlug.providers/route.tsxapps/console/app/routes/_shell.w.$workspaceSlug.settings.danger/route.tsxapps/console/app/routes/_shell.w.$workspaceSlug.settings.members/invite-schema.tsapps/console/app/routes/_shell.w.$workspaceSlug.settings.members/route.tsxapps/console/app/routes/_shell.w.$workspaceSlug.settings/danger-zone.tsxapps/console/app/routes/_shell.w.$workspaceSlug.settings/general.tsxapps/console/app/routes/_shell.w.$workspaceSlug.settings/members.tsxapps/console/app/routes/_shell.w.$workspaceSlug.settings/route.tsxapps/console/app/routes/_shell.w.$workspaceSlug.traces.($traceId)/details.tsxapps/console/app/routes/_shell.w.$workspaceSlug.traces.($traceId)/route.tsxapps/console/app/routes/_shell.w.$workspaceSlug.traces/list.tsxapps/console/app/routes/_shell.w.$workspaceSlug.traces/route.tsxapps/console/app/routes/_shell.w.$workspaceSlug.traces/search-params.tsapps/console/app/routes/_shell.w.$workspaceSlug.traces/types.tsapps/console/app/routes/_shell.w.$workspaceSlug.traces/utils.tsapps/console/app/routes/_shell.w.$workspaceSlug/route.tsxapps/console/app/routes/_shell.w.create/form.tsxapps/console/app/routes/_shell.w.create/route.tsxapps/console/app/routes/_shell/route.tsxapps/console/app/routes/_shell/sidebar-agent.tsxapps/console/app/routes/_shell/sidebar-branch.tsxapps/console/app/routes/_shell/sidebar-nav.tsxapps/console/app/routes/_shell/sidebar-platform.tsxapps/console/app/routes/_shell/sidebar-playground.tsxapps/gateway/src/index.tsapps/gateway/src/lib/hooks.test.tsapps/gateway/src/lib/hooks.tspackages/shared-api/middlewares/auth.tspackages/shared-api/package.json
💤 Files with no reviewable changes (15)
- apps/console/app/routes/_shell.agent.$agentSlug.branch.$branchSlug.models/schema.ts
- apps/api/src/modules/branches/index.ts
- apps/api/src/modules/agents/types.ts
- apps/api/src/modules/branches/types.ts
- apps/console/app/mocks/routes/models.ts
- apps/console/app/routes/_shell.agent.$agentSlug.branches/create.tsx
- apps/console/app/routes/_shell.agent.$agentSlug.branch.$branchSlug.models/form.tsx
- apps/api/src/modules/providers/types.ts
- apps/console/app/mocks/routes/branches.ts
- apps/console/app/routes/_shell.agent.$agentSlug.branches/table.tsx
- apps/api/prisma/migrations/20260417014449_typebox_to_zod/migration.sql
- apps/console/app/routes/_shell.agent.$agentSlug.branch.$branchSlug.models/route.tsx
- apps/console/app/routes/_shell.agent.$agentSlug.branches/route.tsx
- apps/console/app/routes/_shell.agent.$agentSlug.branches/delete.tsx
- apps/console/app/mocks/routes/agents.ts
| async ({ body, prismaClient, organizationId, userId, authClient }) => { | ||
| const agentSlug = slugFromString(body.name, 3); | ||
| const workspaceSlug = body.slug ?? slugFromString(body.name); | ||
|
|
||
| const { data: team, error: createTeamError } = await authClient!.organization.createTeam({ | ||
| name: `${body.name}'s Team`, | ||
| organizationId: organizationId!, | ||
| agentSlug, | ||
| workspaceSlug, | ||
| }); |
There was a problem hiding this comment.
Normalize blank slug values before falling back.
body.slug ?? slugFromString(body.name) treats "" as a real slug, so a request with slug: "" skips the fallback and tries to create an empty-slug workspace/team. Tighten the schema or switch this to a trimmed truthy check.
Suggested fix
- const workspaceSlug = body.slug ?? slugFromString(body.name);
+ const workspaceSlug = body.slug?.trim() || slugFromString(body.name);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async ({ body, prismaClient, organizationId, userId, authClient }) => { | |
| const agentSlug = slugFromString(body.name, 3); | |
| const workspaceSlug = body.slug ?? slugFromString(body.name); | |
| const { data: team, error: createTeamError } = await authClient!.organization.createTeam({ | |
| name: `${body.name}'s Team`, | |
| organizationId: organizationId!, | |
| agentSlug, | |
| workspaceSlug, | |
| }); | |
| async ({ body, prismaClient, organizationId, userId, authClient }) => { | |
| const workspaceSlug = body.slug?.trim() || slugFromString(body.name); | |
| const { data: team, error: createTeamError } = await authClient!.organization.createTeam({ | |
| name: `${body.name}'s Team`, | |
| organizationId: organizationId!, | |
| workspaceSlug, | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/api/src/modules/workspaces/index.ts` around lines 32 - 39, The code uses
body.slug ?? slugFromString(body.name) which treats an empty string as a valid
slug; instead normalize and trim the incoming slug and only use it if non-empty.
Update the creation flow around workspaceSlug (and usages like
authClient!.organization.createTeam) to compute workspaceSlug by trimming
body.slug and falling back to slugFromString(body.name) when the trimmed value
is falsy (empty/whitespace), ensuring you pass a valid slug to createTeam and
related calls.
| "/:workspaceSlug", | ||
| async ({ prismaClient, params }) => { | ||
| await prismaClient.agents.softDelete({ slug: params.agentSlug }); | ||
| const { id } = await prismaClient.workspaces.findFirstOrThrow({ | ||
| where: { slug: params.workspaceSlug }, | ||
| select: { id: true }, | ||
| }); | ||
| await prismaClient.workspaces.softDelete({ id }); | ||
| return status(204); |
There was a problem hiding this comment.
Tear down the backing auth team when deleting a workspace.
DELETE /:workspaceSlug only soft-deletes the app record. The Better Auth team created in POST / remains alive with its memberships, and it can still reserve the same slug, which makes recreate-after-delete fail against auth while the workspace looks deleted in Prisma.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/api/src/modules/workspaces/index.ts` around lines 105 - 112, The DELETE
handler for "/:workspaceSlug" only soft-deletes the workspace row (uses
prismaClient.workspaces.findFirstOrThrow and prismaClient.workspaces.softDelete)
but does not remove the backing auth team created on POST, leaving memberships
and the slug reserved; modify the handler to look up the workspace's associated
auth team identifier (e.g., workspaces.authTeamId or the relation created by the
POST / flow), call the appropriate teardown (e.g., auth client or prisma delete
for the team and its memberships) before/while performing the workspace
softDelete, and ensure errors from tearing down the auth team are handled/logged
(try/catch) so the workspace delete remains consistent and the slug is freed for
recreation.
| async function defaultWorkspaceMiddleware() { | ||
| const { data: workspaces } = await api.workspaces.get(); | ||
|
|
||
| // FUTURE fade-in the next page | ||
| // FUTURE remember last agent and branch in session storage | ||
| if (agents && agents.length > 0) { | ||
| throw redirect(`/agent/${agents[0].slug}/branch/main`); | ||
| if (workspaces && workspaces.length > 0) { | ||
| const preferred = | ||
| workspaces.find((w) => w.slug === "default") ?? workspaces[0]; | ||
| throw redirect(`/w/${preferred.slug}`); | ||
| } | ||
| throw redirect("/agent/create"); | ||
|
|
||
| const { data: created } = await api.workspaces.post({ | ||
| name: "Default Workspace", | ||
| slug: "default", | ||
| }); | ||
| throw redirect(`/w/${created?.slug ?? "default"}`); | ||
| } |
There was a problem hiding this comment.
API error on workspaces.get() silently falls through to workspace creation.
Eden Treaty's .get() returns { data, error } rather than throwing. When api.workspaces.get() fails (5xx, network timeout), data is null and the if (workspaces && workspaces.length > 0) guard is false — so the code proceeds to POST a new "Default Workspace". An existing user who hits a transient API failure could have a spurious workspace created under their organization.
🛡️ Proposed fix
async function defaultWorkspaceMiddleware() {
- const { data: workspaces } = await api.workspaces.get();
+ const { data: workspaces, error } = await api.workspaces.get();
+
+ if (error) throw error;
if (workspaces && workspaces.length > 0) {
const preferred =
workspaces.find((w) => w.slug === "default") ?? workspaces[0];
throw redirect(`/w/${preferred.slug}`);
}Throwing error here lets React Router's error boundary surface the failure rather than masking it with an unintended workspace creation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/console/app/routes/_shell._index/route.tsx` around lines 5 - 19, The
code in defaultWorkspaceMiddleware calls api.workspaces.get() but ignores the
returned error and proceeds to create a workspace on transient failures; after
awaiting api.workspaces.get() inspect the returned error (the { data, error }
shape) and if error is present throw it (or throw a new Error when data is
null/unexpected) so the React Router error boundary surfaces the failure; only
call api.workspaces.post(...) and redirect to created?.slug after confirming no
error and that workspaces is an empty array.
| useEffect(() => { | ||
| if (!user) return; | ||
| void authService.getOrganization().then(({ members }) => { | ||
| setRole(members.find((m) => m.userId === user.userId)?.role); | ||
| return null; | ||
| }); | ||
| }, [user]); |
There was a problem hiding this comment.
Missing .catch(): authService.getOrganization() failure permanently hides "API Keys" from owners/admins.
If the call rejects, role stays undefined, canManage stays false, and the "API Keys" nav item never appears for that session — with no indication to the user.
🔧 Proposed fix
void authService.getOrganization().then(({ members }) => {
setRole(members.find((m) => m.userId === user.userId)?.role);
return null;
- });
+ }).catch(() => {
+ // leave role undefined; API Keys remains hidden
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| useEffect(() => { | |
| if (!user) return; | |
| void authService.getOrganization().then(({ members }) => { | |
| setRole(members.find((m) => m.userId === user.userId)?.role); | |
| return null; | |
| }); | |
| }, [user]); | |
| useEffect(() => { | |
| if (!user) return; | |
| void authService.getOrganization().then(({ members }) => { | |
| setRole(members.find((m) => m.userId === user.userId)?.role); | |
| return null; | |
| }).catch(() => { | |
| // leave role undefined; API Keys remains hidden | |
| }); | |
| }, [user]); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/console/app/routes/_shell/sidebar-nav.tsx` around lines 32 - 38, The
useEffect calling authService.getOrganization() lacks error handling so a
rejected promise leaves role undefined and hides "API Keys"; update the chain on
the getOrganization call (in the useEffect around authService.getOrganization,
setRole, members) to add a .catch handler that logs the error and sets a safe
fallback for setRole (e.g., use user.role if available or preserve prior role)
so canManage isn't permanently false; ensure the promise still returns null at
the end.
| void Promise.all([ | ||
| api.workspaces({ workspaceSlug: slug }).presets.get(), | ||
| gateway.models.get({ query: { endpoints: true } }), | ||
| ]).then(([presets, models]) => { | ||
| if (cancelled) return null; | ||
| const presetSlugs = (presets.data ?? []).map((p) => ({ alias: p.slug })); | ||
| const canonical = (models.data?.data ?? []).map((m) => ({ alias: m.id })); | ||
| setEntries([...presetSlugs, ...canonical]); | ||
| return null; | ||
| }); |
There was a problem hiding this comment.
Unhandled rejection from Promise.all will silently swallow errors.
With no .catch(), any rejection (network error, auth failure) from either presets.get() or gateway.models.get() escapes as an unhandled promise rejection. The entries state stays at its previous/stale value and the user sees no feedback.
🔧 Proposed fix
void Promise.all([
api.workspaces({ workspaceSlug: slug }).presets.get(),
gateway.models.get({ query: { endpoints: true } }),
]).then(([presets, models]) => {
if (cancelled) return null;
const presetSlugs = (presets.data ?? []).map((p) => ({ alias: p.slug }));
const canonical = (models.data?.data ?? []).map((m) => ({ alias: m.id }));
setEntries([...presetSlugs, ...canonical]);
return null;
- });
+ }).catch(() => {
+ if (!cancelled) setEntries([]);
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| void Promise.all([ | |
| api.workspaces({ workspaceSlug: slug }).presets.get(), | |
| gateway.models.get({ query: { endpoints: true } }), | |
| ]).then(([presets, models]) => { | |
| if (cancelled) return null; | |
| const presetSlugs = (presets.data ?? []).map((p) => ({ alias: p.slug })); | |
| const canonical = (models.data?.data ?? []).map((m) => ({ alias: m.id })); | |
| setEntries([...presetSlugs, ...canonical]); | |
| return null; | |
| }); | |
| void Promise.all([ | |
| api.workspaces({ workspaceSlug: slug }).presets.get(), | |
| gateway.models.get({ query: { endpoints: true } }), | |
| ]).then(([presets, models]) => { | |
| if (cancelled) return null; | |
| const presetSlugs = (presets.data ?? []).map((p) => ({ alias: p.slug })); | |
| const canonical = (models.data?.data ?? []).map((m) => ({ alias: m.id })); | |
| setEntries([...presetSlugs, ...canonical]); | |
| return null; | |
| }).catch(() => { | |
| if (!cancelled) setEntries([]); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/console/app/routes/_shell/sidebar-playground.tsx` around lines 24 - 33,
The Promise.all call that invokes api.workspaces(...).presets.get() and
gateway.models.get() lacks error handling, which can cause unhandled rejections
and leave entries stale; wrap the Promise.all(...) chain with a .catch handler
(after the .then) that checks the cancelled flag and then updates local state
appropriately (e.g., call setEntries([]) or set an error state / show a
user-facing message) and logs the error; ensure you still return null and do not
swallow the error silently. Use the existing symbols
api.workspaces(...).presets.get(), gateway.models.get(), setEntries, and
cancelled to locate and implement the fix.
|
@claude what do you think about codex and coderabbit review comments? |
|
Claude finished @heiwen's task in 1m 28s —— View job Assessment of Codex + CodeRabbit feedbackI verified each finding against the current branch. My read: a couple are real (and related), one "critical" is a false positive, and the rest are polish. 🔴 Worth fixing before merge1. Codex P1 — preset lookup is dead when
So preset slugs currently resolve to
I'd go with (a) — also fixes CodeRabbit's 2. CodeRabbit — trace endpoints don't clamp to the authed workspace — 3. CodeRabbit — workspace delete leaves the Better Auth team orphaned — 4. CodeRabbit — 🟡 Polish — worth a follow-up pass, not a blocker
🟢 Nits
❌ False positiveCodeRabbit "Critical" on 📦 Codex P2 — MSW model-catalog handlerWorth checking. RecommendationHappy to pick up items 1–4 on this PR (they're mostly mechanical and hang together via the |
|
@claude keep the rest for later |
|
Claude finished @heiwen's task in 7m 1s —— View job Tasks
Summary
Items explicitly deferred per your instruction: workspaceId stamping on API keys, Verification
|
- Scope GET /v1/traces and GET /v1/traces/:traceId by the authenticated workspaceSlug instead of a caller-supplied ?workspace= query param, so workspace-A API keys can no longer enumerate workspace-B traces. - Restore the MSW model-catalog handler at /gateway/v1/models so the console-only dev flow (shell loader + playground) still resolves canonical models. - Return 204 (not 200) from the workspace/preset DELETE mock handlers to match the real API contract. Co-authored-by: Heinrich Wendel <heiwen@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/console/app/mocks/routes/workspaces.ts (1)
56-81: ⚡ Quick win
POST /presetsduplicate-slug check may miss presets added viadb.presetsdirectly.The duplicate-slug guard on line 63 checks only
workspace.presets.some(p => p.slug === body.slug). If any code path creates a preset indb.presetswithout pushing it intoworkspace.presets[], the guard will miss it and the creation will silently shadow the orphaned record. Within this mock file the two-step write (create indb.presets→ push toworkspace.presets) is the only path, so this is low risk now, but coupling the guard to the embedded array rather thandb.presetsis fragile.Consider guarding against
db.presets.findFirst(q => q.where({ slug: body.slug, workspace_id: workspace.slug }))instead, which is the single source of truth for the PATCH/DELETE handlers.♻️ Proposed refactor
- if (workspace.presets.some((p) => p.slug === body.slug)) + const existingPreset = db.presets.findFirst((q) => + q.where({ slug: body.slug, workspace_id: workspace.slug }), + ); + if (existingPreset) return new HttpResponse("Preset with the same slug already exists", { status: 409 });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/console/app/mocks/routes/workspaces.ts` around lines 56 - 81, The duplicate-slug check in the POST /presets route handler currently inspects workspace.presets (in the http.post handler callback) which can miss presets present in db.presets; change the guard to query the canonical store by calling db.presets.findFirst(...) with a where filter for slug: body.slug and workspace_id: workspace.slug (or use workspace.id if appropriate) and treat a non-null result as a 409 conflict before creating the new preset.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/console/app/mocks/routes/workspaces.ts`:
- Around line 47-53: The GET handler for
"/api/v1/workspaces/:workspaceSlug/presets" currently returns the embedded
workspace.presets array which isn't updated when the PATCH handler calls
db.presets.update, causing edits to disappear; change the GET to read
authoritative presets from db.presets (e.g., query db.presets.findMany filtered
by workspaceId or workspaceSlug) or, after db.presets.update in the PATCH
handler, write the updated preset back into the owning workspace record's
presets array so the workspace.presets and db.presets stay in sync; update the
code around the http.get route handler and the PATCH logic that calls
db.presets.update to ensure both sources reflect the same data (use workspace.id
/ params.workspaceSlug and preset.id to locate and replace the item).
---
Nitpick comments:
In `@apps/console/app/mocks/routes/workspaces.ts`:
- Around line 56-81: The duplicate-slug check in the POST /presets route handler
currently inspects workspace.presets (in the http.post handler callback) which
can miss presets present in db.presets; change the guard to query the canonical
store by calling db.presets.findFirst(...) with a where filter for slug:
body.slug and workspace_id: workspace.slug (or use workspace.id if appropriate)
and treat a non-null result as a 409 conflict before creating the new preset.
🪄 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: 2a11e426-d493-424f-90b5-74d255d7e6e8
📒 Files selected for processing (5)
apps/api/src/modules/traces/index.tsapps/console/app/mocks/browser.tsapps/console/app/mocks/routes/workspaces.tsapps/console/app/routes/_shell.w.$workspaceSlug.traces.($traceId)/route.tsxapps/console/app/routes/_shell.w.$workspaceSlug.traces/route.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/console/app/routes/_shell.w.$workspaceSlug.traces.($traceId)/route.tsx
- apps/api/src/modules/traces/index.ts
- apps/console/app/mocks/browser.ts
- apps/console/app/routes/_shell.w.$workspaceSlug.traces/route.tsx
| http.get<{ workspaceSlug: string }>( | ||
| "/api/v1/workspaces/:workspaceSlug/presets", | ||
| ({ params }) => { | ||
| const workspace = db.workspaces.findFirst((q) => q.where({ slug: params.workspaceSlug })); | ||
| if (!workspace) return new HttpResponse("Not found", { status: 404 }); | ||
| return HttpResponse.json(workspace.presets); | ||
| }, |
There was a problem hiding this comment.
PATCH updates db.presets but GET /presets reads from workspace.presets[] — edits are invisible to subsequent reads.
GET /workspaces/:workspaceSlug/presets (line 52) returns workspace.presets, which is the embedded array on the workspace record. However, the PATCH handler (lines 95–101) only calls db.presets.update(...) and never writes back to workspace.presets. Any edit to a preset's name or model will be lost the next time the list is fetched, breaking edit-then-read test flows.
🐛 Proposed fix — sync the workspace's embedded preset array after update
const updated = await db.presets.update(preset, {
data(p) {
if (body.name !== undefined) p.name = body.name;
if (body.model !== undefined) p.model = body.model;
p.updated_at = new Date();
},
});
+
+ // Keep the workspace's embedded presets list in sync so GET /presets reflects the edit.
+ const ws = db.workspaces.findFirst((q) => q.where({ slug: params.workspaceSlug }));
+ if (ws) {
+ await db.workspaces.update(ws, {
+ data(w) {
+ const idx = w.presets.findIndex((p) => p.slug === params.presetSlug);
+ if (idx !== -1) w.presets[idx] = updated;
+ },
+ });
+ }
return HttpResponse.json(updated);Also applies to: 83-105
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/console/app/mocks/routes/workspaces.ts` around lines 47 - 53, The GET
handler for "/api/v1/workspaces/:workspaceSlug/presets" currently returns the
embedded workspace.presets array which isn't updated when the PATCH handler
calls db.presets.update, causing edits to disappear; change the GET to read
authoritative presets from db.presets (e.g., query db.presets.findMany filtered
by workspaceId or workspaceSlug) or, after db.presets.update in the PATCH
handler, write the updated preset back into the owning workspace record's
presets array so the workspace.presets and db.presets stay in sync; update the
code around the http.get route handler and the PATCH logic that calls
db.presets.update to ensure both sources reflect the same data (use workspace.id
/ params.workspaceSlug and preset.id to locate and replace the item).
1 participant
Closes #455.
Summary
agents+branches; introduceworkspaces(per-org) andpresets(per-workspace,name+slug+model)/w/:workspaceSlug/*;POST /v1/workspacescreates the Better Auth team inlineGET /v1/tracesis top-level with optional?workspace=slug; GreptimeDb tags are nowhebo.workspace.slug/hebo.preset.slug{ workspaceId, workspaceSlug, createdByUserId }metadata; only org owners / admins can create them (hook inapps/auth/src/lib/api-key.ts)memberrole)Test plan
bun run lint(all workspaces)bun run typecheck(all workspaces)bun run test(gateway hook tests updated; all pass)bun run build(console + services)/w/create, redirected to/w/my-first-workspace, opened Presets + Traces pagesFollow-ups explicitly deferred
paramscolumn (Q-A)POST /v1/organizationsfor agent-driven sign-up (Q-I)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Changes