-
-
Notifications
You must be signed in to change notification settings - Fork 985
feat(webapp): allow marking env vars as secret after creation #2969
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
WalkthroughRemoved the Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
Review CompleteYour review story is ready! Comment !reviewfast on this PR to re-generate the story. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx (1)
425-524: Close dialog and clear input value after makeSecret succeeds.The input field uses
defaultValue={variable.value}, so when makeSecret succeeds and marks the variable as secret, the previous value remains visible in the uncontrolled input. Additionally, the dialog doesn't close after makeSecret completes (only after edit actions). Add a handler to track the secret state change and close the dialog:Suggested fix
const fetcher = useFetcher<typeof action>(); const makeSecretFetcher = useFetcher<typeof action>(); const lastSubmission = fetcher.data as any; const isLoading = fetcher.state !== "idle"; const isMakingSecret = makeSecretFetcher.state !== "idle"; + const isSecretNow = variable.isSecret || makeSecretFetcher.data?.success; // Close dialog on successful submission useEffect(() => { if (lastSubmission?.success && fetcher.state === "idle") { setIsOpen(false); } }, [lastSubmission?.success, fetcher.state]); + + useEffect(() => { + if (makeSecretFetcher.data?.success && makeSecretFetcher.state === "idle") { + setIsOpen(false); + } + }, [makeSecretFetcher.data?.success, makeSecretFetcher.state]); const [form, { id, environmentId, value }] = useForm({ id: `edit-environment-variable-${variable.id}-${variable.environment.id}`, // TODO: type this lastSubmission: lastSubmission as any, onValidate({ formData }) { return parse(formData, { schema }); }, shouldRevalidate: "onSubmit", });Then update the input and switch to reflect the immediate secret state:
<Input {...conform.input(value, { type: "text" })} placeholder={variable.isSecret ? "Set new secret value" : "Not set"} - defaultValue={variable.value} + key={`${variable.id}-${variable.environment.id}-${isSecretNow ? "secret" : "plain"}`} + defaultValue={isSecretNow ? "" : variable.value} type={"text"} /><Switch variant="medium" label={<span className="text-text-bright">Secret value</span>} - checked={variable.isSecret} - disabled={variable.isSecret || isMakingSecret} + checked={isSecretNow} + disabled={isSecretNow || isMakingSecret} onCheckedChange={(checked) => { if (checked) { handleMakeSecret(); } }} /> <Hint> - {variable.isSecret + {isSecretNow ? "This variable is secret and cannot be changed back." : "Once enabled, the value will be hidden and cannot be revealed again."} </Hint>
🧹 Nitpick comments (1)
apps/webapp/app/v3/environmentVariables/repository.ts (1)
92-115: Prefer a type alias over an interface for Repository.Since this change extends the interface, consider converting it to a type alias to match the TS style guideline.
♻️ Proposed refactor
-export interface Repository { +export type Repository = { create(projectId: string, options: CreateEnvironmentVariables): Promise<CreateResult>; edit(projectId: string, options: EditEnvironmentVariable): Promise<Result>; editValue(projectId: string, options: EditEnvironmentVariableValue): Promise<Result>; getProject(projectId: string): Promise<ProjectEnvironmentVariable[]>; /** * Get the environment variables for a given environment, it does NOT return values for secret variables */ getEnvironmentWithRedactedSecrets( projectId: string, environmentId: string ): Promise<EnvironmentVariableWithSecret[]>; /** * Get the environment variables for a given environment */ getEnvironment(projectId: string, environmentId: string): Promise<EnvironmentVariable[]>; /** * Return all env vars, including secret variables with values. Should only be used for executing tasks. */ getEnvironmentVariables(projectId: string, environmentId: string): Promise<EnvironmentVariable[]>; delete(projectId: string, options: DeleteEnvironmentVariable): Promise<Result>; deleteValue(projectId: string, options: DeleteEnvironmentVariableValue): Promise<Result>; makeSecret(projectId: string, options: MakeSecretEnvironmentVariableValue): Promise<Result>; -} +};As per coding guidelines: Use types over interfaces for TypeScript.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsxapps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.tsapps/webapp/app/v3/environmentVariables/repository.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
**/*.{ts,tsx}: Always import tasks from@trigger.dev/sdk, never use@trigger.dev/sdk/v3or deprecatedclient.defineJobpattern
Every Trigger.dev task must be exported and have a uniqueidproperty with no timeouts in the run function
Files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsxapps/webapp/app/v3/environmentVariables/repository.tsapps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsxapps/webapp/app/v3/environmentVariables/repository.tsapps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Import from
@trigger.dev/coreusing subpaths only, never import from root
Files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsxapps/webapp/app/v3/environmentVariables/repository.tsapps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts
apps/webapp/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Access all environment variables through the
envexport ofenv.server.tsinstead of directly accessingprocess.envin the Trigger.dev webapp
Files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsxapps/webapp/app/v3/environmentVariables/repository.tsapps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: When importing from@trigger.dev/corein the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webappAccess environment variables via
envexport fromapps/webapp/app/env.server.ts, never useprocess.envdirectly
Files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsxapps/webapp/app/v3/environmentVariables/repository.tsapps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts
**/*.{js,ts,jsx,tsx,json,md,yaml,yml}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier before committing
Files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsxapps/webapp/app/v3/environmentVariables/repository.tsapps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
**/*.ts: When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs
Do not use high-cardinality attributes in OTEL metrics such as UUIDs/IDs (envId, userId, runId, projectId, organizationId), unbounded integers (itemCount, batchSize, retryCount), timestamps (createdAt, startTime), or free-form strings (errorMessage, taskName, queueName)
When exporting OTEL metrics via OTLP to Prometheus, be aware that the exporter automatically adds unit suffixes to metric names (e.g., 'my_duration_ms' becomes 'my_duration_ms_milliseconds', 'my_counter' becomes 'my_counter_total'). Account for these transformations when writing Grafana dashboards or Prometheus queries
Files:
apps/webapp/app/v3/environmentVariables/repository.tsapps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts
🧠 Learnings (6)
📓 Common learnings
Learnt from: julienvanbeveren
Repo: triggerdotdev/trigger.dev PR: 2417
File: apps/webapp/app/routes/api.v1.projects.$projectRef.envvars.$slug.import.ts:56-61
Timestamp: 2025-08-19T09:49:07.011Z
Learning: In the Trigger.dev codebase, environment variables should default to `isSecret: false` when not explicitly marked as secrets in the syncEnvVars functionality. This is the intended behavior for both regular variables and parent variables.
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-11-27T16:26:58.661Z
Learning: Applies to apps/webapp/app/**/*.{ts,tsx} : Access all environment variables through the `env` export of `env.server.ts` instead of directly accessing `process.env` in the Trigger.dev webapp
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 2155
File: hosting/docker/.env.example:4-7
Timestamp: 2025-06-06T23:55:01.933Z
Learning: In the trigger.dev project, .env.example files should contain actual example secret values rather than placeholders, as these help users understand the expected format. The files include clear warnings about not using these defaults in production and instructions for generating proper secrets.
📚 Learning: 2025-12-08T15:19:56.823Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 2760
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx:278-281
Timestamp: 2025-12-08T15:19:56.823Z
Learning: In apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx, the tableState search parameter uses intentional double-encoding: the parameter value contains a URL-encoded URLSearchParams string, so decodeURIComponent(value("tableState") ?? "") is required to fully decode it before parsing with new URLSearchParams(). This pattern allows bundling multiple filter/pagination params as a single search parameter.
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx
📚 Learning: 2025-11-27T16:26:58.661Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-11-27T16:26:58.661Z
Learning: Applies to apps/webapp/app/**/*.{ts,tsx} : Access all environment variables through the `env` export of `env.server.ts` instead of directly accessing `process.env` in the Trigger.dev webapp
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsxapps/webapp/app/v3/environmentVariables/repository.tsapps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts
📚 Learning: 2025-04-17T10:27:25.337Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 1923
File: packages/core/src/v3/schemas/api.ts:813-827
Timestamp: 2025-04-17T10:27:25.337Z
Learning: Creating secret environment variables is restricted to the dashboard UI only, and not allowed via the API/SDK for now. The `EnvironmentVariableWithSecret` type in the API schema is for reading/displaying purposes only.
Applied to files:
apps/webapp/app/v3/environmentVariables/repository.tsapps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts
📚 Learning: 2026-01-15T11:50:06.067Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-15T11:50:06.067Z
Learning: Applies to apps/webapp/**/*.{ts,tsx} : Access environment variables via `env` export from `apps/webapp/app/env.server.ts`, never use `process.env` directly
Applied to files:
apps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts
📚 Learning: 2025-08-19T09:49:07.011Z
Learnt from: julienvanbeveren
Repo: triggerdotdev/trigger.dev PR: 2417
File: apps/webapp/app/routes/api.v1.projects.$projectRef.envvars.$slug.import.ts:56-61
Timestamp: 2025-08-19T09:49:07.011Z
Learning: In the Trigger.dev codebase, environment variables should default to `isSecret: false` when not explicitly marked as secrets in the syncEnvVars functionality. This is the intended behavior for both regular variables and parent variables.
Applied to files:
apps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts
🧬 Code graph analysis (3)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx (1)
apps/webapp/app/v3/environmentVariables/repository.ts (2)
MakeSecretEnvironmentVariableValue(50-53)MakeSecretEnvironmentVariableValue(54-54)
apps/webapp/app/v3/environmentVariables/repository.ts (1)
apps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts (1)
projectId(584-624)
apps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts (1)
apps/webapp/app/v3/environmentVariables/repository.ts (3)
MakeSecretEnvironmentVariableValue(50-53)MakeSecretEnvironmentVariableValue(54-54)Result(63-70)
🔇 Additional comments (5)
apps/webapp/app/v3/environmentVariables/repository.ts (1)
50-54: MakeSecretEnvironmentVariableValue schema looks good.The payload shape aligns with the new action and keeps validation consistent.
apps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts (2)
8-18: No review needed for the type import.
634-702: makeSecret flow looks solid.Guards cover missing project, missing value, and already-secret states before the update.
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx (2)
31-31: No review needed for this import change.
109-198: Schema and action wiring for makeSecret are consistent.The discriminated union and action handler stay aligned with the repository method.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
f717869 to
3a2939b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx (2)
406-418: ResetisSecretwhen the dialog opens to avoid unintended irreversible changes.
If a user toggles to secret and cancels, the state staystrue; a later save can silently mark it secret.🛠️ Suggested fix
const [isOpen, setIsOpen] = useState(false); const [isSecret, setIsSecret] = useState(variable.isSecret); const fetcher = useFetcher<typeof action>(); const lastSubmission = fetcher.data as any; const isLoading = fetcher.state !== "idle"; + useEffect(() => { + if (isOpen) { + setIsSecret(variable.isSecret); + } + }, [isOpen, variable.isSecret]);
439-474: Add a confirmation gate before making a variable secret.
The action is irreversible, but the current submit flow doesn’t confirm the change.🛠️ Suggested fix
const isLoading = fetcher.state !== "idle"; + const shouldConfirmSecret = !variable.isSecret && isSecret; @@ - <fetcher.Form method="post" {...form.props}> + <fetcher.Form + method="post" + {...form.props} + onSubmit={(event) => { + if ( + shouldConfirmSecret && + !window.confirm("Making this value secret is irreversible. Continue?") + ) { + event.preventDefault(); + } + }} + >
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsxapps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.tsapps/webapp/app/v3/environmentVariables/repository.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
**/*.{ts,tsx}: Always import tasks from@trigger.dev/sdk, never use@trigger.dev/sdk/v3or deprecatedclient.defineJobpattern
Every Trigger.dev task must be exported and have a uniqueidproperty with no timeouts in the run function
Files:
apps/webapp/app/v3/environmentVariables/repository.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsxapps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/app/v3/environmentVariables/repository.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsxapps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Import from
@trigger.dev/coreusing subpaths only, never import from root
Files:
apps/webapp/app/v3/environmentVariables/repository.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsxapps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts
apps/webapp/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Access all environment variables through the
envexport ofenv.server.tsinstead of directly accessingprocess.envin the Trigger.dev webapp
Files:
apps/webapp/app/v3/environmentVariables/repository.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsxapps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: When importing from@trigger.dev/corein the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webappAccess environment variables via
envexport fromapps/webapp/app/env.server.ts, never useprocess.envdirectly
Files:
apps/webapp/app/v3/environmentVariables/repository.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsxapps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
**/*.ts: When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs
Do not use high-cardinality attributes in OTEL metrics such as UUIDs/IDs (envId, userId, runId, projectId, organizationId), unbounded integers (itemCount, batchSize, retryCount), timestamps (createdAt, startTime), or free-form strings (errorMessage, taskName, queueName)
When exporting OTEL metrics via OTLP to Prometheus, be aware that the exporter automatically adds unit suffixes to metric names (e.g., 'my_duration_ms' becomes 'my_duration_ms_milliseconds', 'my_counter' becomes 'my_counter_total'). Account for these transformations when writing Grafana dashboards or Prometheus queries
Files:
apps/webapp/app/v3/environmentVariables/repository.tsapps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts
**/*.{js,ts,jsx,tsx,json,md,yaml,yml}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier before committing
Files:
apps/webapp/app/v3/environmentVariables/repository.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsxapps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts
🧠 Learnings (8)
📓 Common learnings
Learnt from: julienvanbeveren
Repo: triggerdotdev/trigger.dev PR: 2417
File: apps/webapp/app/routes/api.v1.projects.$projectRef.envvars.$slug.import.ts:56-61
Timestamp: 2025-08-19T09:49:07.011Z
Learning: In the Trigger.dev codebase, environment variables should default to `isSecret: false` when not explicitly marked as secrets in the syncEnvVars functionality. This is the intended behavior for both regular variables and parent variables.
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 1923
File: packages/core/src/v3/schemas/api.ts:813-827
Timestamp: 2025-04-17T10:27:25.337Z
Learning: Creating secret environment variables is restricted to the dashboard UI only, and not allowed via the API/SDK for now. The `EnvironmentVariableWithSecret` type in the API schema is for reading/displaying purposes only.
📚 Learning: 2025-08-14T18:35:44.370Z
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 2390
File: apps/webapp/app/env.server.ts:764-765
Timestamp: 2025-08-14T18:35:44.370Z
Learning: The BoolEnv helper in apps/webapp/app/utils/boolEnv.ts uses z.preprocess with inconsistent default value types across the codebase - some usages pass boolean defaults (correct) while others pass string defaults (incorrect), leading to type confusion. The helper should enforce boolean-only defaults or have clearer documentation.
Applied to files:
apps/webapp/app/v3/environmentVariables/repository.ts
📚 Learning: 2025-08-19T09:49:07.011Z
Learnt from: julienvanbeveren
Repo: triggerdotdev/trigger.dev PR: 2417
File: apps/webapp/app/routes/api.v1.projects.$projectRef.envvars.$slug.import.ts:56-61
Timestamp: 2025-08-19T09:49:07.011Z
Learning: In the Trigger.dev codebase, environment variables should default to `isSecret: false` when not explicitly marked as secrets in the syncEnvVars functionality. This is the intended behavior for both regular variables and parent variables.
Applied to files:
apps/webapp/app/v3/environmentVariables/repository.tsapps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts
📚 Learning: 2025-11-27T16:26:58.661Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-11-27T16:26:58.661Z
Learning: Applies to apps/webapp/app/**/*.{ts,tsx} : Access all environment variables through the `env` export of `env.server.ts` instead of directly accessing `process.env` in the Trigger.dev webapp
Applied to files:
apps/webapp/app/v3/environmentVariables/repository.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsxapps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts
📚 Learning: 2026-01-15T11:50:06.067Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-15T11:50:06.067Z
Learning: Applies to apps/webapp/**/*.{ts,tsx} : Access environment variables via `env` export from `apps/webapp/app/env.server.ts`, never use `process.env` directly
Applied to files:
apps/webapp/app/v3/environmentVariables/repository.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsxapps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts
📚 Learning: 2025-11-27T16:26:37.432Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T16:26:37.432Z
Learning: Applies to {packages/core,apps/webapp}/**/*.{ts,tsx} : Use zod for validation in packages/core and apps/webapp
Applied to files:
apps/webapp/app/v3/environmentVariables/repository.ts
📚 Learning: 2025-12-08T15:19:56.823Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 2760
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx:278-281
Timestamp: 2025-12-08T15:19:56.823Z
Learning: In apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx, the tableState search parameter uses intentional double-encoding: the parameter value contains a URL-encoded URLSearchParams string, so decodeURIComponent(value("tableState") ?? "") is required to fully decode it before parsing with new URLSearchParams(). This pattern allows bundling multiple filter/pagination params as a single search parameter.
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx
📚 Learning: 2025-08-14T12:13:20.455Z
Learnt from: myftija
Repo: triggerdotdev/trigger.dev PR: 2392
File: packages/cli-v3/src/utilities/gitMeta.ts:195-218
Timestamp: 2025-08-14T12:13:20.455Z
Learning: In the GitMeta schema (packages/core/src/v3/schemas/common.ts), all fields are intentionally optional to handle partial data from various deployment contexts (local, GitHub Actions, GitHub App). Functions like getGitHubAppMeta() are designed to work with missing environment variables rather than validate their presence.
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx
🧬 Code graph analysis (1)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx (1)
apps/webapp/app/components/primitives/Switch.tsx (1)
Switch(64-119)
🔇 Additional comments (2)
apps/webapp/app/v3/environmentVariables/repository.ts (1)
50-55: LGTM forisSecretpreprocessing.
Accepts form string payloads while keeping boolean validation intact.apps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts (1)
363-443: Secret flag persistence looks consistent.
The isSecret update happens inside the same transaction as the secret write, which is the right place for it.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
…ation Move the secret toggle into the edit form so it submits on Save instead of firing a separate request immediately. Remove the standalone makeSecret action/method and include isSecret as an optional field on the existing editValue flow.
3a2939b to
d509417
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@apps/webapp/app/routes/_app.orgs`.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx:
- Around line 460-474: Add a submission confirmation that blocks making a
variable secret unless the user confirms: implement an onSubmit handler on the
surrounding form (or addFormSubmit handler function) that checks if the new
isSecret state is true and the original variable.isSecret is false, then call
window.confirm(...) and abort submission if it returns false; wire this handler
to the form submit event and keep the existing Switch and setIsSecret logic
unchanged so toggling still updates isSecret but finalizing the irreversible
change requires confirm.
- Line 407: The isSecret local state (created with useState(variable.isSecret))
isn't reset when the dialog is reopened, causing stale toggle UI; update the
component to reset isSecret to variable.isSecret whenever the edit dialog opens
by adding an effect that runs when the dialog open flag (e.g., isOpen or open)
changes—call setIsSecret(variable.isSecret) inside a useEffect that depends on
[variable.isSecret, <dialogOpenFlag>] so the toggle reflects the current
variable.isSecret each time the dialog is shown.
🧹 Nitpick comments (1)
apps/webapp/app/v3/environmentVariables/repository.ts (1)
54-54:z.preprocesscoercesundefinedtofalse, making.optional()ineffective.The current pattern converts any input that isn't
"true"ortrueintofalse, includingundefined. This means the schema can never returnundefinedforisSecret, defeating the purpose of.optional().If the intention is to distinguish "not provided" from "explicitly false" (important for an irreversible action), consider:
Suggested fix using nullish handling
- isSecret: z.preprocess((val) => val === "true" || val === true, z.boolean()).optional(), + isSecret: z.preprocess( + (val) => (val === undefined ? undefined : val === "true" || val === true), + z.boolean().optional() + ),Alternatively, if the form always sends a value (as it does currently), the
.optional()is misleading and could be removed:- isSecret: z.preprocess((val) => val === "true" || val === true, z.boolean()).optional(), + isSecret: z.preprocess((val) => val === "true" || val === true, z.boolean()),
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsxapps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.tsapps/webapp/app/v3/environmentVariables/repository.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
**/*.{ts,tsx}: Always import tasks from@trigger.dev/sdk, never use@trigger.dev/sdk/v3or deprecatedclient.defineJobpattern
Every Trigger.dev task must be exported and have a uniqueidproperty with no timeouts in the run function
Files:
apps/webapp/app/v3/environmentVariables/repository.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/app/v3/environmentVariables/repository.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Import from
@trigger.dev/coreusing subpaths only, never import from root
Files:
apps/webapp/app/v3/environmentVariables/repository.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx
apps/webapp/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Access all environment variables through the
envexport ofenv.server.tsinstead of directly accessingprocess.envin the Trigger.dev webapp
Files:
apps/webapp/app/v3/environmentVariables/repository.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: When importing from@trigger.dev/corein the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webappAccess environment variables via
envexport fromapps/webapp/app/env.server.ts, never useprocess.envdirectly
Files:
apps/webapp/app/v3/environmentVariables/repository.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
**/*.ts: When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs
Do not use high-cardinality attributes in OTEL metrics such as UUIDs/IDs (envId, userId, runId, projectId, organizationId), unbounded integers (itemCount, batchSize, retryCount), timestamps (createdAt, startTime), or free-form strings (errorMessage, taskName, queueName)
When exporting OTEL metrics via OTLP to Prometheus, be aware that the exporter automatically adds unit suffixes to metric names (e.g., 'my_duration_ms' becomes 'my_duration_ms_milliseconds', 'my_counter' becomes 'my_counter_total'). Account for these transformations when writing Grafana dashboards or Prometheus queries
Files:
apps/webapp/app/v3/environmentVariables/repository.ts
**/*.{js,ts,jsx,tsx,json,md,yaml,yml}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier before committing
Files:
apps/webapp/app/v3/environmentVariables/repository.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx
🧠 Learnings (7)
📓 Common learnings
Learnt from: julienvanbeveren
Repo: triggerdotdev/trigger.dev PR: 2417
File: apps/webapp/app/routes/api.v1.projects.$projectRef.envvars.$slug.import.ts:56-61
Timestamp: 2025-08-19T09:49:07.011Z
Learning: In the Trigger.dev codebase, environment variables should default to `isSecret: false` when not explicitly marked as secrets in the syncEnvVars functionality. This is the intended behavior for both regular variables and parent variables.
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-11-27T16:26:58.661Z
Learning: Applies to apps/webapp/app/**/*.{ts,tsx} : Access all environment variables through the `env` export of `env.server.ts` instead of directly accessing `process.env` in the Trigger.dev webapp
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 2155
File: hosting/docker/.env.example:4-7
Timestamp: 2025-06-06T23:55:01.933Z
Learning: In the trigger.dev project, .env.example files should contain actual example secret values rather than placeholders, as these help users understand the expected format. The files include clear warnings about not using these defaults in production and instructions for generating proper secrets.
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 1923
File: packages/core/src/v3/schemas/api.ts:813-827
Timestamp: 2025-04-17T10:27:25.337Z
Learning: Creating secret environment variables is restricted to the dashboard UI only, and not allowed via the API/SDK for now. The `EnvironmentVariableWithSecret` type in the API schema is for reading/displaying purposes only.
📚 Learning: 2025-08-14T18:35:44.370Z
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 2390
File: apps/webapp/app/env.server.ts:764-765
Timestamp: 2025-08-14T18:35:44.370Z
Learning: The BoolEnv helper in apps/webapp/app/utils/boolEnv.ts uses z.preprocess with inconsistent default value types across the codebase - some usages pass boolean defaults (correct) while others pass string defaults (incorrect), leading to type confusion. The helper should enforce boolean-only defaults or have clearer documentation.
Applied to files:
apps/webapp/app/v3/environmentVariables/repository.ts
📚 Learning: 2025-08-19T09:49:07.011Z
Learnt from: julienvanbeveren
Repo: triggerdotdev/trigger.dev PR: 2417
File: apps/webapp/app/routes/api.v1.projects.$projectRef.envvars.$slug.import.ts:56-61
Timestamp: 2025-08-19T09:49:07.011Z
Learning: In the Trigger.dev codebase, environment variables should default to `isSecret: false` when not explicitly marked as secrets in the syncEnvVars functionality. This is the intended behavior for both regular variables and parent variables.
Applied to files:
apps/webapp/app/v3/environmentVariables/repository.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx
📚 Learning: 2025-11-27T16:26:58.661Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-11-27T16:26:58.661Z
Learning: Applies to apps/webapp/app/**/*.{ts,tsx} : Access all environment variables through the `env` export of `env.server.ts` instead of directly accessing `process.env` in the Trigger.dev webapp
Applied to files:
apps/webapp/app/v3/environmentVariables/repository.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx
📚 Learning: 2026-01-15T11:50:06.067Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-15T11:50:06.067Z
Learning: Applies to apps/webapp/**/*.{ts,tsx} : Access environment variables via `env` export from `apps/webapp/app/env.server.ts`, never use `process.env` directly
Applied to files:
apps/webapp/app/v3/environmentVariables/repository.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx
📚 Learning: 2025-12-08T15:19:56.823Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 2760
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx:278-281
Timestamp: 2025-12-08T15:19:56.823Z
Learning: In apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx, the tableState search parameter uses intentional double-encoding: the parameter value contains a URL-encoded URLSearchParams string, so decodeURIComponent(value("tableState") ?? "") is required to fully decode it before parsing with new URLSearchParams(). This pattern allows bundling multiple filter/pagination params as a single search parameter.
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx
📚 Learning: 2025-08-14T12:13:20.455Z
Learnt from: myftija
Repo: triggerdotdev/trigger.dev PR: 2392
File: packages/cli-v3/src/utilities/gitMeta.ts:195-218
Timestamp: 2025-08-14T12:13:20.455Z
Learning: In the GitMeta schema (packages/core/src/v3/schemas/common.ts), all fields are intentionally optional to handle partial data from various deployment contexts (local, GitHub Actions, GitHub App). Functions like getGitHubAppMeta() are designed to work with missing environment variables rather than validate their presence.
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx
🧬 Code graph analysis (1)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx (1)
apps/webapp/app/components/primitives/Switch.tsx (1)
Switch(64-119)
🔇 Additional comments (2)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx (2)
441-441: LGTM!The hidden input correctly sends
isSecretas a string value that the Zod schema can preprocess.
31-31: LGTM!The
Hintcomponent import is appropriately added for the new secret toggle UI.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
....orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx
Show resolved
Hide resolved
....orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary
isSecretfield to theeditValuerepository method and Zod schemamakeSecretaction in favor of the unified form-based approachImplements: https://feedback.trigger.dev/p/ability-to-set-a-non-secret-env-var-to-secret
Tested on local:
Before saving:


After saving: