fix: extend detectFirewallAuditArtifacts to find audit files in non-flattened agent artifact by Copilot · Pull Request #29360 · github/gh-aw

Copilot · 2026-04-30T17:54:45Z

Summary

detectFirewallAuditArtifacts previously only searched two locations:

sandbox/firewall/audit/ — the post-flattening path after flattenUnifiedArtifact runs
firewall-audit*/ directories — legacy separate firewall-audit-logs artifact

It did not look inside the non-flattened unified agent artifact directory, so policy-manifest.json and audit.jsonl would not be found when gh aw audit is run on a directory populated via gh run download without going through flattenUnifiedArtifact (e.g. a user manually downloads artifacts and points gh aw audit at that directory, or a cached run skips the download step).

Changes

pkg/cli/firewall_policy.go — Refactored detectFirewallAuditArtifacts to add two new lookup steps between the primary path and the legacy fallback:

Step 2 (new structure): agent/sandbox/firewall/audit/ — actions/upload-artifact v4+ strips the /tmp/gh-aw/ common prefix, so files land directly at sandbox/firewall/audit/ inside the artifact directory
Step 3 (old structure): agent/tmp/gh-aw/sandbox/firewall/audit/ — older artifact uploads preserved the full /tmp/gh-aw/ path inside the artifact

Uses findArtifactDir to resolve the agent directory, handling:

agent (current name)
agent-artifacts (legacy name)
<hash>-agent (workflow_call prefix)

Also extracted the per-directory stat+assign logic into a checkDir closure to eliminate the repetitive double-check pattern.

pkg/cli/firewall_policy_test.go — Adds 4 new sub-tests to TestDetectFirewallAuditArtifacts:

agent artifact new structure (not yet flattened)
agent artifact old structure with tmp/gh-aw prefix (not yet flattened)
agent-artifacts legacy artifact name (not yet flattened)
workflow_call prefixed agent artifact (not yet flattened)

✨ PR Review Safe Output Test - Run 25181816514

💥 [THE END] — Illustrated by Smoke Claude · ● 261.7K · ◷

…lattened agent artifact Previously detectFirewallAuditArtifacts only looked in: - sandbox/firewall/audit/ (post-flattening) - firewall-audit*/ directories (legacy separate artifact) It did not check inside the non-flattened unified agent artifact directory (agent/, agent-artifacts/, or *-agent/ for workflow_call), so the files would not be found when audit is run on a directory populated via `gh run download` without going through flattenUnifiedArtifact. Adds lookup for: - agent/sandbox/firewall/audit/ (new structure, v4+ prefix stripping) - agent/tmp/gh-aw/sandbox/firewall/audit/ (old structure, full path preserved) - agent-artifacts/... (legacy artifact name) - <hash>-agent/... (workflow_call prefixed names) Also adds 4 new test cases for these additional lookup paths. Agent-Logs-Url: https://github.com/github/gh-aw/sessions/2303d4dd-1c37-4812-aa7c-d740e1a19b96 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Agent-Logs-Url: https://github.com/github/gh-aw/sessions/2303d4dd-1c37-4812-aa7c-d740e1a19b96 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Copilot

Pull request overview

Extends detectFirewallAuditArtifacts so gh aw audit can locate firewall audit artifacts inside a non-flattened unified agent artifact directory (e.g., after gh run download but before flattenUnifiedArtifact runs).

Changes:

Refactors detectFirewallAuditArtifacts to check additional agent artifact layouts (agent/sandbox/... and agent/tmp/gh-aw/...) before falling back to legacy firewall-audit*.
Reuses findArtifactDir to support agent, agent-artifacts, and workflow_call-prefixed *-agent directories.
Adds new subtests covering the non-flattened artifact directory structures and naming variants.

Show a summary per file

File	Description
pkg/cli/firewall_policy.go	Adds new lookup steps for non-flattened unified agent artifact layouts; refactors repeated stat logic into `checkDir`.
pkg/cli/firewall_policy_test.go	Adds subtests validating detection across new/old agent artifact layouts and naming variants.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Files reviewed: 2/2 changed files
Comments generated: 2

+		// New artifact structure (actions/upload-artifact v4+, /tmp/gh-aw/ prefix stripped):
+		//   agent/sandbox/firewall/audit/
+		if !checkDir(filepath.Join(agentDir, "sandbox", "firewall", "audit"), "agent/sandbox/firewall/audit") {
+			// Old artifact structure (/tmp/gh-aw/ prefix preserved inside the artifact):
+			//   agent/tmp/gh-aw/sandbox/firewall/audit/
+			checkDir(filepath.Join(agentDir, "tmp", "gh-aw", "sandbox", "firewall", "audit"), "agent/tmp/gh-aw/sandbox/firewall/audit")
+		}
+		if manifestPath != "" && auditJSONLPath != "" {
 			return
 		}


+		// New artifact structure (actions/upload-artifact v4+, /tmp/gh-aw/ prefix stripped):
+		//   agent/sandbox/firewall/audit/
+		if !checkDir(filepath.Join(agentDir, "sandbox", "firewall", "audit"), "agent/sandbox/firewall/audit") {
+			// Old artifact structure (/tmp/gh-aw/ prefix preserved inside the artifact):
+			//   agent/tmp/gh-aw/sandbox/firewall/audit/
+			checkDir(filepath.Join(agentDir, "tmp", "gh-aw", "sandbox", "firewall", "audit"), "agent/tmp/gh-aw/sandbox/firewall/audit")


github-actions · 2026-04-30T18:06:11Z

🧪 Test Quality Sentinel Report

Test Quality Score: 100/100

✅ Excellent test quality

Metric	Value
New/modified tests analyzed	4
✅ Design tests (behavioral contracts)	4 (100%)
⚠️ Implementation tests (low value)	0 (0%)
Tests with error/edge cases	4 (100%)
Duplicate test clusters	0
Test inflation detected	No
🚨 Coding-guideline violations	None

Test Classification Details

Test	File	Classification	Issues Detected
`agent artifact new structure (not yet flattened)`	`pkg/cli/firewall_policy_test.go`	✅ Design	None
`agent artifact old structure with tmp/gh-aw prefix (not yet flattened)`	`pkg/cli/firewall_policy_test.go`	✅ Design	None
`agent-artifacts legacy artifact name (not yet flattened)`	`pkg/cli/firewall_policy_test.go`	✅ Design	None
`workflow_call prefixed agent artifact (not yet flattened)`	`pkg/cli/firewall_policy_test.go`	✅ Design	None

Flagged Tests — Requires Review

None. All new tests are clean.

Language Support

Tests analyzed:

🟨 JavaScript (*.test.cjs, *.test.js): 0 tests

📋 Detailed Analysis Notes

All 4 new sub-tests are added to the existing TestDetectFirewallAuditArtifacts table-driven function. Each covers a distinct real-world artifact directory structure:

New structure — agent/sandbox/firewall/audit/ (upload-artifact v4+ prefix-stripped layout)
Old structure with prefix — agent/tmp/gh-aw/sandbox/firewall/audit/ (legacy full-path preservation)
Legacy artifact name — agent-artifacts/sandbox/firewall/audit/ (old artifact name before rename)
workflow_call prefix — abc123-agent/sandbox/firewall/audit/ (hash-prefixed artifact naming)

Each test:

Uses require.NoError for setup assertions (correct use of require for critical setup)
Asserts on both observable outputs (foundManifest and foundAudit) with descriptive messages
Covers a distinct structural variant (no duplication)
Lines added: test +68, production +54 → inflation ratio 1.26:1 (well below 2:1 threshold)

Verdict

✅ Check passed. 0% of new tests are implementation tests (threshold: 30%). All 4 new tests enforce behavioral contracts by verifying the observable outputs of detectFirewallAuditArtifacts across distinct real-world artifact directory structures.

References: §25181134281

🧪 Test quality analysis by Test Quality Sentinel · ● 519.2K · ◷

github-actions

✅ Test Quality Sentinel: 100/100. Test quality is excellent — 0% of new tests are implementation tests (threshold: 30%). All 4 new sub-tests verify observable outputs of detectFirewallAuditArtifacts across distinct real-world artifact directory structures.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

github-actions · 2026-04-30T18:07:07Z

Commit pushed: e7dba46

🏗️ ADR gate enforced by Design Decision Gate 🏗️

github-actions · 2026-04-30T18:07:07Z

🏗️ Design Decision Gate — ADR Required

This PR makes significant changes to core business logic (122 new lines in pkg/cli/) but does not have a linked Architecture Decision Record (ADR).

AI has analyzed the PR diff and generated a draft ADR to help you get started:

📄 Draft ADR: docs/adr/29360-multi-path-artifact-discovery-for-firewall-audit.md

The draft captures the key architectural decision: a fixed four-step search order in detectFirewallAuditArtifacts that handles flattened and non-flattened artifact layouts, both old and new actions/upload-artifact path conventions, and multiple agent artifact naming schemes.

What to do next

Review the draft ADR committed to your branch — it was generated from the PR diff
Complete any missing sections — add context the AI couldn't infer, refine the decision rationale, and verify the alternatives listed reflect what was actually considered
Reference the ADR in this PR body by adding a line such as:

ADR: ADR-29360: Multi-Path Artifact Discovery Strategy for Firewall Audit Command

Once an ADR is linked in the PR body, this gate will re-run and verify the implementation matches the decision.

Why ADRs Matter

"AI made me procrastinate on key design decisions. Because refactoring was cheap, I could always say 'I'll deal with this later.' Deferring decisions corroded my ability to think clearly."

ADRs create a searchable, permanent record of why the codebase looks the way it does. Future contributors (and your future self) will thank you.

📋 Michael Nygard ADR Format Reference

An ADR must contain these four sections to be considered complete:

Context — What is the problem? What forces are at play?
Decision — What did you decide? Why?
Alternatives Considered — What else could have been done?
Consequences — What are the trade-offs (positive and negative)?

All ADRs are stored in docs/adr/ as Markdown files numbered by PR number (e.g., 29360-multi-path-artifact-discovery-for-firewall-audit.md for PR #29360).

🔒 This PR cannot merge until an ADR is linked in the PR body.

References: §25181134127

🏗️ ADR gate enforced by Design Decision Gate 🏗️ · ● 130K · ◷

github-actions

💥 Automated smoke test review - all systems nominal! The multi-path artifact discovery refactor looks solid. The checkDir closure eliminates the duplicated stat+assign pattern cleanly, and the 4-step search order is well-documented. See inline comments for two minor suggestions.

💥 [THE END] — Illustrated by Smoke Claude · ● 261.7K

github-actions · 2026-04-30T18:29:01Z

+	// checkDir probes dir for policy-manifest.json and audit.jsonl, populating the
+	// return variables for any files not yet found. Returns true when both are found.
+	checkDir := func(dir, label string) bool {
+		if manifestPath == "" {


The checkDir closure is a clean approach that reduces duplication nicely. One suggestion: consider logging the resolved agentDir path once (before probing steps 2 & 3) so debugging is easier when the audit files aren't found — the label strings are hardcoded as agent/... but the actual dir may be agent-artifacts or abc123-agent.

github-actions · 2026-04-30T18:29:01Z

+		// files land at agent/sandbox/firewall/audit/ inside the downloaded artifact dir.
+		dir := t.TempDir()
+		auditDir := filepath.Join(dir, "agent", "sandbox", "firewall", "audit")
+		require.NoError(t, os.MkdirAll(auditDir, 0755))


Good table-driven test coverage for the new artifact layouts. Consider adding a test case where both the primary path AND one of the non-flattened paths exist simultaneously — verifying that the primary path takes priority (step 1 wins) would give extra confidence in the search order contract.

pelikhan · 2026-04-30T18:31:11Z

@copilot review all comments

…ir logs - Guard against findArtifactDir returning a file path: check os.Stat IsDir() before probing for audit files inside the agent artifact directory - Derive log labels from filepath.Base(agentDir) so debug output reflects the actual on-disk name (agent-artifacts, abc123-agent, etc.) instead of the hardcoded "agent/..." string - Add test: file named "agent" does not panic or falsely match Agent-Logs-Url: https://github.com/github/gh-aw/sessions/5534ec16-9018-4e51-8a41-c13438eecaf6 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Copilot · 2026-04-30T18:37:15Z

@copilot review all comments

Both reviewer comments addressed in 7c6b5c6:

Added an os.Stat + IsDir() guard on agentDir before probing for audit files — a file named agent (or agent-artifacts) is now skipped with a log message, not silently traversed.
Labels passed to checkDir are now derived from filepath.Base(agentDir) (e.g. agent-artifacts/sandbox/firewall/audit, abc123-agent/sandbox/firewall/audit) instead of the hardcoded "agent/..." string.

A new subtest (file named 'agent' does not panic or falsely match) covers the first fix.

github-actions · 2026-04-30T18:40:57Z

✅ smoke-ci: safeoutputs CLI comment + comment-memory run (25182884234)

Generated by Smoke CI for issue #29360 · ◷

github-actions · 2026-04-30T18:40:59Z

Comment Memory

CI lights the path
Green checks bloom at dawn
Quiet bots still sing

Note

This comment is managed by comment memory.

It stores persistent context for this thread in the code block at the top of this comment.
Edit only the text inside the backtick fences; workflow metadata and the footer are regenerated automatically.

Learn more about comment memory

Generated by Smoke CI for issue #29360 · ◷

Copilot AI and others added 2 commits April 30, 2026 17:53

revert: remove unrelated frontmatter-full.md documentation change

4d219d1

Agent-Logs-Url: https://github.com/github/gh-aw/sessions/2303d4dd-1c37-4812-aa7c-d740e1a19b96 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Copilot AI assigned Copilot and pelikhan Apr 30, 2026

Copilot created this pull request from a session on behalf of pelikhan April 30, 2026 17:55 View session

Copilot finished work on behalf of pelikhan April 30, 2026 17:55

Copilot AI requested a review from pelikhan April 30, 2026 17:55

pelikhan marked this pull request as ready for review April 30, 2026 18:00

Copilot AI review requested due to automatic review settings April 30, 2026 18:00

Copilot started reviewing on behalf of pelikhan April 30, 2026 18:01 View session

Copilot AI reviewed Apr 30, 2026

View reviewed changes

github-actions Bot approved these changes Apr 30, 2026

View reviewed changes

docs: add draft ADR-29360 for multi-path artifact discovery strategy

e7dba46

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

github-actions Bot mentioned this pull request Apr 30, 2026

Smoke Test: Claude - 25181816514 #29366

Closed

github-actions Bot reviewed Apr 30, 2026

View reviewed changes

Copilot started work on behalf of pelikhan April 30, 2026 18:31 View session

Copilot finished work on behalf of pelikhan April 30, 2026 18:38

Merge branch 'main' into copilot/fix-detect-firewall-audit-artifacts

6c59355

pelikhan merged commit 1feccc4 into main Apr 30, 2026
19 checks passed

pelikhan deleted the copilot/fix-detect-firewall-audit-artifacts branch April 30, 2026 18:38

This was referenced Apr 30, 2026

[aw] No-Op Runs #29134

Open

[aw-failures] Fix: Smoke Claude safe_outputs fails when resolve_pull_request_review_thread targets already-resolved thread #29372

Closed

github-actions Bot mentioned this pull request Apr 30, 2026

[aw-failures] [aw] Failure Investigator (6h) - Issue Group #29232

Open

Copilot AI mentioned this pull request Apr 30, 2026

fix(smoke-claude): resolve only unresolved review threads in test #16 #29373

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: extend detectFirewallAuditArtifacts to find audit files in non-flattened agent artifact#29360

fix: extend detectFirewallAuditArtifacts to find audit files in non-flattened agent artifact#29360
pelikhan merged 5 commits intomainfrom
copilot/fix-detect-firewall-audit-artifacts

Copilot AI commented Apr 30, 2026 •

edited by github-actions Bot

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

github-actions Bot commented Apr 30, 2026

Uh oh!

github-actions Bot left a comment

Uh oh!

github-actions Bot commented Apr 30, 2026

Uh oh!

github-actions Bot commented Apr 30, 2026

Uh oh!

github-actions Bot left a comment

Uh oh!

github-actions Bot Apr 30, 2026

Uh oh!

github-actions Bot Apr 30, 2026

Uh oh!

pelikhan commented Apr 30, 2026

Uh oh!

Copilot AI commented Apr 30, 2026

Uh oh!

Uh oh!

github-actions Bot commented Apr 30, 2026

Uh oh!

github-actions Bot commented Apr 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Copilot AI commented Apr 30, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Copilot's findings

Uh oh!

github-actions Bot commented Apr 30, 2026

🧪 Test Quality Sentinel Report

Test Quality Score: 100/100

Test Classification Details

Flagged Tests — Requires Review

Language Support

Verdict

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Apr 30, 2026

Uh oh!

github-actions Bot commented Apr 30, 2026

🏗️ Design Decision Gate — ADR Required

What to do next

Why ADRs Matter

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

pelikhan commented Apr 30, 2026

Uh oh!

Copilot AI commented Apr 30, 2026

Uh oh!

Uh oh!

github-actions Bot commented Apr 30, 2026

Uh oh!

github-actions Bot commented Apr 30, 2026

Comment Memory

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Copilot AI commented Apr 30, 2026 •

edited by github-actions Bot

Loading