Open
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughRewires four dashboard connector list pages to use a new centralized api-key-connectors feature and adds types, params, hooks, server loaders/prefetchers, TRPC procedures, and UI components to support connector listing by resource type. Changes
Sequence Diagram(s)sequenceDiagram
participant Page as Client Page
participant Loader as Server Loader (connectorsParamsLoader)
participant Prefetch as Server Prefetch
participant TRPC as TRPC Router (getManyByTypeInternal)
participant DB as Prisma DB
Page->>Loader: read pagination/search params
Page->>Prefetch: request prefetch for ResourceType
Prefetch->>TRPC: call getManyByTypeInternal(params + resourceType)
TRPC->>DB: fetch paginated connectors (where resourceType, include user)
DB-->>TRPC: return rows + counts
TRPC-->>Prefetch: return data (cached)
Prefetch-->>Page: hydrate prefetched data
Page->>Page: render ConnectorsList (columns, search, pagination)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/features/api-key-connectors/types.ts (1)
32-34: Consider using PascalCase for the exported type name.The type
paginatedConnectorsByTypeInputuses camelCase, while other types in this file (ConnectorResponse) and TypeScript convention typically use PascalCase for type aliases.✏️ Suggested fix
-export type paginatedConnectorsByTypeInput = z.infer< +export type PaginatedConnectorsByTypeInput = z.infer< typeof paginatedConnectorInputSchema >;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/api-key-connectors/types.ts` around lines 32 - 34, The exported type name paginatedConnectorsByTypeInput should use PascalCase: rename it to PaginatedConnectorsByTypeInput and update its declaration to export type PaginatedConnectorsByTypeInput = z.infer<typeof paginatedConnectorInputSchema>; then search and update all usages/imports of paginatedConnectorsByTypeInput across the codebase (tests, consumers, and index exports) to the new PaginatedConnectorsByTypeInput identifier so imports and references remain consistent with ConnectorResponse and TS naming conventions.src/features/api-key-connectors/server/prefetch.ts (1)
1-3: Add'server-only'directive to prevent client-side imports.This is a server-side prefetch utility that should not be imported on the client. Adding the directive ensures bundler errors if accidentally imported client-side.
🛡️ Suggested fix
+import "server-only"; + import type { inferInput } from "@trpc/tanstack-react-query"; import { ResourceType } from "@/generated/prisma"; import { prefetch, trpc } from "@/trpc/server";As per coding guidelines: "Mark server-side files with 'server-only' directive".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/api-key-connectors/server/prefetch.ts` around lines 1 - 3, Add the top-of-file "server-only" directive (a single line containing 'server-only') as the very first line to prevent client-side imports; update src features referenced symbols such as inferInput, ResourceType, prefetch, and trpc remain unchanged — just insert the directive before the existing imports so the bundler will error if this server-side prefetch utility is imported on the client.src/features/api-key-connectors/components/connectors.tsx (1)
107-117: Inconsistent use of hardcoded string vs enum forresourceType.
DeviceArtifactConnectorsContaineruses the string"Device Artifact"for display, while other containers useResourceType.*enum values directly. This works becauseConnectorsContaineracceptsResourceType | string, but it's inconsistent.Consider creating a display-name mapping to keep the prop type strict while still showing user-friendly names:
💡 Suggested approach
// At top of file or in a shared utility const resourceTypeDisplayNames: Record<ResourceType, string> = { [ResourceType.Asset]: "Asset", [ResourceType.DeviceArtifact]: "Device Artifact", [ResourceType.Remediation]: "Remediation", [ResourceType.Vulnerability]: "Vulnerability", }; // Then in ConnectorsHeader, use: // title={`${resourceTypeDisplayNames[resourceType as ResourceType] ?? resourceType} Connectors`}This would allow
ConnectorsContainerto always receiveResourceTypewhile still displaying "Device Artifact" with a space.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/api-key-connectors/components/connectors.tsx` around lines 107 - 117, DeviceArtifactConnectorsContainer currently passes the literal string "Device Artifact" into ConnectorsContainer which accepts ResourceType | string; change this to pass the ResourceType enum (ResourceType.DeviceArtifact) and add a display-name mapping (e.g., resourceTypeDisplayNames: Record<ResourceType,string>) used by ConnectorsHeader (or whatever renders the title) to convert ResourceType.DeviceArtifact into the user-friendly "Device Artifact"; update ConnectorsContainer prop typing to require ResourceType and ensure ConnectorsHeader uses resourceTypeDisplayNames[resourceType] ?? resourceType for its title.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/features/api-key-connectors/components/columns.tsx`:
- Around line 53-58: The status logic in the columns renderer (where text and
color are set based on row.original.apiKeyId) mislabels connectors that have a
valid integrationId as expired; update the condition in that component (the
block that sets text and color for status) to treat a row as Active when either
row.original.apiKeyId or row.original.integrationId is present, and mark Expired
only when both are missing, keeping the same color mapping (e.g., bg-green-300
for Active, bg-red-400 for Expired).
In `@src/features/api-key-connectors/server/params-loader.ts`:
- Around line 1-2: Add the "server-only" directive as the very first line of the
file and update the local import to use the project alias: keep the createLoader
import from "nuqs/server" but replace the relative connectors import
(connectorsParams from "../params") with the aliased path connectorsParams from
"@/features/api-key-connectors/params"; ensure the "server-only" string literal
appears before any imports so the file is explicitly marked server-side.
---
Nitpick comments:
In `@src/features/api-key-connectors/components/connectors.tsx`:
- Around line 107-117: DeviceArtifactConnectorsContainer currently passes the
literal string "Device Artifact" into ConnectorsContainer which accepts
ResourceType | string; change this to pass the ResourceType enum
(ResourceType.DeviceArtifact) and add a display-name mapping (e.g.,
resourceTypeDisplayNames: Record<ResourceType,string>) used by ConnectorsHeader
(or whatever renders the title) to convert ResourceType.DeviceArtifact into the
user-friendly "Device Artifact"; update ConnectorsContainer prop typing to
require ResourceType and ensure ConnectorsHeader uses
resourceTypeDisplayNames[resourceType] ?? resourceType for its title.
In `@src/features/api-key-connectors/server/prefetch.ts`:
- Around line 1-3: Add the top-of-file "server-only" directive (a single line
containing 'server-only') as the very first line to prevent client-side imports;
update src features referenced symbols such as inferInput, ResourceType,
prefetch, and trpc remain unchanged — just insert the directive before the
existing imports so the bundler will error if this server-side prefetch utility
is imported on the client.
In `@src/features/api-key-connectors/types.ts`:
- Around line 32-34: The exported type name paginatedConnectorsByTypeInput
should use PascalCase: rename it to PaginatedConnectorsByTypeInput and update
its declaration to export type PaginatedConnectorsByTypeInput = z.infer<typeof
paginatedConnectorInputSchema>; then search and update all usages/imports of
paginatedConnectorsByTypeInput across the codebase (tests, consumers, and index
exports) to the new PaginatedConnectorsByTypeInput identifier so imports and
references remain consistent with ConnectorResponse and TS naming conventions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 41069a9f-826c-4444-bf02-64fff5d01bec
📒 Files selected for processing (13)
src/app/(dashboard)/(rest)/connectors/assets/page.tsxsrc/app/(dashboard)/(rest)/connectors/deviceArtifacts/page.tsxsrc/app/(dashboard)/(rest)/connectors/remediations/page.tsxsrc/app/(dashboard)/(rest)/connectors/vulnerabilities/page.tsxsrc/features/api-key-connectors/components/columns.tsxsrc/features/api-key-connectors/components/connectors.tsxsrc/features/api-key-connectors/hooks/use-connectors.tssrc/features/api-key-connectors/params.tssrc/features/api-key-connectors/server/params-loader.tssrc/features/api-key-connectors/server/prefetch.tssrc/features/api-key-connectors/server/routers.tssrc/features/api-key-connectors/types.tssrc/features/assets/components/asset.tsx
timrcm
requested changes
Mar 23, 2026
Contributor
timrcm
left a comment
timrcm
left a comment
There was a problem hiding this comment.
Note Cassidy's feedback in Slack -- it looks like there might still be some issues with this after all.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/app/api/trpc/[trpc]/route.ts (1)
12-14: Consider structured logging and environment-aware verbosity.The error logging is functional, but for production use:
- Logging the full
errorobject may expose sensitive data or stack traces in production logs.- A more structured format would improve observability and log parsing.
♻️ Optional: Structured error logging
onError: ({ path, error }) => { - console.error(path, error); + console.error(`[tRPC Error] ${path}:`, { + code: error.code, + message: error.message, + ...(process.env.NODE_ENV === "development" && { stack: error.stack }), + }); },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/api/trpc/`[trpc]/route.ts around lines 12 - 14, The onError handler currently logs the raw error object (onError: ({ path, error }) => { console.error(path, error); }) which can leak sensitive info and is unstructured; update the handler to emit structured logs (an object with path, error.message, error.name, optional status/code) and gate stack traces behind an environment check (e.g., include error.stack only when NODE_ENV !== 'production'), and redact any sensitive properties before logging or use an existing structured logger instance if available; ensure the handler still captures the path and a concise message for observability while avoiding full error dumps in production.src/features/api-key-connectors/components/connectors.tsx (1)
1-1: Rename this component file to PascalCase for consistency.This component lives in
src/features/.../components/but the filename isconnectors.tsx; please rename it toConnectors.tsxto match repo conventions.As per coding guidelines, "Use PascalCase for component file names and kebab-case for utility/helper file names".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/api-key-connectors/components/connectors.tsx` at line 1, Rename the component file from connectors.tsx to Connectors.tsx and update all imports referencing it to the new filename; ensure the exported React component (Connectors) and any default export usages still match after the rename so imports like import Connectors from "…/components/Connectors" resolve correctly, and run project/type-check to catch any missed import paths.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/features/api-key-connectors/components/connectors.tsx`:
- Line 85: Remove the debug console logging of connector payloads: delete the
console.log("useSuspenseConnectorsByResourceType:", connectors) statement in the
connectors.tsx implementation (the useSuspenseConnectorsByResourceType flow that
handles the connectors variable) so fetched connector data is not printed to the
browser console; ensure no equivalent debug logs remain in the same module or
helper functions that expose connector payloads.
---
Nitpick comments:
In `@src/app/api/trpc/`[trpc]/route.ts:
- Around line 12-14: The onError handler currently logs the raw error object
(onError: ({ path, error }) => { console.error(path, error); }) which can leak
sensitive info and is unstructured; update the handler to emit structured logs
(an object with path, error.message, error.name, optional status/code) and gate
stack traces behind an environment check (e.g., include error.stack only when
NODE_ENV !== 'production'), and redact any sensitive properties before logging
or use an existing structured logger instance if available; ensure the handler
still captures the path and a concise message for observability while avoiding
full error dumps in production.
In `@src/features/api-key-connectors/components/connectors.tsx`:
- Line 1: Rename the component file from connectors.tsx to Connectors.tsx and
update all imports referencing it to the new filename; ensure the exported React
component (Connectors) and any default export usages still match after the
rename so imports like import Connectors from "…/components/Connectors" resolve
correctly, and run project/type-check to catch any missed import paths.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 56631c87-204d-4f97-adf6-2e0ac36f1c78
📒 Files selected for processing (2)
src/app/api/trpc/[trpc]/route.tssrc/features/api-key-connectors/components/connectors.tsx
| resourceType, | ||
| }); | ||
|
|
||
| console.log("useSuspenseConnectorsByResourceType:", connectors); |
There was a problem hiding this comment.
Remove connector payload debug logging before merge.
Line 85 logs fetched connector data in the browser console. This is a production data-exposure/noise risk and should be removed.
Suggested fix
- console.log("useSuspenseConnectorsByResourceType:", connectors);📝 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
| console.log("useSuspenseConnectorsByResourceType:", connectors); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/features/api-key-connectors/components/connectors.tsx` at line 85, Remove
the debug console logging of connector payloads: delete the
console.log("useSuspenseConnectorsByResourceType:", connectors) statement in the
connectors.tsx implementation (the useSuspenseConnectorsByResourceType flow that
handles the connectors variable) so fetched connector data is not printed to the
browser console; ensure no equivalent debug logs remain in the same module or
helper functions that expose connector payloads.
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
3 participants
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.
New table views for each of the 4 connector types (asset, device artifact, remediation, vulnerability)
Summary by CodeRabbit