diff --git a/CHANGELOG.md b/CHANGELOG.md index 679ef99..6add02a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,15 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## Unreleased + +### Fixed + +- **CLI**: Fix `deployments deploy` to exit non-zero and record audit status "error" when a deployment fails [[#119]] +- **Core**: Externalize `pino` so the audit log writes to file instead of printing to the console [[#119]] + +[#119]: https://github.com/studiometa/forge-tools/pull/119 + ## 0.4.3 - 2026.04.09 ### Added diff --git a/packages/cli/src/commands/deployments/handlers.test.ts b/packages/cli/src/commands/deployments/handlers.test.ts index ea34ed3..8fcd5af 100644 --- a/packages/cli/src/commands/deployments/handlers.test.ts +++ b/packages/cli/src/commands/deployments/handlers.test.ts @@ -127,7 +127,7 @@ describe("deploymentsDeploy", () => { ); }); - it("should display failed status when deployment fails", async () => { + it("should display failed status and exit non-zero when deployment fails", async () => { const { deploySiteAndWait } = await import("@studiometa/forge-core"); vi.mocked(deploySiteAndWait).mockResolvedValue({ data: { status: "failed", log: "Error: npm install failed.", elapsed_ms: 2000 }, @@ -140,7 +140,27 @@ describe("deploymentsDeploy", () => { }); await deploymentsDeploy(ctx); - expect(vi.mocked(console.log)).toHaveBeenCalledWith(expect.stringContaining("failed")); + expect(vi.mocked(console.error)).toHaveBeenCalledWith(expect.stringContaining("failed")); + expect(processExitSpy).toHaveBeenCalledWith(1); + }); + + it("should output the log before failing when deployment fails without streaming", async () => { + const { deploySiteAndWait } = await import("@studiometa/forge-core"); + vi.mocked(deploySiteAndWait).mockResolvedValue({ + data: { status: "failed", log: "Error: npm install failed.", elapsed_ms: 2000 }, + }); + + const ctx = createTestContext({ + token: "test", + mockClient: {} as never, + options: { format: "human", server: "10", site: "100" }, + }); + + await deploymentsDeploy(ctx); + expect(vi.mocked(console.log)).toHaveBeenCalledWith( + expect.stringContaining("Error: npm install failed."), + ); + expect(processExitSpy).toHaveBeenCalledWith(1); }); it("should call onProgress during polling", async () => { @@ -207,6 +227,8 @@ describe("deploymentsDeploy", () => { await deploymentsDeploy(ctx); expect(stdoutSpy).toHaveBeenCalledWith("Step 1\n"); expect(stdoutSpy).toHaveBeenCalledWith("Step 2\n"); + // Separator newline so the final status message is not on the last log line. + expect(stdoutSpy).toHaveBeenLastCalledWith("\n"); }); it("should use onProgress when --stream flag is not set", async () => { diff --git a/packages/cli/src/commands/deployments/handlers.ts b/packages/cli/src/commands/deployments/handlers.ts index ad6016e..28c498f 100644 --- a/packages/cli/src/commands/deployments/handlers.ts +++ b/packages/cli/src/commands/deployments/handlers.ts @@ -3,6 +3,7 @@ import { listDeployments, deploySiteAndWait } from "@studiometa/forge-core"; import type { CommandContext } from "../../context.ts"; import { exitWithValidationError, runCommand } from "../../error-handler.ts"; +import { ApiError } from "../../errors.ts"; import { resolveServerId, resolveSiteId } from "../../utils/resolve.ts"; export async function deploymentsList(ctx: CommandContext): Promise { @@ -87,22 +88,27 @@ export async function deploymentsDeploy(ctx: CommandContext): Promise { execCtx, ); - // Clear the progress line (only needed in non-stream mode) - if (!streamLogs) { + // Separate the final status message from preceding output: in stream mode the + // last log chunk has no trailing newline, otherwise clear the progress line. + if (streamLogs) { + process.stdout.write("\n"); + } else { process.stderr.write("\n"); } const elapsedSec = (result.data.elapsed_ms / 1000).toFixed(1); + // Output full log if we didn't stream it (needed for both success and failure). + if (!streamLogs && result.data.log) { + ctx.formatter.output(result.data.log); + } + if (result.data.status === "success") { ctx.formatter.success(`Deployment succeeded for site ${site_id} (${elapsedSec}s).`); } else { - ctx.formatter.error(`Deployment failed for site ${site_id} (${elapsedSec}s).`); - } - - // Only output full log if we didn't stream it - if (!streamLogs && result.data.log) { - ctx.formatter.output(result.data.log); + // Throw so the failure propagates to the exit code (non-zero) and the audit log + // records status "error" instead of "success". + throw new ApiError(`Deployment failed for site ${site_id} (${elapsedSec}s).`); } }, ctx.formatter); } diff --git a/packages/core/vite.config.ts b/packages/core/vite.config.ts index 4f59131..4c124ec 100644 --- a/packages/core/vite.config.ts +++ b/packages/core/vite.config.ts @@ -5,7 +5,10 @@ import { createBuildConfig, createTestConfig } from "../../vite.config.base.ts"; export default defineConfig({ build: createBuildConfig({ entry: { index: "./src/index.ts" }, - external: [/^@studiometa\/forge-api/], + // Keep pino external so Node resolves its real (node) build at runtime. + // Bundling it makes Vite pick pino's "browser" build, which logs to the + // console instead of the audit file. + external: [/^@studiometa\/forge-api/, "pino"], }), test: createTestConfig({ name: "core",