-
Notifications
You must be signed in to change notification settings - Fork 28
Add noop message parsing to logs and audit commands #4428
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.
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
Conversation
- Add NoopReport struct to track noop messages - Add NoopCount field to WorkflowRun struct - Add Noops field to ProcessedRun and RunSummary - Create extractNoopsFromRun function in logs_metrics.go - Update logs command to extract and display noop stats - Add "Noops" column to logs overview table - Update audit command to include noop messages - Add tests for noop extraction (logs_noop_test.go) Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds noop message parsing to the logs and audit commands. Noop messages are transparency indicators logged when safe-outputs are configured but produce no GitHub API actions (e.g., running in read-only or staged mode).
Key changes:
- Adds
NoopReportstruct andNoopCountfield to track noop messages across workflow runs - Implements
extractNoopsFromRun()function following the same pattern asextractMissingToolsFromRun() - Enhances logs overview table with a "Noops" column showing per-run and total counts
- Adds "No-Op Messages" section to audit reports displaying full message text and timestamps
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
pkg/cli/logs_noop_test.go |
Comprehensive unit tests for noop extraction including edge cases (malformed JSON, multiple noops, empty output) |
pkg/cli/logs_metrics.go |
New extractNoopsFromRun() function that parses noop entries from agent_output.json artifact files |
pkg/cli/logs.go |
Adds NoopReport struct, NoopCount field to WorkflowRun, integrates noop data into ProcessedRun, RunSummary, and DownloadResult, and updates logs overview table with "Noops" column |
pkg/cli/audit_report.go |
Adds Noops field to AuditData struct and populates it from processed run data |
pkg/cli/audit.go |
Extracts noops during audit workflow run processing and includes them in ProcessedRun and RunSummary |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| FirewallAnalysis: result.FirewallAnalysis, | ||
| MissingTools: result.MissingTools, | ||
| Noops: result.Noops, | ||
| MCPFailures: result.MCPFailures, |
Copilot
AI
Nov 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ProcessedRun struct is missing the JobDetails field in its initialization. The JobDetails field exists in the struct (line 71) and is populated in result.JobDetails from cached summaries (line 900), but when constructing a ProcessedRun from a fresh download result, the field is not being assigned.
This should be:
processedRun := ProcessedRun{
Run: run,
AccessAnalysis: result.AccessAnalysis,
FirewallAnalysis: result.FirewallAnalysis,
MissingTools: result.MissingTools,
Noops: result.Noops,
MCPFailures: result.MCPFailures,
JobDetails: result.JobDetails,
}Note: This is a pre-existing bug not introduced by this PR, but should be fixed for completeness.
| MCPFailures: result.MCPFailures, | |
| MCPFailures: result.MCPFailures, | |
| JobDetails: result.JobDetails, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in commit 3db46d7. Added JobDetails field to ProcessedRun initialization.
| FirewallAnalysis: summary.FirewallAnalysis, | ||
| MissingTools: summary.MissingTools, | ||
| Noops: summary.Noops, | ||
| MCPFailures: summary.MCPFailures, |
Copilot
AI
Nov 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The JobDetails field is missing from the DownloadResult initialization when loading from cached summary. The RunSummary struct contains JobDetails (see line 140), but it's not being transferred to the result here.
This should include:
result := DownloadResult{
Run: summary.Run,
Metrics: summary.Metrics,
AccessAnalysis: summary.AccessAnalysis,
FirewallAnalysis: summary.FirewallAnalysis,
MissingTools: summary.MissingTools,
Noops: summary.Noops,
MCPFailures: summary.MCPFailures,
JobDetails: summary.JobDetails,
LogsPath: runOutputDir,
}Note: This is a pre-existing bug not introduced by this PR, but should be fixed for completeness.
| MCPFailures: summary.MCPFailures, | |
| MCPFailures: summary.MCPFailures, | |
| JobDetails: summary.JobDetails, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in commit 3db46d7. Added JobDetails field to DownloadResult initialization when loading from cached summary.
|
@copilot lint |
…alizations Address code review feedback by adding missing JobDetails field to: - ProcessedRun initialization from DownloadResult (line 718) - DownloadResult initialization from cached summary (line 899) This ensures job details are properly propagated through the data structures. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Lint completed successfully. All validations passed ✅ |
|
@copilot fix JavaScript tests |
JavaScript tests are already passing ✅ All 1067 tests in 58 test files pass successfully:
No fixes needed - the test suite is healthy. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Updated documentation based on merged pull requests from the last 24 hours. Features documented: - --progress flag for run command (PR #4450) - Noop message parsing in logs/audit commands (PR #4428) - close-discussion safe output type (PR #4411) - debug-agentic-workflow custom agent (PR #4449) - Audit command suggestion in run output (PR #4445) Changes: - Updated docs/setup/cli.md to document --progress flag and noop parsing - Updated docs/reference/safe-outputs.md to add close-discussion safe output - Updated docs/reference/custom-agents.md to mention debug-agentic-workflow 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Noop Stats Implementation - Complete ✅
Summary
Successfully implemented noop message parsing for the logs and audit commands. The implementation follows the same pattern as the existing missing-tool feature.
Recent Updates
✅ Fixed missing JobDetails fields (commit: addressing code review feedback)
Implementation Complete
NoopReportstruct similar toMissingToolReportin pkg/cli/logs.goNoopCountfield toWorkflowRunstructNoopsfield toRunSummary,ProcessedRun,DownloadResultstructsextractNoopsFromRunfunction in pkg/cli/logs_metrics.goextractNoopsFromRunand populate dataTesting Status
✅ All tests pass
make test-unit✅TestExtractNoopsFromRun✅TestExtractMissingToolsFromRun✅ (compatibility check)make build✅make lint✅make fmt✅Changes Made
Files Modified:
pkg/cli/logs.go- Added NoopReport struct, NoopCount field, Noops column, fixed JobDetails propagationpkg/cli/logs_metrics.go- Added extractNoopsFromRun functionpkg/cli/audit.go- Added noop extraction and reportingpkg/cli/audit_report.go- Added noops to AuditData structFiles Created:
pkg/cli/logs_noop_test.go- Comprehensive tests for noop extractionHow It Works
extractNoopsFromRunfunction reads theagent_output.jsonartifact filetype === "noop"from the structured JSONExample Output
Logs Overview Table:
Audit Report (Markdown):
JSON Output:
{ "noops": [ { "message": "This workflow is running in read-only mode. No changes will be made.", "timestamp": "2024-01-15T12:00:00Z", "workflow_name": "test-workflow", "run_id": 123456 }, { "message": "Analysis complete. See report above.", "timestamp": "2024-01-15T12:05:00Z", "workflow_name": "test-workflow", "run_id": 123456 } ] }Implementation Notes
extractMissingToolsFromRunagent_output.jsonOriginal prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.