Skip to content

Fix SC2015 shellcheck errors in generated "Collect usage artifact files" step - #49845

Merged
pelikhan merged 5 commits into
mainfrom
copilot/fix-action-error-91538435540
Aug 2, 2026
Merged

Fix SC2015 shellcheck errors in generated "Collect usage artifact files" step#49845
pelikhan merged 5 commits into
mainfrom
copilot/fix-action-error-91538435540

Conversation

Copilot AI commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

TestPullRequestTargetCheckoutFalseWithImports/strict_mode was failing because strict-mode compilation runs shellcheck on generated YAML, and shellcheck flagged SC2015 on A && B || C patterns in the "Collect usage artifact files" step emitted by notify_comment.go.

Changes

  • pkg/workflow/notify_comment.go: Replace all A && B || C shell patterns with proper if/then/fi constructs:

    - "          [ -f /tmp/gh-aw/aw_info.json ] && cp /tmp/gh-aw/aw_info.json /tmp/gh-aw/usage/aw_info.json || true\n",
    + "          if [ -f /tmp/gh-aw/aw_info.json ]; then cp /tmp/gh-aw/aw_info.json /tmp/gh-aw/usage/aw_info.json; fi\n",

    Covers 14 patterns: the echo "FOUND/MISSING" diagnostic line inside the for loop, 7 [ -f ] && cp || true file copies, and 6 [ -s ] && cp || true token-usage copies.

  • .github/workflows/*.lock.yml: Recompiled all 269 workflows to propagate the updated shell syntax.


Generated by 👨‍🍳 PR Sous Chef · gpt54 · 13.2 AIC · ⌖ 8.74 AIC · ⊞ 8.3K ·
Comment /souschef to run again


Generated by 👨‍🍳 PR Sous Chef · gpt54 · 11.1 AIC · ⌖ 8 AIC · ⊞ 8.3K ·
Comment /souschef to run again

Replace `A && B || C` patterns with `if/then/fi` constructs in the
shell script generated by notify_comment.go. ShellCheck SC2015 warns
that `A && B || C` is not if-then-else because C may run when A is
true but B fails. The strict-mode compile test was failing because
of these patterns in the generated lock files.

Recompile all 269 workflows to update generated .lock.yml files.

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title Fix SC2015 shellcheck errors in Collect usage artifact files step Fix SC2015 shellcheck errors in generated "Collect usage artifact files" step Aug 2, 2026
Copilot AI requested a review from pelikhan August 2, 2026 20:08
@pelikhan
pelikhan marked this pull request as ready for review August 2, 2026 20:09
Copilot AI review requested due to automatic review settings August 2, 2026 20:09

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates generated usage-artifact collection shell code to avoid ShellCheck SC2015 warnings.

Changes:

  • Replaces A && B || C patterns with explicit conditionals.
  • Regenerates 269 workflow lock files.
Show a summary per file
File Description
pkg/workflow/notify_comment.go Updates generated shell commands.
.github/workflows/*.lock.yml (269 files) Propagates updated shell syntax.

Review details

Tip

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

  • Files reviewed: 179/270 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread pkg/workflow/notify_comment.go Outdated
Comment on lines +144 to +148
" if [ -f /tmp/gh-aw/aw_info.json ]; then cp /tmp/gh-aw/aw_info.json /tmp/gh-aw/usage/aw_info.json; fi\n",
" if [ -f /tmp/gh-aw/aw-info.jsonl ]; then cp /tmp/gh-aw/aw-info.jsonl /tmp/gh-aw/usage/aw-info.jsonl; fi\n",
" if [ -f /tmp/gh-aw/agent_usage.json ]; then cp /tmp/gh-aw/agent_usage.json /tmp/gh-aw/usage/agent_usage.json; fi\n",
" if [ -f /tmp/gh-aw/agent_usage.jsonl ]; then cp /tmp/gh-aw/agent_usage.jsonl /tmp/gh-aw/usage/agent_usage.jsonl; fi\n",
" if [ -f /tmp/gh-aw/detection_usage.jsonl ]; then cp /tmp/gh-aw/detection_usage.jsonl /tmp/gh-aw/usage/detection_usage.jsonl; fi\n",
@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot Please continue triage on this PR.

Remaining visible blockers (newest first):

Failed checks:

Run details: https://github.com/github/gh-aw/actions/runs/30765233636

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 13.2 AIC · ⌖ 8.74 AIC · ⊞ 8.3K ·
Comment /souschef to run again

@@ -1132,21 +1132,21 @@ jobs:
mkdir -p /tmp/gh-aw/usage/agent /tmp/gh-aw/usage/detection

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot refactor to a shell script in actions/setup/sh instead of inlining

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Extracted the inline shell code into actions/setup/sh/collect_usage_artifact_files.sh and updated the "Collect usage artifact files" step to invoke it via:

run: bash "${RUNNER_TEMP}/gh-aw/actions/collect_usage_artifact_files.sh"

Tests in notify_comment_test.go were updated to check the script file content directly for the logic assertions (copy commands, -s checks, priority ordering, node invocation) instead of the inline YAML steps. All 269 workflows were recompiled. Commit: 6bd6900.

- Add || true after cp commands inside if/then/fi blocks in
  notify_comment.go to preserve best-effort behavior under bash -e
- Sync pkg/workflow/js/safe_outputs_tools.json push_to_pull_request_branch
  tool to match actions/setup/js/safe_outputs_tools.json (adds repo
  parameter, updates x-safe-outputs-target-requirements, updates desc)

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

Copilot AI commented Aug 2, 2026

Copy link
Copy Markdown
Contributor Author

@copilot Please continue triage on this PR.

Remaining visible blockers (newest first):...

Done. Two changes in the latest commit:

  1. Best-effort cp behavior (reviewer feedback): Added || true inside each if/then/fi branch so that a cp failure under bash -e doesn't abort the rest of the collection step — e.g. if [ -f src ]; then cp src dst || true; fi.

  2. TestSafeOutputsToolsJSONInSync / impacted-go-tests failures: Commit 048649d on main updated actions/setup/js/safe_outputs_tools.json for push_to_pull_request_branch (adding a repo parameter, updating x-safe-outputs-target-requirements, and updating the description) without updating the mirror copy in pkg/workflow/js/safe_outputs_tools.json. The sync test therefore failed after the merge. Both copies are now identical.

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI requested a review from pelikhan August 2, 2026 21:13
@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot Please continue triage on this PR.

Remaining visible blockers (newest first):

  • reviewer follow-up appears addressed by your latest reply; please verify the maintainer-visible concerns are fully resolved
  • if the branch is behind or needs metadata refresh, update the branch
  • then run the pr-finisher skill and leave a concise maintainer handoff

Run details: https://github.com/github/gh-aw/actions/runs/30767332278

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 11.1 AIC · ⌖ 8 AIC · ⊞ 8.3K ·
Comment /souschef to run again

@github-actions

github-actions Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Thanks for working on this! 🙌 The shell syntax fix in pkg/workflow/notify_comment.go is solid—replacing A && B || C patterns with proper if/then/fi constructs to satisfy shellcheck is the right call.

However, there are a couple of things that would strengthen this:

  • Add test coverage — The PR references a failing test (TestPullRequestTargetCheckoutFalseWithImports/strict_mode), but there's no new test added to verify the fix works or prevent regression. Unit tests covering the shell output from notify_comment.go would confirm the patterns are now compliant.
  • Large mechanical diff — 270 of 271 changed files are auto-generated lock files. While lock-file regeneration is expected after a source change, consider whether there's a way to separate or minimize this bulk to make the actual fix (the Go source change) easier to review. Reviewers should focus on the 1 source file, not scroll through hundreds of generated files.

If you'd like help, you can assign this prompt to your agent:

Add unit tests for pkg/workflow/notify_comment.go that verify the shell commands generated in the "Collect usage artifact files" step use proper if/then/fi constructs instead of A && B || C patterns. Confirm the test passes with strict-mode compilation and shellcheck validation enabled.

Generated by ✅ Contribution Check · auto · 46.3 AIC · ⌖ 17.4 AIC · ⊞ 8.8K ·

@pelikhan
pelikhan merged commit 009999a into main Aug 2, 2026
7 of 8 checks passed
@pelikhan
pelikhan deleted the copilot/fix-action-error-91538435540 branch August 2, 2026 21:33
Copilot stopped work on behalf of gh-aw-bot due to an error August 2, 2026 21:34
@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

🎉 This pull request is included in a new release.

Release: v0.84.3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants