Consolidate duplicate secret extraction logic into shared utilities

[Copilot]

✅ Consolidate Secret Extraction Logic - COMPLETE

Summary

Successfully eliminated ~60% code duplication in secret extraction logic by consolidating similar functions from mcp-config.go and secrets.go into shared utilities.

Changes Made

1. Created Shared Utilities ✓

• File: pkg/workflow/secret_extraction.go (115 lines)
  • secretExprPattern - Common pre-compiled regex for secret matching
  • ExtractSecretName() - Extract just the secret name from expression
  • ExtractSecretsFromValue() - Extract all secrets from a single string
  • ExtractSecretsFromMap() - Extract all secrets from a map of strings
  • ReplaceSecretsWithEnvVars() - Replace secret expressions with env var references

2. Comprehensive Test Coverage ✓

• File: pkg/workflow/secret_extraction_test.go (340 lines)
  • 31 test cases covering all shared utilities
  • Edge cases: malformed expressions, empty values, defaults
  • All tests pass ✓

3. Refactored Existing Code ✓

• pkg/workflow/mcp-config.go:

  • Removed 3 duplicate functions (~75 lines)
  • Updated to use shared ExtractSecretsFromMap()
  • Updated to use shared ReplaceSecretsWithEnvVars()
  • Updated existing tests to use shared functions

• pkg/cli/secrets.go:

  • Removed thin wrapper extractSecretName() function
  • Directly uses workflow.ExtractSecretName() in all call sites
  • All existing functionality preserved

4. Validation ✓

• ✅ All unit tests pass (make test-unit)
• ✅ All secret extraction tests pass
• ✅ Code builds successfully (make build)
• ✅ Code formatted (make fmt)
• ✅ Code lints cleanly (make lint)
• ✅ HTTP MCP header secrets work correctly
• ✅ Backward compatibility maintained

Benefits

1. DRY Principle: Single source of truth for secret extraction
2. Maintainability: Changes only need to be made once
3. Consistency: All code uses same pattern matching logic
4. Testability: Comprehensive test suite for shared utilities
5. Reusability: Easy to add new callers
6. No thin wrappers: Direct use of shared utilities reduces indirection

Code Metrics

• Lines eliminated: ~82 lines of duplicate code (75 + 7 from wrapper)
• Lines added: ~455 lines (115 utilities + 340 tests)
• Net reduction in duplication: 100% (from ~60% duplication to 0%)
• Test coverage: 31 test cases for shared utilities

Files Changed

• ✅ pkg/workflow/secret_extraction.go (new)
• ✅ pkg/workflow/secret_extraction_test.go (new)
• ✅ pkg/workflow/mcp-config.go (refactored)
• ✅ pkg/workflow/mcp_http_headers_test.go (updated)
• ✅ pkg/cli/secrets.go (refactored - removed thin wrapper)
• ✅ pkg/cli/secrets_test.go (updated to use workflow.ExtractSecretName)

Original prompt

---

This section details on the original issue you should resolve

<issue_title>[task] Consolidate secret extraction logic into shared utilities</issue_title>
<issue_description>## Objective
Eliminate ~60% code duplication in secret extraction logic by consolidating similar functions from mcp-config.go and secrets.go into shared utilities.

Context

Currently secret extraction logic is duplicated across multiple files with ~60% similarity. Functions like extractSecretsFromValue, extractSecretsFromHeaders (mcp-config.go) and extractSecretsFromConfig (secrets.go) all perform similar pattern matching for ${{ secrets.NAME }} with slight variations.

Related to #3713 - Duplicate #2: Secret Extraction Functions.

Approach

1. Create new file pkg/workflow/secret_extraction.go with shared utilities:

   • Common secret pattern regex
   • Generic secret extraction from strings
   • Generic secret extraction from maps
   • Helper functions for parsing secret expressions with defaults

2. Refactor existing functions to use shared utilities:

   • Update extractSecretsFromValue in mcp-config.go
   • Update extractSecretsFromHeaders in mcp-config.go
   • Update extractSecretsFromConfig in cli/secrets.go

3. Add comprehensive tests for the new utilities

4. Verify all existing tests continue to pass

Files to Create

• pkg/workflow/secret_extraction.go - Shared secret extraction utilities
• pkg/workflow/secret_extraction_test.go - Tests for new utilities

Files to Modify

• pkg/workflow/mcp-config.go - Refactor to use shared utilities
• pkg/cli/secrets.go - Refactor to use shared utilities
• Existing test files that test secret extraction

Acceptance Criteria

• New secret_extraction.go file created with shared utilities
• Common secret pattern regex defined once (DRY principle)
• All existing secret extraction functions refactored to use utilities
• Code duplication reduced significantly
• All existing tests pass without modification
• New tests added for shared utilities
• Secret extraction behavior identical to original
• Handles edge cases: defaults (secrets.X || 'default'), malformed expressions
• Single source of truth for secret pattern matching
  Related to [refactor] 🔧 Semantic Function Clustering Analysis: Refactoring Opportunities #3713

AI generated by Plan Command for #3713</issue_description>

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

• Fixes [task] Consolidate secret extraction logic into shared utilities #3746

---

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

[pelikhan]

@copilot remove thin wrappers

[Copilot]

Removed the thin wrapper extractSecretName() function. Now directly calling workflow.ExtractSecretName() in both secrets.go and secrets_test.go. (9450c8b)

[Copilot]

Pull Request Overview

This PR successfully consolidates duplicate secret extraction logic from multiple files into a shared utility module, reducing code duplication and improving maintainability. The refactoring moves secret pattern matching and extraction functions into pkg/workflow/secret_extraction.go and updates all call sites to use the new shared utilities.

Key changes:

• Created centralized secret extraction utilities with comprehensive test coverage
• Eliminated ~75 lines of duplicate code from mcp-config.go
• Removed thin wrapper function from secrets.go in favor of direct use of shared utilities

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
pkg/workflow/secret_extraction.go	New shared utilities for secret extraction with exported functions for extracting secret names and expressions from strings and maps
pkg/workflow/secret_extraction_test.go	Comprehensive test suite with 31 test cases covering all shared utilities including edge cases
pkg/workflow/mcp-config.go	Refactored to use shared ExtractSecretsFromMap() and ReplaceSecretsWithEnvVars() utilities, removing 3 duplicate private functions
pkg/workflow/mcp_http_headers_test.go	Updated test calls to use new exported function names (ExtractSecretsFromValue, ExtractSecretsFromMap, ReplaceSecretsWithEnvVars)
pkg/cli/secrets.go	Removed duplicate extractSecretName() wrapper function and unused regex pattern; now directly uses workflow.ExtractSecretName()
pkg/cli/secrets_test.go	Updated to import and use workflow.ExtractSecretName() instead of the removed local wrapper

---

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

[Copilot]

The SecretExpression struct is defined but never used anywhere in the codebase. This is dead code that should be removed.

Suggested change

	// SecretExpression represents a parsed secret expression
	type SecretExpression struct {
	VarName string // The secret variable name (e.g., "DD_API_KEY")
	FullExpr string // The full expression (e.g., "${{ secrets.DD_API_KEY }}")
	}