Fix SC2015 ambiguous shell pattern in generated firewall log copy step by Copilot · Pull Request #54747 · github/gh-aw

Copilot · 2026-08-22T06:18:01Z

Static analysis flagged shellcheck SC2015 across 275 compiled workflows: the auto-generated "Copy detection firewall logs" step used [ -d X ] && mkdir -p Y && cp -r X/. Y/ || true, where the trailing || true can silently swallow failures from mkdir -p/cp -r (not just a false -d test), masking real log-copy errors.

Compiler fix

In pkg/workflow/threat_detection_steps.go, rewrote both generated lines from A && B && C || true to explicit if [ -d X ]; then B && C; fi, so a missing directory is a no-op while genuine mkdir/cp failures are no longer suppressed.

// Before
fmt.Sprintf("          [ -d %s ] && mkdir -p %s/logs && cp -r %s/. %s/logs/ || true\n", ...)

// After
fmt.Sprintf("          if [ -d %s ]; then mkdir -p %s/logs && cp -r %s/. %s/logs/; fi\n", ...)

Regenerated workflows

Ran make recompile to propagate the single source change into all 275 affected .lock.yml files with firewall/threat-detection enabled.

Fixes [static-analysis] Report - 2026-08-22 #54728

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 33 AIC · ⌖ 8.29 AIC · ⊞ 9.5K · ◷
_{Comment /souschef to run again}

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

github-actions · 2026-08-22T06:27:28Z

PR Triage

Category: chore
Risk: low
Priority score: 30/100 (impact 10, urgency 5, quality 15)
Recommended action: defer
CI status: pending

Generated by 🔧 PR Triage Agent · auto · 82.9 AIC · ⌖ 6.65 AIC · ⊞ 8.3K · ◷

github-actions · 2026-08-22T09:03:36Z

Thanks for addressing the shellcheck SC2015 linting issue across all 275 workflows! 🚀 The fix is well-focused and correctly rewrites the ambiguous shell pattern from A && B && C || true to explicit if [ -d X ]; then B && C; fi, which prevents silent error masking in the firewall log-copy step.

Here's what would help this PR cross the finish line:

Add test coverage — The compiler change in pkg/workflow/threat_detection_steps.go currently lacks unit tests. Please add:
1. A unit test in pkg/workflow/ (e.g. threat_detection_test.go) verifying that buildCopyDetectionFirewallLogsStep() generates the new if [ -d ... ]; then ... ; fi pattern (not the old && || true form).
2. Optionally, a generated-workflow snapshot/integration test confirming a regenerated .lock.yml contains the corrected step with no SC2015 warnings via shellcheck.

Once test coverage is added, this PR should be ready for review!

If you'd like to hand this to your coding agent:

Add unit test coverage for the SC2015 shellcheck fix in pkg/workflow/threat_detection_steps.go.

Test requirements:
1. Create or update a test in pkg/workflow/ (e.g., threat_detection_test.go or compiler_test.go) that calls buildCopyDetectionFirewallLogsStep() and verifies:
   - The returned string contains "if [ -d" (new pattern)
   - The returned string does NOT contain "|| true" (old pattern is gone)
   - Both the /logs and /audit copy blocks are fixed

2. Optional: Generate a sample workflow with threat-detection enabled and verify no SC2015 linting violations appear in the generated firewall log-copy step.

Ensure tests pass with `go test ./pkg/workflow/...` (or the project's test command).

Generated by ✅ Contribution Check · auto · 51 AIC · ⌖ 5.08 AIC · ⊞ 9.2K · ◷

github-actions · 2026-08-22T12:26:09Z

PR Triage

Category: chore | Risk: low | Priority: low | Score: 23/100
Recommended action: defer

Draft. Mechanical shellcheck SC2015 fix across 276 generated lockfiles. Blocked mergeable state.

_{Automated triage — run 32572524009}

Generated by 🔧 PR Triage Agent · auto · 65.4 AIC · ⌖ 6.04 AIC · ⊞ 8.3K · ◷

Copilot

Pull request overview

Replaces the ambiguous firewall-log copy shell chain with explicit conditionals so copy failures remain visible.

Changes:

Updates the compiler-generated logs and audit copy commands.
Regenerates all 275 affected workflow lock files.

Show a summary per file

File	Description
`pkg/workflow/threat_detection_steps.go`	Generates explicit conditional copy commands.
`.github/workflows/*.lock.yml` (275 files)	Propagates the corrected generated step across affected workflows.

Review details

Files reviewed: 276/276 changed files
Comments generated: 1
Review effort level: Balanced

+		fmt.Sprintf("          if [ -d %s ]; then mkdir -p %s/logs && cp -r %s/. %s/logs/; fi\n", proxyLogsDir, detectionFirewallLogsDir, proxyLogsDir, detectionFirewallLogsDir),
+		fmt.Sprintf("          if [ -d %s ]; then mkdir -p %s/audit && cp -r %s/. %s/audit/; fi\n", auditDir, detectionFirewallLogsDir, auditDir, detectionFirewallLogsDir),


github-actions · 2026-08-22T13:01:33Z

✅ PR Code Quality Reviewer completed the code quality review.

🔎 Code quality review by PR Code Quality Reviewer

github-actions · 2026-08-22T13:01:34Z

✅ Design Decision Gate 🏗️ completed the design decision gate check.

No ADR enforcement needed: PR does not have the implementation label and has only 2 new lines of code in business logic directories (threshold: 100).

🏗️ ADR gate enforced by Design Decision Gate 🏗️

github-actions · 2026-08-22T13:01:44Z

🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer

github-actions · 2026-08-22T13:01:57Z

✅ Test Quality Sentinel completed test quality analysis.

No test files were added or modified in this PR. This PR contains only workflow lock file updates (.lock.yml). Test Quality Sentinel skipped.

🧪 Test quality analysis by Test Quality Sentinel

github-actions · 2026-08-22T13:02:24Z

✅ Ponytail Reviewer completed successfully!

Warning

Threat Detection Engine Failure — The analysis engine could not complete. This is a tooling failure, not a security finding.

What happened

The threat detection engine failed to produce results.

Review the workflow run logs for details.

Generated by Ponytail Reviewer for #54747

github-actions · 2026-08-22T13:05:12Z

Comment Memory

reviewed_at: 2026-08-22T00:00:00Z
review_event: COMMENT
top_themes:
  - existing review already covers missing regression assertion for explicit if-form generation
  - no additional blocking correctness or maintainability issues found in changed lines
files_reviewed:
  - pkg/workflow/threat_detection_steps.go
  - pkg/workflow/threat_detection_test.go
  - .github/workflows/*.lock.yml
comment_count: 0

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

🔎 Code quality review by PR Code Quality Reviewer · gpt54 · 8.69 AIC · ⌖ 8.6 AIC · ⊞ 7K · ◷
_{Comment /review to run again}

github-actions

Verdict

No new blocking issues from this patch.

The only actionable gap I found is already covered by an existing review comment.

The compiler change from A && B && C || true to explicit if guards is directionally correct and the regenerated workflow output matches it. I did not find an additional correctness, performance, or maintainability problem in the changed lines beyond the already-raised test regression gap.

🔎 Code quality review by PR Code Quality Reviewer · gpt54 · 8.69 AIC · ⌖ 8.6 AIC · ⊞ 7K
_{Comment /review to run again}

github-actions

Skills-Based Review 🧠

Applied /diagnosing-bugs — the SC2015 fix is correct and the root cause is properly addressed at the source.

📋 Summary

What's good

✅ Root-cause fix is in the generator (threat_detection_steps.go), not just patched in individual workflows — all 275 regenerated .lock.yml files are consistent mechanical output.
✅ The rewrite from A && B && C || true to if [ -d X ]; then B && C; fi correctly scopes the no-op to a missing directory only, while letting real mkdir/cp failures propagate — exactly what SC2015 demands.
✅ The two-line pattern (logs + audit) is handled symmetrically.

One open item

An existing review comment already requests a regression test asserting the generated script uses the if form and does not contain || true. That test would lock in the fix and prevent future regressions. Approving here because the production code is correct; the test gap is already tracked.

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 · 30.3 AIC · ⌖ 9.73 AIC · ⊞ 7.6K
_{Comment /matt to run again}

github-actions

The SC2015 fix is correct and consistently applied across all lock files.

The old pattern [ -d dir ] && mkdir && cp || true had a subtle bug: the || true swallowed failures from mkdir and cp, not just the directory-existence test. The replacement if [ -d dir ]; then mkdir -p ... && cp -r ...; fi is semantically correct: skips when the directory is absent and correctly propagates errors from the copy/mkdir commands.

No other concerns.

🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · sonnet46 · 26.1 AIC · ⌖ 8.96 AIC · ⊞ 6.2K

gh-aw-bot · 2026-08-22T13:46:33Z

@copilot this PR still needs forward progress before maintainer review.

Failed checks to address:

impacted-go-tests — https://github.com/github/gh-aw/actions/runs/32556989804/job/97033227271
impacted-go-tests — https://github.com/github/gh-aw/actions/runs/32573858109/job/97034898559
lint-go-custom — https://github.com/github/gh-aw/actions/runs/32556989804/job/97033227342
lint-go-custom — https://github.com/github/gh-aw/actions/runs/32573858109/job/97034898707
Unit tests (M-R) — https://github.com/github/gh-aw/actions/runs/32573858109/job/97035043070

Please address the remaining failures, refresh the branch if needed, and use the pr-finisher skill before pushing the next revision.

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 33 AIC · ⌖ 8.29 AIC · ⊞ 9.5K · ◷
_{Comment /souschef to run again}

Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>

Copilot · 2026-08-22T14:00:41Z

@copilot this PR still needs forward progress before maintainer review.

Failed checks to address:...

Added regression assertions in b01f982 requiring explicit directory guards and rejecting || true. Local build, lint, focused test, and full test suite pass. The impacted-test CI failure is a missing origin/main merge base; custom-lint failures are pre-existing unrelated findings (including pkg/cli/trial_runner.go).

pelikhan · 2026-08-22T15:10:55Z

@copilot resolve the merge conflicts on this branch.

# Conflicts: # pkg/workflow/threat_detection_test.go Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Copilot · 2026-08-22T15:29:18Z

@copilot resolve the merge conflicts on this branch.

Resolved and pushed in merge commit 745924d0cc. The regression assertions were moved into the firewall-focused test file created by the current main branch.

Initial plan

feda680

Copilot AI assigned Copilot and pelikhan Aug 22, 2026

Copilot AI linked an issue Aug 22, 2026 that may be closed by this pull request

[static-analysis] Report - 2026-08-22 #54728

Closed

Copilot started work on behalf of pelikhan August 22, 2026 06:18 View session

Fix SC2015 in firewall log copy step: use if/then instead of &&/||

be9ad3e

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

github-actions Bot added pr-action:defer pr-agent:copilot pr-priority:low pr-risk:low pr-type:chore labels Aug 22, 2026

Copilot AI changed the title ~~[WIP] Report findings from static analysis scan~~ Fix SC2015 ambiguous shell pattern in generated firewall log copy step Aug 22, 2026

Copilot finished work on behalf of pelikhan August 22, 2026 06:27

Copilot AI requested a review from pelikhan August 22, 2026 06:27

This was referenced Aug 22, 2026

[PR Triage Report] PR Triage Report — 2026-08-22 (Run 32556532240) #54751

Closed

[Contribution Check Report] Contribution Check — 2026-08-22 #54682

Open

github-actions Bot mentioned this pull request Aug 22, 2026

[PR Triage Report] PR Triage Report — Run 32572524009 (20 PRs) #54789

Closed

pelikhan marked this pull request as ready for review August 22, 2026 12:45

Copilot AI balanced review requested due to automatic review settings August 22, 2026 12:45

Copilot started reviewing on behalf of pelikhan August 22, 2026 12:46 View session

Copilot AI reviewed Aug 22, 2026

View reviewed changes

github-actions Bot mentioned this pull request Aug 22, 2026

[pr-sous-chef] PR Sous Chef run report #54661

Closed

github-actions Bot reviewed Aug 22, 2026

View reviewed changes

github-actions Bot approved these changes Aug 22, 2026

View reviewed changes

github-actions Bot mentioned this pull request Aug 22, 2026

[aw] Ponytail Reviewer produced no safe outputs #54801

Open

github-actions Bot added the needs-review label Aug 22, 2026

Copilot started work on behalf of gh-aw-bot August 22, 2026 13:46 View session

Add firewall log copy regression test

b01f982

Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>

Copilot finished work on behalf of gh-aw-bot August 22, 2026 14:01

Copilot AI requested a review from gh-aw-bot August 22, 2026 14:01

Copilot started work on behalf of pelikhan August 22, 2026 15:11 View session

Merge origin/main into copilot/static-analysis-2026-08-22

745924d

# Conflicts: # pkg/workflow/threat_detection_test.go Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Copilot finished work on behalf of pelikhan August 22, 2026 15:29

pelikhan merged commit 151b20e into main Aug 22, 2026
31 checks passed

pelikhan deleted the copilot/static-analysis-2026-08-22 branch August 22, 2026 16:05

		fmt.Sprintf(" if [ -d %s ]; then mkdir -p %s/logs && cp -r %s/. %s/logs/; fi\n", proxyLogsDir, detectionFirewallLogsDir, proxyLogsDir, detectionFirewallLogsDir),
		fmt.Sprintf(" if [ -d %s ]; then mkdir -p %s/audit && cp -r %s/. %s/audit/; fi\n", auditDir, detectionFirewallLogsDir, auditDir, detectionFirewallLogsDir),

Uh oh!

Conversation

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

Uh oh!

Compiler fix

Regenerated workflows

Uh oh!

github-actions Bot commented Aug 22, 2026

PR Triage

Uh oh!

github-actions Bot commented Aug 22, 2026

Uh oh!

github-actions Bot commented Aug 22, 2026

PR Triage

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Review details

Uh oh!

github-actions Bot commented Aug 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Aug 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Aug 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Aug 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Aug 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Aug 22, 2026

Comment Memory

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Verdict

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Skills-Based Review 🧠

What's good

One open item

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

gh-aw-bot commented Aug 22, 2026

Uh oh!

Copilot AI commented Aug 22, 2026

Uh oh!

pelikhan commented Aug 22, 2026

Uh oh!

Copilot AI commented Aug 22, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Copilot AI commented Aug 22, 2026 •

edited by github-actions Bot

Loading

github-actions Bot commented Aug 22, 2026 •

edited

Loading

github-actions Bot commented Aug 22, 2026 •

edited

Loading

github-actions Bot commented Aug 22, 2026 •

edited

Loading

github-actions Bot commented Aug 22, 2026 •

edited

Loading

github-actions Bot commented Aug 22, 2026 •

edited

Loading