Skip to content

logger: write warnings to stderr, not stdout #197

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
selimdev00 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

logger: write warnings to stderr, not stdout #197

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/utils/logger.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,7 @@ export function info(message: string): void {

export function warn(message: string): void {
if (quiet) return;
console.log(chalk.yellow(message));
// Warnings go to stderr so they never corrupt machine-readable stdout
// (e.g. `--json` output or a piped value); stdout is reserved for results.
console.error(chalk.yellow(message));
}
6 changes: 3 additions & 3 deletions tests/platform/resolve.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ describe("resolveProjectPlatform", () => {
})
);

const logSpy = vi.spyOn(console, "log").mockImplementation(() => {});
const logSpy = vi.spyOn(console, "error").mockImplementation(() => {});

const { resolveProjectPlatform } = await import("../../src/platform/resolve.js");
const platform = await resolveProjectPlatform(testDir);
Expand All @@ -113,7 +113,7 @@ describe("resolveProjectPlatform", () => {
})
);

const logSpy = vi.spyOn(console, "log").mockImplementation(() => {});
const logSpy = vi.spyOn(console, "error").mockImplementation(() => {});

const { resolveProjectPlatform } = await import("../../src/platform/resolve.js");
const platform = await resolveProjectPlatform(testDir);
Expand All @@ -128,7 +128,7 @@ describe("resolveProjectPlatform", () => {
await mkdir(join(testDir, "bmalph"), { recursive: true });
await writeFile(join(testDir, "bmalph/config.json"), "not valid json{{{");

const logSpy = vi.spyOn(console, "log").mockImplementation(() => {});
const logSpy = vi.spyOn(console, "error").mockImplementation(() => {});

const { resolveProjectPlatform } = await import("../../src/platform/resolve.js");
const platform = await resolveProjectPlatform(testDir);
Expand Down
22 changes: 11 additions & 11 deletions tests/utils/config.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ describe("config", () => {
it("returns null and warns when config file has invalid structure", async () => {
await writeFile(join(testDir, "bmalph/config.json"), JSON.stringify({ garbage: true }));

const warnSpy = vi.spyOn(console, "log").mockImplementation(() => {});
const warnSpy = vi.spyOn(console, "error").mockImplementation(() => {});

const result = await readConfig(testDir);

Expand Down Expand Up @@ -123,7 +123,7 @@ platform: claude-code

it("returns null and warns for malformed YAML", async () => {
await writeFile(join(testDir, "_bmad/config.yaml"), "planning_artifacts: [invalid");
const warnSpy = vi.spyOn(console, "log").mockImplementation(() => {});
const warnSpy = vi.spyOn(console, "error").mockImplementation(() => {});
const result = await readBmadConfig(testDir);
expect(result).toBeNull();
expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining("Error reading BMAD config"));
Expand All @@ -132,7 +132,7 @@ platform: claude-code

it("returns null and warns for non-string planning_artifacts field", async () => {
await writeFile(join(testDir, "_bmad/config.yaml"), "planning_artifacts: 123\n");
const warnSpy = vi.spyOn(console, "log").mockImplementation(() => {});
const warnSpy = vi.spyOn(console, "error").mockImplementation(() => {});
const result = await readBmadConfig(testDir);
expect(result).toBeNull();
expect(warnSpy).toHaveBeenCalledWith(
Expand All @@ -143,7 +143,7 @@ platform: claude-code

it("returns null and warns for boolean planning_artifacts field", async () => {
await writeFile(join(testDir, "_bmad/config.yaml"), "planning_artifacts: true\n");
const warnSpy = vi.spyOn(console, "log").mockImplementation(() => {});
const warnSpy = vi.spyOn(console, "error").mockImplementation(() => {});
const result = await readBmadConfig(testDir);
expect(result).toBeNull();
expect(warnSpy).toHaveBeenCalledWith(
Expand All @@ -154,7 +154,7 @@ platform: claude-code

it("returns null and warns for object planning_artifacts field", async () => {
await writeFile(join(testDir, "_bmad/config.yaml"), "planning_artifacts:\n foo: bar\n");
const warnSpy = vi.spyOn(console, "log").mockImplementation(() => {});
const warnSpy = vi.spyOn(console, "error").mockImplementation(() => {});
const result = await readBmadConfig(testDir);
expect(result).toBeNull();
expect(warnSpy).toHaveBeenCalledWith(
Expand All @@ -165,7 +165,7 @@ platform: claude-code

it("returns null and warns for empty YAML file", async () => {
await writeFile(join(testDir, "_bmad/config.yaml"), "");
const warnSpy = vi.spyOn(console, "log").mockImplementation(() => {});
const warnSpy = vi.spyOn(console, "error").mockImplementation(() => {});
const result = await readBmadConfig(testDir);
expect(result).toBeNull();
expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining("Error reading BMAD config"));
Expand All @@ -174,7 +174,7 @@ platform: claude-code

it("returns null and warns when YAML parses to a scalar", async () => {
await writeFile(join(testDir, "_bmad/config.yaml"), "just a string\n");
const warnSpy = vi.spyOn(console, "log").mockImplementation(() => {});
const warnSpy = vi.spyOn(console, "error").mockImplementation(() => {});
const result = await readBmadConfig(testDir);
expect(result).toBeNull();
expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining("Error reading BMAD config"));
Expand All @@ -183,7 +183,7 @@ platform: claude-code

it("returns null and warns when YAML parses to an array", async () => {
await writeFile(join(testDir, "_bmad/config.yaml"), "- item1\n- item2\n");
const warnSpy = vi.spyOn(console, "log").mockImplementation(() => {});
const warnSpy = vi.spyOn(console, "error").mockImplementation(() => {});
const result = await readBmadConfig(testDir);
expect(result).toBeNull();
expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining("Error reading BMAD config"));
Expand All @@ -192,7 +192,7 @@ platform: claude-code

it("returns null and warns for non-string platform field", async () => {
await writeFile(join(testDir, "_bmad/config.yaml"), "platform: 123\n");
const warnSpy = vi.spyOn(console, "log").mockImplementation(() => {});
const warnSpy = vi.spyOn(console, "error").mockImplementation(() => {});
const result = await readBmadConfig(testDir);
expect(result).toBeNull();
expect(warnSpy).toHaveBeenCalledWith(
Expand All @@ -203,7 +203,7 @@ platform: claude-code

it("returns null and warns for non-string project_name field", async () => {
await writeFile(join(testDir, "_bmad/config.yaml"), "project_name: true\n");
const warnSpy = vi.spyOn(console, "log").mockImplementation(() => {});
const warnSpy = vi.spyOn(console, "error").mockImplementation(() => {});
const result = await readBmadConfig(testDir);
expect(result).toBeNull();
expect(warnSpy).toHaveBeenCalledWith(
Expand All @@ -214,7 +214,7 @@ platform: claude-code

it("returns null and warns for non-string output_folder field", async () => {
await writeFile(join(testDir, "_bmad/config.yaml"), "output_folder: 42\n");
const warnSpy = vi.spyOn(console, "log").mockImplementation(() => {});
const warnSpy = vi.spyOn(console, "error").mockImplementation(() => {});
const result = await readBmadConfig(testDir);
expect(result).toBeNull();
expect(warnSpy).toHaveBeenCalledWith(
Expand Down
18 changes: 15 additions & 3 deletions tests/utils/logger.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,17 @@ vi.mock("chalk", () => ({

describe("logger", () => {
let consoleSpy: ReturnType<typeof vi.spyOn>;
let consoleErrorSpy: ReturnType<typeof vi.spyOn>;

beforeEach(() => {
vi.resetModules();
consoleSpy = vi.spyOn(console, "log").mockImplementation(() => {});
consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {});
});

afterEach(() => {
consoleSpy.mockRestore();
consoleErrorSpy.mockRestore();
vi.restoreAllMocks();
});

Expand Down Expand Up @@ -100,6 +103,7 @@ describe("logger", () => {
warn("warning should not appear");

expect(consoleSpy).not.toHaveBeenCalled();
expect(consoleErrorSpy).not.toHaveBeenCalled();
});

it("resumes normal output when quiet is disabled", async () => {
Expand Down Expand Up @@ -127,13 +131,21 @@ describe("logger", () => {
});

describe("warn", () => {
it("outputs message with yellow color", async () => {
it("outputs message with yellow color to stderr", async () => {
const { warn } = await import("../../src/utils/logger.js");

warn("warning message");

expect(consoleSpy).toHaveBeenCalledWith(expect.stringContaining("[yellow]"));
expect(consoleSpy).toHaveBeenCalledWith(expect.stringContaining("warning message"));
expect(consoleErrorSpy).toHaveBeenCalledWith(expect.stringContaining("[yellow]"));
expect(consoleErrorSpy).toHaveBeenCalledWith(expect.stringContaining("warning message"));
});

it("does not write warnings to stdout", async () => {
const { warn } = await import("../../src/utils/logger.js");

warn("warning message");

expect(consoleSpy).not.toHaveBeenCalled();
});
});
});
14 changes: 7 additions & 7 deletions tests/utils/state.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ describe("state", () => {
JSON.stringify({ garbage: true })
);

const warnSpy = vi.spyOn(console, "log").mockImplementation(() => {});
const warnSpy = vi.spyOn(console, "error").mockImplementation(() => {});

const result = await readState(testDir);

Expand Down Expand Up @@ -230,7 +230,7 @@ describe("state", () => {
await mkdir(join(testDir, ".ralph"), { recursive: true });
await writeFile(join(testDir, ".ralph/status.json"), '"just a string"');

const warnSpy = vi.spyOn(console, "log").mockImplementation(() => {});
const warnSpy = vi.spyOn(console, "error").mockImplementation(() => {});

const result = await readRalphStatus(testDir);

Expand All @@ -250,7 +250,7 @@ describe("state", () => {
await mkdir(join(testDir, ".ralph"), { recursive: true });
await writeFile(join(testDir, ".ralph/status.json"), '{"loopCount": "not-a-number"}');

const warnSpy = vi.spyOn(console, "log").mockImplementation(() => {});
const warnSpy = vi.spyOn(console, "error").mockImplementation(() => {});

const result = await readRalphStatus(testDir);

Expand Down Expand Up @@ -278,7 +278,7 @@ describe("state", () => {
})
);

const warnSpy = vi.spyOn(console, "log").mockImplementation(() => {});
const warnSpy = vi.spyOn(console, "error").mockImplementation(() => {});

const result = await readRalphStatus(testDir);

Expand All @@ -305,7 +305,7 @@ describe("state", () => {
})
);

const warnSpy = vi.spyOn(console, "log").mockImplementation(() => {});
const warnSpy = vi.spyOn(console, "error").mockImplementation(() => {});

const result = await readRalphStatus(testDir);

Expand All @@ -332,7 +332,7 @@ describe("state", () => {
})
);

const warnSpy = vi.spyOn(console, "log").mockImplementation(() => {});
const warnSpy = vi.spyOn(console, "error").mockImplementation(() => {});

const result = await readRalphStatus(testDir);

Expand Down Expand Up @@ -435,7 +435,7 @@ describe("state", () => {
})
);

const warnSpy = vi.spyOn(console, "log").mockImplementation(() => {});
const warnSpy = vi.spyOn(console, "error").mockImplementation(() => {});

const result = await readRalphStatus(testDir);

Expand Down