refactor(workflow): deduplicate logic, extract cross-engine helpers, fix interface bypass

[Copilot]

• Issue 7: Merged safe_output_config.go (single function) into safe_outputs_config.go; deleted thin file
• Issue 6: Eliminated parseToolCalls; call site now uses parseToolCallsWithSequence (sequence return value explicitly discarded with comment)
• Issue 4: Added shouldRewriteLocalhostToDocker helper in mcp_config_utils.go; replaced 4 inline guard expressions
• Issue 10: Defined noOpCacheMemoryRenderer in mcp_config_utils.go; removed redundant engine methods from Claude and Gemini
• Issue 5: Moved generateSquidLogsUploadStep / generateFirewallLogParsingStep from copilot_installer.go to engine_firewall_support.go
• Issue 2: Extracted defaultGetSquidLogsSteps to engine_firewall_support.go; all three engines delegate to it
• Issue 9: Added GetFirewallLogsCollectionStep to WorkflowExecutor interface + BaseEngine default; replaced 4-block type-switch (including duplicate CodexEngine case) with single polymorphic call
• Issue 1: Created missing_issue_reporting.go with shared IssueReportingConfig, buildIssueReportingJob, parseIssueReportingConfig; type aliases for backward compatibility; each missing_*.go slimmed to ~30 lines
• Recompiled all 161 workflow lock files (no changes — output identical)

Original prompt

---

This section details on the original issue you should resolve

<issue_title>[refactor] Semantic Function Clustering Analysis: Duplicates, Misplaced Logic, and Fragmented Files in pkg/workflow</issue_title>
<issue_description>Automated semantic analysis of the pkg/ directory (563 non-test Go files across 18 packages, 280 in pkg/workflow/) identified 10 concrete refactoring opportunities. Findings are ordered by estimated impact.

Summary

• Total Go files analyzed: 563 (280 in workflow, ~192 in cli, 37 in parser, and smaller utility packages)
• Near-duplicate file pairs: 2 (structural duplicates with token-only differences)
• Triplicated/duplicated function bodies: 3 groups
• Misplaced functions: 2 (wrong files based on their domain/consumers)
• Fragmented file sets: 2 (thin files that should be merged)
• Missing helper abstractions: 2 (repeated expressions/patterns across 4–15+ call sites)
• Interface bypass patterns: 1 (type-switches where polymorphism should be used)

---

Issue 1: Near-Duplicate Files — missing_data.go and missing_tool.go

Files: pkg/workflow/missing_data.go, pkg/workflow/missing_tool.go

Problem: These two ~163-line files are structurally identical. The only differences are token-level (variable name, log prefix, env var prefix, default title prefix string, script path). The struct definitions (MissingDataConfig / MissingToolConfig) are identical except for a comment word. The parse*Config and build*Job functions share 100% of their control-flow logic and field-access patterns.

Recommendation: Introduce a single generic config type (e.g., IssueReportingConfig) with a parameter struct covering the varying fields, and a single shared buildIssueReportingJob / parseIssueReportingConfig function. Type aliases MissingDataConfig / MissingToolConfig can remain for backward compatibility. Estimated elimination: ~120 lines of duplicated logic.

---

Issue 2: Triplicated GetSquidLogsSteps Across Three Engine Files

Files:

• pkg/workflow/claude_engine.go (line 470)
• pkg/workflow/codex_engine.go (line 354)
• pkg/workflow/copilot_logs.go (line 450)

Problem: All three GetSquidLogsSteps implementations are byte-for-byte identical except for the logger variable. All three call isFirewallEnabled, generateSquidLogsUploadStep, and generateFirewallLogParsingStep with the same arguments and same conditional structure. Similarly, GetFirewallLogsCollectionStep is a no-op stub duplicated in both claude_engine.go and codex_engine.go.

Recommendation: Move a shared DefaultGetSquidLogsSteps(workflowData *WorkflowData, log *logger.Logger) []GitHubActionStep helper to engine_firewall_support.go (which already exists for cross-engine firewall logic). Each engine calls it with its own logger, reducing three 18-line functions to three 3-line wrappers. Alternatively, promote GetSquidLogsSteps to BaseEngine since the logic is engine-agnostic.

---

Issue 3: Near-Duplicate RenderMCPConfig in claude_mcp.go and gemini_mcp.go

Files:

• pkg/workflow/claude_mcp.go (lines 12–69)
• pkg/workflow/gemini_mcp.go (lines 12–64)

Problem: Diffing the two functions shows they differ only in receiver type, logger variable name, and one minor comment. The MCPRendererOptions struct passed in both uses identical fields (IncludeCopilotFields: false, InlineArgs: false, Format: "json"). The MCPToolRenderers struct construction is completely parallel with the same eight function closures in the same order. (Note: copilot_mcp.go legitimately differs by using IncludeCopilotFields: true, InlineArgs: true.)

Recommendation: Extract a shared renderJSONMCPConfigForEngine helper that accepts MCPRendererOptions, a config path string, and engine-specific overrides. Both Claude and Gemini engines call it with their specific logger. This can live in mcp_config_utils.go.

---

Issue 4: Repeated rewriteLocalhost Guard Expression (4+ Call Sites)

Files:

• pkg/workflow/codex_mcp.go (lines 172–174 and 195–197 — appears twice)
• pkg/workflow/copilot_mcp.go (lines 90–92)
• pkg/workflow/mcp_config_custom.go (lines 53–55)

Problem: The following expression is repeated verbatim at 4 locations across 3 files, with an additional 14+ variant occurrences of the nil-guard chain across the package:

rewriteLocalhost := workflowData != nil && (workflowData.SandboxConfig == nil ||
    workflowData.SandboxConfig.Agent == nil ||
    !workflowData.SandboxConfig.Agent.Disabled)

Recommendation: Add shouldRewriteLocalhostToDocker(workflowData *WorkflowData) bool in mcp_config_utils.go or sandbox.go. All call sites replace the 3-line guard with a single readable predicate. This clarifies the semantics: firewall enabled = rewrite localhost.

---

Issue 5: Misplaced Firewall Step Generators in copilot_installer.go

File: `pkg/workflow/copilot_instal...

• Fixes [refactor] Semantic Function Clustering Analysis: Duplicates, Misplaced Logic, and Fragmented Files in pkg/workflow #18543

---

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

[pelikhan]

@copilot recompile

[Copilot]

Pull request overview

This PR refactors pkg/workflow/ to eliminate structural duplication and improve code organization, addressing 8 of 10 issues identified by semantic analysis. The refactoring consolidates duplicate logic, extracts cross-engine helpers, and fixes an interface bypass pattern (including a duplicate *CodexEngine block).

Changes:

• Unified missing_data.go and missing_tool.go into shared missing_issue_reporting.go (~260 lines eliminated)
• Extracted firewall helpers (generateSquidLogsUploadStep, generateFirewallLogParsingStep, defaultGetSquidLogsSteps) to engine_firewall_support.go
• Added GetFirewallLogsCollectionStep to WorkflowExecutor interface, replacing type-assertion chains with polymorphic calls
• Extracted shouldRewriteLocalhostToDocker and noOpCacheMemoryRenderer to mcp_config_utils.go
• Removed duplicate parseToolCalls function (78 lines) from claude_logs.go
• Merged safe_output_config.go into safe_outputs_config.go

Reviewed changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
pkg/workflow/missing_issue_reporting.go	New shared implementation for missing-data and missing-tool config/job building with type aliases for backward compatibility
pkg/workflow/missing_data.go	Reduced from ~163 to ~31 lines by delegating to shared functions
pkg/workflow/missing_tool.go	Reduced from ~163 to ~31 lines by delegating to shared functions
pkg/workflow/engine_firewall_support.go	Added 3 firewall-related helper functions extracted from copilot_installer.go and engine files
pkg/workflow/copilot_installer.go	Removed 2 firewall helper functions (moved to engine_firewall_support.go), cleaned up unused imports
pkg/workflow/mcp_config_utils.go	Added shouldRewriteLocalhostToDocker helper and noOpCacheMemoryRenderer no-op function
pkg/workflow/codex_mcp.go	Uses shouldRewriteLocalhostToDocker helper (2 call sites)
pkg/workflow/copilot_mcp.go	Uses shouldRewriteLocalhostToDocker helper
pkg/workflow/mcp_config_custom.go	Uses shouldRewriteLocalhostToDocker helper
pkg/workflow/claude_mcp.go	Uses noOpCacheMemoryRenderer, removed duplicate method
pkg/workflow/gemini_mcp.go	Uses noOpCacheMemoryRenderer, removed duplicate method
pkg/workflow/claude_logs.go	Removed 78-line duplicate parseToolCalls function, replaced call with parseToolCallsWithSequence
pkg/workflow/claude_engine.go	Uses defaultGetSquidLogsSteps helper
pkg/workflow/codex_engine.go	Uses defaultGetSquidLogsSteps helper
pkg/workflow/copilot_logs.go	Uses defaultGetSquidLogsSteps helper
pkg/workflow/compiler_yaml_main_job.go	Fixed duplicate *CodexEngine block by using polymorphic GetFirewallLogsCollectionStep interface method
pkg/workflow/agentic_engine.go	Added GetFirewallLogsCollectionStep to WorkflowExecutor interface and default implementation in BaseEngine
pkg/workflow/safe_outputs_config.go	Absorbed parseBaseSafeOutputConfig from deleted safe_output_config.go, added strings import
pkg/workflow/safe_output_config.go	Deleted (function moved to safe_outputs_config.go)

---

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

[Copilot]

@copilot recompile

Recompiled all 161 workflow files — no lock file changes (output is identical to pre-refactoring, confirming the code changes don't affect workflow compilation).

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

• https://api.github.com/graphql
  • Triggering command: /usr/bin/gh /usr/bin/gh api graphql -f query=query($owner: String!, $name: String!) { repository(owner: $owner, name: $name) { hasDiscussionsEnabled } } -f owner=github -f name=gh-aw (http block)
• https://api.github.com/repos/actions/checkout/git/ref/tags/11bd71901bbe5b1630ceea73d27597364c9af683
  • Triggering command: /usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/11bd71901bbe5b1630ceea73d27597364c9af683 --jq .object.sha (http block)
• https://api.github.com/repos/actions/checkout/git/ref/tags/v6
  • Triggering command: /usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v6 --jq .object.sha (http block)
• https://api.github.com/repos/actions/github-script/git/ref/tags/v7
  • Triggering command: /usr/bin/gh gh api /repos/actions/github-script/git/ref/tags/v7 --jq .object.sha (http block)
• https://api.github.com/repos/actions/setup-go/git/ref/tags/4dc6199c7b1a012772edbd06daecab0f50c9053c
  • Triggering command: /usr/bin/gh gh api /repos/actions/setup-go/git/ref/tags/4dc6199c7b1a012772edbd06daecab0f50c9053c --jq .object.sha (http block)
• https://api.github.com/repos/actions/setup-go/git/ref/tags/v5
  • Triggering command: /usr/bin/gh gh api /repos/actions/setup-go/git/ref/tags/v5 --jq .object.sha (http block)
• https://api.github.com/repos/actions/setup-node/git/ref/tags/v6
  • Triggering command: /usr/bin/gh gh api /repos/actions/setup-node/git/ref/tags/v6 --jq .object.sha 8f^{commit} (http block)
  • Triggering command: /usr/bin/gh gh api /repos/actions/setup-node/git/ref/tags/v6 --jq .object.sha (http block)
• https://api.github.com/repos/astral-sh/setup-uv/git/ref/tags/eac588ad8def6316056a12d4907a9d4d84ff7a3b
  • Triggering command: /usr/bin/gh gh api /repos/astral-sh/setup-uv/git/ref/tags/eac588ad8def6316056a12d4907a9d4d84ff7a3b --jq .object.sha (http block)
• https://api.github.com/repos/docker/build-push-action/git/ref/tags/v6
  • Triggering command: /usr/bin/gh gh api /repos/docker/build-push-action/git/ref/tags/v6 --jq .object.sha (http block)
• https://api.github.com/repos/docker/login-action/git/ref/tags/v3
  • Triggering command: /usr/bin/gh gh api /repos/docker/login-action/git/ref/tags/v3 --jq .object.sha (http block)
• https://api.github.com/repos/docker/metadata-action/git/ref/tags/v5
  • Triggering command: /usr/bin/gh gh api /repos/docker/metadata-action/git/ref/tags/v5 --jq .object.sha (http block)
• https://api.github.com/repos/docker/setup-buildx-action/git/ref/tags/v3
  • Triggering command: /usr/bin/gh gh api /repos/docker/setup-buildx-action/git/ref/tags/v3 --jq .object.sha (http block)
• https://api.github.com/repos/github/gh-aw/git/ref/tags/a70c5eada06553e3510ac27f2c3bda9d3705bccb
  • Triggering command: /usr/bin/gh gh api /repos/github/gh-aw/git/ref/tags/a70c5eada06553e3510ac27f2c3bda9d3705bccb --jq .object.sha (http block)
• https://api.github.com/repos/githubnext/agentics/git/ref/tags/
  • Triggering command: /usr/bin/gh gh api /repos/githubnext/agentics/git/ref/tags/# --jq .object.sha (http block)

If you need me to access, download, or install something from one of these locations, you can either:

• Configure Actions setup steps to set up my environment, which run before the firewall is enabled
• Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)