refactor(core): move dotenv helpers to core

.changeset/pull-skip-invalid-dotenv-keys.md

@@ -0,0 +1,5 @@
+---
+"@qawolf/cli": patch
+---
+
+Skip platform environment variables with keys that cannot be represented in local .env files during flows pull.

src/commands/flows/loadEnvFile.ts

@@ -2,7 +2,7 @@ import { readFile } from "node:fs/promises";
 import { join } from "node:path";
 
 import { isNoEntError } from "~/core/errors.js";
-import { parseDotenv } from "~/domains/flows/dotenv.js";
+import { parseDotenv } from "~/core/dotenv.js";
 
 export async function loadEnvFile(envDir: string): Promise<void> {
   let content: string;

src/domains/flows/dotenv.test.ts → src/core/dotenv.test.ts

@@ -1,6 +1,10 @@
 import { describe, expect, it } from "bun:test";
 
-import { parseDotenv, serializeDotenv } from "./dotenv.js";
+import {
+  parseDotenv,
+  serializeDotenv,
+  serializeDotenvSkippingInvalid,
+} from "./dotenv.js";
 
 describe("serializeDotenv", () => {
   it('emits KEY="value" lines sorted by key with trailing newline', () => {
@@ -41,6 +45,29 @@ describe("serializeDotenv", () => {
   });
 });
 
+describe("serializeDotenvSkippingInvalid", () => {
+  it("skips keys that violate POSIX env-var shape and reports them", () => {
+    expect(
+      serializeDotenvSkippingInvalid({
+        VALID: "ok",
+        "BAD KEY": "x",
+        "1LEADING_DIGIT": "y",
+        "DOTTED.KEY": "z",
+      }),
+    ).toEqual({
+      content: 'VALID="ok"\n',
+      skippedKeys: ["1LEADING_DIGIT", "BAD KEY", "DOTTED.KEY"],
+    });
+  });
+
+  it("returns empty content when every key is invalid", () => {
+    expect(serializeDotenvSkippingInvalid({ "BAD KEY": "x" })).toEqual({
+      content: "",
+      skippedKeys: ["BAD KEY"],
+    });
+  });
+});
+
 describe("parseDotenv", () => {
   it('parses simple KEY="value" lines', () => {
     expect(parseDotenv('TOKEN="abc"\nURL="https://example.com"\n')).toEqual({
@@ -61,7 +88,7 @@ describe("parseDotenv", () => {
     expect(parseDotenv('A="1"\r\n  B="2"  \r\n')).toEqual({ A: "1", B: "2" });
   });
 
-  it('unescapes \\\\, \\", \\n, \\r, \\t inside values', () => {
+  it('unescapes \\\\, ", \\n, \\r, \\t inside values', () => {
     expect(
       parseDotenv(
         'BACK="a\\\\b"\nCR="a\\rb"\nNL="a\\nb"\nQUOTE="a\\"b"\nTAB="a\\tb"\n',

src/domains/flows/dotenv.ts → src/core/dotenv.ts

@@ -7,17 +7,42 @@ import { flowsMessages } from "~/core/messages/index.js";
 const envKeyPattern = /^[A-Za-z_][A-Za-z0-9_]*$/;
 const lineRe = /^([A-Za-z_][A-Za-z0-9_]*)="((?:[^"\\]|\\.)*)"$/;
 
+export type SerializeDotenvSkippingInvalidResult = {
+  readonly content: string;
+  // Keys that are not valid POSIX shell identifiers (e.g. they contain a `.`
+  // or a space). A `.env` file cannot represent them, so pull callers can
+  // choose to skip them while surfacing the names to the user.
+  readonly skippedKeys: readonly string[];
+};
+
+function isDotenvKey(key: string): boolean {
+  return envKeyPattern.test(key);
+}
+
 export function serializeDotenv(vars: Record<string, string>): string {
   const keys = Object.keys(vars).sort();
   for (const key of keys) {
-    if (!envKeyPattern.test(key)) {
+    if (!isDotenvKey(key)) {
       throw new Error(flowsMessages.dotenv.invalidKey(key));
     }
   }
   if (keys.length === 0) return "";
   return `${keys.map((key) => `${key}=${quote(vars[key] ?? "")}`).join("\n")}\n`;
 }
 
+export function serializeDotenvSkippingInvalid(
+  vars: Record<string, string>,
+): SerializeDotenvSkippingInvalidResult {
+  const keys = Object.keys(vars).sort();
+  const validVars: Record<string, string> = {};
+  const skippedKeys: string[] = [];
+  for (const key of keys) {
+    if (isDotenvKey(key)) validVars[key] = vars[key] ?? "";
+    else skippedKeys.push(key);
+  }
+  return { content: serializeDotenv(validVars), skippedKeys };
+}
+
 function quote(value: string): string {
   const escaped = value
     .replaceAll("\\", "\\\\")

src/core/messages/flows.ts

@@ -82,6 +82,13 @@ export const flowsMessages = {
   dotenv: {
     invalidKey: (key: string) =>
       `Cannot serialize env var with invalid key: ${JSON.stringify(key)}`,
+    skippedKeys: (keys: readonly string[]) =>
+      `Skipped ${pluralize(
+        keys.length,
+        "environment variable",
+      )} with key(s) that are not valid POSIX shell identifiers (cannot be written to .env): ${keys
+        .map((key) => JSON.stringify(key))
+        .join(", ")}`,
     unparseableLine: (line: string) =>
       `Cannot parse .env line: ${JSON.stringify(line)}`,
   },

src/domains/flows/pull/envVars.test.ts

@@ -32,6 +32,26 @@ describe("writeEnvFile", () => {
 
     expect(await pathExists(join(workDir, ".env"))).toBe(false);
   });
+
+  it("writes valid vars, drops invalid keys, and reports them", async () => {
+    const { skippedKeys } = await writeEnvFile(workDir, {
+      TOKEN: "abc",
+      "BRIAN_2.0_AUTH_TOKEN": "secret",
+    });
+
+    expect(skippedKeys).toEqual(["BRIAN_2.0_AUTH_TOKEN"]);
+    const body = await readFile(join(workDir, ".env"), "utf8");
+    expect(body).toBe('TOKEN="abc"\n');
+  });
+
+  it("does not write an empty .env when every key is invalid", async () => {
+    const { skippedKeys } = await writeEnvFile(workDir, {
+      "BRIAN_2.0_AUTH_TOKEN": "secret",
+    });
+
+    expect(skippedKeys).toEqual(["BRIAN_2.0_AUTH_TOKEN"]);
+    expect(await pathExists(join(workDir, ".env"))).toBe(false);
+  });
 });
 
 describe("writeEnvFile (memory fs)", () => {

src/domains/flows/pull/envVars.ts

@@ -3,15 +3,19 @@ import { join } from "node:path";
 import { makeDefaultFs } from "~/shell/fs.js";
 import type { Fs } from "~/shell/fs.js";
 
-import { serializeDotenv } from "~/domains/flows/dotenv.js";
+import { serializeDotenvSkippingInvalid } from "~/core/dotenv.js";
 
 export async function writeEnvFile(
   envDir: string,
   vars: Record<string, string>,
   fs: Fs = makeDefaultFs(),
-): Promise<void> {
-  if (Object.keys(vars).length === 0) return;
-  await fs.writeFile(join(envDir, ".env"), serializeDotenv(vars), {
-    mode: 0o600,
-  });
+): Promise<{ skippedKeys: readonly string[] }> {
+  if (Object.keys(vars).length === 0) return { skippedKeys: [] };
+  const { content, skippedKeys } = serializeDotenvSkippingInvalid(vars);
+  // Only write when there is at least one valid var. An all-invalid set
+  // serializes to "" and we leave no empty .env behind.
+  if (content !== "") {
+    await fs.writeFile(join(envDir, ".env"), content, { mode: 0o600 });
+  }
+  return { skippedKeys };
 }

src/domains/flows/pull/handler.test.ts

@@ -114,6 +114,7 @@ describe("handleFlowsPull json mode output", () => {
       "flowCount",
       "flowsWithTeamStorageRefs",
       "manifestPath",
+      "skippedEnvVarKeys",
     ]);
     expect(payload).toEqual({
       assetsDir: expect.stringContaining("/assets"),
@@ -127,6 +128,7 @@ describe("handleFlowsPull json mode output", () => {
       ),
       flowCount: 2,
       envVarCount: 2,
+      skippedEnvVarKeys: [],
       flowsWithTeamStorageRefs: [],
       manifestPath: join(destDir, manifestFilename),
     });

src/domains/flows/pull/handler.ts

@@ -111,6 +111,10 @@ export async function handleFlowsPull(
         ),
     );
 
+    if (result.skippedEnvVarKeys.length > 0) {
+      ctx.ui.warn(flowsMessages.dotenv.skippedKeys(result.skippedEnvVarKeys));
+    }
+
     if (ctx.ui.mode === "json") {
       ctx.ui.output(
         {
@@ -120,6 +124,7 @@ export async function handleFlowsPull(
           fetchedAt: fetched.bundleFetchedAt.toISOString(),
           flowCount: result.flowCount,
           envVarCount: result.envVarCount,
+          skippedEnvVarKeys: result.skippedEnvVarKeys,
           flowsWithTeamStorageRefs: result.flowsWithTeamStorageRefs,
           assetDownloadedCount: assetResult.downloadedCount,
           assetReusedCount: assetResult.reusedCount,

src/domains/flows/pull/stage.test.ts

@@ -47,6 +47,7 @@ describe("stageBundle", () => {
       flowCount: 2,
       envVarCount: 1,
       flowsWithTeamStorageRefs: [],
+      skippedEnvVarKeys: [],
     });
     expect(await readFile(join(destDir, "checkout.flow.ts"), "utf8")).toBe(
       "// checkout\n",

src/domains/flows/pull/stage.ts

@@ -30,6 +30,7 @@ type StageBundleResult = {
   flowCount: number;
   envVarCount: number;
   flowsWithTeamStorageRefs: string[];
+  skippedEnvVarKeys: readonly string[];
 };
 
 export async function stageBundle(
@@ -61,7 +62,11 @@ export async function stageBundle(
       ...args.envVars,
       TEAM_STORAGE_DIR: args.assetsAbs,
     };
-    await writeEnvFile(tmpDir, effectiveEnvVars, fs);
+    const { skippedKeys: skippedEnvVarKeys } = await writeEnvFile(
+      tmpDir,
+      effectiveEnvVars,
+      fs,
+    );
     const manifest = await buildManifest(
       {
         envId: args.envId,
@@ -93,8 +98,11 @@ export async function stageBundle(
     return {
       envDir: args.destAbs,
       flowCount: manifest.flows.length,
-      envVarCount: Object.keys(effectiveEnvVars).length,
+      // Skipped keys never made it into the .env, so don't count them.
+      envVarCount:
+        Object.keys(effectiveEnvVars).length - skippedEnvVarKeys.length,
       flowsWithTeamStorageRefs,
+      skippedEnvVarKeys,
     };
   } catch (err) {
     await removeTempDir(tmpDir, registry, fs).catch(() => {});