From 4da7f3a3c3641c3e3718438861146a32a7222600 Mon Sep 17 00:00:00 2001 From: Titouan Mathis Date: Wed, 1 Jul 2026 14:08:51 +0200 Subject: [PATCH 1/4] Fix deploy command to exit non-zero on deployment failure The `deployments deploy` handler printed a failure message but returned normally when the deployment failed, so the process exited 0 and the audit log recorded status "success". Throw an error on non-success so the failure propagates to the exit code and audit log via the existing error handling. Co-authored-by: Claude --- .../src/commands/deployments/handlers.test.ts | 24 +++++++++++++++++-- .../cli/src/commands/deployments/handlers.ts | 15 +++++++----- 2 files changed, 31 insertions(+), 8 deletions(-) diff --git a/packages/cli/src/commands/deployments/handlers.test.ts b/packages/cli/src/commands/deployments/handlers.test.ts index ea34ed3..23a85a4 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 () => { diff --git a/packages/cli/src/commands/deployments/handlers.ts b/packages/cli/src/commands/deployments/handlers.ts index ad6016e..d610c52 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 { @@ -94,15 +95,17 @@ export async function deploymentsDeploy(ctx: CommandContext): Promise { 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); } From 8d4b58e7325c4f8a2fc7acc473e8a9689140c25c Mon Sep 17 00:00:00 2001 From: Titouan Mathis Date: Wed, 1 Jul 2026 14:09:23 +0200 Subject: [PATCH 2/4] Update changelog for deploy exit code fix Co-authored-by: Claude --- CHANGELOG.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 679ef99..05eef0b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,14 @@ 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]] + +[#119]: https://github.com/studiometa/forge-tools/pull/119 + ## 0.4.3 - 2026.04.09 ### Added From c14b67270232bfda4e4a4b742f976508ff5e8341 Mon Sep 17 00:00:00 2001 From: Titouan Mathis Date: Wed, 1 Jul 2026 14:14:44 +0200 Subject: [PATCH 3/4] Add newline separator before deploy status message in stream mode Streamed log chunks have no trailing newline, so the success/failure message was printed on the same line as the last log line. Emit a separating newline on stdout when streaming. Co-authored-by: Claude --- packages/cli/src/commands/deployments/handlers.test.ts | 2 ++ packages/cli/src/commands/deployments/handlers.ts | 7 +++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/commands/deployments/handlers.test.ts b/packages/cli/src/commands/deployments/handlers.test.ts index 23a85a4..8fcd5af 100644 --- a/packages/cli/src/commands/deployments/handlers.test.ts +++ b/packages/cli/src/commands/deployments/handlers.test.ts @@ -227,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 d610c52..28c498f 100644 --- a/packages/cli/src/commands/deployments/handlers.ts +++ b/packages/cli/src/commands/deployments/handlers.ts @@ -88,8 +88,11 @@ 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"); } From 1ad8166ad331d47d6b3dfc9730788c37b096b1b4 Mon Sep 17 00:00:00 2001 From: Titouan Mathis Date: Wed, 1 Jul 2026 14:24:22 +0200 Subject: [PATCH 4/4] Externalize pino so audit log writes to file not console Vite bundled pino and resolved its browser build, which routes log output to the console via console.log. This printed the audit entry object to stdout after every write command. Externalize pino so Node resolves its real node build at runtime and logs go to the audit file. Co-authored-by: Claude --- CHANGELOG.md | 1 + packages/core/vite.config.ts | 5 ++++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 05eef0b..6add02a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### 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 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",