Expose JSON schemas for audit and logs output#59720
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
| # Recompile all workflow files | ||
| .PHONY: recompile | ||
| recompile: build | ||
| @set -eu; \ |
There was a problem hiding this comment.
🟡 Changes recommended
The generated contracts reject some real populated outputs and omit supported JSON output modes.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds CLI-accessible schemas for audit and logs JSON output, plus reproducible checked-in artifacts.
Changes:
- Adds and registers
gh aw json-schema. - Generates audit/logs schemas during
make recompile. - Adds tests and usage documentation.
File summaries
| File | Description |
|---|---|
.prettierignore |
Excludes generated schemas. |
Makefile |
Regenerates schemas atomically. |
cmd/gh-aw/argument_syntax_test.go |
Tests command syntax. |
cmd/gh-aw/command_groups_test.go |
Tests utility grouping. |
cmd/gh-aw/main.go |
Registers the command. |
docs/src/content/docs/reference/audit.md |
Documents schema generation. |
pkg/cli/json_schema_command.go |
Implements the command. |
pkg/cli/json_schema_command_test.go |
Tests output and artifacts. |
pkg/cli/mcp_schema.go |
Adds output-schema generation. |
schemas/audit.schema.json |
Adds generated audit schema. |
schemas/logs.schema.json |
Adds generated logs schema. |
Review details
- Files reviewed: 11/11 changed files
- Comments generated: 2
- Review effort level: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
| func GenerateOutputSchema[T any]() (*jsonschema.Schema, error) { | ||
| return GenerateSchema[T]() |
| case "audit": | ||
| schema, err = GenerateOutputSchema[AuditData]() | ||
| case "logs": | ||
| schema, err = GenerateOutputSchema[LogsData]() |
There was a problem hiding this comment.
@copilot Fix the code for all comments in this review comment.
When a review comment includes a suggested change, apply the suggestion exactly.
Do not make changes beyond what is described in the linked review comment.
There was a problem hiding this comment.
Implemented in 9684f73. The published schemas now use oneOf to cover audit single-run, single-diff, and multi-diff JSON output, plus logs default and cross-run report JSON output.
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. See the comment below for the result and any generated ADR draft.
|
|
✅ Ponytail Reviewer completed successfully! Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "ab.chatgpt.com"See Network Configuration for more information.
|
|
✅ Test Quality Sentinel completed test quality analysis. Test Quality Sentinel skipped because pre-fetch PR data was unavailable: unable to fetch test file diff
|
|
✅ PR Code Quality Reviewer completed the code quality review. Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "github.com"See Network Configuration for more information.
|
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Ponytail pass focused only on removable complexity in changed lines.
net: -7 lines possible.
Warning
Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
ab.chatgpt.com
To allow these domains, add them to the network.allowed list in your workflow frontmatter:
network:
allowed:
- defaults
- "ab.chatgpt.com"See Network Configuration for more information.
Generated by ✂️ Ponytail Reviewer for #59720 · codex · gpt53codex · 9.25 AIC · ⌖ 3.98 AIC · ⊞ 11.8K
Comment /ponytail to run again
| } | ||
|
|
||
| // GenerateOutputSchema generates a JSON schema for a structured command output type. | ||
| func GenerateOutputSchema[T any]() (*jsonschema.Schema, error) { |
There was a problem hiding this comment.
pkg/cli/mcp_schema.go:L42: yagni: is a pass-through wrapper with one caller path. Call directly from and delete the wrapper.
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
🏗️ ADR required — draft added for PR #59720I did not find an existing ADR in the PR body or on the PR branch that covers this change with all required Michael Nygard sections. Evidence reviewed
What I inferred as the architectural decision
Action taken
Next action for the author
|
Comment MemoryNote This comment is managed by comment memory.It stores persistent context for this thread in the code block at the top of this comment. Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "github.com"See Network Configuration for more information.
|
There was a problem hiding this comment.
Request changes
The new json-schema surface is still publishing schemas that do not match the real JSON emitted by the CLI, so consumers would validate against the wrong contract.
Blocking themes
The implementation generates logs directly from LogsData, but several nested report types intentionally override MarshalJSON to preserve legacy wire keys. Reflection cannot see those custom wire shapes, so the checked-in schema drifts from actual gh aw logs --json output. That makes the new command actively misleading for automation instead of useful.
Warning
Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
github.com
To allow these domains, add them to the network.allowed list in your workflow frontmatter:
network:
allowed:
- defaults
- "github.com"See Network Configuration for more information.
🔎 Code quality review by PR Code Quality Reviewer · copilot · gpt54 · 46.8 AIC · ⌖ 7.41 AIC · ⊞ 21.8K
Comment /review to run again
| switch name { | ||
| case "audit": | ||
| schema, err = GenerateOutputSchema[AuditData]() | ||
| case "logs": |
There was a problem hiding this comment.
The new json-schema logs output is not the contract that gh aw logs --json actually emits, because LogsData contains fields with custom MarshalJSON implementations whose wire names differ from the struct field tags.
💡 Why this blocks the feature
GenerateOutputSchema[LogsData]() reflects the Go struct layout, but several nested types deliberately rewrite their JSON shape at marshal time. ToolUsageSummary is the obvious example: the schema generator sees ToolUsageStatsBase as json:"-" plus fields like name/total_calls, while MarshalJSON reconstructs a different object and other nested types such as MCPFailureSummary, MCPServerCrossRunHealth, and MCPServerHealthDetail do the same thing. That means the checked-in schema can claim required or renamed properties that never appear in real logs --json output, so downstream validators will reject valid CLI output.
This needs to generate schemas from the real wire structs or explicit schema-specific DTOs, not directly from the runtime structs with custom marshalers.
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /codebase-design and /grill-with-docs — requesting changes on two correctness gaps in the generated schemas that were flagged in the prior review round and don't appear resolved yet.
📋 Key Themes & Highlights
Key Themes
- Schema/runtime mismatch:
jsonschema.For[T]reflects Go struct tags, but several types (ToolUsageSummary,MCPFailureSummary,DomainAnalysis, etc.) implement customMarshalJSON. The generatedschemas/logs.schema.jsonrequires bogusToolName/CallCountfields undertool_usagethat never appear in realgh aw logs --jsonoutput. - Incomplete coverage:
gh aw audit --jsonin multi-run diff mode emitsAuditDiff, notAuditData— the newjson-schema auditcommand and its docs don't cover or caveat this.
Positive Highlights
- ✅ Solid CLI hygiene:
ExactArgs(1),ValidArgs, no partial stdout on failure, deterministic newline-terminated output — all covered by tests. - ✅
make recompilefreshness check (TestGeneratedOutputSchemasAreCurrent) is a good guard against schema drift. - ✅ Clean separation between
GenerateOutputSchema/MarshalOutputSchema/GenerateNamedOutputSchema.
@copilot please address the review comments above.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · copilot · sonnet50 · 55.3 AIC · ⌖ 14.9 AIC · ⊞ 10.3K
Comment /matt to run again
|
|
||
| // GenerateOutputSchema generates a JSON schema for a structured command output type. | ||
| func GenerateOutputSchema[T any]() (*jsonschema.Schema, error) { | ||
| return GenerateSchema[T]() |
There was a problem hiding this comment.
[/codebase-design] The generated tool_usage schema in schemas/logs.schema.json requires ToolName and CallCount properties that never appear in real runtime JSON — ToolUsageSummary.MarshalJSON (pkg/cli/logs_report_tools.go) emits only name/total_calls/runs/etc. and never serializes the embedded ToolUsageStatsBase.
💡 Evidence and fix
Verified directly against the checked-in schema:
"tool_usage": { "items": { "properties": { "ToolName": ..., "CallCount": ..., "name": ..., "total_calls": ... },
"required": ["ToolName", "CallCount", "name", "total_calls", "runs"] } }No gh aw logs --json output will ever contain ToolName/CallCount, yet a schema-validating consumer would reject valid output for missing those "required" fields, and accept output missing the real fields since neither is checked properly by consumers relying on reflection-derived shape.
jsonschema.For[T] reflects raw struct tags and does not account for types implementing MarshalJSON/UnmarshalJSON (also affects MCPFailureSummary, DomainAnalysis, MCPServerCrossRunHealth, MCPServerHealthDetail). Before merging, either:
- Exclude/tag-embed
ToolUsageStatsBasewithjson:"-"at the schema-generation level so reflection doesn't see it, or - Generate schemas from the wire-format shadow structs used inside each
MarshalJSON(e.g. reflect on the anonymous struct inToolUsageSummary.MarshalJSON) instead of the public type, or - Add a test asserting round-trip: marshal a populated instance of each
MarshalJSON-bearing type reachable fromAuditData/LogsData, and confirm every emitted JSON key is present in the generated schema'sproperties(and vice versa forrequired).
This was already raised in an earlier review comment on pkg/cli/mcp_schema.go and does not appear to have been addressed.
@copilot please address this.
| case "audit": | ||
| schema, err = GenerateOutputSchema[AuditData]() | ||
| case "logs": | ||
| schema, err = GenerateOutputSchema[LogsData]() |
There was a problem hiding this comment.
[/grill-with-docs] gh aw json-schema audit only documents/covers the single-run AuditData shape, but gh aw audit --json with multiple comparison run IDs emits AuditDiff objects (see pkg/cli/audit_diff_command.go:166, computeAuditDiff), which this schema doesn't describe at all.
💡 Why this matters
A consumer that fetches the "audit schema" via gh aw json-schema audit and then runs gh aw audit --json in diff mode will get output the schema can't validate — silently misleading. The docs in docs/src/content/docs/reference/audit.md say "The audit schema describes gh aw audit --json" without qualifying that this excludes the multi-run diff/comparison mode.
Options:
- Add a second schema name (e.g.
audit-diff) coveringAuditDiff, or a oneOf/anyOf schema combining both shapes underjson-schema audit. - At minimum, narrow the docs and
--helptext to clarify the schema only applies to single-run (non-diff)--jsonoutput, so users don't assume full coverage.
This mirrors an unresolved comment from the earlier review round on pkg/cli/mcp_schema.go:66.
@copilot please address this.
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot this PR is blocked on requested changes. Please address the newest review feedback before continuing:
Also refresh the branch from base if needed and run the Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "github.com"See Network Configuration for more information.
|
…ma-command Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Fixed in 701431d. Explicit wire schemas now cover the custom JSON marshalers used by logs, audit health, and cross-run reports; populated fixtures validate them. All audit JSON variants remain in the |
|
@copilot the author replied that the review concerns are fixed in Please verify the latest commit against the unresolved feedback and, if the issues are addressed, run the Outstanding review feedback:
Recent author follow-up: #59720 (comment) Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "github.com"See Network Configuration for more information.
|
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
|
@copilot the author replied that the blocking review concerns are fixed in Please verify the latest commit against the unresolved feedback and, if the issues are addressed, run the Outstanding review feedback:
Recent author follow-up: #59720 (comment) Run: https://github.com/github/gh-aw/actions/runs/34357701418 Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "github.com"See Network Configuration for more information.
|
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Verified the latest schema contracts and their populated fixtures: |
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Verified on the current branch: |
|
@copilot the author has already followed up on the blocking schema-review comments, but the requested-changes state is still open. Please verify the latest head commit against these unresolved review threads and, if they are satisfied, run the
Recent author confirmations:
Run: https://github.com/github/gh-aw/actions/runs/34360227396 Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "github.com"See Network Configuration for more information.
|
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Verified and completed in 77014fb. The schema generator now covers every custom-marshaled output reachable from |
Adds CLI access to schemas generated directly from the structured output types for
audit --jsonandlogs --json. Checked-in schemas now regenerate withmake recompile.CLI
gh aw json-schema auditgh aw json-schema logsSchema generation
GenerateOutputSchema[AuditData]()andGenerateOutputSchema[LogsData]()Generated artifacts
schemas/audit.schema.jsonschemas/logs.schema.jsonmake recompileCoverage and documentation
Warning
Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
github.comTo allow these domains, add them to the
network.allowedlist in your workflow frontmatter:See Network Configuration for more information.
branch refresh requested by PR Sous Chef run https://github.com/github/gh-aw/actions/runs/34357701418
Warning
Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
github.comTo allow these domains, add them to the
network.allowedlist in your workflow frontmatter:See Network Configuration for more information.