Skip to content

fix(cli): derive repo log levels #407

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

Merged
moncher-dev merged 3 commits into from
Jun 22, 2026
Merged

fix(cli): derive repo log levels #407

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b9031e5
chore(cli): open issue 386 work branch
Jun 22, 2026
b59df63
fix(cli): derive repo log levels
Jun 22, 2026
25f49a5
fix(cli): harden repo log level filter
Jun 22, 2026
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
5 changes: 5 additions & 0 deletions .changeset/repo-logs-level-filter.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@gh-symphony/cli": patch
---

Fix `gh-symphony repo logs --level` to derive levels from structured event types, include turn failures in error results, validate unsupported level values, and report empty filtered results clearly for issue #386.
189 changes: 188 additions & 1 deletion packages/cli/src/commands/logs.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,10 @@ async function createConfigFixture(): Promise<string> {
"utf8"
);

for (const project of [createProject("tenant-a"), createProject("tenant-b")]) {
for (const project of [
createProject("tenant-a"),
createProject("tenant-b"),
]) {
const projectDir = join(configDir, "projects", project.projectId);
await mkdir(projectDir, { recursive: true });
await writeFile(
Expand Down Expand Up @@ -250,6 +253,190 @@ describe("logs command", () => {
);
});

it("filters scanned events by derived --level values", async () => {
const configDir = await createConfigFixture();
await writeRunEvents(configDir, "tenant-a", "run-1", [
{
at: "2026-03-16T00:00:00.000Z",
event: "run-started",
issueIdentifier: "acme/platform#1",
projectId: "tenant-a",
},
{
at: "2026-03-16T00:01:00.000Z",
event: "run-failed",
issueIdentifier: "acme/platform#2",
projectId: "tenant-a",
attempt: 1,
lastError: "worker failed",
},
{
at: "2026-03-16T00:02:00.000Z",
event: "run-retried",
issueIdentifier: "acme/platform#3",
projectId: "tenant-a",
attempt: 2,
retryKind: "backoff",
nextRetryAt: "2026-03-16T00:05:00.000Z",
},
{
at: "2026-03-16T00:03:00.000Z",
event: "turn_failed",
issueIdentifier: "acme/platform#4",
projectId: "tenant-a",
turnCount: 1,
startedAt: "2026-03-16T00:02:30.000Z",
durationMs: 30_000,
tokenUsage: {
inputTokens: 10,
outputTokens: 5,
totalTokens: 15,
},
error: "turn failed",
},
]);
const stdout = captureWrites(process.stdout);

try {
await logsCommand(["--level", "error"], {
configDir,
verbose: false,
json: false,
noColor: false,
});
} finally {
stdout.restore();
}

const output = stdout.output();
expect(output).toContain("run-failed acme/platform#2 worker failed");
expect(output).toContain("turn_failed acme/platform#4 turn failed");
expect(output).not.toContain("run-started");
expect(output).not.toContain("run-retried");
});

it("filters a specific run by derived warning level", async () => {
const configDir = await createConfigFixture();
await writeRunEvents(configDir, "tenant-a", "run-1", [
{
at: "2026-03-16T00:00:00.000Z",
event: "hook-failed",
projectId: "tenant-a",
hook: "after-create",
error: "hook failed",
},
{
at: "2026-03-16T00:01:00.000Z",
event: "run-suppressed",
issueIdentifier: "acme/platform#2",
projectId: "tenant-a",
reason: "lease already active",
},
]);
const stdout = captureWrites(process.stdout);

try {
await logsCommand(["--run", "run-1", "--level", "warn"], {
configDir,
verbose: false,
json: false,
noColor: false,
});
} finally {
stdout.restore();
}

const output = stdout.output();
expect(output).toContain("run-suppressed acme/platform#2");
expect(output).not.toContain("hook-failed");
});

Comment on lines +352 to +353

@moncher-dev moncher-dev Jun 22, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved in aaa1855: added a regression test for --run <id> --level error where the run has events but none match the derived level.

it("prints a notice when --level matches no scanned events", async () => {
const configDir = await createConfigFixture();
await writeRunEvents(configDir, "tenant-a", "run-1", [
{
at: "2026-03-16T00:00:00.000Z",
event: "run-started",
issueIdentifier: "acme/platform#1",
projectId: "tenant-a",
},
]);
const stdout = captureWrites(process.stdout);
const stderr = captureWrites(process.stderr);

try {
await logsCommand(["--level", "error"], {
configDir,
verbose: false,
json: false,
noColor: false,
});
} finally {
stdout.restore();
stderr.restore();
}

expect(stdout.output()).toBe("");
expect(stderr.output()).toBe(
"No events matched the provided filters (--level error).\n"
);
});

it("prints a notice when --level matches no events for a specific run", async () => {
const configDir = await createConfigFixture();
await writeRunEvents(configDir, "tenant-a", "run-1", [
{
at: "2026-03-16T00:00:00.000Z",
event: "run-started",
issueIdentifier: "acme/platform#1",
projectId: "tenant-a",
},
]);
const stdout = captureWrites(process.stdout);
const stderr = captureWrites(process.stderr);

try {
await logsCommand(["--run", "run-1", "--level", "error"], {
configDir,
verbose: false,
json: false,
noColor: false,
});
} finally {
stdout.restore();
stderr.restore();
}

expect(stdout.output()).toBe("");
expect(stderr.output()).toBe(
"No events matched the provided filters (--level error).\n"
);
});

it("rejects unsupported --level values", async () => {
const configDir = await createConfigFixture();
const stdout = captureWrites(process.stdout);
const stderr = captureWrites(process.stderr);

try {
await logsCommand(["--level", "debug"], {
configDir,
verbose: false,
json: false,
noColor: false,
});
} finally {
stdout.restore();
stderr.restore();
}

expect(stdout.output()).toBe("");
expect(stderr.output()).toBe(
'Unknown --level "debug". Valid values: error, warn, info.\n'
);
expect(process.exitCode).toBe(1);
});

it("sorts scanned events chronologically across project run directories", async () => {
const configDir = await createConfigFixture();
await writeRunEvents(configDir, "tenant-a", "run-later", [
Expand Down
55 changes: 51 additions & 4 deletions packages/cli/src/commands/logs.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import {
} from "../project-selection.js";

type LoggedEvent = Record<string, unknown> & { at?: string };
type LogLevel = "error" | "warn" | "info";
const LOG_LEVELS: readonly LogLevel[] = ["error", "warn", "info"];

function parseLogsArgs(args: string[]): {
follow: boolean;
Expand Down Expand Up @@ -56,6 +58,16 @@ const handler = async (
options: GlobalOptions
): Promise<void> => {
const parsed = parseLogsArgs(args);
const level = parseLogLevel(parsed.level);
if (parsed.level && !level) {
process.stderr.write(
`Unknown --level "${parsed.level}". Valid values: ${LOG_LEVELS.join(
", "
)}.\n`
);
process.exitCode = 1;
return;
}
const runtimeRoot = resolve(options.configDir);

// If --run is specified, read that run's events
Expand All @@ -78,14 +90,19 @@ const handler = async (
try {
const content = await readFile(eventsPath, "utf8");
const lines = content.trim().split("\n").filter(Boolean);
let matchedEvents = 0;
for (const line of lines) {
Comment on lines 90 to 94

@moncher-dev moncher-dev Jun 22, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved in aaa1855: --level is validated once before reading logs. Unsupported values now fail fast with Unknown --level "<value>". Valid values: error, warn, info. and exit code 1.

const event = JSON.parse(line) as LoggedEvent;
if (parsed.projectId && getProjectId(event) !== parsed.projectId)
continue;
if (parsed.level && getLevel(event) !== parsed.level) continue;
if (level && getLevel(event) !== level) continue;
if (parsed.issue && getIssueIdentifier(event) !== parsed.issue)
continue;
process.stdout.write(formatEvent(event) + "\n");
matchedEvents += 1;
}
if (level && matchedEvents === 0) {
process.stderr.write(noMatchedEventsMessage(level));
}
} catch {
process.stderr.write(`No events found for run: ${parsed.run}\n`);
Expand Down Expand Up @@ -147,6 +164,7 @@ const handler = async (
? [join(runtimeRoot, "projects", parsed.projectId, "runs")]
: await listProjectRunRoots(runtimeRoot);
let foundRuns = false;
let matchedEvents = 0;
const events: LoggedEvent[] = [];
Comment on lines 166 to 168

@moncher-dev moncher-dev Jun 22, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved in aaa1855: validation now happens once before both the --run path and scan path, so unsupported levels never reach either filtering path.


try {
Expand All @@ -165,10 +183,11 @@ const handler = async (
const event = JSON.parse(line) as LoggedEvent;
if (parsed.projectId && getProjectId(event) !== parsed.projectId)
continue;
if (parsed.level && getLevel(event) !== parsed.level) continue;
if (level && getLevel(event) !== level) continue;
if (parsed.issue && getIssueIdentifier(event) !== parsed.issue)
continue;
events.push(event);
matchedEvents += 1;
}
} catch {
// Skip runs without events
Expand All @@ -183,6 +202,10 @@ const handler = async (
process.stderr.write("No runs found. Start the orchestrator first.\n");
return;
}
if (level && matchedEvents === 0) {
process.stderr.write(noMatchedEventsMessage(level));
return;
}

events.sort((left, right) =>
String(left.at ?? "").localeCompare(String(right.at ?? ""))
Expand All @@ -208,8 +231,32 @@ function getProjectId(event: LoggedEvent): string | undefined {
return typeof event.projectId === "string" ? event.projectId : undefined;
}

function getLevel(event: LoggedEvent): string | undefined {
return typeof event.level === "string" ? event.level : undefined;
function getLevel(event: LoggedEvent): LogLevel {

@hojinzs hojinzs Jun 22, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit (non-blocking): getLevel now returns a closed error | warn | info set, but parsed.level is still an arbitrary string compared with getLevel(event) !== parsed.level. A misspelled or unsupported value (--level Error, --level debug) silently falls through to the "No events matched" notice. Since the whole point of #386 was that operators get misled by empty output, consider validating --level against the three accepted values up front and surfacing them (e.g. Unknown --level "debug". Valid values: error, warn, info.). Not required for this fix.

Generated by Claude Code

@moncher-dev moncher-dev Jun 22, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Accepted and resolved in aaa1855: --level now validates against error, warn, and info up front, with a clear error and nonzero exit for unsupported values.

switch (event.event) {
case "run-failed":
case "turn_failed":
case "worker-error":
case "hook-failed":
return "error";
Comment on lines +236 to +240

@chatgpt-codex-connector chatgpt-codex-connector Bot Jun 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Include turn_failed in error-level filtering

When a worker turn fails, the orchestrator writes a structured run event with event: "turn_failed" (checked packages/orchestrator/src/service.ts:2347-2351), but this derived-level switch falls through to info for that event. In runs where the failure recorded in events.ndjson is a turn failure, gh-symphony repo logs --level error hides the failure and may report no error matches, which defeats the new level filter for a real runtime failure. Add turn_failed to the error cases.

Useful? React with 👍 / 👎.

@moncher-dev moncher-dev Jun 22, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved in aaa1855: turn_failed is now classified as error, and the CLI logs regression test asserts --level error includes turn_failed output.

case "run-suppressed":
case "run-retried":
return "warn";
default:
return "info";
}
}
Comment on lines +234 to 247

@moncher-dev moncher-dev Jun 22, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved in aaa1855: added turn_failed to the derived error-level cases and covered it in logs.test.ts.


function parseLogLevel(level: string | undefined): LogLevel | undefined {
if (!level) {
return undefined;
}
return LOG_LEVELS.includes(level as LogLevel)
? (level as LogLevel)
: undefined;
}

function noMatchedEventsMessage(level: LogLevel): string {
return `No events matched the provided filters (--level ${level}).\n`;
}

function getIssueIdentifier(event: LoggedEvent): string {
Expand Down
Loading