[refactor] Semantic function clustering: duplicate SHA validation, workflow reconstruction, and git root logic

[github-actions]

Analysis of 711 non-test Go source files across 22 packages in pkg/ revealed three concrete refactoring opportunities involving true functional duplication (excluding intentional WASM build-tag variants, which are correctly split).

---

1. Duplicate isValidFullSHA across unrelated packages

Severity: Medium | Effort: Low

The same 40-character hex SHA validation logic exists independently in two packages:

Location	Regex variable	Implementation
pkg/workflow/features_validation.go:102	shaRegex	len(s) != 40 guard + shaRegex.MatchString(s)
pkg/actionpins/actionpins.go:201	fullSHARegex	fullSHARegex.MatchString(s) only

Both compile the identical pattern ^[0-9a-f]{40}$ but with subtle divergence: the workflow copy adds an explicit length pre-check before the regex; actionpins does not.

pkg/gitutil/gitutil.go already exports IsHexString() (hex character check without length), making it the natural home for a shared IsValidFullSHA().

Recommendation: Add IsValidFullSHA(s string) bool to pkg/gitutil and replace both private copies. This closes the behavioral divergence and puts the function next to the related IsHexString utility.

// pkg/gitutil/gitutil.go
var fullSHARegex = regexp.MustCompile(`^[0-9a-f]{40}$`)

// IsValidFullSHA checks if s is a valid 40-character lowercase hexadecimal SHA.
func IsValidFullSHA(s string) bool {
    return fullSHARegex.MatchString(s)
}

---

2. Duplicated workflow file reconstruction logic

Severity: Medium | Effort: Low

Two private functions assemble a workflow file from frontmatter + markdown body with identical --- fencing:

parser/workflow_update.go:88 — reconstructWorkflowFile(frontmatterYAML, markdownContent string)

lines = append(lines, "---")
lines = append(lines, strings.Split(frontmatterStr, "\n")...)
lines = append(lines, "---")
if markdownContent != "" { lines = append(lines, markdownContent) }
return strings.Join(lines, "\n"), nil

cli/imports.go:158 — reconstructWorkflowFileFromMap(frontmatter map[string]any, markdown string)

// Marshals map to YAML first, then:
lines = append(lines, "---")
lines = append(lines, strings.Split(frontmatterStr, "\n")...)
lines = append(lines, "---")
if markdown != "" { lines = append(lines, markdown) }
return strings.Join(lines, "\n"), nil

The cli function additionally marshals the map and unquotes the on key before delegating to the same assembly pattern.

Recommendation: Export parser.ReconstructWorkflowFile(frontmatterYAML, markdown string) (string, error) (capitalize r). Update cli/imports.go::reconstructWorkflowFileFromMap to call it after marshaling, eliminating the duplicated assembly loop.

---

3. Divergent workflow-spec path detection

Severity: Low | Effort: Low

Two functions answer the same question — "is this path a workflow spec reference?" — with different logic:

Location	Logic
parser/remote_fetch.go:205 — isWorkflowSpec(path string)	Strips #section and @ref before checking; handles edge cases
cli/imports.go:433 — isWorkflowSpecFormat(path string)	strings.Contains(path, "@") only

A path like shared/mcp.md@v1.0#Section would be classified differently if the section part contains an @. The parser implementation is more robust.

cli/imports.go already imports the parser package for ExtractFrontmatterFromContent, ParseImportDirective, and ExtractMarkdownContent, so the dependency already exists.

Recommendation: Export parser.IsWorkflowSpec(path string) bool and replace cli/imports.go::isWorkflowSpecFormat with it to ensure consistent, robust detection.

---

Context

• Files analyzed: 711 non-test .go files across 22 packages
• WASM variants excluded: *_wasm.go files with //go:build js || wasm are correctly split platform variants (e.g., console/console_wasm.go, workflow/github_cli_wasm.go) — not duplicates
• Other reviewed patterns (no action needed):
  • getEffective*Token functions in workflow/github_token.go — intentionally distinct secret chains
  • update{Issue,PullRequest,Discussion}Config helpers — thin wrappers over generic parseUpdateEntityConfigTyped[T]
  • cli/version.go::GetVersion vs workflow/version.go::GetVersion — different package scopes, kept in sync via SetVersion

Implementation checklist

• Add gitutil.IsValidFullSHA and update features_validation.go + actionpins.go
• Export parser.ReconstructWorkflowFile and update cli/imports.go
• Export parser.IsWorkflowSpec and replace cli/imports.go::isWorkflowSpecFormat
• Run existing tests after each change to confirm no regressions

References: §24603615719

Generated by Semantic Function Refactoring · ● 361K · ◷

• expires on Apr 20, 2026, 11:31 AM UTC