fix(cli): keep check from waiting on project history another process holds (#5759)

* fix(cli): keep check from waiting on project history another process holds

* fix(cli): name the history skip instead of commenting it

packages/cli/src/commands/check.ts

@@ -55,11 +55,12 @@ export interface CheckCommandDependencies {
 const DEFAULT_COMMAND_DEPENDENCIES: CheckCommandDependencies = {
   resolveProject,
   async runPipeline(project, options) {
-    const capture = await captureArtifactRevision(project.dir).catch(warnNotRecorded);
+    const capturing = captureArtifactRevision(project.dir).catch(warnNotRecorded);
     const report = await runCheckPipeline(project, options);
     const images = (files: readonly string[]) => files.map((file) => resolve(project.dir, file));
     reportArtifacts("check", "frame", images(report.snapshots.files));
     reportArtifacts("check", "finding", images(report.snapshots.findingFiles));
+    const capture = await capturing;
     if (!capture) return report;
     try {
       return { ...report, saved: await saveCheckReport(project, options, report, capture) };

packages/cli/src/commands/checkArtifacts.test.ts

@@ -71,7 +71,7 @@ it("only warns when the report cannot be recorded; the exit status is the check'
   }
 });
 
-it("still reports the check when another process holds history", async () => {
+it("still reports the check when another process holds history, without waiting for it", async () => {
   const project = mkdtempSync(join(tmpdir(), "hf-check-history-held-"));
   const log = vi.spyOn(console, "log").mockImplementation(() => undefined);
   const warn = vi.spyOn(console, "warn").mockImplementation(() => undefined);
@@ -80,7 +80,8 @@ it("still reports the check when another process holds history", async () => {
     "<html><body>Missing composition metadata</body></html>",
   );
   const previousWait = historyDeps.ownerWaitMs;
-  historyDeps.ownerWaitMs = 0;
+  // A wait for the holder outlasts the test: recording the report must not take it.
+  historyDeps.ownerWaitMs = 60_000;
   const held = await openProjectHistory({
     projectDir: project,
     historyRoot: historyDeps.historyRoot,

packages/cli/src/commands/checkCapture.test.ts

@@ -0,0 +1,52 @@
+import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import { runCommand } from "citty";
+import { expect, it, vi } from "vitest";
+import check from "./check.js";
+import { consumeCommandResult } from "../utils/commandResult.js";
+import type { CheckReport } from "../utils/checkPipeline.js";
+
+// The history capture answers only once the check has started: a check that waits for it never runs.
+const gate = vi.hoisted(() => {
+  let open = () => {};
+  const opened = new Promise<void>((resolve) => (open = resolve));
+  return { opened, open: () => open() };
+});
+
+vi.mock("../utils/artifactCapture.js", () => ({
+  captureArtifactRevision: async () => {
+    await gate.opened;
+    return undefined;
+  },
+}));
+
+vi.mock("../utils/checkPipeline.js", async (importOriginal) => {
+  const actual = await importOriginal<typeof import("../utils/checkPipeline.js")>();
+  return {
+    ...actual,
+    runCheckPipeline: (...args: Parameters<typeof actual.runCheckPipeline>) => {
+      gate.open();
+      return actual.runCheckPipeline(...args);
+    },
+  };
+});
+
+it("runs the check while its history capture is still under way", async () => {
+  const project = mkdtempSync(join(tmpdir(), "hf-check-capture-"));
+  const log = vi.spyOn(console, "log").mockImplementation(() => undefined);
+  writeFileSync(
+    join(project, "index.html"),
+    "<html><body>Missing composition metadata</body></html>",
+  );
+  try {
+    await runCommand(check, { rawArgs: [project, "--json"] });
+    const output = JSON.parse(String(log.mock.calls.at(-1)?.[0])) as CheckReport;
+    expect(output.browserSkipped).toBe(true);
+    expect(output).not.toHaveProperty("saved");
+  } finally {
+    log.mockRestore();
+    consumeCommandResult();
+    rmSync(project, { recursive: true, force: true });
+  }
+});

packages/cli/src/utils/artifactCapture.ts

@@ -1,9 +1,15 @@
 import { beginArtifactCapture } from "@hyperframes/studio-server/history";
 import { withOwner } from "./historyOwner.js";
 
+const SKIP_BUSY_HISTORY = { ownerWaitMs: 0 };
+
 export async function captureArtifactRevision(projectDir: string) {
-  return withOwner(projectDir, async (owner) => {
-    await owner.checkpoint();
-    return beginArtifactCapture(projectDir);
-  });
+  return withOwner(
+    projectDir,
+    async (owner) => {
+      await owner.checkpoint();
+      return beginArtifactCapture(projectDir);
+    },
+    SKIP_BUSY_HISTORY,
+  );
 }

packages/cli/src/utils/historyOwner.ts

@@ -172,14 +172,17 @@ function previewOwner(route: (path: string) => string): Owner {
   };
 }
 
-async function directOwner(projectDir: string): Promise<{ owner: Owner; turn: Turn | null }> {
+async function directOwner(
+  projectDir: string,
+  ownerWaitMs = historyDeps.ownerWaitMs,
+): Promise<{ owner: Owner; turn: Turn | null }> {
   let turn: Turn | null = null;
   const history = await openProjectHistory({
     projectDir,
     historyRoot: historyDeps.historyRoot,
     now: () => historyDeps.now(),
     pruneGoneProjectsBudgetMs: 1000,
-    ownerWaitMs: historyDeps.ownerWaitMs,
+    ownerWaitMs,
     closedWindowAfterOwnership: () => {
       turn = readTurn(projectDir);
       return turn
@@ -214,7 +217,10 @@ async function directOwner(projectDir: string): Promise<{ owner: Owner; turn: Tu
 }
 
 /** One owner: a preview that keeps this project's history, else the engine, which refuses a second opener. */
-async function connect(projectDir: string): Promise<{ owner: Owner; turn: Turn | null }> {
+async function connect(
+  projectDir: string,
+  ownerWaitMs?: number,
+): Promise<{ owner: Owner; turn: Turn | null }> {
   const server = await historyDeps.findServer(projectDir);
   if (server) {
     const route = (path: string) => studioApiUrl(server, `history${path}`);
@@ -225,16 +231,17 @@ async function connect(projectDir: string): Promise<{ owner: Owner; turn: Turn |
     );
     if (status !== 404) return { owner: previewOwner(route), turn: readTurn(projectDir) };
   }
-  return directOwner(projectDir);
+  return directOwner(projectDir, ownerWaitMs);
 }
 
 /** Runs `task` with the project's history. ponytail: with no preview, a command mid-turn splits the turn in two. */
 export async function withOwner<T>(
   dir: string | undefined,
   task: (owner: Owner, turn: Turn | null, projectDir: string) => Promise<T>,
+  { ownerWaitMs }: { ownerWaitMs?: number } = {},
 ): Promise<T> {
   const { dir: projectDir } = resolveProject(dir);
-  const { owner, turn } = await connect(projectDir);
+  const { owner, turn } = await connect(projectDir, ownerWaitMs);
   try {
     return await task(owner, turn, projectDir);
   } finally {