Conversation
UpdateNotification() wrote to stdout via fmt.Printf, corrupting JSON output when --json was passed to commands like search. Machine consumers got invalid JSON with trailing human-readable text. Three-layer fix: 1. Move UpdateNotification output to os.Stderr (safety net for any caller that bypasses the main.go guard). 2. Add isStructuredOutput() that detects all machine-readable modes: --json, -j, and --format json/sarif/markdown. 3. Skip the update check and trailing newline entirely in main() when structured output is requested, honouring the contract that stdout = pure structured data and stderr = empty. Tests: - Unit: isStructuredOutput table test (9 cases) - Unit: UpdateNotification writes only to stderr - Integration: versioned binary (1.0.0) with seeded cache (9.9.9) exercises the real update path for search --json, list -j, and audit --format json — asserts valid JSON stdout + empty stderr. Closes #129
Restructure contribution guidelines to clarify expectations: - Issues are the primary entry point for all contributions - PRs are for small fixes; features require issue + proposal first - Implementation is handled by maintainer after proposal approval - All development must use devcontainer (make devc) Add PR template to surface checklist when contributors open PRs. Add proposals/ directory with TEMPLATE.md for feature proposals.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request addresses an issue where structured output (JSON, SARIF, Markdown) was being corrupted by human-readable update notifications and trailing newlines. Key changes include the introduction of an isStructuredOutput helper to detect machine-readable modes, redirecting update notifications to stderr, and updating the project's contribution guidelines and templates. Feedback was provided regarding the isStructuredOutput function, which currently fails to account for the --flag=value syntax common in Go CLIs, potentially leading to missed detections.
| func isStructuredOutput(args []string) bool { | ||
| if hasFlag(args, "--json") || hasFlag(args, "-j") { | ||
| return true | ||
| } | ||
| for i, arg := range args { | ||
| if arg == "--format" && i+1 < len(args) { | ||
| switch args[i+1] { | ||
| case "json", "sarif", "markdown": | ||
| return true | ||
| } | ||
| } | ||
| } | ||
| return false | ||
| } |
There was a problem hiding this comment.
The isStructuredOutput function fails to detect structured output modes when flags use the --flag=value syntax (e.g., --format=json or --json=true). This is a common syntax in Go CLIs. When detection fails, main() will print a trailing newline to stdout and execute the update check, which contradicts the PR's goal of ensuring a clean payload for machine consumers.
Additionally, the current implementation of hasFlag (used for --json and -j) only performs an exact match, missing cases like --json=true.
func isStructuredOutput(args []string) bool {
for i, arg := range args {
if arg == "--json" || arg == "-j" || arg == "--json=true" || arg == "-j=true" {
return true
}
if arg == "--format" && i+1 < len(args) {
switch args[i+1] {
case "json", "sarif", "markdown":
return true
}
}
switch arg {
case "--format=json", "--format=sarif", "--format=markdown":
return true
}
}
return false
}1 participant
Bug Fixes
--json,-j, and--format json/sarif/markdownmodes could emit a trailing human-readable update notification into stdout, producing invalid JSON for downstream consumers. The update check is now skipped entirely in structured-output modes, and the notification itself writes to stderr as a safety net. Refs: search --json prints update notice after JSON on stdout in non-current versions, breaking machine consumers #129