From e2bfdf2224fe8ddf25e3a7b691b05d6d9cf8a6ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel?= Date: Sat, 17 Oct 2026 16:52:46 +0000 Subject: [PATCH] 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 | 3 +- .../cli/src/commands/checkArtifacts.test.ts | 5 +- .../cli/src/commands/checkCapture.test.ts | 52 +++++++++++++++++++ packages/cli/src/utils/artifactCapture.ts | 14 +++-- packages/cli/src/utils/historyOwner.ts | 17 ++++-- 5 files changed, 79 insertions(+), 12 deletions(-) create mode 100644 packages/cli/src/commands/checkCapture.test.ts diff --git a/packages/cli/src/commands/check.ts b/packages/cli/src/commands/check.ts index 2b32f9cae0..e981a77800 100644 --- a/packages/cli/src/commands/check.ts +++ b/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) }; diff --git a/packages/cli/src/commands/checkArtifacts.test.ts b/packages/cli/src/commands/checkArtifacts.test.ts index 5093395768..79439f7b34 100644 --- a/packages/cli/src/commands/checkArtifacts.test.ts +++ b/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 () => { "Missing composition metadata", ); 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, diff --git a/packages/cli/src/commands/checkCapture.test.ts b/packages/cli/src/commands/checkCapture.test.ts new file mode 100644 index 0000000000..b7f9db8949 --- /dev/null +++ b/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((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(); + return { + ...actual, + runCheckPipeline: (...args: Parameters) => { + 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"), + "Missing composition metadata", + ); + 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 }); + } +}); diff --git a/packages/cli/src/utils/artifactCapture.ts b/packages/cli/src/utils/artifactCapture.ts index 87f730f05e..a65d3672f4 100644 --- a/packages/cli/src/utils/artifactCapture.ts +++ b/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, + ); } diff --git a/packages/cli/src/utils/historyOwner.ts b/packages/cli/src/utils/historyOwner.ts index 34d1852a07..735eae6b9f 100644 --- a/packages/cli/src/utils/historyOwner.ts +++ b/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( dir: string | undefined, task: (owner: Owner, turn: Turn | null, projectDir: string) => Promise, + { ownerWaitMs }: { ownerWaitMs?: number } = {}, ): Promise { 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 {