refactor(P1): consolidate YAML single-quote escapers, MCP config base, and workflow validation helpers

[Copilot]

Three confirmed duplicate/scattered-helper patterns from the semantic function clustering analysis (P1 items). Eliminates byte-identical function bodies and ~90%-parallel validation files by extracting shared helpers.

P1.1 — Single canonical YAML single-quote escaper

Two functions (escapeSingleQuotedYAMLString, escapeYAMLSingleQuotedScalar) were byte-identical, with the same one-liner also inlined at 6 additional sites. Canonical escapeYAMLSingleQuotedScalar now lives in yaml_env_helpers.go; both definitions and all inline sites route through it.

escapeSingleQuote in redact_secrets.go uses backslash-style '→\' (different semantics) — intentionally left separate.

P1.2 — Shared RenderMCPConfig base for 5 JSON-MCP engines

Claude, Gemini, Antigravity, Crush, and OpenCode each had an identical 3-line body: log + comment + renderDefaultJSONMCPConfig(…, PATH). New helper in mcp_renderer_factory.go:

func renderJSONMCPConfigForEngine(engineName string, sb *strings.Builder, tools map[string]any, mcpTools []string, workflowData *WorkflowData, configPath string) error {
    mcpRenderingLog.Printf("Rendering MCP config for %s: tool_count=%d, mcp_tool_count=%d", engineName, len(tools), len(mcpTools))
    return renderDefaultJSONMCPConfig(sb, tools, mcpTools, workflowData, configPath)
}

Each engine file is now a one-liner delegation; 5 per-engine logger.New(…) vars removed.

P1.3 — Shared call/dispatch workflow validation helpers

call_workflow_validation.go and dispatch_workflow_validation.go were ~90% parallel (~200 lines each), with byte-identical error templates and duplicated read→parse→check-trigger logic.

Extracted to validation_helpers.go:

• formatWorkflowNotFoundError(workflowType, workflowName, workflowsDir string) error
• formatMissingTriggerError(workflowType, workflowName, triggerName string) error

Extracted to dispatch_workflow_file_resolver.go:

• workflowFileValidation struct + readAndValidateWorkflowFileTrigger(fileResult, triggerName) — reads lock/yml (priority order), parses YAML, checks the on: trigger; preserves read-vs-parse error distinction for callers

call_workflow_validation.go also has its duplicated lockExists/ymlExists branches collapsed into the single readAndValidateWorkflowFileTrigger call, matching the cleaner pattern already present in the dispatch file.

[github-actions]

Hey @Copilot 👋 — great to see the core team agent tackling the semantic function clustering refactor! Consolidating helpers like escapeYAMLSingleQuotedScalar, renderJSONMCPConfigWithLog, and the workflow validation extractors is a solid cleanup that will reduce noise across the codebase.

A couple of things to address before this is ready for review:

• Complete the implementation — the PR is still in WIP/draft state with only 2 lines changed (+1/-1). The three planned tasks (P1.1, P1.2, P1.3) are all unchecked. The actual refactor work needs to land in the diff.
• Add tests — once the helpers are extracted (e.g. escapeYAMLSingleQuotedScalar, renderJSONMCPConfigWithLog, formatWorkflowNotFoundError), unit tests covering their behaviour would help protect against regressions from the deduplication.

Once the implementation is complete, consider assigning this prompt to your coding agent to finish up:

Complete the refactor described in PR #36033 across three tasks:

P1.1: Add `escapeYAMLSingleQuotedScalar` to `yaml_env_helpers.go`, remove the duplicate definitions in `central_slash_command_workflow.go` and `observability_otlp.go`, and update all inline call sites in the workflow package to use the shared helper.

P1.2: Add a `renderJSONMCPConfigWithLog` helper to `mcp_renderer_factory.go` and collapse the 5 engine-specific MCP files to one-liner delegations that call this helper.

P1.3: Extract `formatWorkflowNotFoundError`, `formatMissingTriggerError`, and `readAndValidateWorkflowFileTrigger` into a new `validation_helpers.go` file, then deduplicate `call_workflow_validation.go` and `dispatch_workflow_validation.go` to use those helpers.

After each extraction, add unit tests for the new helper functions covering the main happy-path and error cases. Verify the build passes (`go build ./...`) before marking the PR ready for review.

Generated by ✅ Contribution Check · sonnet46 2.9M · ◷

[Copilot]

Pull request overview

Refactors three duplicate/scattered patterns: consolidates the YAML single-quoted-scalar escaper into a single canonical helper, introduces a shared renderJSONMCPConfigForEngine base for the five standard JSON-MCP engines, and extracts shared call/dispatch workflow validation helpers (error formatters + a read/parse/trigger-check helper).

Changes:

• Adds escapeYAMLSingleQuotedScalar to yaml_env_helpers.go and routes 8 call sites (including former duplicates in observability_otlp.go and central_slash_command_workflow.go) through it.
• Adds renderJSONMCPConfigForEngine to mcp_renderer_factory.go; Claude/Gemini/Antigravity/Crush/OpenCode MCP files collapse to one-liner delegations and drop per-engine loggers.
• Adds formatWorkflowNotFoundError/formatMissingTriggerError to validation_helpers.go and workflowFileValidation/readAndValidateWorkflowFileTrigger to dispatch_workflow_file_resolver.go; both validators now share read/parse/trigger logic and error templates.

Show a summary per file

File	Description
pkg/workflow/yaml_env_helpers.go	Adds canonical escapeYAMLSingleQuotedScalar.
pkg/workflow/observability_otlp.go	Removes duplicate definition of the escaper.
pkg/workflow/central_slash_command_workflow.go	Replaces inline escaper and removes local helper.
pkg/workflow/cache.go, compiler_yaml.go, compiler_experiments.go, engine_helpers.go, expression_nodes.go, service_ports.go	Route inline '→'' replacements through the shared helper.
pkg/workflow/mcp_renderer_factory.go	Adds shared renderJSONMCPConfigForEngine base.
pkg/workflow/{claude,gemini,antigravity,crush,opencode}_mcp.go	Collapse to single-line delegations; drop per-engine loggers/imports.
pkg/workflow/validation_helpers.go	Adds shared error formatters for missing workflow/trigger.
pkg/workflow/dispatch_workflow_file_resolver.go	Adds workflowFileValidation + readAndValidateWorkflowFileTrigger.
pkg/workflow/call_workflow_validation.go	Uses shared trigger helper + error formatters; drops yaml import.
pkg/workflow/dispatch_workflow_validation.go	Same consolidation; drops os/yaml imports.
.github/workflows/daily-model-inventory.lock.yml	Regenerated body_hash only.

Copilot's findings

Tip

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

• Files reviewed: 20/20 changed files
• Comments generated: 0

[github-actions]

@copilot review all comments and address unresolved review feedback.
Please finish the refactor implementation, add the missing tests, then rerun checks.

Generated by 👨‍🍳 PR Sous Chef · gpt54 9M · ◷

[github-actions]

@copilot review all comments and address unresolved review feedback.
Please finish the refactor implementation, add the missing tests, then rerun checks.

Generated by 👨‍🍳 PR Sous Chef · gpt54 9M · ◷