fix(vis): tear down run services and report preflight errors

packages/tooling/vis/__tests__/commands/run/run-teardown-on-throw.test.ts

@@ -0,0 +1,104 @@
+import { beforeEach, describe, expect, it, vi } from "vitest";
+
+import runExecute from "../../../src/commands/run/handler";
+
+const state = vi.hoisted(() => {
+    return {
+        cleanup: { finish: vi.fn(), install: vi.fn() },
+        createRunRuntime: vi.fn(),
+    };
+});
+
+vi.mock(import("../../../src/commands/run/run-cleanup"), () => {
+    return { createRunCleanup: () => state.cleanup };
+});
+
+vi.mock(import("../../../src/commands/run/run-runtime"), () => {
+    return { createRunRuntime: state.createRunRuntime };
+});
+
+vi.mock(import("../../../src/commands/run/run-dry-run"), () => {
+    return { printDryRunPlan: () => {} };
+});
+
+vi.mock(import("../../../src/commands/run/run-task-graph"), () => {
+    return { finalizeRunTaskGraph: ({ taskGraph }: { taskGraph: unknown }) => taskGraph };
+});
+
+vi.mock(import("../../../src/commands/run/run-services"), () => {
+    return {
+        wireRunServices: async ({ initialTasks, taskGraph }: { initialTasks: unknown; taskGraph: unknown }) => {
+            return { ephemeralPidFiles: [], initialTasks, injectedServiceIdSet: new Set(), registryStartedCount: 0, registryStartedIds: [], registryStartedPids: new Map(), serviceDockStore: null, serviceEnvByTaskId: new Map(), serviceEventBridge: null, serviceResult: {}, serviceRunDir: undefined, taskGraph };
+        },
+    };
+});
+
+vi.mock(import("../../../src/commands/run/run-preflight"), () => {
+    return { runStartupPreflights: async () => {} };
+});
+
+vi.mock(import("../../../src/commands/run/run-selection"), () => {
+    return { resolveRunSelection: async () => { return { affectedFiles: [], currentOs: "linux", forwardedArgs: [], projectsWithTarget: [], projectTargetIndex: new Map(), target: "build" }; } };
+});
+
+vi.mock(import("../../../src/commands/run/run-tasks"), () => {
+    return { buildRunTasks: () => { return { initialTasks: [], persistentTasks: [], taskGraph: { tasks: {} } }; } };
+});
+
+vi.mock(import("../../../src/runtime/command-runtime"), () => {
+    return { resolveCommandRuntime: () => {} };
+});
+
+vi.mock(import("../../../src/util/vis-paths"), () => {
+    return { cleanupLegacyTaskRunnerLayout: () => {} };
+});
+
+vi.mock(import("../../../src/config/workspace"), () => {
+    return {
+        buildProjectGraph: () => { return {}; },
+        discoverWorkspace: () => { return { config: {}, fileOwners: new Map(), packageJsons: [], projectOptions: {}, workspace: {} }; },
+        loadVisTaskConfigsForWorkspace: async () => { return {}; },
+    };
+});
+
+const toolbox = (): unknown => {
+    return {
+        argument: undefined,
+        logger: { error: () => {}, info: () => {}, log: () => {}, warn: () => {} },
+        options: { dryRun: false, stopServices: false },
+        visConfig: {},
+        workspaceRoot: "/ws",
+    };
+};
+
+describe("vis run teardown", () => {
+    beforeEach(() => {
+        state.cleanup.finish.mockReset().mockResolvedValue(undefined);
+        state.cleanup.install.mockReset();
+        state.createRunRuntime.mockReset();
+    });
+
+    it("tears down services booted by this run when runtime setup throws", async () => {
+        expect.assertions(3);
+
+        state.createRunRuntime.mockRejectedValue(new Error("attestation failed"));
+
+        await expect((runExecute as unknown as (tb: unknown) => Promise<void>)(toolbox())).rejects.toThrow("attestation failed");
+
+        expect(state.cleanup.install).toHaveBeenCalledTimes(1);
+        expect(state.cleanup.finish).toHaveBeenCalledTimes(1);
+    });
+
+    it("tears down services on a dry run, which returns before the runtime is built", async () => {
+        expect.assertions(2);
+
+        const tb = toolbox() as { options: { dryRun: boolean } };
+
+        tb.options.dryRun = true;
+
+        await (runExecute as unknown as (tb: unknown) => Promise<void>)(tb);
+
+        expect(state.createRunRuntime).not.toHaveBeenCalled();
+        expect(state.cleanup.finish).toHaveBeenCalledTimes(1);
+    });
+});

packages/tooling/vis/__tests__/release/core/orchestrator-preflight-guard.test.ts

@@ -0,0 +1,141 @@
+import { execFileSync } from "node:child_process";
+import { mkdirSync, mkdtempSync, writeFileSync } from "node:fs";
+import { rm } from "node:fs/promises";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+
+import { buildContext, publishContext } from "../../../src/release/core/orchestrator";
+import type { PublishResult } from "../../../src/release/core/package-managers/interface";
+import type { PublishContext } from "../../../src/release/core/version-actions/interface";
+import { VersionActions } from "../../../src/release/core/version-actions/interface";
+
+const remote = vi.hoisted(() => {
+    return {
+        detectRemoteProvider: vi.fn(),
+        detectRepoSlug: vi.fn(),
+    };
+});
+
+vi.mock(import("../../../src/release/core/remote/detect"), async (importOriginal) => {
+    const actual = await importOriginal();
+
+    return {
+        ...actual,
+        createRemoteClient: () => {
+            return { detectRepoSlug: remote.detectRepoSlug };
+        },
+        detectRemoteProvider: remote.detectRemoteProvider,
+    };
+});
+
+// Only the network-facing push check is stubbed; the remote is still a real
+// git remote so `git remote get-url --push origin` runs for real.
+vi.mock(import("../../../src/release/core/git"), async (importOriginal) => {
+    const actual = await importOriginal();
+
+    return {
+        ...actual,
+        getCurrentBranch: async () => "main",
+        resolvePushCredentials: async () => undefined,
+        verifyPushAccess: async () => undefined,
+    };
+});
+
+class StubVersionActions extends VersionActions {
+    public readonly id = "stub";
+
+    public readonly calls: string[] = [];
+
+    public async readPublishedVersion(): Promise<string | undefined> {
+        return undefined;
+    }
+
+    public async publish(context: PublishContext): Promise<PublishResult> {
+        this.calls.push(context.pkg.name);
+
+        return { alreadyPublished: false, output: "ok", published: true };
+    }
+}
+
+const writeJson = (path: string, value: unknown): void => {
+    writeFileSync(path, `${JSON.stringify(value, null, 4)}\n`);
+};
+
+const setupFixture = (): string => {
+    const cwd = mkdtempSync(join(tmpdir(), "vis-orch-preflight-"));
+
+    writeJson(join(cwd, "package.json"), { name: "fixture-root", packageManager: "pnpm@10.0.0", private: true, version: "0.0.0", workspaces: ["packages/*"] });
+    writeFileSync(join(cwd, "pnpm-workspace.yaml"), "packages:\n  - 'packages/*'\n");
+
+    mkdirSync(join(cwd, "packages", "a"), { recursive: true });
+    writeJson(join(cwd, "packages", "a", "package.json"), { name: "@scope/a", version: "1.0.0" });
+
+    mkdirSync(join(cwd, ".vis", "release"), { recursive: true });
+    writeFileSync(join(cwd, ".vis", "release", "feat.md"), `---\n"@scope/a": minor\n---\nbody\n`);
+    writeFileSync(
+        join(cwd, "vis.config.cjs"),
+        `module.exports = ${JSON.stringify({ release: { acknowledgeUnstable: true, defaultManaged: true } }, null, 4)};\n`,
+    );
+
+    execFileSync("git", ["init", "-q"], { cwd });
+    execFileSync("git", ["remote", "add", "origin", "https://example.invalid/acme/repo.git"], { cwd });
+
+    return cwd;
+};
+
+describe("orchestrator: publish preflight reports failures instead of rejecting", () => {
+    let cwd: string;
+
+    beforeEach(() => {
+        cwd = setupFixture();
+        remote.detectRemoteProvider.mockReset();
+        remote.detectRepoSlug.mockReset();
+    });
+
+    afterEach(async () => {
+        await rm(cwd, { force: true, recursive: true });
+    });
+
+    it("records a _preflight failure when the release-PR check cannot reach the remote", async () => {
+        expect.hasAssertions();
+
+        const actions = new StubVersionActions();
+        const ctx = await buildContext({
+            config: { acknowledgeUnstable: true, defaultManaged: true },
+            cwd,
+            firstRelease: true,
+        });
+
+        // Only the version-pr channel runs the open-release-PR check.
+        Object.assign(ctx, { channel: { mode: "version-pr" } });
+        remote.detectRemoteProvider.mockRejectedValue(new Error("remote unreachable"));
+
+        const result = await publishContext(ctx, { noPush: true, noTag: true, publishActionsOverride: actions });
+
+        expect(actions.calls).toStrictEqual([]);
+        expect(result.published).toStrictEqual([]);
+        expect(result.failed).toStrictEqual([expect.objectContaining({ name: "_preflight", reason: expect.stringContaining("remote unreachable") })]);
+    });
+
+    it("records a _preflight failure when the push-rule check cannot resolve the repo", async () => {
+        expect.hasAssertions();
+
+        const actions = new StubVersionActions();
+        const ctx = await buildContext({
+            config: { acknowledgeUnstable: true, defaultManaged: true },
+            cwd,
+            firstRelease: true,
+        });
+
+        remote.detectRemoteProvider.mockResolvedValue("github");
+        remote.detectRepoSlug.mockRejectedValue(new Error("slug lookup failed"));
+
+        const result = await publishContext(ctx, { noTag: true, publishActionsOverride: actions });
+
+        expect(actions.calls).toStrictEqual([]);
+        expect(result.published).toStrictEqual([]);
+        expect(result.failed).toStrictEqual([expect.objectContaining({ name: "_preflight", reason: expect.stringContaining("slug lookup failed") })]);
+    });
+});

packages/tooling/vis/src/commands/run/handler.ts

@@ -124,34 +124,9 @@ const execute = async ({ argument, logger, options, visConfig, workspaceRoot: ws
         serviceRunDir,
     } = services;
 
-    taskGraph = finalizeRunTaskGraph({ logger, options, projectOptions, taskGraph });
-
-    if (options.dryRun) {
-        printDryRunPlan({ config, logger, persistentTasks, taskGraph });
-
-        return;
-    }
-
-    const runtime = await createRunRuntime({
-        affectedFiles,
-        config,
-        currentOs,
-        initialTasks,
-        invocationCwd,
-        logger,
-        options,
-        persistentTasks,
-        projectsWithTarget,
-        serviceEnvByTaskId,
-        serviceResult,
-        target,
-        taskGraph,
-        visConfig,
-        workspaceRoot,
-    });
-
-    await runtime.hooks.callHook("run:before", { tasks: initialTasks, workspaceRoot });
-
+    // Arm before anything that can throw: services may already be booted by
+    // now, so a failure in finalizing, the dry run, the runtime hooks or the
+    // modes must still tear them down.
     const stopServicesOnExit = options.stopServices;
     const cleanup = createRunCleanup({
         ephemeralPidFiles,
@@ -166,6 +141,34 @@ const execute = async ({ argument, logger, options, visConfig, workspaceRoot: ws
     cleanup.install();
 
     try {
+        taskGraph = finalizeRunTaskGraph({ logger, options, projectOptions, taskGraph });
+
+        if (options.dryRun) {
+            printDryRunPlan({ config, logger, persistentTasks, taskGraph });
+
+            return;
+        }
+
+        const runtime = await createRunRuntime({
+            affectedFiles,
+            config,
+            currentOs,
+            initialTasks,
+            invocationCwd,
+            logger,
+            options,
+            persistentTasks,
+            projectsWithTarget,
+            serviceEnvByTaskId,
+            serviceResult,
+            target,
+            taskGraph,
+            visConfig,
+            workspaceRoot,
+        });
+
+        await runtime.hooks.callHook("run:before", { tasks: initialTasks, workspaceRoot });
+
         if (runtime.isTTY) {
             const { spawnContext } = runtime;
             const { dynamic, lifeCycle, stdinRegistry, store, taskExecutor } = createInteractiveSession({

packages/tooling/vis/src/release/core/orchestrator/publish-preflight.ts

@@ -148,13 +148,26 @@ export const runPublishPreflight = async (
     // a developer runs `vis release publish` while CI is mid-release-PR cycle.
     if (!options.dryRun && context.channel?.mode === "version-pr") {
         const { createRemoteClient, detectRemoteProvider } = await import("../remote/detect");
-        const provider = await detectRemoteProvider(context.cwd, runner, context.config.provider);
-        const client = createRemoteClient(provider, {
-            githubHost: context.config.githubHost,
-            gitlabHost: context.config.gitlabHost,
-            httpProxy: context.config.httpProxy,
-        });
-        const repo = await client.detectRepoSlug(context.cwd, runner);
+        let repo: string | undefined;
+
+        try {
+            const provider = await detectRemoteProvider(context.cwd, runner, context.config.provider);
+            const client = createRemoteClient(provider, {
+                githubHost: context.config.githubHost,
+                gitlabHost: context.config.gitlabHost,
+                httpProxy: context.config.httpProxy,
+            });
+
+            repo = await client.detectRepoSlug(context.cwd, runner);
+        } catch (error) {
+            result.failed.push({
+                name: "_preflight",
+                reason: `Could not read the remote to check for an open release PR: ${(error as Error).message}`,
+            });
+
+            return true;
+        }
+
         const branch = context.config.versionPr?.branch ?? "vis-release/version-packages";
 
         if (repo) {
@@ -196,7 +209,15 @@ export const runPublishPreflight = async (
     // Fail before publishing when the pushes later in this run would be
     // rejected, so packages don't land on the registry without their tags.
     if (!options.dryRun && !options.noPush) {
-        const pushError = await verifyReleasePushes(context, runner, { noTag: options.noTag === true });
+        let pushError: string | undefined;
+
+        try {
+            pushError = await verifyReleasePushes(context, runner, { noTag: options.noTag === true });
+        } catch (error) {
+            result.failed.push({ name: "_preflight", reason: `Could not verify the release pushes: ${(error as Error).message}` });
+
+            return true;
+        }
 
         if (pushError !== undefined) {
             result.failed.push({ name: "_pushAccess", reason: pushError });