feat(webapp): allow marking env vars as secret after creation - #2969
feat(webapp): allow marking env vars as secret after creation#2969JoanLaRosa wants to merge 1 commit into
Conversation
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. WalkthroughThe environment variable edit flow now tracks and submits Merge Risk: ⚪ Minimal · up to This localized change adds an irreversible option to mark an environment variable as secret during editing. No actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Description checkExplanation The description explains the implementation, irreversible behavior, testing performed, and provides screenshots. It does not include the repository checklist or a separate Changelog section, but the key change and validation details are present.
✨ Finishing Touches🧪 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 |
Review CompleteYour review story is ready! Comment !reviewfast on this PR to re-generate the story. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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.
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.
3a2939b to
d509417
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…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.
d509417 to
8c97d18
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
@matt-aitken your approval was submitted against
New head is |
| revealAll: boolean; | ||
| }) { | ||
| const [isOpen, setIsOpen] = useState(false); | ||
| const [isSecret, setIsSecret] = useState(variable.isSecret); |
There was a problem hiding this comment.
🟡 Secret toggle stays on after cancelling the edit dialog
The isSecret state lives on the panel component, which stays mounted when the dialog closes (useState at line 812). Toggling Secret value on and then cancelling leaves it on; reopening the row shows it still enabled, so an unrelated later Save marks the variable secret irreversibly.
Prompt for agents
In EditEnvironmentVariablePanel the `isSecret` state is initialized once from `variable.isSecret` via useState, but the component does not unmount when the Dialog closes, so the toggle value persists across open/close cycles. Because marking a variable secret is irreversible, a user who toggles the switch on, cancels the dialog, then later reopens and saves an unrelated change will silently and permanently make the variable secret. Reset `isSecret` back to `variable.isSecret` whenever the dialog transitions to closed (e.g. in the onOpenChange handler), so a cancelled toggle does not leak into a later save.
Was this helpful? React with 👍 or 👎 to provide feedback.
| environmentId: z.string(), | ||
| value: z.string(), | ||
| lastUpdatedBy: EnvironmentVariableUpdaterSchema.optional(), | ||
| isSecret: z.preprocess((val) => val === "true" || val === true, z.boolean()).optional(), |
There was a problem hiding this comment.
🟡 Missing .server-changes note for new user-facing feature
This PR adds a user-facing feature (marking env vars secret after creation) and touches only webapp files, but includes no .server-changes/ file. CONTRIBUTING.md and AGENTS.md require one for user-facing server-only changes.
Prompt for agents
Per CONTRIBUTING.md and AGENTS.md, a user-facing change that only touches server apps (apps/webapp) requires a .server-changes/ markdown file so it appears in release notes. This PR adds a user-visible feature (allow marking environment variables as secret after creation) but adds no such file. Add a .server-changes/ file with frontmatter area: webapp and type: feature, and a concise one-line user-facing description.
Was this helpful? React with 👍 or 👎 to provide feedback.
2 participants
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: