fix(env): keep legacy env files holding values no layer received (HACK-1695)

docs/env.md

@@ -1046,6 +1046,17 @@ That migrates the repo into:
 - `.hack/hack.env.<overlay>.yaml`
 - `.hack.secret.key` when needed
 
+Doctor offers legacy dotenv files for cleanup only when every parsed entry was
+delivered to an env layer. Files containing undeclared or undelivered entries stay
+on disk; the cleanup report lists their key names and omits their values. This
+also applies to untracked overlays such as `.env.bak` and `.env.example`.
+Migration refuses a legacy `.env.default` overlay before writing env layers,
+because it would overwrite the base `hack.env.default.yaml` layer. Rename that
+legacy overlay and update its project references before retrying.
+If a legacy dotenv file cannot be read, migration refuses cleanup and leaves
+legacy artifacts in place. Env layers may already have been written; check read
+access and preserve the remaining entries before cleaning up manually.
+
 `hack doctor` also warns when a repo still depends on the old format.
 For modern env repos, `hack doctor` also warns when materialized compatibility output in
 `.hack/.env` or `.hack/.env.state.json` is stale and points to `hack env materialize`.

src/commands/doctor.ts

@@ -3546,12 +3546,18 @@ async function runDoctorFix(opts: {
 
   await maybeInstallMutagenForDoctorFix();
   await maybeUntrackGeneratedFiles({ startDir: opts.startDir });
-  await maybeEnsureHackDirGitignore({ startDir: opts.startDir });
+  // Let env migration refuse before creating an unrelated missing gitignore.
+  if (!opts.migrateEnvConfig) {
+    await maybeEnsureHackDirGitignore({ startDir: opts.startDir });
+  }
   await maybeRemovePlaintextComposeEnvOverride({ startDir: opts.startDir });
   await maybeRepairHackd();
 
   const dockerOk = await dockerInfoOk();
   if (!dockerOk) {
+    if (opts.migrateEnvConfig) {
+      await maybeEnsureHackDirGitignore({ startDir: opts.startDir });
+    }
     note(
       "Docker is not reachable; daemon repairs were still applied, but Docker-backed fixes were skipped.",
       "doctor"
@@ -3587,6 +3593,7 @@ async function runDoctorFix(opts: {
   await maybeMigrateDnsmasq();
   if (opts.migrateEnvConfig) {
     await maybeMigrateProjectEnvConfig({ startDir: opts.startDir });
+    await maybeEnsureHackDirGitignore({ startDir: opts.startDir });
   }
 
   renderSkippedDoctorFixSteps();
@@ -3728,6 +3735,13 @@ async function maybeMigrateProjectEnvConfig(opts: {
   }
   note(migrationNotes.join("\n"), "doctor");
 
+  for (const retained of migrated.retainedEnvFiles) {
+    note(
+      `Kept ${retained.path}: keys not preserved in env layers: ${retained.keys.join(", ")} (values omitted).`,
+      "env cleanup"
+    );
+  }
+
   let blockedCleanupCandidates = [...migrated.blockedCleanupCandidates];
   if (migrated.composeEnvFileReferences.length > 0) {
     note(

src/lib/project-env-config.ts

@@ -39,6 +39,7 @@ import {
   mergeManagedGitignoreBlock,
   pathExists,
   readAtomicWriteTargetText,
+  readOptionalTextFile,
   readTextFile,
   writeTextFile,
   writeTextFileAtomicIfChanged,
@@ -4898,6 +4899,10 @@ export async function migrateLegacyProjectEnv(opts: {
   readonly cleanupCandidates: readonly string[];
   readonly blockedCleanupCandidates: readonly string[];
   readonly composeEnvFileReferences: readonly LegacyComposeEnvFileReference[];
+  readonly retainedEnvFiles: readonly {
+    readonly path: string;
+    readonly keys: readonly string[];
+  }[];
 }> {
   await assertLegacyProjectDirectory({ projectDir: opts.projectDir });
   await assertLegacyProjectInputFamily({ projectRoot: opts.projectRoot });
@@ -4911,6 +4916,7 @@ export async function migrateLegacyProjectEnv(opts: {
       cleanupCandidates: [],
       blockedCleanupCandidates: [],
       composeEnvFileReferences: [],
+      retainedEnvFiles: [],
     };
   }
 
@@ -4923,12 +4929,24 @@ export async function migrateLegacyProjectEnv(opts: {
   const overlayNames = await discoverLegacyOverlayNames({
     projectDir: opts.projectDir,
   });
+  if (overlayNames.includes("default")) {
+    throw new ClosedCliError(
+      "Refusing legacy env migration: .env.default would overwrite the base hack.env.default.yaml layer. Rename the legacy default overlay and update its project references before retrying. No env migration files were written."
+    );
+  }
   const keyInfo = baseResolved.values.some(
     (value) => value.source === "keychain"
   )
     ? await ensureProjectEnvSecretKey({ projectRoot: opts.projectRoot })
     : null;
 
+  // An overlay may inherit an equal base value without writing its own entry.
+  // Keep the full resolved delivery per source file, including inherited values.
+  const deliveredValues = new Map<string, ReadonlyMap<string, string | null>>();
+  deliveredValues.set(
+    resolve(opts.projectDir, PROJECT_ENV_FILENAME),
+    new Map(baseResolved.values.map((entry) => [entry.key, entry.value]))
+  );
   const wroteFiles: string[] = [];
   const defaultConfig = buildMigratedProjectEnvConfig({
     environment: "default",
@@ -4958,6 +4976,10 @@ export async function migrateLegacyProjectEnv(opts: {
       projectName: opts.projectName,
       envName: overlayName,
     });
+    deliveredValues.set(
+      resolve(opts.projectDir, `${PROJECT_ENV_FILENAME}.${overlayName}`),
+      new Map(overlayResolved.values.map((entry) => [entry.key, entry.value]))
+    );
     const overlayConfig = buildMigratedProjectEnvConfig({
       environment: overlayName,
       values: buildMigratedValues({
@@ -5013,6 +5035,7 @@ export async function migrateLegacyProjectEnv(opts: {
     overlayNames,
     materialize: opts.materialize,
     configCleanupCandidates: configCleanup.cleanupCandidates,
+    deliveredValues,
     composeEnvFileReferences,
   });
 
@@ -5023,6 +5046,7 @@ export async function migrateLegacyProjectEnv(opts: {
     updatedProjectConfig: configCleanup.changed,
     cleanupCandidates: cleanupCandidates.allowed,
     blockedCleanupCandidates: cleanupCandidates.blocked,
+    retainedEnvFiles: cleanupCandidates.retainedEnvFiles,
     composeEnvFileReferences,
   };
 }
@@ -5138,16 +5162,37 @@ function buildMigratedValues(opts: {
   return values;
 }
 
+/** Cleanup requires readable contents, even when legacy resolution treated a read failure as absence. */
+async function readLegacyDotenvCleanupEntries(opts: {
+  readonly path: string;
+}): Promise<Readonly<Record<string, string>>> {
+  const source = await readOptionalTextFile(opts.path);
+  if (source.kind !== "text") {
+    throw new ClosedCliError(
+      `Refusing legacy env cleanup: could not read ${opts.path}. No legacy files were removed; env layers may already have been written. Check read access before cleaning up manually (values omitted).`
+    );
+  }
+  return parseDotEnv(source.text);
+}
+
 async function collectLegacyProjectEnvCleanupCandidates(opts: {
   readonly projectRoot: string;
   readonly projectDir: string;
   readonly overlayNames: readonly string[];
   readonly materialize: boolean;
+  readonly deliveredValues: ReadonlyMap<
+    string,
+    ReadonlyMap<string, string | null>
+  >;
   readonly configCleanupCandidates: readonly string[];
   readonly composeEnvFileReferences: readonly LegacyComposeEnvFileReference[];
 }): Promise<{
   readonly allowed: readonly string[];
   readonly blocked: readonly string[];
+  readonly retainedEnvFiles: readonly {
+    readonly path: string;
+    readonly keys: readonly string[];
+  }[];
 }> {
   const candidates = new Set<string>();
   candidates.add(resolve(opts.projectDir, PROJECT_ENV_CONTRACT_FILENAME));
@@ -5166,10 +5211,25 @@ async function collectLegacyProjectEnvCleanupCandidates(opts: {
   const blockedPaths = new Set(
     opts.composeEnvFileReferences.map((reference) => reference.resolvedPath)
   );
+  const retainedEnvFiles: { path: string; keys: string[] }[] = [];
   const allowed: string[] = [];
   const blocked: string[] = [];
   for (const path of candidates) {
     if (await pathExists(path)) {
+      // Value retention is independent of Compose references: repairing Compose
+      // must never promote a file holding undelivered values into cleanup.
+      const delivered = opts.deliveredValues.get(path);
+      if (delivered) {
+        const entries = await readLegacyDotenvCleanupEntries({ path });
+        const keys = Object.entries(entries)
+          .filter(([key, value]) => delivered.get(key) !== value)
+          .map(([key]) => key)
+          .sort();
+        if (keys.length > 0) {
+          retainedEnvFiles.push({ path, keys });
+          continue;
+        }
+      }
       if (blockedPaths.has(path)) {
         blocked.push(path);
       } else {
@@ -5178,6 +5238,7 @@ async function collectLegacyProjectEnvCleanupCandidates(opts: {
     }
   }
   return {
+    retainedEnvFiles,
     allowed: allowed.sort((left, right) => left.localeCompare(right)),
     blocked: blocked.sort((left, right) => left.localeCompare(right)),
   };