Conversation
|
Caution Review failedPull request was closed or merged during review 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 organization-scoped agent management: DB migrations and schema for organization-agent joins, new services and pages for org-scoped agents, UI/forms/actions to manage agent↔organization links, ACL permission for agents, and consistent timestamp updates across several DB updates. Changes
Sequence DiagramsequenceDiagram
participant User as User (Org Member)
participant Browser as Browser
participant OrgPage as Org Settings Page (Server)
participant ACL as Org ACL Service
participant DB as Database
participant AgentSvc as Agent Service
User->>Browser: request org agent settings page
Browser->>OrgPage: SSR request with params
OrgPage->>ACL: load org, user, activeMember
ACL-->>OrgPage: permissions (canManageAgents)
alt not authorized
OrgPage-->>Browser: notFound()
else authorized
OrgPage->>DB: fetch agent (with organizations, databases)
DB-->>OrgPage: agent + relations
OrgPage->>OrgPage: verify org ownership/scoping
OrgPage-->>Browser: render page (adminView if owned)
User->>Browser: submit org-assignment form
Browser->>AgentSvc: updateAgentOrganizationsAction(payload)
AgentSvc->>DB: fetch agent + current orgs + projects
AgentSvc->>DB: insert/delete organization_agent rows
alt orgs removed
AgentSvc->>DB: clear database.projectId & backupPolicy (withUpdatedAt)
AgentSvc->>DB: delete retention/alert/storage policies
end
DB-->>AgentSvc: success
AgentSvc-->>Browser: success response
Browser->>Browser: show toast & refresh
end
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
...s/wrappers/dashboard/organization/tabs/organization-channels-tab/organization-agents-tab.tsx
Fixed
Show fixed
Hide fixed
...s/wrappers/dashboard/organization/tabs/organization-channels-tab/organization-agents-tab.tsx
Fixed
Show fixed
Hide fixed
...s/wrappers/dashboard/organization/tabs/organization-channels-tab/organization-agents-tab.tsx
Fixed
Show fixed
Hide fixed
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 21
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/features/agents/components/agent.form.tsx (1)
67-70:⚠️ Potential issue | 🟠 MajorMake the fallback redirect org-aware.
This form can now create organization-owned agents, but the default success path still pushes to the admin detail route. That route now 404s for agents with
organizationId, so org-scoped creates will land on a dead page unless every caller overridesonSuccess.Proposed fix
- } else { - router.push(`/dashboard/agents/${data.id}`); + } else if (props.organization) { + router.push("/dashboard/settings?tab=agents"); + } else { + router.push(`/dashboard/agents/${data.id}`); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/agents/components/agent.form.tsx` around lines 67 - 70, The fallback redirect in the success handler currently always does router.push(`/dashboard/agents/${data.id}`) which 404s for org-owned agents; update the fallback in the component (the props.onSuccess branch in agent.form.tsx) to check data.organizationId and, if present, push to the org-scoped agent route (include data.organizationId and data.id in the URL), otherwise keep the existing admin route; modify the code around props.onSuccess and router.push to choose the route based on data.organizationId.src/components/wrappers/dashboard/agent/button-delete-agent/button-delete-agent.tsx (1)
22-27:⚠️ Potential issue | 🟠 MajorChoose the post-delete redirect from the current scope.
This component is now used from organization settings as well, but success still hardcodes
"/dashboard/agents". After an org-scoped delete, that sends users to the global agents screen instead of back to the organization UI, and may even route them to a page they cannot access. Base the redirect onprops.organizationId.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/wrappers/dashboard/agent/button-delete-agent/button-delete-agent.tsx` around lines 22 - 27, The onSuccess handler currently always calls router.push("/dashboard/agents") after delete; change it to choose the post-delete redirect based on props.organizationId so org-scoped deletes return to the organization UI. In the onSuccess of the mutationFn (where deleteAgentAction is called and props.agentId/props.organizationId are available), replace the hardcoded router.push("/dashboard/agents") with a conditional that pushes the organization-specific route when props.organizationId is set (e.g. the org agents list) and falls back to the global "/dashboard/agents" otherwise; update any tests/consumers that assume the previous hardcoded redirect if needed.src/components/wrappers/dashboard/organization/tabs/organization-tabs.tsx (1)
84-111: 🛠️ Refactor suggestion | 🟠 MajorGuard the new tab with
canManageAgents.The ACL layer already exposes a dedicated agent-management permission, but this tab is rendered under the existing notification/storage gate instead of its own flag. That works only while those permissions happen to match; if they diverge, agent management will be shown or hidden behind the wrong ACL.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/wrappers/dashboard/organization/tabs/organization-tabs.tsx` around lines 84 - 111, The Agents tab is currently shown/hidden under the wrong permission; update organization-tabs.tsx to guard the Agents tab with the dedicated canManageAgents flag: wrap (or conditionally render) the TabsTrigger with value="agents" and the corresponding TabsContent value="agents" (which renders OrganizationAgentsTab with prop agents) behind canManageAgents instead of the notification/storage permission. Ensure both the tab trigger and its content check canManageAgents so visibility is consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/`(customer)/dashboard/(admin)/agents/[agentId]/page.tsx:
- Around line 33-38: The organizations DB query (db.query.organization.findMany
that assigns organizations) runs before the notFound() guards; move the
organizations lookup so it executes after all validation/guard checks (the
notFound() calls that validate agentId and ownership) to avoid doing heavy work
on 404 paths, and apply the same relocation for the second organizations lookup
block (the other db.query.organization.findMany instance) so both are executed
only after the guards pass.
In `@app/`(customer)/dashboard/(admin)/agents/page.tsx:
- Line 19: The current filter uses the nullable field
drizzleDb.schemas.agent.organizationId (where:
and(not(eq(drizzleDb.schemas.agent.isArchived, true)),
isNull(drizzleDb.schemas.agent.organizationId))) which is incorrect for the new
many-to-many org-agent model; replace this nullable check with a membership
check against the join table (organization_agents) — e.g., change the where
clause to filter agents that do not have a related row in
drizzleDb.schemas.organization_agents (use a NOT EXISTS subquery or a left join
to organization_agents and assert the join key is null) so the admin agent list
reflects join-table association instead of agents.organizationId.
In `@app/`(customer)/dashboard/(organization)/projects/[projectId]/page.tsx:
- Around line 60-72: Remove the retired inline query block by deleting the
commented-out availableDatabases declaration and its db.query.database.findMany
call (the commented lines referencing availableDatabases and
db.query.database.findMany), since the new service call is the single source of
truth; ensure no leftover references to availableDatabases remain in this file
(page.tsx) so only the service-based data flow is used.
In `@app/`(customer)/dashboard/(organization)/settings/agents/[agentId]/page.tsx:
- Line 63: The isOwned check currently uses the truthiness of
agent.organizationId which only indicates presence, not ownership; update the
variable (isOwned) to compare agent.organizationId === organization.id so it
only returns true when the current organization actually owns the agent
(referencing agent and organization.id), or if the original intent was merely to
detect presence, rename isOwned to hasOwner to clarify intent.
- Line 82: The ButtonDeleteAgent is being passed organizationId={organization.id
?? null} but ButtonDeleteAgentProps expects string | undefined; replace the null
with undefined (or just pass organization.id directly) so organizationId is a
string or undefined — update the JSX at ButtonDeleteAgent to use organization.id
(or organization.id ?? undefined) and keep agentId as-is.
In
`@src/components/wrappers/dashboard/admin/settings/storage/storage-s3/s3-form.action.ts`:
- Around line 44-45: Remove the "// `@ts-ignore`" on the .set({ ...data }) call
and fix the update payload by mapping StorageSwitchSchema fields to actual
settings table columns instead of a nonexistent "storage" column; follow the
pattern used in updateS3SettingsAction by wrapping the update with
withUpdatedAt() and pass an explicit object of valid column names (e.g.,
enabled/region/bucket or whatever fields your settings table expects) to
.set(...) so the types align and no ts-ignore is needed.
In
`@src/components/wrappers/dashboard/agent/button-delete-agent/delete-agent.action.ts`:
- Around line 66-69: The success metadata is using the wrong key; update
actionSuccess.messageParams so it uses agentId instead of projectId. Locate the
actionSuccess object in delete-agent.action.ts (the actionSuccess block) and
change messageParams from {projectId: agentId} to {agentId: agentId} (or
shorthand {agentId}) so templates/log consumers receive the expected agentId
key.
- Around line 28-48: The current flow deletes a single organization_agents row
then always archives the agent (in the block that uses organizationId and in the
admin path), which lets one org disable an agent for all orgs and leaves stale
associations; change the logic in delete-agent.action.ts to branch: if
organizationId is provided, run a transaction that deletes only the matching
organizationAgent row (drizzleDb.schemas.organizationAgent) and does not archive
the agent, and if no organizationId is provided (global delete/admin path) run a
transaction that archives the agent row (drizzleDb.schemas.agent: set
isArchived, slug, deletedAt) and deletes all related organizationAgent rows
atomically; ensure you use the existing db transaction API and the same schema
identifiers (organizationAgent, agent) and where predicates (organizationId,
agentId) so associations are cleaned up in one atomic operation for each path.
In
`@src/components/wrappers/dashboard/organization/tabs/organization-channels-tab/organization-agents-tab.tsx`:
- Around line 32-37: Remove the commented-out ChannelAddEditModal block: delete
the commented JSX lines referencing ChannelAddEditModal and its props (kind,
organization, open/isAddModalOpen, onOpenChangeAction/setIsAddModalOpen) from
organization-agents-tab.tsx; if the modal functionality is intended for future
work, create a tracking issue instead of leaving the dead code commented in
place.
- Line 19: The state pair isAddModalOpen / setIsAddModalOpen in the
OrganizationAgentsTab component is unused; either remove the declaration to
eliminate dead code or wire it up to the intended add-agent modal: implement
openAddModal/closeAddModal handlers that call setIsAddModalOpen(true/false),
pass isAddModalOpen to the modal’s open/visible prop and set onClose to call
setIsAddModalOpen(false), and ensure any "Add" button calls openAddModal; if
removing, also delete any unused imports related to the modal.
In `@src/db/migrations/0049_chief_terrax.sql`:
- Around line 1-5: The migration for organization_agents is missing the
created_at column and the composite primary key specified in the schema; update
the CREATE TABLE for organization_agents to include a created_at TIMESTAMP WITH
TIME ZONE NOT NULL DEFAULT now(), make ("organization_id","agent_id") the
PRIMARY KEY instead of a UNIQUE constraint, and add explicit foreign key
constraints for organization_id and agent_id referencing the respective parent
tables (preserve column names organization_id and agent_id). Ensure the column
type and default match the schema's created_at definition so relation queries
expecting created_at and the composite PK succeed.
In `@src/db/schema/08_agent.ts`:
- Around line 58-64: The AgentWith type is inconsistent: databases is optional
but organizations is required, which causes type errors when agents are loaded
without eager-loading organizations; update the AgentWith definition (the
AgentWith type in src/db/schema/08_agent.ts) to make organizations optional and
allow null (e.g., organizations?: { organizationId: string; agentId: string }[]
| null) so both relations follow the same optional/nullable pattern as
databases.
- Around line 24-35: The join table organizationAgent (organization_agents) is
missing createdAt/updatedAt columns used elsewhere; update the pgTable
definition for organizationAgent to add timestamp columns (e.g., createdAt and
updatedAt with appropriate defaults/precision) or use the shared ...timestamps
helper if available, ensuring the new columns are declared alongside
organizationId and agentId and remain compatible with the existing unique()
index and cascade references.
In `@src/db/services/agent.ts`:
- Around line 22-33: The query that builds the agent list currently only joins
organizationAgent and thus omits agents where agent.organizationId ===
organizationId; update the WHERE logic in the query (the block using
organizationAgent, agent, organizationId) to include agents that are either
linked via the organization_agents join OR directly owned via
agent.organizationId equals organizationId (i.e., add an OR condition that
checks eq(agent.organizationId, organizationId)). Also relax the archive check
on agent.isArchived from eq(agent.isArchived, false) to exclude only true (e.g.,
not(eq(agent.isArchived, true))) so NULL legacy rows are included.
In `@src/db/services/database.ts`:
- Around line 8-31: The current implementation fetches databases via
db.query.database.findMany into availableDatabases and then filters by
organization in memory; instead push the organization predicate into the
database query by extending the where clause passed to
db.query.database.findMany (the same call that currently uses isNull/eq on
projectId) to also require that the related agent either has
agent.organizationId equal to organizationId or that an agent.organizations
entry exists with organizationId equal to organizationId (use a join/EXISTS
style predicate), so only eligible Database rows are returned and you can remove
the post-query Array.filter that checks agent and agent.organizations; update
the where builder in db.query.database.findMany (same call that returns
availableDatabases) to include this agent/organizations predicate while keeping
the existing projectId logic and with: settings for agent, project, backups,
restorations.
In `@src/features/agents/agents.action.ts`:
- Around line 28-35: The two-step write creating the agent and then inserting
into organizationAgent must be atomic; wrap the db.insert to
drizzleDb.schemas.agent and the subsequent insert to
drizzleDb.schemas.organizationAgent in a single transaction so that either both
commits or neither do. Locate the code that uses
db.insert(...).values({...parsedInput.data, slug, organizationId:
parsedInput.organizationId}).returning() (symbols: createdAgent, parsedInput,
drizzleDb.schemas.agent, drizzleDb.schemas.organizationAgent) and replace the
separate inserts with a transactional block using the project's Drizzle
transaction API (begin/transaction/tx method) so that the agent creation and
organizationAgent join insert execute under the same transaction and errors roll
back both operations.
In `@src/features/agents/components/agent-organizations.action.ts`:
- Around line 49-55: Replace the sequential single-row inserts inside the loop
that iterates organizationsToAdd with a single batch insert: build an array of
value objects using organizationsToAdd and agentId and call
db.insert(drizzleDb.schemas.organizationAgent).values(batchArray) once instead
of awaiting each insert inside the for loop (refer to organizationsToAdd,
agentId, and drizzleDb.schemas.organizationAgent to locate the code).
- Around line 20-106: The handler in agent-organizations.action.ts performs
multiple DB changes (inserts to drizzleDb.schemas.organizationAgent, delete from
organizationAgent, update drizzleDb.schemas.database, and deletes from
retentionPolicy, alertPolicy, storagePolicy) without a transaction; wrap all
related operations (the loop that inserts organizationAgent rows, the delete of
organizationAgent, the subsequent query of organization, the update of database
and deletes of retentionPolicy/alertPolicy/storagePolicy) inside a single
Drizzle transaction so they either all commit or all rollback on error; use the
transaction object (e.g., tx.insert, tx.delete, tx.update, tx.query.*) instead
of the global db for those calls and ensure errors rethrow or return a failure
response so the transaction can roll back.
In `@src/features/agents/components/agent-organizations.schema.ts`:
- Around line 3-4: AgentOrganizationSchema currently accepts any string for the
organizations array; change the validation to require UUIDs by updating the
organizations element schema in AgentOrganizationSchema to use z.string().uuid()
(or z.string().uuid({ version: "..." }) if a specific UUID version is required)
so malformed IDs are rejected at form validation instead of failing later in the
write path.
In `@src/features/agents/components/agent.dialog.tsx`:
- Around line 62-85: The Organizations tab currently mounts
AgentOrganisationForm even when agent is undefined causing create flows to
submit an empty id; change the adminView Tabs so the TabsTrigger and TabsContent
for value="organizations" are only rendered when agent?.id exists (or otherwise
disable/hide the "organizations" trigger until after the first save). Locate the
adminView branch that renders Tabs, the TabsTrigger with value="organizations"
and the TabsContent that mounts AgentOrganisationForm (which reads
defaultValues?.id) and wrap those pieces in a conditional that checks agent?.id
(or make agent required for adminView) so the form is not mounted for new
agents.
In `@src/lib/auth/auth.ts`:
- Line 33: Replace the "//TODO: capture errors in a monitoring service" comment
in src/lib/auth/auth.ts by initializing a monitoring client (e.g., Sentry) at
module load and calling its capture method inside the auth error handling paths:
import and call Sentry.init(...) (using an env var like SENTRY_DSN) near the top
of the module, then in each catch block or error handler in this file (the
functions that perform authentication/verification) call
Sentry.captureException(err) with contextual tags (user id, function name) and
rethrow or return the appropriate error; ensure the monitoring init is guarded
by config so it can be disabled in tests and that sensitive data is not sent.
---
Outside diff comments:
In
`@src/components/wrappers/dashboard/agent/button-delete-agent/button-delete-agent.tsx`:
- Around line 22-27: The onSuccess handler currently always calls
router.push("/dashboard/agents") after delete; change it to choose the
post-delete redirect based on props.organizationId so org-scoped deletes return
to the organization UI. In the onSuccess of the mutationFn (where
deleteAgentAction is called and props.agentId/props.organizationId are
available), replace the hardcoded router.push("/dashboard/agents") with a
conditional that pushes the organization-specific route when
props.organizationId is set (e.g. the org agents list) and falls back to the
global "/dashboard/agents" otherwise; update any tests/consumers that assume the
previous hardcoded redirect if needed.
In `@src/components/wrappers/dashboard/organization/tabs/organization-tabs.tsx`:
- Around line 84-111: The Agents tab is currently shown/hidden under the wrong
permission; update organization-tabs.tsx to guard the Agents tab with the
dedicated canManageAgents flag: wrap (or conditionally render) the TabsTrigger
with value="agents" and the corresponding TabsContent value="agents" (which
renders OrganizationAgentsTab with prop agents) behind canManageAgents instead
of the notification/storage permission. Ensure both the tab trigger and its
content check canManageAgents so visibility is consistent.
In `@src/features/agents/components/agent.form.tsx`:
- Around line 67-70: The fallback redirect in the success handler currently
always does router.push(`/dashboard/agents/${data.id}`) which 404s for org-owned
agents; update the fallback in the component (the props.onSuccess branch in
agent.form.tsx) to check data.organizationId and, if present, push to the
org-scoped agent route (include data.organizationId and data.id in the URL),
otherwise keep the existing admin route; modify the code around props.onSuccess
and router.push to choose the route based on data.organizationId.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: acf3ce39-9fe4-4c91-8d8c-1c52a956b92d
📒 Files selected for processing (41)
app/(customer)/dashboard/(admin)/agents/[agentId]/page.tsxapp/(customer)/dashboard/(admin)/agents/page.tsxapp/(customer)/dashboard/(organization)/projects/[projectId]/page.tsxapp/(customer)/dashboard/(organization)/settings/agents/[agentId]/page.tsxapp/(customer)/dashboard/(organization)/settings/agents/page.tsxapp/(customer)/dashboard/(organization)/settings/page.tsxapp/api/agent/[agentId]/backup/upload/status/route.tsapp/api/agent/[agentId]/restore/route.tsapp/api/agent/[agentId]/status/helpers.tssrc/components/wrappers/common/bread-crumbs/bread-crumbs.tsxsrc/components/wrappers/dashboard/admin/settings/email/email-form/email-form.action.tssrc/components/wrappers/dashboard/admin/settings/notification/settings-notification.action.tssrc/components/wrappers/dashboard/admin/settings/storage/settings-storage.action.tssrc/components/wrappers/dashboard/admin/settings/storage/storage-s3/s3-form.action.tssrc/components/wrappers/dashboard/agent/agent-card/agent-card.tsxsrc/components/wrappers/dashboard/agent/button-delete-agent/button-delete-agent.tsxsrc/components/wrappers/dashboard/agent/button-delete-agent/delete-agent.action.tssrc/components/wrappers/dashboard/database/import/upload-backup-zone.tsxsrc/components/wrappers/dashboard/organization/tabs/organization-channels-tab/organization-agents-tab.tsxsrc/components/wrappers/dashboard/organization/tabs/organization-channels-tab/organization-notifiers-tab.tsxsrc/components/wrappers/dashboard/organization/tabs/organization-tabs.tsxsrc/db/index.tssrc/db/migrations/0049_chief_terrax.sqlsrc/db/migrations/meta/0049_snapshot.jsonsrc/db/migrations/meta/_journal.jsonsrc/db/schema/07_database.tssrc/db/schema/08_agent.tssrc/db/schema/12_storage-channel.tssrc/db/schema/13_storage-policy.tssrc/db/services/agent.tssrc/db/services/database.tssrc/features/agents/agents.action.tssrc/features/agents/agents.schema.tssrc/features/agents/components/agent-organizations.action.tssrc/features/agents/components/agent-organizations.form.tsxsrc/features/agents/components/agent-organizations.schema.tssrc/features/agents/components/agent.dialog.tsxsrc/features/agents/components/agent.form.tsxsrc/lib/acl/organization-acl.tssrc/lib/auth/auth.tssrc/utils/init.ts
💤 Files with no reviewable changes (3)
- src/components/wrappers/dashboard/organization/tabs/organization-channels-tab/organization-notifiers-tab.tsx
- src/components/wrappers/dashboard/database/import/upload-backup-zone.tsx
- src/db/schema/12_storage-channel.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-and-test
🔇 Additional comments (17)
src/utils/init.ts (1)
107-108: Good timestamp consistency on settings update.Using
withUpdatedAthere keepsupdatedAtin sync whendefaultStorageChannelIdis initialized.src/components/wrappers/dashboard/admin/settings/email/email-form/email-form.action.ts (1)
22-24: Looks correct.The
.set(...)payload now updates timestamps consistently while preserving the same settings fields.app/api/agent/[agentId]/backup/upload/status/route.ts (1)
73-76: Timestamp-wrapped status promotion looks good.Applying
withUpdatedAton backup success transition is consistent with the rest of the mutation paths.src/components/wrappers/dashboard/admin/settings/storage/storage-s3/s3-form.action.ts (1)
23-23:withUpdatedAtusage here is good.This keeps S3-related settings writes aligned with timestamp update conventions.
app/api/agent/[agentId]/status/helpers.ts (1)
186-187: Restoration status transitions are now consistently timestamped.The
failedandongoingupdates now correctly carryupdatedAtin all branches.Also applies to: 197-198, 204-205
app/api/agent/[agentId]/restore/route.ts (1)
66-67: Good update payload change.Wrapping restoration status updates with
withUpdatedAtkeeps this route consistent with other status mutation handlers.src/components/wrappers/dashboard/admin/settings/storage/settings-storage.action.ts (1)
26-30: This is a solid change.Explicit column mapping is preserved and
updatedAtis now updated atomically with the storage settings fields.src/components/wrappers/dashboard/admin/settings/notification/settings-notification.action.ts (1)
28-30: Nice consistency improvement.
defaultNotificationChannelIdupdates now also refreshupdatedAt, matching the rest of the settings actions.src/features/agents/agents.schema.ts (1)
6-6: Whitespace-only change; no functional impact.
No schema behavior/type impact from this segment.src/db/index.ts (1)
29-29: No functional effect from this import change in current state.
This segment does not introduce behavior changes by itself.src/db/schema/13_storage-policy.ts (1)
4-4: Import cleanup looks correct.
This keeps only the symbol actually used by the schema/relation definitions.src/components/wrappers/common/bread-crumbs/bread-crumbs.tsx (1)
47-47: Allowing “settings” as a navigable breadcrumb is consistent with the new flow.
This change is coherent with organization settings navigation.src/components/wrappers/dashboard/agent/agent-card/agent-card.tsx (1)
18-18: Route switching byorganizationViewis clean and straightforward.
Good extension ofAgentCardfor shared usage across admin/org contexts.Also applies to: 38-38
src/lib/acl/organization-acl.ts (1)
10-10:canManageAgentsintegration is consistent with the ACL pattern.
Type and computation are aligned and preserve existing role semantics.Also applies to: 35-35
app/(customer)/dashboard/(organization)/settings/page.tsx (1)
22-22: Organization agents data wiring is correct.
Fetching organization-scoped agents and passing them to tabs aligns with the new settings surface.Also applies to: 39-39, 92-92
src/features/agents/components/agent-organizations.form.tsx (1)
27-35: Nice default-value mapping for shared agents.Prefilling the form from
defaultValues.organizationskeeps the multiselect aligned with the join-table shape and makes editing existing assignments straightforward.src/features/agents/components/agent-organizations.action.ts (1)
88-98: Explicit policy deletions may be redundant due to cascade constraints.Per the schema (context snippet 4),
retentionPolicy,alertPolicy, andstoragePolicyall haveonDelete: 'cascade'referencingdatabase.id. If you delete the database row, these policies are automatically removed.However, here you're only updating the database rows (setting
projectIdto null), not deleting them, so explicit deletes are necessary. Consider adding a comment explaining this to prevent future confusion.
app/(customer)/dashboard/(organization)/projects/[projectId]/page.tsx
Outdated
Show resolved
Hide resolved
app/(customer)/dashboard/(organization)/settings/agents/[agentId]/page.tsx
Show resolved
Hide resolved
app/(customer)/dashboard/(organization)/settings/agents/[agentId]/page.tsx
Show resolved
Hide resolved
src/components/wrappers/dashboard/admin/settings/storage/storage-s3/s3-form.action.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/`(customer)/dashboard/(organization)/settings/page.tsx:
- Around line 81-96: The server action that performs organization deletion (the
handler invoked by DeleteOrganizationButton) must enforce the same "no projects"
rule as the UI: before calling the deletion, query the organization's projects
(e.g., via organizationSlug) and if any exist return/throw an error (HTTP 400 or
thrown Error) preventing deletion; update the server action (the
deleteOrganization / handleDeleteOrganization function used by
DeleteOrganizationButton) to check projects.length > 0 (or a count query) and
reject with a clear message so direct calls cannot bypass the UI constraint.
In
`@src/components/wrappers/dashboard/organization/tabs/organization-channels-tab/organization-agents-tab.tsx`:
- Around line 10-13: The prop type for OrganizationAgentsTabProps is incorrect:
change the agents property from Agent[] to the richer AgentWith[] (the type
expected by AgentCard) or ensure the data passed into OrganizationAgentsTab (and
any data-fetching code) includes the organizations relation so AgentCard's data:
AgentWith requirement is satisfied; update OrganizationAgentsTabProps to agents:
AgentWith[] and adjust upstream fetch/transform logic so each agent includes an
organizations array used by AgentCard.
In `@src/db/migrations/0050_dark_saracen.sql`:
- Around line 10-30: The migration currently backfills organization_agents only
via projects→databases→agent_id; also insert rows for agents that have a direct
agents.organization_id. Update the migration (0050_dark_saracen.sql) to also
INSERT INTO organization_agents (organization_id, agent_id) SELECT
organization_id, id FROM agents WHERE organization_id IS NOT NULL ON CONFLICT
(organization_id, agent_id) DO NOTHING (or add an equivalent loop that iterates
agents with non-null organization_id and inserts into organization_agents),
ensuring you reference the agents table, agents.organization_id, agents.id
(agent_id) and use the same ON CONFLICT clause as the existing inserts.
In `@src/db/schema/08_agent.ts`:
- Line 18: The agent ownership field agent.organizationId becomes stale when an
agent is removed from its owner org because delete-agent.action.ts currently
only deletes the organizationAgent join row; update the deletion logic to either
clear agent.organizationId (set to null) whenever the owner organization is
removed from an agent, or enforce admin-only deletion that removes all
organizationAgent rows and then deletes the agent record; specifically modify
the code in delete-agent.action.ts (and any service that removes
organizationAgent rows) to check if the removed organizationId equals
agent.organizationId and if so update the Agent record to nullify organizationId
(or alternatively cascade-remove all organizationAgent rows and delete Agent
only for admins), and ensure the access check in
settings/agents/[agentId]/page.tsx remains valid by keeping agent.organizationId
consistent with organizationAgent entries.
In `@src/db/services/agent.ts`:
- Around line 6-42: The function getOrganizationAgents currently casts the query
result to Agent[] despite selecting an aggregated databases field; update the
function signature and return type to AgentWithDatabases[] (or a matching type
that includes the databases array), import AgentWithDatabases from the schema
(e.g. from src/db/schema/08_agent.ts), remove the unsafe as unknown as Agent[]
cast, and ensure the sql<> aggregation for databases maps to Database[] so the
DB query result matches the new AgentWithDatabases return type.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: baa85dc0-bfe2-4bce-a9a1-cedadfdb9a8d
📒 Files selected for processing (13)
app/(customer)/dashboard/(organization)/settings/agents/[agentId]/page.tsxapp/(customer)/dashboard/(organization)/settings/page.tsxsrc/components/wrappers/dashboard/agent/button-delete-agent/button-delete-agent.tsxsrc/components/wrappers/dashboard/organization/delete-organization-button.tsxsrc/components/wrappers/dashboard/organization/tabs/organization-channels-tab/organization-agents-tab.tsxsrc/db/migrations/0050_dark_saracen.sqlsrc/db/migrations/0051_young_senator_kelly.sqlsrc/db/migrations/meta/0050_snapshot.jsonsrc/db/migrations/meta/0051_snapshot.jsonsrc/db/migrations/meta/_journal.jsonsrc/db/schema/08_agent.tssrc/db/services/agent.tssrc/features/agents/components/agent.form.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-and-test
🔇 Additional comments (8)
src/db/migrations/0051_young_senator_kelly.sql (1)
1-3: > Likely an incorrect or invalid review comment.src/components/wrappers/dashboard/organization/tabs/organization-channels-tab/organization-agents-tab.tsx (1)
2-3: Unused imports and state already flagged.The unused
EmptyStatePlaceholderimport,useStateimport, andisAddModalOpen/setIsAddModalOpenstate have been flagged in previous reviews. Please address these by either removing them or implementing the intended modal functionality.Also applies to: 19-19
app/(customer)/dashboard/(organization)/settings/agents/[agentId]/page.tsx (3)
63-63:isOwnedownership check issue already flagged.This was previously flagged:
isOwnedonly checks ifagent.organizationIdis truthy, not whether the current organization owns the agent. Should compare:agent.organizationId === organization.id.
82-82: Type mismatch withnullalready flagged.This was previously flagged: passing
nullwherestring | undefinedis expected. Sinceorganizationis verified on line 32,organization.idwill always be defined here. Useorganization.iddirectly.
55-61: LGTM!The access control logic correctly checks both direct ownership (
agent.organizationId === organization.id) and shared access via the join table (agent.organizations.some(...)). This properly supports the dual-ownership model.src/db/schema/08_agent.ts (2)
59-65:AgentWithtype inconsistency already flagged.Previous review noted that
databasesis optional (databases?: Database[] | null) whileorganizationsis required. This inconsistency can cause type errors when loading agents without eager-loading organizations.
23-36: LGTM on join table implementation!The
organizationAgentjoin table is well-structured:
- Proper cascade deletes on both foreign keys
- Unique constraint prevents duplicate associations
- Timestamps added (addressing prior feedback)
- Relations correctly defined for bidirectional queries
Also applies to: 44-45
src/db/migrations/meta/0051_snapshot.json (1)
1673-1752: LGTM!The
organization_agentsjoin table is correctly defined with:
- Foreign keys to
organizationandagentswith cascade deletes- Unique constraint on
(organization_id, agent_id)preventing duplicate associations- Timestamp columns for auditing
...s/wrappers/dashboard/organization/tabs/organization-channels-tab/organization-agents-tab.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
src/components/wrappers/dashboard/organization/tabs/organization-channels-tab/organization-agents-tab.tsx (1)
8-11:⚠️ Potential issue | 🟠 MajorAlign
agentsprop type withAgentCard’s required data shape.At Line 10,
agentsis typed asAgent[], but this component passes each item toAgentCardat Line 31. IfAgentCardstill requires a richer relation type (for example includingorganizations), this is a type/runtime contract break.🔧 Proposed fix
-import {Agent} from "@/db/schema/08_agent"; +import {AgentWith} from "@/db/schema/08_agent"; export type OrganizationAgentsTabProps = { organization: OrganizationWithMembers; - agents: Agent[]; + agents: AgentWith[]; };#!/bin/bash # Verify AgentCard data contract and this tab prop contract. rg -n -C3 'type\s+AgentCardProps|interface\s+AgentCardProps|data:\s*' src/components/wrappers/dashboard/agent/agent-card/agent-card.tsx rg -n -C3 'export type OrganizationAgentsTabProps|agents:\s*' src/components/wrappers/dashboard/organization/tabs/organization-channels-tab/organization-agents-tab/organization-agents-tab.tsx🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/wrappers/dashboard/organization/tabs/organization-channels-tab/organization-agents-tab.tsx` around lines 8 - 11, The agents prop in OrganizationAgentsTabProps is declared as Agent[] but each item is passed into AgentCard (see AgentCard component/AgentCardProps), so update OrganizationAgentsTabProps (and its agents usage) to match the exact data shape AgentCard expects (for example AgentWithRelations or Agent & { organizations: Organization[] } if AgentCard requires organizations), or change AgentCard to accept the narrower Agent type; ensure the type name you choose (e.g., AgentWithOrganizations or AgentCardProps['data']) replaces Agent[] in OrganizationAgentsTabProps and adjust any map/props calls inside organization-agents-tab to satisfy the new type.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/`(customer)/dashboard/(organization)/projects/[projectId]/page.tsx:
- Line 58: The call to getOrganizationAvailableDatabases uses the stale
auth-derived organization.id; change it to use the freshly fetched DB record
org.id so the service filters against the same DB state—update the invocation
that assigns availableDatabases to pass org.id (keep proj.id unchanged) to match
the earlier DB lookup for org and ensure consistent organization filtering.
In
`@src/components/wrappers/auth/login/forgot-password-form/forgot-password.actions.ts`:
- Line 9: The TODO indicates the password-reset action in this file returns
success: true without actually sending reset instructions; update the
forgot-password action (the exported handler in forgot-password.actions.ts) to
either (A) re-enable and call the actual send/reset delivery logic (e.g., invoke
the existing sendResetEmail/sendPasswordReset or Auth.forgotPassword helper) and
await it, catching and logging errors, then return success: true only on
delivery success, or (B) if delivery is not implemented yet, change the return
to success: false and include an explanatory error message so callers are not
misled; ensure you reference and use the existing request input
(email/identifier) and the local function names used in this file, and handle
exceptions to avoid returning a false success on failure.
In `@src/features/agents/components/agent-organizations.action.ts`:
- Line 112: Fix the grammar in the success notification by changing the message
string "Agent organizations has been successfully updated." to use plural
agreement: "Agent organizations have been successfully updated."; locate the
message property in src/features/agents/components/agent-organizations.action.ts
(the object containing the message key) and update its value accordingly.
- Around line 14-17: The server-side action schema in
agent-organizations.action.ts currently validates id as z.string(), which is
weaker than the companion AgentOrganizationSchema (which uses
z.string().uuid()); update the action's z.object (the one with keys data and id)
to validate id with z.string().uuid() so IDs are enforced as UUIDs, and ensure
any array items in data match the expected type in AgentOrganizationSchema if
they represent IDs as well.
- Around line 72-98: The current database queries use projectIds only and thus
affect databases owned by other agents; update db.query.database.findMany to
also filter by the current agentId (e.g., include a condition on
drizzleDb.schemas.database.agentId or the column representing agent ownership)
so databaseIds only contains databases for this agent, and add the same agentId
condition to the subsequent update and delete statements (the db.update(...) on
drizzleDb.schemas.database and the db.delete(...) calls for retentionPolicy,
alertPolicy, storagePolicy) so each WHERE includes both inArray(...projectIds)
and the agentId equality check.
---
Duplicate comments:
In
`@src/components/wrappers/dashboard/organization/tabs/organization-channels-tab/organization-agents-tab.tsx`:
- Around line 8-11: The agents prop in OrganizationAgentsTabProps is declared as
Agent[] but each item is passed into AgentCard (see AgentCard
component/AgentCardProps), so update OrganizationAgentsTabProps (and its agents
usage) to match the exact data shape AgentCard expects (for example
AgentWithRelations or Agent & { organizations: Organization[] } if AgentCard
requires organizations), or change AgentCard to accept the narrower Agent type;
ensure the type name you choose (e.g., AgentWithOrganizations or
AgentCardProps['data']) replaces Agent[] in OrganizationAgentsTabProps and
adjust any map/props calls inside organization-agents-tab to satisfy the new
type.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 97c0f202-c40f-4cad-8be7-9e387a8dd064
📒 Files selected for processing (6)
app/(customer)/dashboard/(organization)/projects/[projectId]/page.tsxsrc/components/wrappers/auth/login/forgot-password-form/forgot-password.actions.tssrc/components/wrappers/dashboard/admin/channels/organization/channels-organization.schema.tssrc/components/wrappers/dashboard/organization/tabs/organization-channels-tab/organization-agents-tab.tsxsrc/features/agents/components/agent-organizations.action.tssrc/features/agents/components/agent-organizations.schema.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-and-test
🔇 Additional comments (3)
src/components/wrappers/dashboard/organization/tabs/organization-channels-tab/organization-agents-tab.tsx (1)
22-34: Conditional trigger/state rendering is clean and consistent.The
hasAgentsplit is clear: create CTA when populated (Line 28) and empty-state CTA when no agents exist (Line 33). This is good UX for organization-scoped agent management.src/components/wrappers/dashboard/admin/channels/organization/channels-organization.schema.ts (1)
3-5: LGTM!Adding UUID validation to the
organizationsarray elements ensures malformed IDs are rejected at validation time rather than failing later in the data layer. This is consistent with the newly introducedAgentOrganizationSchema.src/features/agents/components/agent-organizations.schema.ts (1)
1-7: LGTM!The schema correctly validates organization IDs as UUIDs, which addresses the prior review feedback. This ensures malformed IDs are rejected at form validation.
| import {action} from "@/lib/safe-actions/actions"; | ||
|
|
||
| //todo: to be continued... | ||
| //TODO: to be continued... |
There was a problem hiding this comment.
TODO marks an incomplete password-reset path that currently returns success without sending reset instructions.
This should not ship as-is: the action can return success: true even though reset delivery is not executed (send logic is commented out), which breaks recovery UX and can mislead clients.
Proposed fix direction
-//TODO: to be continued...
+// Implemented: generate verification token and send reset email before returning success.- // await (
- // await auth.$context
- // ).options.emailAndPassword
- // .sendResetPassword(...)
+ // 1) Create/store verification token with expiry
+ // 2) Build reset URL from `parsedInput.redirectTo` (validated allowlist)
+ // 3) Call sendResetPassword(...)
+ // 4) Return success only after send succeeds🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/components/wrappers/auth/login/forgot-password-form/forgot-password.actions.ts`
at line 9, The TODO indicates the password-reset action in this file returns
success: true without actually sending reset instructions; update the
forgot-password action (the exported handler in forgot-password.actions.ts) to
either (A) re-enable and call the actual send/reset delivery logic (e.g., invoke
the existing sendResetEmail/sendPasswordReset or Auth.forgotPassword helper) and
await it, catching and logging errors, then return success: true only on
delivery success, or (B) if delivery is not implemented yet, change the return
to success: false and include an explanatory error message so callers are not
misled; ensure you reference and use the existing request input
(email/identifier) and the local function names used in this file, and handle
exceptions to avoid returning a false success on failure.
Comment on lines
+72
to
+98
| const databases = await db.query.database.findMany({ | ||
| where: (db, { inArray }) => inArray(db.projectId, projectIds), | ||
| columns: { id: true } | ||
| }); | ||
|
|
||
| const databaseIds = databases.map(d => d.id); | ||
|
|
||
| await db | ||
| .update(drizzleDb.schemas.database) | ||
| .set(withUpdatedAt({ | ||
| backupPolicy: null, | ||
| projectId: null | ||
| })) | ||
| .where(inArray(drizzleDb.schemas.database.projectId, projectIds)) | ||
| .execute(); | ||
|
|
||
| await db.delete(drizzleDb.schemas.retentionPolicy) | ||
| .where(inArray(drizzleDb.schemas.retentionPolicy.databaseId, databaseIds)) | ||
| .execute(); | ||
|
|
||
| await db.delete(drizzleDb.schemas.alertPolicy) | ||
| .where(inArray(drizzleDb.schemas.alertPolicy.databaseId, databaseIds)) | ||
| .execute(); | ||
|
|
||
| await db.delete(drizzleDb.schemas.storagePolicy) | ||
| .where(inArray(drizzleDb.schemas.storagePolicy.databaseId, databaseIds)) | ||
| .execute(); |
There was a problem hiding this comment.
Critical: Database operations affect other agents' databases.
The query at lines 72-75 finds databases by projectId without filtering by the current agentId. This means if multiple agents have databases in the same project, removing an organization from agent A will incorrectly clear backupPolicy/projectId and delete policies for databases belonging to other agents.
🐛 Proposed fix to scope operations to the current agent
if (projectIds.length > 0) {
const databases = await db.query.database.findMany({
- where: (db, { inArray }) => inArray(db.projectId, projectIds),
+ where: (db, { inArray, and, eq }) => and(
+ inArray(db.projectId, projectIds),
+ eq(db.agentId, agentId)
+ ),
columns: { id: true }
});
const databaseIds = databases.map(d => d.id);
+ if (databaseIds.length === 0) {
+ // No databases to update for this agent
+ return;
+ }
+
await db
.update(drizzleDb.schemas.database)
.set(withUpdatedAt({
backupPolicy: null,
projectId: null
}))
- .where(inArray(drizzleDb.schemas.database.projectId, projectIds))
+ .where(inArray(drizzleDb.schemas.database.id, databaseIds))
.execute();📝 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.
Suggested change
| const databases = await db.query.database.findMany({ | |
| where: (db, { inArray }) => inArray(db.projectId, projectIds), | |
| columns: { id: true } | |
| }); | |
| const databaseIds = databases.map(d => d.id); | |
| await db | |
| .update(drizzleDb.schemas.database) | |
| .set(withUpdatedAt({ | |
| backupPolicy: null, | |
| projectId: null | |
| })) | |
| .where(inArray(drizzleDb.schemas.database.projectId, projectIds)) | |
| .execute(); | |
| await db.delete(drizzleDb.schemas.retentionPolicy) | |
| .where(inArray(drizzleDb.schemas.retentionPolicy.databaseId, databaseIds)) | |
| .execute(); | |
| await db.delete(drizzleDb.schemas.alertPolicy) | |
| .where(inArray(drizzleDb.schemas.alertPolicy.databaseId, databaseIds)) | |
| .execute(); | |
| await db.delete(drizzleDb.schemas.storagePolicy) | |
| .where(inArray(drizzleDb.schemas.storagePolicy.databaseId, databaseIds)) | |
| .execute(); | |
| const databases = await db.query.database.findMany({ | |
| where: (db, { inArray, and, eq }) => and( | |
| inArray(db.projectId, projectIds), | |
| eq(db.agentId, agentId) | |
| ), | |
| columns: { id: true } | |
| }); | |
| const databaseIds = databases.map(d => d.id); | |
| if (databaseIds.length === 0) { | |
| // No databases to update for this agent | |
| return; | |
| } | |
| await db | |
| .update(drizzleDb.schemas.database) | |
| .set(withUpdatedAt({ | |
| backupPolicy: null, | |
| projectId: null | |
| })) | |
| .where(inArray(drizzleDb.schemas.database.id, databaseIds)) | |
| .execute(); | |
| await db.delete(drizzleDb.schemas.retentionPolicy) | |
| .where(inArray(drizzleDb.schemas.retentionPolicy.databaseId, databaseIds)) | |
| .execute(); | |
| await db.delete(drizzleDb.schemas.alertPolicy) | |
| .where(inArray(drizzleDb.schemas.alertPolicy.databaseId, databaseIds)) | |
| .execute(); | |
| await db.delete(drizzleDb.schemas.storagePolicy) | |
| .where(inArray(drizzleDb.schemas.storagePolicy.databaseId, databaseIds)) | |
| .execute(); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/features/agents/components/agent-organizations.action.ts` around lines 72
- 98, The current database queries use projectIds only and thus affect databases
owned by other agents; update db.query.database.findMany to also filter by the
current agentId (e.g., include a condition on drizzleDb.schemas.database.agentId
or the column representing agent ownership) so databaseIds only contains
databases for this agent, and add the same agentId condition to the subsequent
update and delete statements (the db.update(...) on drizzleDb.schemas.database
and the db.delete(...) calls for retentionPolicy, alertPolicy, storagePolicy) so
each WHERE includes both inArray(...projectIds) and the agentId equality check.
src/components/wrappers/dashboard/agent/button-delete-agent/delete-agent.action.ts
Fixed
Show fixed
Hide fixed
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (5)
app/(customer)/dashboard/(organization)/settings/agents/[agentId]/page.tsx (1)
67-86:⚠️ Potential issue | 🟠 MajorOnly show owner controls when the current organization actually owns the agent.
const isOwned = agent.organizationIdis truthy for any org-owned agent. SincehasAccessalso grants access throughagent.organizations, a linked org can pass the access check here and still see edit/delete controls for an agent owned by some other organization. Compare againstorganization.idinstead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/`(customer)/dashboard/(organization)/settings/agents/[agentId]/page.tsx around lines 67 - 86, The ownership check uses const isOwned = agent.organizationId which is truthy for any agent with an owner but doesn't verify it matches the current organization; change the logic in the page component to compare agent.organizationId to organization.id (e.g., set isOwned = agent.organizationId === organization.id) so the edit/delete controls (AgentDialog, ButtonDeleteAgent) only render when the current organization actually owns the agent.app/(customer)/dashboard/(admin)/agents/[agentId]/page.tsx (1)
33-49: 🧹 Nitpick | 🔵 TrivialMove the organizations lookup below the
notFound()guards.Invalid IDs and org-owned agents still pay for
organization.findMany({ with: { members: true } })before this page bails out. Defer that query until after the!agentandisOwnerByAnOrganizationchecks so the 404 path stays cheap.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/`(customer)/dashboard/(admin)/agents/[agentId]/page.tsx around lines 33 - 49, The organizations query (the call to db.query.organization.findMany with with: { members: true }) is executed before the notFound guards; move that query below the agent existence and ownership checks so invalid agent IDs and org-owned agents return 404 without running organization.findMany. Concretely: keep the checks for if (!agent) { notFound(); } and the isOwnerByAnOrganization = agent.organizationId / if (isOwnerByAnOrganization) { notFound(); } immediately after loading agent, then run the organizations lookup (the organization.findMany call that populates organizations) only after those guards pass.src/components/wrappers/dashboard/agent/button-delete-agent/delete-agent.action.ts (2)
121-123:⚠️ Potential issue | 🟡 MinorUse
agentIdin the success metadata.
messageParamsstill emits{ projectId: agentId }, which is inconsistent with the rest of this action and will break any consumer expectingagentId.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/wrappers/dashboard/agent/button-delete-agent/delete-agent.action.ts` around lines 121 - 123, The success metadata uses the wrong key; update the actionSuccess.messageParams to emit agentId instead of projectId so consumers get the expected identifier. Locate the actionSuccess block (symbol: actionSuccess) in delete-agent.action.ts and change messageParams from { projectId: agentId } to { agentId: agentId } (or just { agentId }) so it matches the rest of the action and downstream consumers.
29-104:⚠️ Potential issue | 🔴 CriticalSplit org detach from global delete, and make each path atomic.
When
organizationIdis present, this removes oneorganization_agentsrow and then still archives the agent globally a few lines later. That lets one org disable a shared agent for every other org, and a failure mid-flow can leave memberships and cleanup half-applied. This should branch into “detach one org” vs “archive everywhere” and run each branch inside a single transaction.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/wrappers/dashboard/agent/button-delete-agent/delete-agent.action.ts` around lines 29 - 104, The current flow mixes “detach single org” and “archive globally” logic; when organizationId is provided you should only remove the organization-agent link (the db.delete on drizzleDb.schemas.organizationAgent) inside a single transaction and must NOT run the global agent archiving (the db.update on drizzleDb.schemas.agent that sets isArchived/slug/deletedAt); conversely, when organizationIds (or the global delete path) is taken, perform the full cleanup (collect projectIds, null out database.projectId, delete retentionPolicy/alertPolicy/storagePolicy, then archive the agent via update on drizzleDb.schemas.agent) inside one transaction so either all cleanup + archive succeeds or all rolls back; refactor the branches around organizationId vs organizationIds to use explicit transactions (the DB transaction helper in your DB client) and move the agent update into only the global-delete branch.src/db/services/database.ts (1)
12-39:⚠️ Potential issue | 🟠 MajorFilter by organization in SQL instead of post-filtering in memory.
This still fetches every candidate database plus its related agent/project/backup/restoration rows, then drops unrelated orgs in JS. On multi-tenant project pages that scales with all tenants and moves cross-org data through the app layer unnecessarily. Push the organization and archived-agent predicates into the
findManyquery so only eligible rows are loaded.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/db/services/database.ts` around lines 12 - 39, The current code fetches all candidate databases then filters by agent.isArchived and organization membership in memory; change the predicate so db.query.database.findMany only returns eligible rows by adding agent-related conditions into the where clause: include a condition that the related agent is not archived (agent.isArchived = false) and that either agent.organizationId = organizationId OR the agent.organizations relation contains an entry with organizationId (use the relational/or filter APIs your query builder exposes), while preserving the existing projectId logic, with/with includes (agent, project, backups, restorations) and orderBy; update the where passed to db.query.database.findMany (the call in availableDatabases) and remove the post-query JS filter on availableDatabases and agent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/`(customer)/dashboard/(organization)/settings/agents/[agentId]/page.tsx:
- Around line 64-65: Remove the two debug console.log calls that print
"oooooooooooooooooo" and "iciii" along with organization.id from the route's
page.tsx; locate the page component where those console.log(...) calls occur and
delete them (or replace with a proper server-side logger at an appropriate level
if you need audit logging) so organization IDs are not leaked to server logs on
every request.
In `@app/api/agent/`[agentId]/restore/route.ts:
- Line 66: Validate body.status at runtime before casting and use strict
equality when checking for "failed": add a runtime guard (e.g.,
isRestorationStatus(value) or a small switch/allowed-values check) that verifies
body.status is one of the RestorationStatus enum members before calling
withUpdatedAt({ status: body.status as RestorationStatus }) in the restore
route; if invalid, return a 400 error. Also change the loose equality check
(currently body.status == "failed") to strict equality (===) against the
validated RestorationStatus value so only the explicit "failed" status triggers
failure handling. Ensure you update the code paths that write to the DB and send
notifications to rely on the validated status variable.
In `@package.json`:
- Around line 66-67: Update the Zod option objects in your schema helpers to use
the Zod 4 standardized "error" key instead of the deprecated "message" key:
locate the helper functions in src/lib/zod.ts (the schema helpers referenced and
any usages with createSelectSchema) and replace option objects like { message:
"..." } with { error: "..." } for calls such as z.enum(..., {...}),
z.string().email({...}), .min(..., {...}), and .regex(..., {...}); ensure all
helper exports and any createSelectSchema invocations are updated consistently
to avoid deprecation warnings.
In
`@src/components/wrappers/dashboard/agent/button-delete-agent/delete-agent.action.ts`:
- Around line 39-93: The cleanup currently gathers projectIds from organizations
and then operates on all databases in those projects; restrict the scope to only
databases owned by the agent by adding an agent predicate using the agentId
variable: when calling db.query.database.findMany (and when computing
databaseIds) include a predicate that the database.agentId equals agentId (e.g.
combine inArray(db.projectId, projectIds) with eq(db.agentId, agentId)), and
likewise tighten the db.update(drizzleDb.schemas.database).where(...) and each
db.delete(...).where(...) to include the same agentId predicate (or use the
filtered databaseIds from the constrained findMany) so only this agent’s
database rows and their retentionPolicy/alertPolicy/storagePolicy records are
touched.
In `@src/db/migrations/0052_cute_punisher.sql`:
- Line 1: The migration adds a NOT NULL "verified" column to the two_factor
table but omits a DEFAULT, which will fail on existing rows; update the ALTER
TABLE in 0052_cute_punisher.sql to add DEFAULT false (so existing rows get
false) and then update the Drizzle schema's two_factor column definition (the
"verified" column in the table/schema declaration in 02_user.ts) to include
.default(false) so the runtime schema matches the migration.
---
Duplicate comments:
In `@app/`(customer)/dashboard/(admin)/agents/[agentId]/page.tsx:
- Around line 33-49: The organizations query (the call to
db.query.organization.findMany with with: { members: true }) is executed before
the notFound guards; move that query below the agent existence and ownership
checks so invalid agent IDs and org-owned agents return 404 without running
organization.findMany. Concretely: keep the checks for if (!agent) { notFound();
} and the isOwnerByAnOrganization = agent.organizationId / if
(isOwnerByAnOrganization) { notFound(); } immediately after loading agent, then
run the organizations lookup (the organization.findMany call that populates
organizations) only after those guards pass.
In `@app/`(customer)/dashboard/(organization)/settings/agents/[agentId]/page.tsx:
- Around line 67-86: The ownership check uses const isOwned =
agent.organizationId which is truthy for any agent with an owner but doesn't
verify it matches the current organization; change the logic in the page
component to compare agent.organizationId to organization.id (e.g., set isOwned
= agent.organizationId === organization.id) so the edit/delete controls
(AgentDialog, ButtonDeleteAgent) only render when the current organization
actually owns the agent.
In
`@src/components/wrappers/dashboard/agent/button-delete-agent/delete-agent.action.ts`:
- Around line 121-123: The success metadata uses the wrong key; update the
actionSuccess.messageParams to emit agentId instead of projectId so consumers
get the expected identifier. Locate the actionSuccess block (symbol:
actionSuccess) in delete-agent.action.ts and change messageParams from {
projectId: agentId } to { agentId: agentId } (or just { agentId }) so it matches
the rest of the action and downstream consumers.
- Around line 29-104: The current flow mixes “detach single org” and “archive
globally” logic; when organizationId is provided you should only remove the
organization-agent link (the db.delete on drizzleDb.schemas.organizationAgent)
inside a single transaction and must NOT run the global agent archiving (the
db.update on drizzleDb.schemas.agent that sets isArchived/slug/deletedAt);
conversely, when organizationIds (or the global delete path) is taken, perform
the full cleanup (collect projectIds, null out database.projectId, delete
retentionPolicy/alertPolicy/storagePolicy, then archive the agent via update on
drizzleDb.schemas.agent) inside one transaction so either all cleanup + archive
succeeds or all rolls back; refactor the branches around organizationId vs
organizationIds to use explicit transactions (the DB transaction helper in your
DB client) and move the agent update into only the global-delete branch.
In `@src/db/services/database.ts`:
- Around line 12-39: The current code fetches all candidate databases then
filters by agent.isArchived and organization membership in memory; change the
predicate so db.query.database.findMany only returns eligible rows by adding
agent-related conditions into the where clause: include a condition that the
related agent is not archived (agent.isArchived = false) and that either
agent.organizationId = organizationId OR the agent.organizations relation
contains an entry with organizationId (use the relational/or filter APIs your
query builder exposes), while preserving the existing projectId logic, with/with
includes (agent, project, backups, restorations) and orderBy; update the where
passed to db.query.database.findMany (the call in availableDatabases) and remove
the post-query JS filter on availableDatabases and agent.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4e756f06-1b5c-4d7d-8554-f68d84044575
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (16)
Makefileapp/(customer)/dashboard/(admin)/agents/[agentId]/page.tsxapp/(customer)/dashboard/(organization)/projects/page.tsxapp/(customer)/dashboard/(organization)/settings/agents/[agentId]/page.tsxapp/api/agent/[agentId]/backup/helpers.tsapp/api/agent/[agentId]/restore/route.tsapp/api/agent/[agentId]/status/route.tspackage.jsonsrc/components/wrappers/common/empty-state-placeholder.tsxsrc/components/wrappers/dashboard/agent/button-delete-agent/button-delete-agent.tsxsrc/components/wrappers/dashboard/agent/button-delete-agent/delete-agent.action.tssrc/db/migrations/0052_cute_punisher.sqlsrc/db/migrations/meta/0052_snapshot.jsonsrc/db/migrations/meta/_journal.jsonsrc/db/schema/02_user.tssrc/db/services/database.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-and-test
🔇 Additional comments (7)
Makefile (1)
19-19: Good improvement: absolute bind-mount path is safer for Docker Compose runs.Line 19’s switch to
$$(pwd)/...makes the mount path resolution more robust than a relative path.app/api/agent/[agentId]/backup/helpers.ts (1)
2-16: Archived-agent guard is correctly enforced in agent lookup.At Line 15, adding
agent.isArchived = falseto theagent.idpredicate makes the middleware treat archived agents as not found while preserving the existing 404 flow.app/api/agent/[agentId]/status/route.ts (1)
6-50: Status endpoint lookup now correctly excludes archived agents.Line 49’s combined predicate is a solid hardening step and aligns existence checks with active-agent semantics.
app/api/agent/[agentId]/restore/route.ts (1)
8-39: Restore route correctly enforces active-agent lookup.Line 38’s
agent.id AND isArchived = falsecondition is consistent with the archived-agent handling introduced in related routes.src/db/migrations/meta/_journal.json (1)
347-375: LGTM!The new migration journal entries for migrations 0049–0052 are correctly formatted and sequential. Timestamps are in increasing order, and the structure matches existing entries.
src/db/migrations/meta/0052_snapshot.json (1)
555-612: Snapshot correctly reflects the schema changes.The
two_factortable definition in the snapshot accurately captures the newverifiedcolumn asbooleanwithnotNull: true. This auto-generated snapshot is consistent with both the Drizzle schema and migration SQL.Note: If the migration is updated to include a
DEFAULT false, regenerate this snapshot to include the default value.src/db/schema/02_user.ts (1)
96-104: No changes needed; the schema is correctly aligned with better-auth's twoFactor plugin.The
verifiedcolumn does not require a.default(false)because better-auth's twoFactor plugin explicitly handles initialization of theverifiedfield when creating records. The schema and migration are already properly configured.> Likely an incorrect or invalid review comment.
app/(customer)/dashboard/(organization)/settings/agents/[agentId]/page.tsx
Outdated
Show resolved
Hide resolved
Comment on lines
+66
to
+67
| "drizzle-orm": "0.45.2", | ||
| "drizzle-zod": "0.8.3", |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
# Check if the referenced files exist and inspect their content
cat -n src/lib/zod.ts 2>/dev/null | head -20Repository: Portabase/portabase
Length of output: 952
🏁 Script executed:
# Also check the schema files mentioned
cat -n src/db/schema/07_database.ts 2>/dev/null | head -15
cat -n src/db/schema/08_agent.ts 2>/dev/null | head -15Repository: Portabase/portabase
Length of output: 1745
🏁 Script executed:
# Check line 105 in package.json
sed -n '100,110p' package.jsonRepository: Portabase/portabase
Length of output: 342
🌐 Web query:
Zod 4.3.6 API breaking changes from Zod 3 message vs error parameter
💡 Result:
In Zod v4 (including 4.3.6), error customization was unified under a single error option, and the old message option is still accepted but deprecated. (zod.dev)
What changed vs Zod 3
-
Per-check message
- Zod 3:
z.string().min(5, { message: "Too short" }) - Zod 4:
z.string().min(5, { error: "Too short" })(preferred) (zod.dev)
- Zod 3:
-
Type-level messages (
required_error,invalid_type_error)- Zod 3:
z.string({ required_error: "...", invalid_type_error: "..." }) - Zod 4:
z.string({ error: (issue) => issue.input === undefined ? "..." : "..." })(zod.dev)
- Zod 3:
-
errorMapreplacement- Zod 3:
errorMap: (issue, ctx) => ({ message: ... }) - Zod 4:
error: (issue) => "..."(can return a string; returnundefinedto fall back to defaults/next map) (zod.dev)
- Zod 3:
Citations:
- 1: https://zod.dev/v4?id=2x-reduction-in-core-bundle-size
- 2: https://zod.dev/v4?id=2x-reduction-in-core-bundle-size
- 3: https://zod.dev/v4?id=2x-reduction-in-core-bundle-size
- 4: https://zod.dev/v4?id=2x-reduction-in-core-bundle-size
🌐 Web query:
drizzle-zod 0.8.3 compatibility with Zod 4 createSelectSchema
💡 Result:
drizzle-zod@0.8.3 is compatible with Zod 4 for createSelectSchema (and the other create*Schema helpers): its peerDependencies explicitly allow zod ^3.25.0 || ^4.0.0, so a project using zod@4.x satisfies the supported range. [1]
Practical notes:
- With Zod 4, use the v4 entry when you need to reference
zdirectly in refinements/overrides (e.g.import { z } from "zod/v4"). [2] - Drizzle’s docs also note that
drizzle-zodis deprecated in favor of the built-in integration (createSelectSchemafromdrizzle-orm/zod), which is the recommended path going forward. [2]
Sources: [1] drizzle-zod@0.8.3 package.json (peerDependencies) [2] Drizzle ORM docs (drizzle-orm/zod, Zod v4 import, deprecation note)
No breaking changes; migrate helpers to Zod 4 syntax as a best practice.
The upgrade to zod@4.3.6 maintains backward compatibility with the { message: ... } syntax currently used in src/lib/zod.ts (lines 6, 8, 12), and drizzle-zod@0.8.3 explicitly supports both Zod 3 and Zod 4. Your schema helpers and createSelectSchema usage will continue to work without modification.
However, Zod 4 standardizes on the error parameter (the message option is deprecated). Consider updating the helpers to use the new syntax:
z.enum(values, { error: "Field required" })
z.string().email({ error: "Invalid email" })
.min(8, { error: "..." })
.regex(pattern, { error: "..." })
This is optional but recommended to follow Zod 4 best practices and avoid deprecation warnings.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` around lines 66 - 67, Update the Zod option objects in your
schema helpers to use the Zod 4 standardized "error" key instead of the
deprecated "message" key: locate the helper functions in src/lib/zod.ts (the
schema helpers referenced and any usages with createSelectSchema) and replace
option objects like { message: "..." } with { error: "..." } for calls such as
z.enum(..., {...}), z.string().email({...}), .min(..., {...}), and .regex(...,
{...}); ensure all helper exports and any createSelectSchema invocations are
updated consistently to avoid deprecation warnings.
src/components/wrappers/dashboard/agent/button-delete-agent/delete-agent.action.ts
Show resolved
Hide resolved
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor