From d443e1ea42a1388be48368866e04174feb13e6ab Mon Sep 17 00:00:00 2001 From: tcerqueira Date: Mon, 29 Jun 2026 13:38:01 +0100 Subject: [PATCH 1/3] 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/3] 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/3] 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; }