Skip to content

Add --format json to miren logs (MIR-802)#675

Merged
evanphx merged 1 commit intomainfrom
mir-802-add-json-to-miren-logs-for-agent-use
Mar 13, 2026
Merged

Add --format json to miren logs (MIR-802)#675
evanphx merged 1 commit intomainfrom
mir-802-add-json-to-miren-logs-for-agent-use

Conversation

@evanphx
Copy link
Copy Markdown
Contributor

@evanphx evanphx commented Mar 13, 2026

Summary

  • Adds --format json flag to all four log subcommands (app, sandbox, build, system)
  • Outputs newline-delimited JSON (JSONL) — one JSON object per log entry — for easy parsing by agents and scripts
  • Each line contains timestamp, stream, source (optional), message, and attributes (optional)
  • Follows the existing FormatOptions / --format json pattern used by other commands

Test plan

  • Unit tests for JSON output (valid JSON, field correctness, omitempty, JSONL format)
  • All existing tests pass (hack/it cli/commands — 94 tests)
  • Lint clean
  • Manual: miren logs --format json | jq . produces valid JSON per line
  • Manual: miren logs --format json -f streams correctly

All four log subcommands (app, sandbox, build, system) now accept
--format json, producing newline-delimited JSON (JSONL) that agents
and scripts can parse. Each line is a JSON object with timestamp,
stream, source, message, and attributes fields.
@evanphx evanphx requested a review from a team as a code owner March 13, 2026 18:40
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 13, 2026

📝 Walkthrough

Walkthrough

This pull request introduces JSON output formatting to the logs command. A new logPrinter helper function creates printer functions that format log entries as JSON when enabled. Log streaming functions (streamLogs, streamLogChunks, legacyLogs) now accept a custom printer function parameter instead of directly outputting logs. The JSON mode flag is threaded through the log dispatch flow and all command entry points (LogsApp, LogsSandbox, LogsBuild, LogsSystem) pass it to dispatchLogs. A test function validates JSON and JSONL output formatting across various log entry scenarios.

📝 Coding Plan
  • Generate coding plan for human review comments

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (2)
cli/commands/logs_test.go (1)

171-177: Prefer structured key checks over raw substring checks.

The current assertions can false-positive if "source" or "attributes" appears inside message values. Unmarshal into map[string]any and assert key absence directly.

♻️ Suggested test hardening
 		raw := strings.TrimSpace(buf.String())
-		if strings.Contains(raw, "source") {
-			t.Errorf("expected source to be omitted, got: %s", raw)
-		}
-		if strings.Contains(raw, "attributes") {
-			t.Errorf("expected attributes to be omitted, got: %s", raw)
-		}
+		var obj map[string]any
+		if err := json.Unmarshal([]byte(raw), &obj); err != nil {
+			t.Fatalf("invalid JSON output: %v\nraw: %s", err, raw)
+		}
+		if _, ok := obj["source"]; ok {
+			t.Errorf("expected source to be omitted, got: %s", raw)
+		}
+		if _, ok := obj["attributes"]; ok {
+			t.Errorf("expected attributes to be omitted, got: %s", raw)
+		}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cli/commands/logs_test.go` around lines 171 - 177, The test currently checks
for "source" and "attributes" using substring searches on raw (buf.String()),
which can false-positive; instead unmarshal the trimmed JSON string into a
map[string]any (e.g., var m map[string]any) and assert that the keys "source"
and "attributes" are not present (using _, ok := m["source"] / m["attributes"]
and fail the test if ok is true). Replace the strings.Contains(raw, ...) checks
with these structured key-absence assertions so the test verifies keys rather
than arbitrary substrings.
cli/commands/logs.go (1)

282-285: Avoid silent drop on JSON marshal failure.

Returning without any signal makes entry loss invisible during troubleshooting. Emit an explicit stderr diagnostic at minimum.

🛠️ Suggested reliability tweak
 	data, err := json.Marshal(entry)
 	if err != nil {
+		fmt.Fprintf(ctx.Stderr, "failed to encode log entry as JSON: %v\n", err)
 		return
 	}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cli/commands/logs.go` around lines 282 - 285, The code currently swallows
json.Marshal errors by simply returning after "data, err := json.Marshal(entry)"
which silently drops log entries; update the error path in the function
containing that line to emit an explicit diagnostic (e.g., write a descriptive
message including err to stderr or use the existing logger) instead of a bare
return, and consider returning the error to the caller if the function's
signature allows; locate the "json.Marshal(entry)" call and replace the silent
"return" with an explicit os.Stderr or logger output that includes err and
context about the failed entry.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@cli/commands/logs_test.go`:
- Around line 171-177: The test currently checks for "source" and "attributes"
using substring searches on raw (buf.String()), which can false-positive;
instead unmarshal the trimmed JSON string into a map[string]any (e.g., var m
map[string]any) and assert that the keys "source" and "attributes" are not
present (using _, ok := m["source"] / m["attributes"] and fail the test if ok is
true). Replace the strings.Contains(raw, ...) checks with these structured
key-absence assertions so the test verifies keys rather than arbitrary
substrings.

In `@cli/commands/logs.go`:
- Around line 282-285: The code currently swallows json.Marshal errors by simply
returning after "data, err := json.Marshal(entry)" which silently drops log
entries; update the error path in the function containing that line to emit an
explicit diagnostic (e.g., write a descriptive message including err to stderr
or use the existing logger) instead of a bare return, and consider returning the
error to the caller if the function's signature allows; locate the
"json.Marshal(entry)" call and replace the silent "return" with an explicit
os.Stderr or logger output that includes err and context about the failed entry.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: edabd1bb-05bc-4b25-8480-205e51830217

📥 Commits

Reviewing files that changed from the base of the PR and between 350ba22 and b2edbbe.

📒 Files selected for processing (2)
  • cli/commands/logs.go
  • cli/commands/logs_test.go

@evanphx evanphx merged commit 6bf6259 into main Mar 13, 2026
13 checks passed
@evanphx evanphx deleted the mir-802-add-json-to-miren-logs-for-agent-use branch March 13, 2026 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants