Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis pull request introduces a centralized environment settings management system by creating an Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/apps/dashboard/lib/trpc/routers/deploy/environment-settings/get.ts (1)
30-35:⚠️ Potential issue | 🟠 MajorRemove type assertion and validate
sentinelConfigwith a zod schema.The type assertion
as SentinelConfigviolates the no-type-assertion rule. Add schema validation to ensure type safety without casts.Proposed fix
import { and, db, eq } from "@/lib/db"; import { environmentBuildSettings, environmentRuntimeSettings } from "@unkey/db/src/schema"; import { z } from "zod"; import { workspaceProcedure } from "../../../trpc"; import type { SentinelConfig } from "./sentinel/update-middleware"; +const sentinelConfigSchema = z.object({ + policies: z.array( + z.object({ + id: z.string(), + name: z.string(), + enabled: z.boolean(), + keyauth: z.object({ keySpaceIds: z.array(z.string()) }), + }), + ), +}); + export const getEnvironmentSettings = workspaceProcedure .input(z.object({ environmentId: z.string() })) .query(async ({ ctx, input }) => { @@ -29,9 +39,7 @@ export const getEnvironmentSettings = workspaceProcedure // Without that length check this will Buffer.from gives "", and JSON.parse("") throws 500. sentinelConfig: runtimeSettings.sentinelConfig?.length - ? (JSON.parse( - Buffer.from(runtimeSettings.sentinelConfig).toString(), - ) as SentinelConfig) + ? sentinelConfigSchema.parse( + JSON.parse(Buffer.from(runtimeSettings.sentinelConfig).toString()), + ) : undefined,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/apps/dashboard/lib/trpc/routers/deploy/environment-settings/get.ts` around lines 30 - 35, Replace the unsafe cast of runtimeSettings.sentinelConfig to SentinelConfig with a zod validation step: define a SentinelConfigSchema (using zod) that matches the SentinelConfig shape, parse the Buffer.from(runtimeSettings.sentinelConfig).toString() JSON and run SentinelConfigSchema.safeParse on the result, and then set sentinelConfig to the parsed data when safeParse.success is true (or handle/log/return undefined on failure) instead of using "as SentinelConfig"; update the code paths that read sentinelConfig to expect undefined when validation fails.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/[projectId]/(overview)/settings/components/advanced-settings/env-vars/index.tsx:
- Around line 118-134: Remove the unsafe type assertions in the env var
mutations: stop casting the result of toTrpcType(...) and stop casting v.id;
instead rely on the return type of toTrpcType as-is (it already yields
"recoverable" | "writeonly") when calling collection.envVars.insert and
collection.envVars.update, and add an explicit runtime guard before calling
collection.envVars.update to ensure v.id is a defined string (e.g., skip or
throw if v.id is undefined) so you no longer need the v.id as string assertion.
In
`@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/[projectId]/(overview)/settings/components/runtime-settings/port-settings.tsx:
- Around line 18-29: The form's defaultValues from useForm (resolver:
zodResolver(portSchema)) only apply on mount so when defaultValue (from
useEnvironmentSettings/useLiveQuery) changes the form stays stale; add a
useEffect that watches defaultValue and calls the form reset method (from
useForm) with the new value (e.g., reset({ port: defaultValue })) so useWatch/
currentPort and formState (isValid/isSubmitting/errors) reflect the updated
environment value; place the effect near the useForm/useWatch setup and
reference reset, defaultValue, useEffect, useWatch, and portSchema.
In
`@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/[projectId]/(overview)/settings/environment-provider.tsx:
- Around line 37-41: Replace the raw Error thrown in useEnvironmentSettings with
a fault-created error and correct the provider name: import fault from 'fault'
(or the project's fault import pattern), check the EnvironmentContext as you do
now, and when it's missing throw fault("useEnvironmentSettings must be used
within EnvironmentSettingsProvider") instead of new Error(...); ensure the
change references useEnvironmentSettings and EnvironmentContext and uses the
fault helper to construct the error with the corrected provider name
EnvironmentSettingsProvider.
---
Outside diff comments:
In `@web/apps/dashboard/lib/trpc/routers/deploy/environment-settings/get.ts`:
- Around line 30-35: Replace the unsafe cast of runtimeSettings.sentinelConfig
to SentinelConfig with a zod validation step: define a SentinelConfigSchema
(using zod) that matches the SentinelConfig shape, parse the
Buffer.from(runtimeSettings.sentinelConfig).toString() JSON and run
SentinelConfigSchema.safeParse on the result, and then set sentinelConfig to the
parsed data when safeParse.success is true (or handle/log/return undefined on
failure) instead of using "as SentinelConfig"; update the code paths that read
sentinelConfig to expect undefined when validation fails.
What does this PR do?
This PR moves all the deploy settings into tanstackdb, also creates a nice DI for accessing environmentId(in the future we'll have different apps/envs) so this structure will help us a lot. We are not gonna change anything in the setting forms we'll just tell settings where to read and commit their mesages.
Also fixed some UI issues reported by @chronark.
By default current settings form persists and reads everything from
productionenvironment.How should this be tested?