Deduplicate GitHub Script step scaffold in safe_outputs_steps.go

[Copilot]

buildGitHubScriptStep and buildGitHubScriptStepWithoutDownload were near-identical implementations of the same YAML step scaffold (name/id/uses/env/with/script), differing only in artifact download handling and an optional if: condition — leaving two places to update and room for drift.

Changes

• Added buildGitHubScriptStepCommon(data, config, includeDownload bool) []string holding the single copy of the scaffold: step metadata, optional StepCondition, env vars, token selection, and script rendering (inline or require()).
• Reduced both public builders to thin wrappers that keep their existing debug logging and pass includeDownload true/false.

Behavior is unchanged; the download-variant still emits buildAgentOutputDownloadSteps first, and StepCondition is honored in both paths (previously only wired in the no-download variant, which was the only caller setting it). Compiled .lock.yml files are byte-identical.

func (c *Compiler) buildGitHubScriptStep(data *WorkflowData, config GitHubScriptStepConfig) []string {
	safeOutputsStepsLog.Printf(...)
	return c.buildGitHubScriptStepCommon(data, config, true)
}

func (c *Compiler) buildGitHubScriptStepWithoutDownload(data *WorkflowData, config GitHubScriptStepConfig) []string {
	safeOutputsStepsLog.Printf(...)
	return c.buildGitHubScriptStepCommon(data, config, false)
}

• Fixes [duplicate-code] Duplicate Code: Repeated GitHub Script Step Scaffold #50104

---

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

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

[github-actions]

✅ Test Quality Sentinel completed test quality analysis.

No test files were added or modified in this PR. PR #50105 is a production code refactoring (safe_outputs_steps.go deduplication with net −36 lines) with no test changes. Test Quality Sentinel analysis skipped.

[github-actions]

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

[github-actions]

✅ PR Code Quality Reviewer completed the code quality review.

[github-actions]

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

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

[Copilot]

Pull request overview

Deduplicates GitHub Script step generation while preserving wrapper behavior.

Changes:

• Extracts the shared scaffold into one helper.
• Keeps download handling configurable.
• Applies optional step conditions consistently.

Show a summary per file

File	Description
pkg/workflow/safe_outputs_steps.go	Consolidates duplicated step-building logic.

Review details

Tip

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

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

[github-actions]

Clean, correct refactoring. The deduplication is well-structured and the StepCondition fix (now active in both paths) is a genuine improvement. No issues found.

🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · sonnet46 · 15.6 AIC · ⌖ 9.27 AIC · ⊞ 5.4K

[github-actions]

Skills-Based Review 🧠

Applied /tdd and /codebase-design — one suggestion on test coverage.

📋 Summary

Positive Highlights

• ✅ Clean extraction of buildGitHubScriptStepCommon — single source of truth, no drift risk
• ✅ Both public methods correctly reduced to thin logging wrappers
• ✅ The StepCondition fix (now honored in the download path too) is a real correctness improvement
• ✅ Compiled .lock.yml output is byte-identical — safe to land

One Gap

The StepCondition behavior in the download path is newly enabled but untested. Inline comment added with a suggested test skeleton.

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

Comments that could not be inline-anchored

pkg/workflow/safe_outputs_steps.go:177

[/tdd] StepCondition is now active for the download path (buildGitHubScriptStep) for the first time, but there is no test covering this behavior.

The PR description notes this as an intentional fix ("previously only wired in the no-download variant"). A unit test asserting that if: always() appears in the generated YAML when StepCondition is set via the download path would prevent future regression.

<details>
<summary>💡 Suggested test skeleton</summary>

func TestBuildGitHu…

</details>

[github-actions]

Verdict: COMMENT

Clean, low-risk deduplication with no behavior change.

Analysis

• Verified the only caller of buildGitHubScriptStep (safe_outputs_jobs.go:106) never sets StepCondition, confirming the PR's claim that behavior is unchanged for that path.
• Ran TestBuildGitHubScriptStep* suite — all pass.
• The shared buildGitHubScriptStepCommon correctly gates the artifact-download steps behind includeDownload, matching prior per-function logic.
• No correctness, concurrency, or security issues found in the diff. Naming and structure are reasonable; no dead code or duplication introduced.

🔎 Code quality review by PR Code Quality Reviewer · auto · 33.9 AIC · ⌖ 5.03 AIC · ⊞ 7.9K
_{Comment /review to run again}

[gh-aw-bot]

@copilot quick triage for this PR:

• Please address unresolved review threads, newest first:
  • copilot-pull-request-reviewer: Deduplicate GitHub Script step scaffold in safe_outputs_steps.go #50105 (comment)
• The branch refresh was requested for this run.
• After fixing the above, run the pr-finisher skill and resolve addressed threads.

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

[github-actions]

🎉 This pull request is included in a new release.

Release: v0.84.4