Refactor: Eliminate duplicate safe output env logic in workflow helpers

[Copilot]

Problem

Safe output environment configuration logic was duplicated across multiple workflow helper files, making maintenance difficult and error-prone. When adding new environment variables, developers had to update the logic in multiple places, increasing the risk of inconsistent behavior.

The duplicate code pattern appeared in:

• create_pr_review_comment.go (lines 40-48)
• create_discussion.go (lines 73-82)

Each file independently implemented the same logic for handling GITHUB_AW_SAFE_OUTPUTS_STAGED and GITHUB_AW_TARGET_REPO_SLUG environment variables:

if c.trialMode || data.SafeOutputs.Staged {
    customEnvVars = append(customEnvVars, "          GITHUB_AW_SAFE_OUTPUTS_STAGED: \"true\"\n")
}
if data.SafeOutputs.CreateX.TargetRepoSlug != "" {
    customEnvVars = append(customEnvVars, fmt.Sprintf("          GITHUB_AW_TARGET_REPO_SLUG: %q\n", ...))
} else if c.trialMode && c.trialLogicalRepoSlug != "" {
    customEnvVars = append(customEnvVars, fmt.Sprintf("          GITHUB_AW_TARGET_REPO_SLUG: %q\n", ...))
}

Solution

Consolidated the duplicate logic by utilizing the existing buildSafeOutputJobEnvVars helper function that was already available in safe_output_helpers.go. This helper was previously used by add_comment.go but not by the other safe output job builders.

The refactored code now uses a single, consistent approach:

// Add common safe output job environment variables (staged/target repo)
customEnvVars = append(customEnvVars, buildSafeOutputJobEnvVars(
    c.trialMode,
    c.trialLogicalRepoSlug,
    data.SafeOutputs.Staged,
    data.SafeOutputs.CreateX.TargetRepoSlug,
)...)

Benefits

Single Source of Truth: All safe output job builders now use the same helper function:

• ✅ add_comment.go (already using helper)
• ✅ create_pr_review_comment.go (now using helper)
• ✅ create_discussion.go (now using helper)

Improved Maintainability: Future environment variable additions only require changes in one location (buildSafeOutputJobEnvVars), not across multiple files.

Consistent Behavior: Eliminates the risk of divergent implementations causing subtle bugs across different safe output types.

Code Reduction: Eliminated 18 lines of duplicate code while maintaining identical functionality.

Testing

Added comprehensive test suite (safe_output_refactor_test.go) with 5 test cases:

• Verifies PR review comment job uses helper correctly
• Verifies discussion job uses helper correctly
• Tests trial mode without explicit target-repo
• Tests scenarios with no staged flag or trial mode
• Verifies explicit target-repo overrides trial mode

All existing tests continue to pass, confirming no breaking changes.

Related Issue

Addresses Pattern 2 from issue #[duplicate-code]: "Staged/target repo env logic reimplemented instead of using helper"

Original prompt

---

This section details on the original issue you should resolve

<issue_title>[duplicate-code] Refactor safe output env duplication in workflow helpers</issue_title>
<issue_description># 🔍 Duplicate Code Detected

Analysis of commit c0280e9

Assignee: @copilot

Summary

Safe output environment configuration logic is duplicated across multiple workflow helpers, increasing the maintenance surface when adding new environment variables.

Duplication Details

Pattern 1: Safe output env injection duplicated for map vs slice

• Severity: Medium
• Occurrences: 2
• Locations:
  • pkg/workflow/safe_output_helpers.go:259
  • pkg/workflow/safe_output_helpers.go:287
• Code Sample:

if data.SafeOutputs == nil {
    return
}

*stepLines = append(*stepLines, "          GITHUB_AW_SAFE_OUTPUTS: ${{ env.GITHUB_AW_SAFE_OUTPUTS }}")
...
if workflowData.SafeOutputs.UploadAssets != nil {
    *stepLines = append(*stepLines, fmt.Sprintf("          GITHUB_AW_ASSETS_BRANCH: %q", workflowData.SafeOutputs.UploadAssets.BranchName))
    *stepLines = append(*stepLines, fmt.Sprintf("          GITHUB_AW_ASSETS_MAX_SIZE_KB: %d", workflowData.SafeOutputs.UploadAssets.MaxSizeKB))
    *stepLines = append(*stepLines, fmt.Sprintf("          GITHUB_AW_ASSETS_ALLOWED_EXTS: %q", strings.Join(workflowData.SafeOutputs.UploadAssets.AllowedExts, ",")))
}

Pattern 2: Staged/target repo env logic reimplemented instead of using helper

• Severity: Medium
• Occurrences: 2
• Locations:
  • pkg/workflow/create_pr_review_comment.go:15
  • pkg/workflow/safe_output_helpers.go:313
• Code Sample:

if c.trialMode || data.SafeOutputs.Staged {
    customEnvVars = append(customEnvVars, "          GITHUB_AW_SAFE_OUTPUTS_STAGED: \"true\"\n")
}
if data.SafeOutputs.CreatePullRequestReviewComments.TargetRepoSlug != "" {
    customEnvVars = append(customEnvVars, fmt.Sprintf("          GITHUB_AW_TARGET_REPO_SLUG: %q\n", data.SafeOutputs.CreatePullRequestReviewComments.TargetRepoSlug))
} else if c.trialMode && c.trialLogicalRepoSlug != "" {
    customEnvVars = append(customEnvVars, fmt.Sprintf("          GITHUB_AW_TARGET_REPO_SLUG: %q\n", c.trialLogicalRepoSlug))
}

Impact Analysis

• Maintainability: Each new safe output knob requires touching multiple sites, increasing the chance of inconsistent behavior.
• Bug Risk: Divergent environment handling can cause stage-only fixes to miss specific job types.
• Code Bloat: Repeated blocks inflate helpers and obscure the intended variations.

Refactoring Recommendations

1. Extract shared safe output env builders

   • Extract a reusable helper that returns the common env entries (map and slice variants) so both call sites share the same implementation.
   • Estimated effort: 3h
   • Benefits: Single source of truth for future env additions.

2. Adopt buildSafeOutputJobEnvVars for PR review comment job

   • Reuse the shared helper (or extend it) instead of open-coding staged/target logic in buildCreateOutputPullRequestReviewCommentJob.
   • Estimated effort: 2h
   • Benefits: Keeps staged handling consistent with other safe output job builders.

Implementation Checklist

• Review duplication findings
• Prioritize refactoring tasks
• Create refactoring plan
• Implement changes
• Update tests
• Verify no functionality broken

Analysis Metadata

• Analyzed Files: 4
• Detection Method: Serena semantic code analysis
• Commit: c0280e9
• Analysis Date: 2025-10-20 11:10:20Z

AI generated by Duplicate Code Detector</issue_description>

Comments on the Issue (you are @copilot in this section)

Fixes #2022

---

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

[github-actions]

Agentic Changeset Generator triggered by this pull request.