Add prisma app env set/list/unset namespace#4
Draft
kristof-siket wants to merge 6 commits intomainfrom
Draft
Conversation
PTL-1475 / Compute Public Beta launch — M5: Configure.
The platform-managed /v1/environment-variables API is now the single source
of truth for app env vars. The CLI's existing `app update-env` / `app list-env`
commands talk to the legacy Foundry-version env-var path, leaving CLI users
without a way to author env vars against the new platform store. This
introduces the `prisma app env` namespace to fill that gap.
New verbs:
- `prisma app env set KEY=VALUE` — create or replace a variable on a project
template (`--class production` / `--class preview`) or a branch override
(`--branch <name>`). Idempotent upsert: existing rows are replaced via PATCH,
new rows are created via POST.
- `prisma app env list` — list metadata for a scope (no values, FR15). Defaults
to `--class production` when no scope flag is provided.
- `prisma app env unset KEY` — looks the row up by natural key and DELETEs it.
Flag rules per spec FR21:
- `--class` and `--branch` are mutually exclusive.
- `set` and `unset` require an explicit scope so the CLI never silently writes
to production.
- `--branch <name>` resolves to a Branch via `GET /v1/projects/{projectId}/branches?gitName=`.
Branch-override writes (`set`/`unset` against `--branch`) currently surface a
focused "feature unavailable" error: the POST/PATCH request bodies on
/v1/environment-variables don't accept a branchId yet. List works against
`--branch` because the GET endpoint already supports the filter. Branch-write
support lands in a follow-up.
Legacy `app update-env` / `app list-env` continue to work but print a
deprecation banner pointing at the new commands. The banner is suppressed
under --json so machine consumers keep their JSON channel clean. Removal is
deferred to a separate decision spike with the Terminal team.
The `@prisma/management-api-sdk` dep is bumped from ^1.24.0 to ^1.27.0 to pull
in the typed client for the new endpoints.
Tests cover: set happy path (POST), set replace (PATCH upsert), set with
both flags, set with neither flag, malformed KEY=VALUE, invalid key shape,
branch-override write feature-unavailable, list metadata-only contract,
list defaulting to production, list with branch resolution, unset happy
path, unset not-found, unset with neither flag, deprecation banner emitted
to stderr, deprecation banner suppressed under --json.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Summary by CodeRabbit
WalkthroughThis PR introduces the ChangesApp Environment Variables Management
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
CodeRabbit chat interactions are restricted to organization members for this repository. Ask an organization member to interact with CodeRabbit, or set |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/cli/tests/app-env.test.ts`:
- Around line 515-618: Add two tests mirroring the existing runAppUpdateEnv
cases but for runAppListEnv: one that calls runAppListEnv(context,
"hello-world") in human mode and asserts stderr.buffer contains "[deprecation]"
and the suggested guidance strings, and another that creates context with flags:
{ json: true } and asserts stderr.buffer does not contain "[deprecation]". In
the test setup reuse the same vi.doMock blocks but ensure the preview provider
mock returned by createPreviewAppProvider includes the method used by
runAppListEnv (e.g., listAppEnv or listEnvs) returning the same shape as
updateAppEnv/listDeployments so the controller resolves correctly; import
runAppListEnv from ../src/controllers/app and
createTempCwd/createTestCommandContext from ./helpers just like the update-env
tests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c17f1916-8c6c-4f2e-910f-b864ceabd27d
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
docs/product/command-spec.mdpackages/cli/package.jsonpackages/cli/src/commands/app/env.tspackages/cli/src/commands/app/index.tspackages/cli/src/controllers/app-env.tspackages/cli/src/controllers/app.tspackages/cli/src/lib/app/env-config.tspackages/cli/src/presenters/app-env.tspackages/cli/src/shell/command-meta.tspackages/cli/src/types/app-env.tspackages/cli/tests/app-env.test.ts
Adds parity coverage for runAppListEnv mirroring the runAppUpdateEnv deprecation tests: the banner is emitted to stderr in human mode and suppressed under --json. Without this guard, a future change that silently drops or alters the list-env deprecation policy would slip through, since only update-env was previously pinned by tests. Addresses CodeRabbit nitpick on PR #4.
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
CodeRabbit chat interactions are restricted to organization members for this repository. Ask an organization member to interact with CodeRabbit, or set |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/cli/tests/app-env.test.ts`:
- Around line 281-435: Tests for mutual exclusion of scope flags are missing for
runAppEnvList and runAppEnvUnset; add negative tests that call
controllers.runAppEnvList and controllers.runAppEnvUnset with both className (or
--class) and branchName (or --branch) supplied and assert the command returns an
error (or throws) and that no HTTP client GET/DELETE is invoked. Specifically,
add one test for runAppEnvList that passes { className: "production",
branchName: "feature" } and expects a validation error and that client.GET was
not called, and another for runAppEnvUnset that passes both scope flags and
expects a validation error and that client.DELETE (or appropriate client method)
was not called; place these alongside the existing app env list tests so they
guard the shared validation rule.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0e61e4fa-4107-41b3-8c23-f566e08f80f7
📒 Files selected for processing (1)
packages/cli/tests/app-env.test.ts
The mutex rule is enforced by a shared validator; pinning it on every verb (set, list, unset) ensures a future refactor that splits the validators (or short-circuits one path) can't regress just a single entry point silently. Addresses CodeRabbit nitpick on PR #4.
|
CodeRabbit chat interactions are restricted to organization members for this repository. Ask an organization member to interact with CodeRabbit, or set |
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
CodeRabbit chat interactions are restricted to organization members for this repository. Ask an organization member to interact with CodeRabbit, or set |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/cli/tests/app-env.test.ts`:
- Around line 571-748: The tests repeat the same
vi.doMock("../src/lib/app/preview-provider", ...) scaffold; extract that into a
reusable helper (e.g., export a setupPreviewProviderMock function from
tests/helpers.ts) that calls vi.doMock and returns or accepts overrides for
createPreviewAppProvider's methods (listApps, listDeployments, updateAppEnv,
listAppEnvNames, etc.), then replace each repeated vi.doMock block in
packages/cli/tests/app-env.test.ts with a single call to
setupPreviewProviderMock(...) (imported from helpers) passing only the method
overrides needed for that test; ensure the helper names the mocked factory
createPreviewAppProvider to match existing code and document that tests should
call/reset the mock between cases.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 979ad4ea-6ab8-4d5d-975b-e9b981eb5e07
📒 Files selected for processing (1)
packages/cli/tests/app-env.test.ts
Centralises the auth/config/preview-provider mock setup that was repeated in each of the four legacy-deprecation tests into a single mockLegacyEnvDependencies helper plus two happy-path response factories. A future change to the seam — e.g. extending the preview provider's contract or moving the link lookup off readLinkedProjectId — now needs to be reflected in exactly one place instead of fanning out across every deprecation test. Addresses CodeRabbit nitpick on PR #4.
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
CodeRabbit chat interactions are restricted to organization members for this repository. Ask an organization member to interact with CodeRabbit, or set |
1 similar comment
|
CodeRabbit chat interactions are restricted to organization members for this repository. Ask an organization member to interact with CodeRabbit, or set |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/cli/tests/app-env.test.ts (1)
614-652: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueConsider consolidating duplicate happy-path factories.
updateAppEnvHappyPathandlistAppEnvNamesHappyPathreturn identical structures. Extracting a shared factory (e.g.,createLegacyEnvResponse()) would eliminate duplication and simplify future response-shape changes.♻️ Consolidation sketch
+const createLegacyEnvResponse = () => ({ + projectId: "proj_123", + app: { + id: "app_1", + name: "hello-world", + region: null, + liveDeploymentId: "dep_1", + liveUrl: null, + }, + deployment: { + id: "dep_1", + status: "running", + url: null, + createdAt: "2026-05-08T10:00:00.000Z", + live: true, + }, + variables: ["FOO"], +}); + -const updateAppEnvHappyPath = () => - vi.fn().mockResolvedValue({ - projectId: "proj_123", - app: { ... }, - deployment: { ... }, - variables: ["FOO"], - }); +const updateAppEnvHappyPath = () => + vi.fn().mockResolvedValue(createLegacyEnvResponse()); -const listAppEnvNamesHappyPath = () => - vi.fn().mockResolvedValue({ - projectId: "proj_123", - app: { ... }, - deployment: { ... }, - variables: ["FOO"], - }); +const listAppEnvNamesHappyPath = () => + vi.fn().mockResolvedValue(createLegacyEnvResponse());🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/cli/tests/app-env.test.ts` around lines 614 - 652, Both updateAppEnvHappyPath and listAppEnvNamesHappyPath return the same mock response; extract the shared object into a single factory (e.g., createLegacyEnvResponse) and have updateAppEnvHappyPath and listAppEnvNamesHappyPath call that factory to remove duplication. Update references in tests to use the new factory where needed and ensure the returned shape (projectId, app, deployment, variables) remains identical so existing assertions keep working.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/cli/tests/app-env.test.ts`:
- Around line 5-10: The afterEach cleanup in the test does not explicitly unmock
the preview-provider module that mockLegacyEnvDependencies registers; add a
vi.doUnmock("../src/lib/app/preview-provider") call inside the existing
afterEach alongside the other vi.doUnmock(...) calls so the mock is removed from
Vitest's registry (ensure this is placed with the existing afterEach block that
currently calls vi.doUnmock("../src/adapters/config"),
vi.doUnmock("../src/lib/auth/guard"), vi.resetModules(), and
vi.restoreAllMocks()).
---
Duplicate comments:
In `@packages/cli/tests/app-env.test.ts`:
- Around line 614-652: Both updateAppEnvHappyPath and listAppEnvNamesHappyPath
return the same mock response; extract the shared object into a single factory
(e.g., createLegacyEnvResponse) and have updateAppEnvHappyPath and
listAppEnvNamesHappyPath call that factory to remove duplication. Update
references in tests to use the new factory where needed and ensure the returned
shape (projectId, app, deployment, variables) remains identical so existing
assertions keep working.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3d2fe338-4e57-4fc1-b3f0-8e3279a750e5
📒 Files selected for processing (1)
packages/cli/tests/app-env.test.ts
Pulls a single legacyEnvProviderResponse() factory out of the two
happy-path mock builders that were returning identical shapes, and
adds vi.doUnmock("../src/lib/app/preview-provider") to afterEach
so a stale registration can't bleed across sibling test files.
Both addresses CodeRabbit nitpicks on PR #4.
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
CodeRabbit chat interactions are restricted to organization members for this repository. Ask an organization member to interact with CodeRabbit, or set |
1 similar comment
|
CodeRabbit chat interactions are restricted to organization members for this repository. Ask an organization member to interact with CodeRabbit, or set |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/cli/tests/app-env.test.ts`:
- Around line 463-558: Add a test that mirrors the existing "set --branch
unavailable" case to assert that runAppEnvUnset rejects when passed branchName
(branch-override writes are unavailable): create a test using createMockClient +
loadControllers + writePrismaConfig + createTestCommandContext, call
controllers.runAppEnvUnset(context, "STRIPE_KEY", { branchName: "feature-auth"
}) and expect it to reject with a feature-unavailable style error (e.g.,
toMatchObject({ code: "FEATURE_UNAVAILABLE" }) or summary containing
"branch-override" or "branch"), and assert client.DELETE was not called;
reference controllers.runAppEnvUnset, createMockClient, loadControllers, and
client.DELETE so the test sits alongside the other unset validation specs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6862c154-37f8-42b4-8858-54d3c2daea76
📒 Files selected for processing (1)
packages/cli/tests/app-env.test.ts
Comment on lines
+463
to
+558
| describe("app env unset", () => { | ||
| it("looks up the row and DELETEs it on the happy path", async () => { | ||
| const client = createMockClient(); | ||
| client.GET.mockResolvedValueOnce({ | ||
| data: { | ||
| data: [makeVariableRow({ id: "envvar_target", key: "STRIPE_KEY" })], | ||
| pagination: { hasMore: false, nextCursor: null }, | ||
| }, | ||
| response: { status: 200 }, | ||
| }); | ||
| client.DELETE.mockResolvedValueOnce({ | ||
| data: undefined, | ||
| response: { status: 204 }, | ||
| }); | ||
|
|
||
| const { controllers, createTempCwd, createTestCommandContext } = | ||
| await loadControllers(client, "proj_123"); | ||
| const cwd = await createTempCwd(); | ||
| await writePrismaConfig(cwd, "proj_123"); | ||
| const { context } = await createTestCommandContext({ cwd }); | ||
|
|
||
| const result = await controllers.runAppEnvUnset(context, "STRIPE_KEY", { | ||
| className: "production", | ||
| }); | ||
|
|
||
| expect(client.DELETE).toHaveBeenCalledWith( | ||
| "/v1/environment-variables/{envVarId}", | ||
| expect.objectContaining({ | ||
| params: { path: { envVarId: "envvar_target" } }, | ||
| }), | ||
| ); | ||
| expect(result.result).toEqual({ | ||
| projectId: "proj_123", | ||
| scope: { kind: "class", class: "production" }, | ||
| key: "STRIPE_KEY", | ||
| }); | ||
| }); | ||
|
|
||
| it("returns a focused not-found error when the row does not exist", async () => { | ||
| const client = createMockClient(); | ||
| client.GET.mockResolvedValueOnce({ | ||
| data: { data: [], pagination: { hasMore: false, nextCursor: null } }, | ||
| response: { status: 200 }, | ||
| }); | ||
|
|
||
| const { controllers, createTempCwd, createTestCommandContext } = | ||
| await loadControllers(client, "proj_123"); | ||
| const cwd = await createTempCwd(); | ||
| await writePrismaConfig(cwd, "proj_123"); | ||
| const { context } = await createTestCommandContext({ cwd }); | ||
|
|
||
| await expect( | ||
| controllers.runAppEnvUnset(context, "STRIPE_KEY", { | ||
| className: "production", | ||
| }), | ||
| ).rejects.toMatchObject({ | ||
| code: "ENV_VARIABLE_NOT_FOUND", | ||
| }); | ||
| expect(client.DELETE).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it("rejects neither --class nor --branch (fail-fast on writes)", async () => { | ||
| const client = createMockClient(); | ||
| const { controllers, createTempCwd, createTestCommandContext } = | ||
| await loadControllers(client, "proj_123"); | ||
| const cwd = await createTempCwd(); | ||
| await writePrismaConfig(cwd, "proj_123"); | ||
| const { context } = await createTestCommandContext({ cwd }); | ||
|
|
||
| await expect( | ||
| controllers.runAppEnvUnset(context, "STRIPE_KEY", {}), | ||
| ).rejects.toMatchObject({ | ||
| summary: expect.stringContaining("requires --class or --branch"), | ||
| }); | ||
| expect(client.DELETE).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it("rejects --class and --branch supplied together", async () => { | ||
| const client = createMockClient(); | ||
| const { controllers, createTempCwd, createTestCommandContext } = | ||
| await loadControllers(client, "proj_123"); | ||
| const cwd = await createTempCwd(); | ||
| await writePrismaConfig(cwd, "proj_123"); | ||
| const { context } = await createTestCommandContext({ cwd }); | ||
|
|
||
| await expect( | ||
| controllers.runAppEnvUnset(context, "STRIPE_KEY", { | ||
| className: "production", | ||
| branchName: "feature-auth", | ||
| }), | ||
| ).rejects.toMatchObject({ | ||
| summary: expect.stringContaining("mutually exclusive"), | ||
| }); | ||
| expect(client.DELETE).not.toHaveBeenCalled(); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add explicit unset --branch feature-unavailable test.
Line 463 onward covers unset validation, but it doesn’t pin the documented “branch-override writes unavailable” behavior for runAppEnvUnset. Add one test mirroring the set --branch unavailable case to prevent regressions.
Suggested test addition
describe("app env unset", () => {
+ it("returns a feature-unavailable error for branch-override writes", async () => {
+ const client = createMockClient();
+ const { controllers, createTempCwd, createTestCommandContext } =
+ await loadControllers(client, "proj_123");
+ const cwd = await createTempCwd();
+ await writePrismaConfig(cwd, "proj_123");
+ const { context } = await createTestCommandContext({ cwd });
+
+ await expect(
+ controllers.runAppEnvUnset(context, "STRIPE_KEY", {
+ branchName: "feature-auth",
+ }),
+ ).rejects.toMatchObject({
+ summary: expect.stringContaining("Branch-override writes are not available yet"),
+ });
+ expect(client.DELETE).not.toHaveBeenCalled();
+ });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/cli/tests/app-env.test.ts` around lines 463 - 558, Add a test that
mirrors the existing "set --branch unavailable" case to assert that
runAppEnvUnset rejects when passed branchName (branch-override writes are
unavailable): create a test using createMockClient + loadControllers +
writePrismaConfig + createTestCommandContext, call
controllers.runAppEnvUnset(context, "STRIPE_KEY", { branchName: "feature-auth"
}) and expect it to reject with a feature-unavailable style error (e.g.,
toMatchObject({ code: "FEATURE_UNAVAILABLE" }) or summary containing
"branch-override" or "branch"), and assert client.DELETE was not called;
reference controllers.runAppEnvUnset, createMockClient, loadControllers, and
client.DELETE so the test sits alongside the other unset validation specs.
|
CodeRabbit chat interactions are restricted to organization members for this repository. Ask an organization member to interact with CodeRabbit, or set |
Documents the asymmetry between set and unset on branch scopes:
`unset --branch` works against an existing override row because
DELETE /v1/environment-variables/{envVarId} addresses the row by id
and doesn't depend on the POST/PATCH body extension that gates the
branch-override write path. The new test guards against a
sympathetic-but-wrong refactor that would route unset --branch
through the same feature-unavailable gate as set --branch.
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
CodeRabbit chat interactions are restricted to organization members for this repository. Ask an organization member to interact with CodeRabbit, or set |
1 similar comment
|
CodeRabbit chat interactions are restricted to organization members for this repository. Ask an organization member to interact with CodeRabbit, or set |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/cli/tests/app-env.test.ts`:
- Around line 165-215: These tests assert only specific client methods (e.g.,
expect(client.POST).not.toHaveBeenCalled()) which can miss leaked calls to other
HTTP methods; update each negative-path test that uses createMockClient() and
controllers.runAppEnvSet (and the similar tests later) to assert that all client
methods on the mock (at minimum client.GET, client.POST, client.PATCH,
client.DELETE) were not called so the tests fail if any stray API call is made.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 02e7df2c-b36f-4c6c-9abe-9263c93b8ecc
📒 Files selected for processing (1)
packages/cli/tests/app-env.test.ts
Comment on lines
+165
to
+215
| it("rejects --class and --branch supplied together", async () => { | ||
| const client = createMockClient(); | ||
| const { controllers, createTempCwd, createTestCommandContext } = | ||
| await loadControllers(client, "proj_123"); | ||
| const cwd = await createTempCwd(); | ||
| await writePrismaConfig(cwd, "proj_123"); | ||
| const { context } = await createTestCommandContext({ cwd }); | ||
|
|
||
| await expect( | ||
| controllers.runAppEnvSet(context, "STRIPE_KEY=sk", { | ||
| className: "production", | ||
| branchName: "feature-auth", | ||
| }), | ||
| ).rejects.toMatchObject({ | ||
| summary: expect.stringContaining("mutually exclusive"), | ||
| }); | ||
| expect(client.POST).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it("rejects neither --class nor --branch (fail-fast on writes)", async () => { | ||
| const client = createMockClient(); | ||
| const { controllers, createTempCwd, createTestCommandContext } = | ||
| await loadControllers(client, "proj_123"); | ||
| const cwd = await createTempCwd(); | ||
| await writePrismaConfig(cwd, "proj_123"); | ||
| const { context } = await createTestCommandContext({ cwd }); | ||
|
|
||
| await expect( | ||
| controllers.runAppEnvSet(context, "STRIPE_KEY=sk", {}), | ||
| ).rejects.toMatchObject({ | ||
| summary: expect.stringContaining("requires --class or --branch"), | ||
| }); | ||
| expect(client.POST).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it("rejects malformed KEY=VALUE", async () => { | ||
| const client = createMockClient(); | ||
| const { controllers, createTempCwd, createTestCommandContext } = | ||
| await loadControllers(client, "proj_123"); | ||
| const cwd = await createTempCwd(); | ||
| await writePrismaConfig(cwd, "proj_123"); | ||
| const { context } = await createTestCommandContext({ cwd }); | ||
|
|
||
| await expect( | ||
| controllers.runAppEnvSet(context, "noequalshere", { | ||
| className: "production", | ||
| }), | ||
| ).rejects.toMatchObject({ | ||
| summary: expect.stringContaining("missing the = separator"), | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Strengthen fail-fast tests by asserting zero API calls across all methods.
These negative-path tests currently assert only a subset of client methods (POST or DELETE), so regressions could still leak GET/PATCH calls without failing the suite.
Proposed test-hardening diff
+function expectNoApiCalls(client: MockClient) {
+ expect(client.GET).not.toHaveBeenCalled();
+ expect(client.POST).not.toHaveBeenCalled();
+ expect(client.PATCH).not.toHaveBeenCalled();
+ expect(client.DELETE).not.toHaveBeenCalled();
+}
+
describe("app env set", () => {
it("rejects --class and --branch supplied together", async () => {
@@
- expect(client.POST).not.toHaveBeenCalled();
+ expectNoApiCalls(client);
});
it("rejects neither --class nor --branch (fail-fast on writes)", async () => {
@@
- expect(client.POST).not.toHaveBeenCalled();
+ expectNoApiCalls(client);
});
it("rejects malformed KEY=VALUE", async () => {
@@
await expect(
controllers.runAppEnvSet(context, "noequalshere", {
className: "production",
}),
).rejects.toMatchObject({
summary: expect.stringContaining("missing the = separator"),
});
+ expectNoApiCalls(client);
});
it("rejects keys that don't match POSIX env-var shape", async () => {
@@
await expect(
controllers.runAppEnvSet(context, "lowercase-key=value", {
className: "production",
}),
).rejects.toMatchObject({
summary: expect.stringContaining("POSIX env-var shape"),
});
+ expectNoApiCalls(client);
});
});
@@
describe("app env unset", () => {
it("rejects neither --class nor --branch (fail-fast on writes)", async () => {
@@
- expect(client.DELETE).not.toHaveBeenCalled();
+ expectNoApiCalls(client);
});
it("rejects --class and --branch supplied together", async () => {
@@
- expect(client.DELETE).not.toHaveBeenCalled();
+ expectNoApiCalls(client);
});
});Also applies to: 524-557
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/cli/tests/app-env.test.ts` around lines 165 - 215, These tests
assert only specific client methods (e.g.,
expect(client.POST).not.toHaveBeenCalled()) which can miss leaked calls to other
HTTP methods; update each negative-path test that uses createMockClient() and
controllers.runAppEnvSet (and the similar tests later) to assert that all client
methods on the mock (at minimum client.GET, client.POST, client.PATCH,
client.DELETE) were not called so the tests fail if any stray API call is made.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Linear: PTL-1475 (Compute Public Beta launch — M5: Configure)
Summary
Adds a
prisma app envnamespace so CLI users can author environment variables against the new platform-managed/v1/environment-variablesAPI. Without this, the CLI has no self-serve write path against the platform-authoritative env-var store;app update-env/app list-envoperate on the legacy Foundry-version env-var path, which is no longer the source of truth after the deploy path was made platform-authoritative in pdp-control-plane#3849.New verbs:
prisma app env set KEY=VALUE— create or replace a variable on a project template (--class production/--class preview) or a branch override (--branch <name>). Idempotent upsert: existing rows are replaced viaPATCH, new rows are created viaPOST.prisma app env list— list metadata for a scope (no values, FR15). Defaults to--class productionwhen no scope flag is provided.prisma app env unset KEY— looks the row up by natural key andDELETEs it.Scope flag rules (FR21)
--class <production|preview>targets a project template.--branch <name>targets a per-branch override (always against thepreviewclass — the platform forbids overrides onproduction). Resolves the name to a Branch ID viaGET /v1/projects/{projectId}/branches?gitName=.--classand--branchare mutually exclusive.set/unsetrequire an explicit scope so the CLI never silently writes to production.listwith neither flag defaults to--class production.Surface policy
pullverb in Beta — under FR15 (never-reveal) the platform never returns stored plaintext, sopullwould always write a zero-value file. Decided in spec § Decisions Log Q17.listoutput is metadata-only (key,id,last updated). The plaintext value never appears in any response body, log, or rendered field anywhere in the namespace.Legacy commands
app update-envandapp list-envcontinue to work for backward compatibility but emit a deprecation banner to stderr pointing at the new commands. The banner is suppressed under--jsonso machine consumers keep their JSON channel clean. Removal is deferred — see the open question below.Branch-override writes (current scope)
The Management API's
POST/PATCHrequest bodies on/v1/environment-variablesdon't yet accept abranchIdfield — branch-override writes are scheduled as a follow-up extension to the env-var route. Until that lands, this PR surfaces a focused "feature unavailable" error when the user runsapp env set/unsetwith--branch. List against--branchworks because theGETendpoint already supports the filter.Diff / import
diffandimportare deferred to follow-up tickets — the spec lists them as part of FR21/FR22 but they're large enough on their own (especiallyimport's file-parsing surface) that bundling them here would have made review unwieldy. Will track as separate PTL tickets.Decision pending
cc @luan @alexey — the legacy
app update-env/app list-envdeprecation in this PR is warning-only. Plan task 3B.1 calls for an explicit decision on whether to remove the legacy commands at the end of Beta or carry them indefinitely. This PR does not remove them; happy to follow up with a removal PR once we align.Coordination
Depends on
@prisma/management-api-sdk≥ 1.27.0 (already published; introduced by pdp-control-plane#3827 + #3831). The dep bump from^1.24.0to^1.27.0is included in this PR. The compute-sdk peer-dep range (>=1.23.0) accepts 1.27.0 unchanged.Test plan
pnpm test— 26 test files, 166 tests pass (151 baseline + 15 new intests/app-env.test.ts)pnpm --filter @prisma/cli build— builds cleanpnpm --filter @prisma/cli exec tsc --noEmit— no new type errors (one preexistingpublish-prep.test.tsdeclaration-file error unaffected by this change)pnpm prisma app env --help,set --help,list --help,unset --help; flag-rule errors render with the expected next-step examplesvenus-flagged workspace — pending preview-deployment availability of the API endpoints. Will verify on dev as soon as the SDK 1.27 bump finishes propagating.Acceptance criteria (AC10 / TC-13)
prisma app env set STRIPE_KEY=sk_test_... --class productionsucceeds; the variable appears inprisma app env list --class production(covered by integration tests against a mockedManagementApiClient).prisma app env set STRIPE_KEY=val --class production --branch feature/authis rejected with a clear mutually-exclusive-flags error.prisma app env set STRIPE_KEY=val(neither flag) is rejected with a clear "requires --class or --branch" error.prisma app env unset STRIPE_KEY --branch feature-authremoves the override (when the API supports it; covered by happy-path test against--class, blocked-path test against--branchfor now).listoutput is metadata-only (assertion: result JSON contains novaluefield).--json.