From d443e1ea42a1388be48368866e04174feb13e6ab Mon Sep 17 00:00:00 2001 From: tcerqueira Date: Mon, 29 Jun 2026 13:38:01 +0100 Subject: [PATCH 1/4] fix(env): emit JSON results and route status text to stderr The mutating `env` subcommands (add, update-value, update-contexts, delete, load) printed their human success confirmation to stdout and never emitted a machine-readable result, so `--json` (and any piped) output was polluted with non-JSON text. Route all confirmation/status text to stderr and, under `--json`, emit a single JSON object per command on stdout: add/update-* report the affected variable shaped exactly like an `env list` item (id, key, masked value, isSecret, contexts), delete reports `{ id, key, deleted }`, and load reports an `{ file, added, updated, skipped }` summary. The shared `shapeEnvVar` helper is reused by `list` so all JSON stays consistent. --- deploy/env.ts | 103 ++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 83 insertions(+), 20 deletions(-) diff --git a/deploy/env.ts b/deploy/env.ts index 08d1b81..107b9d5 100644 --- a/deploy/env.ts +++ b/deploy/env.ts @@ -29,6 +29,48 @@ type EnvCommandContext = GlobalContext & { app?: string; }; +/** + * Shape a backend env var into the public `--json` representation, masking the + * value of secrets and resolving context ids to their human names. Shared by + * `list` and the mutating commands so their JSON output stays identical. + */ +function shapeEnvVar(envVar: EnvVar, contexts: Context[]) { + return { + id: envVar.id, + key: envVar.key, + value: envVar.is_secret ? null : envVar.value, + isSecret: envVar.is_secret, + contexts: envVar.context_ids + ? envVar.context_ids.map((id) => + contexts.find((c) => c.id === id)?.name ?? id + ) + : null, + }; +} + +/** + * Re-read a single env var (and the org contexts) from the backend and shape it + * for `--json` output. Used by the mutating commands to report the persisted + * state of the variable they just changed. + */ +async function fetchShapedEnvVar( + trpcClient: ReturnType, + org: string, + app: string, + key: string, +) { + const envVars = await trpcClient.query("envVarsContexts.list", { + org, + app, + }) as EnvVar[]; + const contexts = await trpcClient.query( + "envVarsContexts.listContexts", + { org }, + ) as Context[]; + const envVar = envVars.find((envVar) => envVar.key === key); + return envVar ? shapeEnvVar(envVar, contexts) : null; +} + const envListCommand = new Command() .description("List all environment variables in an application") .action(actionHandler(async (config, options) => { @@ -48,17 +90,7 @@ const envListCommand = new Command() ) as Context[]; if (options.json) { - writeJsonResult(envVars.map((envVar) => ({ - id: envVar.id, - key: envVar.key, - value: envVar.is_secret ? null : envVar.value, - isSecret: envVar.is_secret, - contexts: envVar.context_ids - ? envVar.context_ids.map((id) => - contexts.find((c) => c.id === id)?.name ?? id - ) - : null, - }))); + writeJsonResult(envVars.map((envVar) => shapeEnvVar(envVar, contexts))); return; } @@ -130,7 +162,12 @@ const envAddCommand = new Command() remove: [], }); - console.log( + if (options.json) { + writeJsonResult(await fetchShapedEnvVar(trpcClient, org, app, variable)); + return; + } + + console.error( `${ green("✔") } Environment variable '${variable}' has been successfully set.`, @@ -172,7 +209,12 @@ const envUpdateValueCommand = new Command() remove: [], }); - console.log( + if (options.json) { + writeJsonResult(await fetchShapedEnvVar(trpcClient, org, app, variable)); + return; + } + + console.error( `${ green("✔") } The value of environment variable '${variable}' has been successfully updated.`, @@ -230,7 +272,12 @@ You can define no contexts, which is the equivalent to "All"`, remove: [], }); - console.log( + if (options.json) { + writeJsonResult(await fetchShapedEnvVar(trpcClient, org, app, variable)); + return; + } + + console.error( `${ green("✔") } The contexts of environment variable '${variable}' have been successfully updated.`, @@ -266,7 +313,12 @@ const envDeleteCommand = new Command() remove: [envVar.id], }); - console.log( + if (options.json) { + writeJsonResult({ id: envVar.id, key: variable, deleted: true }); + return; + } + + console.error( `${ green("✔") } Environment variable '${variable}' has been successfully deleted.`, @@ -357,6 +409,8 @@ const envLoadCommand = new Command() } } + const existingKeys = updateEnvVars.map((envVar) => envVar.key); + if (updateEnvVars.length > 0) { if (options.skipExisting) { updateEnvVars = []; @@ -368,11 +422,11 @@ const envLoadCommand = new Command() "Existing env vars found and prompting is disabled.\nUse --replace to overwrite or --skip-existing to skip.", ); } else { - console.log("The following env vars are already defined:"); + console.error("The following env vars are already defined:"); for (const updateEnvVar of updateEnvVars) { - console.log(` - ${updateEnvVar.key}`); + console.error(` - ${updateEnvVar.key}`); } - console.log(); + console.error(); outer: while (true) { const res = prompt( "Would you like to replace these with your .env file? [y = Yes, n = No, s = Ignore/Skip]", @@ -394,7 +448,7 @@ const envLoadCommand = new Command() } } } - console.log(); + console.error(); } } @@ -405,7 +459,16 @@ const envLoadCommand = new Command() remove: [], }); - console.log( + const added = addEnvVars.map((envVar) => envVar.key); + const updated = updateEnvVars.map((envVar) => envVar.key); + const skipped = existingKeys.filter((key) => !updated.includes(key)); + + if (options.json) { + writeJsonResult({ file, added, updated, skipped }); + return; + } + + console.error( `${green("✔")} .env file '${file}' has been successfully loaded.`, ); })); From 1558e873f7dda22ab95802a053d148c1119a5268 Mon Sep 17 00:00:00 2001 From: tcerqueira Date: Mon, 29 Jun 2026 13:38:09 +0100 Subject: [PATCH 2/4] test(env): cover --json stdout discipline for mutating commands Add an end-to-end test exercising the env add/update-value/delete cycle with `--json --non-interactive`, asserting stdout carries exactly one JSON object per command. Gated on DENO_DEPLOY_TEST_ORG/APP and run from an isolated temp cwd so it never touches the repo's deno.json; skipped when throwaway creds are absent. --- tests/env.test.ts | 81 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 81 insertions(+) create mode 100644 tests/env.test.ts diff --git a/tests/env.test.ts b/tests/env.test.ts new file mode 100644 index 0000000..a49d242 --- /dev/null +++ b/tests/env.test.ts @@ -0,0 +1,81 @@ +import { $ } from "dax"; +import { assertEquals } from "@std/assert"; + +const TOKEN = Deno.env.get("DENO_DEPLOY_TOKEN"); +const ORG = Deno.env.get("DENO_DEPLOY_TEST_ORG"); +const APP = Deno.env.get("DENO_DEPLOY_TEST_APP"); + +// The mutating `env` commands persist real backend state, so these run only +// when a throwaway org/app is supplied via DENO_DEPLOY_TEST_ORG / +// DENO_DEPLOY_TEST_APP (alongside DENO_DEPLOY_TOKEN and DENO_DEPLOY_CLI_SPECIFIER). +const live = Boolean(TOKEN && ORG && APP); + +async function env( + cwd: string, + ...args: string[] +): Promise<{ code: number; stdout: string; stderr: string }> { + const escaped = args.map((a) => $.escapeArg(a)).join(" "); + const result = await $.raw`deno deploy env ${escaped}` + .cwd(cwd) + .noThrow() + .stdout("piped") + .stderr("piped"); + return { code: result.code, stdout: result.stdout, stderr: result.stderr }; +} + +Deno.test({ + name: + "env add/update-value/delete --json emit exactly one JSON object on stdout", + ignore: !live, + fn: async () => { + // Run from a throwaway cwd: resolving --org/--app persists a `deploy` + // object to the working deno.json, which we discard with the temp dir. + const cwd = await Deno.makeTempDir({ prefix: "deno-deploy-env-test-" }); + await Deno.writeTextFile(`${cwd}/deno.json`, "{}\n"); + const key = `AGENT_TEST_${ + crypto.randomUUID().replaceAll("-", "").slice(0, 12).toUpperCase() + }`; + const target = [ + "--org", + ORG!, + "--app", + APP!, + "--json", + "--non-interactive", + ]; + + try { + // add: stdout must be a single JSON object shaped like an `env list` item. + let res = await env(cwd, "add", key, "v1", ...target); + assertEquals(res.code, 0, `add failed; stderr: ${res.stderr}`); + assertEquals( + res.stdout.trim().split("\n").length, + 1, + `add stdout not a single line: ${JSON.stringify(res.stdout)}`, + ); + let parsed = JSON.parse(res.stdout.trim()); + assertEquals(parsed.key, key); + assertEquals(parsed.value, "v1"); + assertEquals(parsed.isSecret, false); + assertEquals(parsed.contexts, null); + + // update-value: the result reflects the new value on clean stdout. + res = await env(cwd, "update-value", key, "v2", ...target); + assertEquals(res.code, 0, `update-value failed; stderr: ${res.stderr}`); + parsed = JSON.parse(res.stdout.trim()); + assertEquals(parsed.key, key); + assertEquals(parsed.value, "v2"); + + // delete: result is an operation summary; the var is gone afterwards. + res = await env(cwd, "delete", key, ...target); + assertEquals(res.code, 0, `delete failed; stderr: ${res.stderr}`); + parsed = JSON.parse(res.stdout.trim()); + assertEquals(parsed.key, key); + assertEquals(parsed.deleted, true); + } finally { + // Best-effort cleanup in case an assertion aborted mid-cycle. + await env(cwd, "delete", key, ...target).catch(() => {}); + await Deno.remove(cwd, { recursive: true }).catch(() => {}); + } + }, +}); From e4f049ceef65e7b1f42ad125ef9d9685ebb299bc Mon Sep 17 00:00:00 2001 From: tcerqueira Date: Mon, 29 Jun 2026 15:44:15 +0100 Subject: [PATCH 3/4] fix(env): return NOT_FOUND envelope instead of null on read-after-write miss fetchShapedEnvVar returned null when the variable could not be read back after a successful add/update-value/update-contexts mutation (e.g. a backend read-after-write miss or key-normalization difference), and the call sites passed that straight to writeJsonResult, printing a bare `null` to stdout and breaking the agent/JSON contract. Fold the miss handling into fetchShapedEnvVar: it now exits via error() with ExitCode.NOT_FOUND / errorCode ENV_VAR_NOT_FOUND (message naming the key) so the command always yields either the documented variable object on stdout or a structured {error:{...}} envelope on stderr. The normal found path is unchanged. --- deploy/env.ts | 31 ++++++++++++++++++++++++++----- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/deploy/env.ts b/deploy/env.ts index 107b9d5..ec647b3 100644 --- a/deploy/env.ts +++ b/deploy/env.ts @@ -2,6 +2,7 @@ import { Command } from "@cliffy/command"; import { parse as dotEnvParse } from "@std/dotenv"; import { error, + ExitCode, isNonInteractive, tablePrinter, writeJsonResult, @@ -51,9 +52,13 @@ function shapeEnvVar(envVar: EnvVar, contexts: Context[]) { /** * Re-read a single env var (and the org contexts) from the backend and shape it * for `--json` output. Used by the mutating commands to report the persisted - * state of the variable they just changed. + * state of the variable they just changed. If the variable can't be read back + * (read-after-write miss, key-normalization difference), this exits via + * {@link error} with a structured `NOT_FOUND` envelope rather than returning a + * value — so callers always emit the var object, never a bare `null`. */ async function fetchShapedEnvVar( + context: GlobalContext, trpcClient: ReturnType, org: string, app: string, @@ -68,7 +73,17 @@ async function fetchShapedEnvVar( { org }, ) as Context[]; const envVar = envVars.find((envVar) => envVar.key === key); - return envVar ? shapeEnvVar(envVar, contexts) : null; + if (!envVar) { + error( + context, + `Environment variable '${key}' could not be read back from the backend after the operation.`, + { + code: ExitCode.NOT_FOUND, + errorCode: "ENV_VAR_NOT_FOUND", + }, + ); + } + return shapeEnvVar(envVar, contexts); } const envListCommand = new Command() @@ -163,7 +178,9 @@ const envAddCommand = new Command() }); if (options.json) { - writeJsonResult(await fetchShapedEnvVar(trpcClient, org, app, variable)); + writeJsonResult( + await fetchShapedEnvVar(options, trpcClient, org, app, variable), + ); return; } @@ -210,7 +227,9 @@ const envUpdateValueCommand = new Command() }); if (options.json) { - writeJsonResult(await fetchShapedEnvVar(trpcClient, org, app, variable)); + writeJsonResult( + await fetchShapedEnvVar(options, trpcClient, org, app, variable), + ); return; } @@ -273,7 +292,9 @@ You can define no contexts, which is the equivalent to "All"`, }); if (options.json) { - writeJsonResult(await fetchShapedEnvVar(trpcClient, org, app, variable)); + writeJsonResult( + await fetchShapedEnvVar(options, trpcClient, org, app, variable), + ); return; } From e9f768d7b49c6b572ff25b8ad54ae40bac83fc67 Mon Sep 17 00:00:00 2001 From: tcerqueira Date: Tue, 30 Jun 2026 12:43:19 +0100 Subject: [PATCH 4/4] fix(env): derive mutation result from in-hand data instead of failing on read-back miss MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous NOT_FOUND hardening (e4f049c) made a SUCCESSFUL add/update in --json mode exit 4 on a backend read-after-write miss, turning a successful write into a hard failure for the CI consumers this PR targets. Replace the post-mutation re-query with results derived from data already in hand: - add: use the server-assigned id returned by the updateEnvVars mutation plus the input we sent (context_ids is always null on add); id is best-effort (null if the backend returns nothing) — a successful create is never reported as a failure. - update-value / update-contexts: construct the result locally from the record fetched before mutating, with the new value / context_ids applied. This removes the redundant post-mutation round-trips and the read-after-write race entirely. update-value now fetches the contexts list (only in --json mode) so context names resolve consistently with `env list`. - Remove the now-unused fetchShapedEnvVar helper and its NOT_FOUND path (supersedes e4f049c); loosen shapeEnvVar's id to string | null and EnvVar.context_ids to string[] | null. - Route the `list` empty-state status line to stderr (status -> stderr). env load keeps its request-derived added/updated/skipped summary: the batch mutation response only returns ids for newly added records (no keys, no per-update/skipped detail), so the request payload is the best available source of truth. Extend tests/env.test.ts to exercise add + update-value + update-contexts and assert each emits one JSON var object on stdout with exit 0. --- deploy/env.ts | 94 +++++++++++++++++++++-------------------------- tests/env.test.ts | 21 ++++++++++- 2 files changed, 61 insertions(+), 54 deletions(-) diff --git a/deploy/env.ts b/deploy/env.ts index ec647b3..42d311d 100644 --- a/deploy/env.ts +++ b/deploy/env.ts @@ -2,7 +2,6 @@ import { Command } from "@cliffy/command"; import { parse as dotEnvParse } from "@std/dotenv"; import { error, - ExitCode, isNonInteractive, tablePrinter, writeJsonResult, @@ -17,7 +16,7 @@ interface EnvVar { key: string; value: string; is_secret: boolean; - context_ids: string[]; + context_ids: string[] | null; } interface Context { @@ -33,9 +32,14 @@ type EnvCommandContext = GlobalContext & { /** * Shape a backend env var into the public `--json` representation, masking the * value of secrets and resolving context ids to their human names. Shared by - * `list` and the mutating commands so their JSON output stays identical. + * `list` and the mutating commands so their JSON output stays identical. `id` + * is allowed to be null for the `add` best-effort case where the backend + * returned no server-assigned id. */ -function shapeEnvVar(envVar: EnvVar, contexts: Context[]) { +function shapeEnvVar( + envVar: Omit & { id: string | null }, + contexts: Context[], +) { return { id: envVar.id, key: envVar.key, @@ -49,43 +53,6 @@ function shapeEnvVar(envVar: EnvVar, contexts: Context[]) { }; } -/** - * Re-read a single env var (and the org contexts) from the backend and shape it - * for `--json` output. Used by the mutating commands to report the persisted - * state of the variable they just changed. If the variable can't be read back - * (read-after-write miss, key-normalization difference), this exits via - * {@link error} with a structured `NOT_FOUND` envelope rather than returning a - * value — so callers always emit the var object, never a bare `null`. - */ -async function fetchShapedEnvVar( - context: GlobalContext, - trpcClient: ReturnType, - org: string, - app: string, - key: string, -) { - const envVars = await trpcClient.query("envVarsContexts.list", { - org, - app, - }) as EnvVar[]; - const contexts = await trpcClient.query( - "envVarsContexts.listContexts", - { org }, - ) as Context[]; - const envVar = envVars.find((envVar) => envVar.key === key); - if (!envVar) { - error( - context, - `Environment variable '${key}' could not be read back from the backend after the operation.`, - { - code: ExitCode.NOT_FOUND, - errorCode: "ENV_VAR_NOT_FOUND", - }, - ); - } - return shapeEnvVar(envVar, contexts); -} - const envListCommand = new Command() .description("List all environment variables in an application") .action(actionHandler(async (config, options) => { @@ -110,7 +77,7 @@ const envListCommand = new Command() } if (envVars.length === 0) { - console.log( + console.error( "There are no environment variables set on this application.", ); return; @@ -162,7 +129,7 @@ const envAddCommand = new Command() app, }) as { id: string }; - await trpcClient.mutation("envVarsContexts.updateEnvVars", { + const created = await trpcClient.mutation("envVarsContexts.updateEnvVars", { org, add: [ { @@ -175,12 +142,20 @@ const envAddCommand = new Command() ], update: [], remove: [], - }); + }) as string[]; if (options.json) { - writeJsonResult( - await fetchShapedEnvVar(options, trpcClient, org, app, variable), - ); + // The mutation returns the server-assigned id(s) of the added record(s); + // everything else is the input we just sent, so the result is derived + // without a read-back. `id` is best-effort (null if the backend returns + // nothing) — a successful create is never reported as a failure. + writeJsonResult(shapeEnvVar({ + id: created?.[0] ?? null, + key: variable, + value, + is_secret: options.secret, + context_ids: null, + }, [])); return; } @@ -216,6 +191,13 @@ const envUpdateValueCommand = new Command() ); } + const contexts = options.json + ? await trpcClient.query( + "envVarsContexts.listContexts", + { org }, + ) as Context[] + : []; + await trpcClient.mutation("envVarsContexts.updateEnvVars", { org, add: [], @@ -227,9 +209,9 @@ const envUpdateValueCommand = new Command() }); if (options.json) { - writeJsonResult( - await fetchShapedEnvVar(options, trpcClient, org, app, variable), - ); + // Derive the result from the in-hand record with the new value applied — + // no read-back, so no read-after-write race. + writeJsonResult(shapeEnvVar({ ...envVar, value }, contexts)); return; } @@ -281,19 +263,23 @@ You can define no contexts, which is the equivalent to "All"`, contextIds.push(context.id); } + const newContextIds = newContexts.length === 0 ? null : contextIds; + await trpcClient.mutation("envVarsContexts.updateEnvVars", { org, add: [], update: [{ id: envVar.id, - context_ids: newContexts.length === 0 ? null : contextIds, + context_ids: newContextIds, }], remove: [], }); if (options.json) { + // Derive the result from the in-hand record and the contexts already + // fetched above, with the new context_ids applied — no read-back. writeJsonResult( - await fetchShapedEnvVar(options, trpcClient, org, app, variable), + shapeEnvVar({ ...envVar, context_ids: newContextIds }, contexts), ); return; } @@ -480,6 +466,10 @@ const envLoadCommand = new Command() remove: [], }); + // `added`/`updated`/`skipped` are derived from the request payload: the + // batch mutation response only returns server-assigned ids for newly added + // records (no keys, no per-update/skipped detail), so the request is the + // best available source of truth for the summary. const added = addEnvVars.map((envVar) => envVar.key); const updated = updateEnvVars.map((envVar) => envVar.key); const skipped = existingKeys.filter((key) => !updated.includes(key)); diff --git a/tests/env.test.ts b/tests/env.test.ts index a49d242..7dde4da 100644 --- a/tests/env.test.ts +++ b/tests/env.test.ts @@ -25,7 +25,7 @@ async function env( Deno.test({ name: - "env add/update-value/delete --json emit exactly one JSON object on stdout", + "env add/update-value/update-contexts/delete --json emit exactly one JSON object on stdout (exit 0)", ignore: !live, fn: async () => { // Run from a throwaway cwd: resolving --org/--app persists a `deploy` @@ -58,14 +58,31 @@ Deno.test({ assertEquals(parsed.value, "v1"); assertEquals(parsed.isSecret, false); assertEquals(parsed.contexts, null); + // The id is derived from the mutation response, not a read-back. + assertEquals(typeof parsed.id, "string"); - // update-value: the result reflects the new value on clean stdout. + // update-value: result is derived from in-hand data with the new value; + // a successful write must report the var object on stdout, never fail. res = await env(cwd, "update-value", key, "v2", ...target); assertEquals(res.code, 0, `update-value failed; stderr: ${res.stderr}`); + assertEquals(res.stdout.trim().split("\n").length, 1); parsed = JSON.parse(res.stdout.trim()); assertEquals(parsed.key, key); assertEquals(parsed.value, "v2"); + // update-contexts (no contexts = "All"): result reflects contexts: null, + // again derived from in-hand data with exit 0. + res = await env(cwd, "update-contexts", key, ...target); + assertEquals( + res.code, + 0, + `update-contexts failed; stderr: ${res.stderr}`, + ); + assertEquals(res.stdout.trim().split("\n").length, 1); + parsed = JSON.parse(res.stdout.trim()); + assertEquals(parsed.key, key); + assertEquals(parsed.contexts, null); + // delete: result is an operation summary; the var is gone afterwards. res = await env(cwd, "delete", key, ...target); assertEquals(res.code, 0, `delete failed; stderr: ${res.stderr}`);