fix(env): emit JSON results and route status text to stderr

deploy/env.ts

@@ -16,7 +16,7 @@ interface EnvVar {
   key: string;
   value: string;
   is_secret: boolean;
-  context_ids: string[];
+  context_ids: string[] | null;
 }
 
 interface Context {
@@ -29,6 +29,30 @@ 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. `id`
+ * is allowed to be null for the `add` best-effort case where the backend
+ * returned no server-assigned id.
+ */
+function shapeEnvVar(
+  envVar: Omit<EnvVar, "id"> & { id: string | null },
+  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,
+  };
+}
+
 const envListCommand = new Command<EnvCommandContext>()
   .description("List all environment variables in an application")
   .action(actionHandler(async (config, options) => {
@@ -48,22 +72,12 @@ const envListCommand = new Command<EnvCommandContext>()
     ) 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;
     }
 
     if (envVars.length === 0) {
-      console.log(
+      console.error(
         "There are no environment variables set on this application.",
       );
       return;
@@ -115,7 +129,7 @@ const envAddCommand = new Command<EnvCommandContext>()
       app,
     }) as { id: string };
 
-    await trpcClient.mutation("envVarsContexts.updateEnvVars", {
+    const created = await trpcClient.mutation("envVarsContexts.updateEnvVars", {
       org,
       add: [
         {
@@ -128,9 +142,24 @@ const envAddCommand = new Command<EnvCommandContext>()
       ],
       update: [],
       remove: [],
-    });
+    }) as string[];
 
-    console.log(
+    if (options.json) {
+      // 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;
+    }
+
+    console.error(
       `${
         green("✔")
       } Environment variable '${variable}' has been successfully set.`,
@@ -162,6 +191,13 @@ const envUpdateValueCommand = new Command<EnvCommandContext>()
       );
     }
 
+    const contexts = options.json
+      ? await trpcClient.query(
+        "envVarsContexts.listContexts",
+        { org },
+      ) as Context[]
+      : [];
+
     await trpcClient.mutation("envVarsContexts.updateEnvVars", {
       org,
       add: [],
@@ -172,7 +208,14 @@ const envUpdateValueCommand = new Command<EnvCommandContext>()
       remove: [],
     });
 
-    console.log(
+    if (options.json) {
+      // 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;
+    }
+
+    console.error(
       `${
         green("✔")
       } The value of environment variable '${variable}' has been successfully updated.`,
@@ -220,17 +263,28 @@ 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: [],
     });
 
-    console.log(
+    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(
+        shapeEnvVar({ ...envVar, context_ids: newContextIds }, contexts),
+      );
+      return;
+    }
+
+    console.error(
       `${
         green("✔")
       } The contexts of environment variable '${variable}' have been successfully updated.`,
@@ -266,7 +320,12 @@ const envDeleteCommand = new Command<EnvCommandContext>()
       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 +416,8 @@ const envLoadCommand = new Command<EnvCommandContext>()
       }
     }
 
+    const existingKeys = updateEnvVars.map((envVar) => envVar.key);
+
     if (updateEnvVars.length > 0) {
       if (options.skipExisting) {
         updateEnvVars = [];
@@ -368,11 +429,11 @@ const envLoadCommand = new Command<EnvCommandContext>()
           "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 +455,7 @@ const envLoadCommand = new Command<EnvCommandContext>()
             }
           }
         }
-        console.log();
+        console.error();
       }
     }
 
@@ -405,7 +466,20 @@ const envLoadCommand = new Command<EnvCommandContext>()
       remove: [],
     });
 
-    console.log(
+    // `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));
+
+    if (options.json) {
+      writeJsonResult({ file, added, updated, skipped });
+      return;
+    }
+
+    console.error(
       `${green("✔")} .env file '${file}' has been successfully loaded.`,
     );
   }));

tests/env.test.ts

@@ -0,0 +1,98 @@
+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/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`
+    // 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);
+      // The id is derived from the mutation response, not a read-back.
+      assertEquals(typeof parsed.id, "string");
+
+      // 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}`);
+      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(() => {});
+    }
+  },
+});