lab_check: work in a fresh directory each time, and name what an earlier check left (hq issue 624)

lab_check cloned into one fixed path, <work>/check, after an rm -rf whose result it ignored; a tree an
earlier check left that the lab's account could not remove made every check fail on "destination path
already exists". Each check now works in a fresh check-* directory it removes when done, and clears
what earlier checks left, naming any path it could not remove.

modules/lab/test/runs.test.ts

@@ -194,3 +194,35 @@ test("the lab asks the package registry for read only", () => {
   assert.ok(m.requires.includes("npm-package-registry"));
   assert.equal(m.contributes?.["npm-package-registry"]?.access, "read");
 });
+
+// lab_check failed for ever on "destination path '<work>/check' already exists": it cloned into one fixed
+// path and ignored whether removing the last one worked (novox/hq issue 624). Each check now works in a
+// fresh directory, and what earlier checks left is cleared or named.
+import { checkDir, clearChecks } from "../tools/runs.ts";
+
+test("each check gets a fresh directory of its own", () => {
+  const work = mkdtempSync(join(tmpdir(), "lab-work-"));
+  const a = checkDir(work);
+  const b = checkDir(work);
+  assert.notEqual(a, b);
+  assert.ok(a.startsWith(join(work, "check-")) && existsSync(a) && existsSync(b));
+  rmSync(work, { recursive: true, force: true });
+});
+
+test("what earlier checks left is cleared, and what cannot be is named rather than tripped over", () => {
+  const work = mkdtempSync(join(tmpdir(), "lab-work-"));
+  for (const d of ["check/src", "check-setup/mesh-lab", "check-abc", "run-x", "keep"]) mkdirSync(join(work, d), { recursive: true });
+  // A tree this account cannot empty: a directory it may not write, with something in it.
+  mkdirSync(join(work, "check-stuck", "locked"), { recursive: true });
+  writeFileSync(join(work, "check-stuck", "locked", "file"), "x");
+  chmodSync(join(work, "check-stuck", "locked"), 0o555);
+  const said = clearChecks(work);
+  chmodSync(join(work, "check-stuck", "locked"), 0o755);
+  for (const gone of ["check", "check-setup", "check-abc"]) assert.equal(existsSync(join(work, gone)), false, gone);
+  assert.ok(existsSync(join(work, "run-x")) && existsSync(join(work, "keep")));
+  if (process.getuid?.() !== 0) {
+    assert.equal(said.length, 1);
+    assert.match(said[0] as string, /check-stuck, left by an earlier check, could not be removed/);
+  }
+  rmSync(work, { recursive: true, force: true });
+});

modules/lab/tools/index.ts

@@ -8,7 +8,7 @@ import { mkdtempSync, rmSync } from "node:fs";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
 import {
-  cleanCredentials, credentialArgs, forgeCredential, installPlan, listRuns, NPM_REPOSITORIES, readStatus, REPOSITORIES, running, start,
+  checkDir, cleanCredentials, clearChecks, credentialArgs, forgeCredential, installPlan, listRuns, NPM_REPOSITORIES, readStatus, REPOSITORIES, running, start,
   stop, tail, writeCredential,
 } from "./runs.js";
 
@@ -30,12 +30,21 @@ export function getLabTools(env: NodeJS.ProcessEnv): ToolDefinition[] {
       run: async () => {
         const forge = forgeOf();
         if (!forge) return { ok: false, output: "the lab's forge is not set: settings for lab, {\"forge\": \"<url>\"}" };
-        const dir = `${work}/check`;
-        spawnSync("rm", ["-rf", dir]);
-        const clone = spawnSync("git", ["clone", "--quiet", "--depth", "1", `${forge}/novox/mesh-lab.git`, dir], { encoding: "utf8" });
-        if (clone.status !== 0) return { ok: false, output: clone.stderr };
-        spawnSync("npm", ["ci", "--no-audit", "--no-fund", "--loglevel=error"], { cwd: dir, encoding: "utf8" });
-        const check = spawnSync("node", ["--experimental-strip-types", "src/cli.ts", "check"], { cwd: dir, encoding: "utf8" });
+        // What earlier checks left is cleared, and what could not be is said; this check works in a fresh
+        // directory of its own either way, and removes it when it is done.
+        const leftovers = clearChecks(work);
+        const dir = checkDir(work);
+        let check: ReturnType<typeof spawnSync>;
+        let problems: string[];
+        try {
+          const clone = spawnSync("git", ["clone", "--quiet", "--depth", "1", `${forge}/novox/mesh-lab.git`, dir], { encoding: "utf8" });
+          if (clone.status !== 0) return { ok: false, output: [String(clone.stderr).trim(), ...leftovers].join("\n") };
+          spawnSync("npm", ["ci", "--no-audit", "--no-fund", "--loglevel=error"], { cwd: dir, encoding: "utf8" });
+          check = spawnSync("node", ["--experimental-strip-types", "src/cli.ts", "check"], { cwd: dir, encoding: "utf8" });
+          problems = setupProblems(forge, `${dir}/setup`, credentialOf(forge));
+        } finally {
+          rmSync(dir, { recursive: true, force: true });
+        }
         // **And what a run would check out and install, before a run finds out** (novox/hq issue 624): every
         // repository it clones answers on the forge, and every one it installs has a lock file where the run
         // installs from.
@@ -43,9 +52,8 @@ export function getLabTools(env: NodeJS.ProcessEnv): ToolDefinition[] {
         // by a running run is not.
         const busy = running(work);
         const left = cleanCredentials(work, busy ? `${work}/.${busy.id}.git-credentials` : undefined);
-        const problems = setupProblems(forge, `${work}/check-setup`, credentialOf(forge));
         const removed = left.length > 0 ? `removed ${left.length} forge credential(s) a run left behind: ${left.join(", ")}` : "";
-        const output = [`${check.stdout}${check.stderr}`.trim(), removed, ...problems].filter(Boolean).join("\n");
+        const output = [`${check.stdout}${check.stderr}`.trim(), removed, ...leftovers, ...problems].filter(Boolean).join("\n");
         return { ok: check.status === 0 && problems.length === 0, output };
       },
     },

modules/lab/tools/runs.ts

@@ -7,7 +7,7 @@
 // for it, and says it was lost rather than pretending it is still going.
 
 import { spawn } from "node:child_process";
-import { existsSync, mkdirSync, readFileSync, readdirSync, rmSync, writeFileSync } from "node:fs";
+import { existsSync, mkdirSync, mkdtempSync, readFileSync, readdirSync, rmSync, writeFileSync } from "node:fs";
 import { join } from "node:path";
 
 /**
@@ -137,6 +137,37 @@ export function cleanCredentials(work: string, keep?: string): string[] {
   return removed;
 }
 
+/**
+ * checkDir is a directory of lab_check's own for one check, fresh each time (novox/hq issue 624). The check
+ * cloned into the one fixed path `<work>/check`, removing it first and ignoring whether that worked; a tree
+ * an earlier check left that this account could not remove — one cloned as another account before the
+ * work directory was the operator's — made every check fail on "destination path already exists".
+ */
+export function checkDir(work: string): string {
+  mkdirSync(work, { recursive: true });
+  return mkdtempSync(join(work, "check-"));
+}
+
+/**
+ * clearChecks removes what earlier checks left in the work directory — the fixed `check` and `check-setup`
+ * of before, and any `check-*` — and says, for each it could not remove, which and why, so a check reports
+ * it rather than tripping over it.
+ */
+export function clearChecks(work: string): string[] {
+  if (!existsSync(work)) return [];
+  const said: string[] = [];
+  for (const name of readdirSync(work)) {
+    if (name !== "check" && name !== "check-setup" && !name.startsWith("check-")) continue;
+    const path = join(work, name);
+    try {
+      rmSync(path, { recursive: true, force: true });
+    } catch (err) {
+      said.push(`${path}, left by an earlier check, could not be removed: ${(err as Error).message}`);
+    }
+  }
+  return said;
+}
+
 /** The arguments that have git offer the credential in that file, and only that one. */
 export function credentialArgs(path: string | undefined): string {
   if (!path) return "";