diff --git a/modules/lab/module.json b/modules/lab/module.json index 5406579d..c9c14770 100644 --- a/modules/lab/module.json +++ b/modules/lab/module.json @@ -104,7 +104,9 @@ ], "env": { "MESH_LAB_WORK": "${dir:work}", - "MESH_LAB_ENV_FILE": "${dir:state}/lab.env" + "MESH_LAB_ENV_FILE": "${dir:state}/lab.env", + "MESH_LAB_FORGE_BINDING": "${dir:state}/package-registry.json", + "MESH_LAB_FORGE_SECRET": "${dir:state}/package-registry.secret" } } ] diff --git a/modules/lab/test/runs.test.ts b/modules/lab/test/runs.test.ts index 816d1baa..f87bf3d2 100644 --- a/modules/lab/test/runs.test.ts +++ b/modules/lab/test/runs.test.ts @@ -194,3 +194,60 @@ 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 '/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 }); +}); + +// As root nothing is locked against removal, so the unremovable tree cannot be made: skipped there, before +// it makes anything (the merge check runs as root). +test("what earlier checks left is cleared, and what cannot be is named rather than tripped over", { + skip: process.getuid?.() === 0 ? "running as root, which no directory mode stops from removing anything" : false, +}, () => { + 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"))); + 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 }); +}); + +// And as root too: what earlier checks left is cleared. +test("leftover checks are cleared, whoever runs the check", () => { + const work = mkdtempSync(join(tmpdir(), "lab-work-")); + for (const d of ["check/src", "check-setup/mesh-lab", "check-abc", "run-x"]) mkdirSync(join(work, d), { recursive: true }); + assert.deepEqual(clearChecks(work), []); + for (const gone of ["check", "check-setup", "check-abc"]) assert.equal(existsSync(join(work, gone)), false, gone); + assert.ok(existsSync(join(work, "run-x"))); + rmSync(work, { recursive: true, force: true }); +}); + +// The lab's tools read the bound credential, so the runtime is given that file by the tools bundle's env +// (ADR 0192), as gitea and umami are given theirs: the runtime's account may then read exactly it, and no +// other credential changes hands (issue 624). +test("the lab's tools bundle names its bound credential and binding in its env", () => { + const m = JSON.parse(readFileSync(join(import.meta.dirname, "..", "module.json"), "utf8")) as { + secrets: Record; binds: Record; + build: { artifacts: { name: string; env?: Record }[] }; + }; + const env = m.build.artifacts.find((a) => a.name === "code")?.env ?? {}; + assert.equal(env.MESH_LAB_FORGE_SECRET, m.secrets["npm-package-registry"]); + assert.equal(env.MESH_LAB_FORGE_BINDING, m.binds["npm-package-registry"]); +}); diff --git a/modules/lab/tools/index.ts b/modules/lab/tools/index.ts index ce665cbf..d13d5cc7 100644 --- a/modules/lab/tools/index.ts +++ b/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\": \"\"}" }; - 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; + 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 }; }, }, diff --git a/modules/lab/tools/runs.ts b/modules/lab/tools/runs.ts index debc0bc0..9d06f18f 100644 --- a/modules/lab/tools/runs.ts +++ b/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 `/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 "";