Add safe output items manifest for audit trail#16842
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot logs command should show aggregate stats in result like number of GitHub items created by safe outputs (safe items) |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
📰 BREAKING: Smoke macOS ARM64 is now investigating this pull request. Sources say the story is developing... |
|
📰 BREAKING: Smoke Copilot is now investigating this pull request. Sources say the story is developing... |
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
|
🧪 Smoke Project is now testing project operations... |
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
|
🧪 Smoke Temporary ID is now testing temporary ID functionality... |
|
📰 DEVELOPING STORY: Smoke macOS ARM64 reports failed. Our correspondents are investigating the incident... |
There was a problem hiding this comment.
Pull request overview
Adds an auditable trail of GitHub items created by safe output handlers by logging each created item into a JSONL manifest, uploading it as an artifact, and surfacing it in gh aw audit as a “Created Items” table (and in --json output).
Changes:
- Introduces a JS manifest helper (
safe_output_manifest.cjs) plus unit tests to append created-item entries to/tmp/safe-output-items.jsonl. - Extends the unified safe output handler manager to invoke an
onItemCreatedcallback and ensure the manifest file exists for upload. - Updates the safe outputs job compiler and workflow lock files to always upload the manifest artifact, and updates the audit command to parse/render created items.
Reviewed changes
Copilot reviewed 152 out of 152 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
actions/setup/js/safe_output_manifest.cjs |
New helper to write created-item JSONL entries and normalize handler results into manifest entries. |
actions/setup/js/safe_output_manifest.test.cjs |
Adds Vitest coverage for manifest logger, ensure-file behavior, and result extraction. |
actions/setup/js/safe_output_unified_handler_manager.cjs |
Adds optional onItemCreated callback plumbing and ensures the manifest exists on early-return paths. |
pkg/workflow/compiler_safe_outputs_job.go |
Appends an if: always() upload-artifact step for the manifest in the consolidated safe outputs job. |
pkg/cli/audit_report.go |
Adds CreatedItems to AuditData and parses safe-output-items.jsonl from the run’s downloaded artifacts directory. |
pkg/cli/audit_report_render.go |
Renders a new “Created Items” section/table when created items are present. |
.github/workflows/audit-workflows.lock.yml |
Adds the manifest upload step to the compiled workflow. |
.github/workflows/artifacts-summary.lock.yml |
Adds the manifest upload step to the compiled workflow. |
.github/workflows/archie.lock.yml |
Adds the manifest upload step to the compiled workflow. |
.github/workflows/ai-moderator.lock.yml |
Adds the manifest upload step to the compiled workflow. |
.github/workflows/agent-persona-explorer.lock.yml |
Adds the manifest upload step to the compiled workflow. |
.github/workflows/agent-performance-analyzer.lock.yml |
Adds the manifest upload step to the compiled workflow. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| core.info(`Exported ${issuesToAssignCopilot.length} issue(s) for copilot assignment: ${issuesToAssignStr}`); | ||
| } else { | ||
| core.setOutput("issues_to_assign_copilot", ""); | ||
| } | ||
|
|
||
| // Ensure the manifest file always exists for artifact upload (even if no items were created). | ||
| // Skip in staged mode — no real items were created so no manifest should be emitted. |
There was a problem hiding this comment.
If main() throws before the normal completion path, the manifest may never be created, so the subsequent actions/upload-artifact step won’t have /tmp/safe-output-items.jsonl to upload (despite the goal of “uploads always”). Consider calling ensureManifestExists() in a finally block (or in the catch block, wrapped so it can’t mask the original failure) to guarantee the file exists even on handler-manager errors.
|
🧪 Smoke Temporary ID is now testing temporary ID functionality... |
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
|
📰 BREAKING: Smoke macOS ARM64 is now investigating this pull request. Sources say the story is developing... |
|
📰 BREAKING: Smoke Copilot is now investigating this pull request. Sources say the story is developing... |
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
|
🧪 Smoke Project is now testing project operations... |
|
📰 DEVELOPING STORY: Smoke macOS ARM64 reports failed. Our correspondents are investigating the incident... |
|
✅ Smoke Project completed successfully. All project operations validated. |
|
Smoke test summary
|
|
Smoke test results for
Overall: PASS ✅
|
There was a problem hiding this comment.
Good PR — the safe output manifest adds useful audit trail capability. Two minor observations about logging consistency in the manifest integration. Overall looks solid.
📰 BREAKING: Report filed by Smoke Copilot for issue #16842
| * @returns {Promise<{success: boolean, results: Array<any>, temporaryIdMap: Object, outputsWithUnresolvedIds: Array<any>, missings: Object}>} | ||
| */ | ||
| async function processMessages(messageHandlers, messages) { | ||
| async function processMessages(messageHandlers, messages, onItemCreated = null) { |
There was a problem hiding this comment.
The onItemCreated callback follows a clean optional pattern with = null default. Consider documenting in the JSDoc that callers can use createManifestLogger() from safe_output_manifest.cjs as the standard implementation for this parameter.
| const createdItem = extractCreatedItemFromResult(messageType, result); | ||
| if (createdItem) { | ||
| core.info(`📝 Manifest: logged ${createdItem.type} → ${createdItem.url}`); | ||
| onItemCreated(createdItem); |
There was a problem hiding this comment.
The array branch (lines 377-384) logs core.info for each item, but the else branch (lines 385-390) doesn't log. This inconsistency might make debugging harder — consider adding the core.info log in the single-item path too for consistency with the array path.
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
Done in 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:
|
Smoke Test Results – Run §22190930520Core Tests #1–10: ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅ Overall: PARTIAL (test #17 skipped – no safe test PR to close)
|
There was a problem hiding this comment.
💥 Automated smoke test review - all systems nominal!
💥 [THE END] — Illustrated by Smoke Claude for issue #16842
actions/setup/js/safe_output_manifest.cjs- JavaScript helper that logs each created GitHub item to a JSONL manifest file at/tmp/safe-output-items.jsonlactions/setup/js/safe_output_manifest.test.cjs- 22 tests for the manifest helper (including staged mode tests)actions/setup/js/safe_output_unified_handler_manager.cjs- staged mode, manifest logging, finally blockactions/setup/js/safe_output_handler_manager.cjs- add manifest logging (this is the script the compiled workflows actually call)pkg/workflow/compiler_safe_outputs_job.go- Add upload-artifact step withif: always(); skip step entirely in staged modepkg/cli/audit_report.go/audit_report_render.go- Parse and display "Created Items" table in audit outputSafeItemsCountstats togh aw logscommandmake recompileto update all 152 lock filescreateManifestLogger()now touches file immediately so it always exists before processing startsisStagedbeforetryblock +finallyto guaranteeensureManifestExists()runs even on handler failurescanner.Err()check in Go JSONL scan loopDate.parse()extractCreatedItemsFromManifest📝 Manifest: logged <type> → <url>debug logging when items are writtenOriginal prompt
🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.
Changeset
✨ PR Review Safe Output Test - Run 22187312231
Changeset
✨ PR Review Safe Output Test - Run 22190175158
Changeset
gh aw auditoutput.✨ PR Review Safe Output Test - Run 22190930520