-
Notifications
You must be signed in to change notification settings - Fork 295
Description
Overview
Recent commit analysis found a repeated codemod implementation pattern in pkg/cli where multiple get*Codemod() constructors duplicate the same frontmatter guard/check/transform scaffold with only field names changed.
This exceeds the duplication threshold via 3+ similar occurrences and increases maintenance risk when codemod behavior or validation flow changes.
Critical Information
- Pattern type: Structural duplication (copy-adapted logic)
- Severity: Medium
- Occurrences: 3 confirmed instances in production code
- Commit analyzed:
04e391b8945b082ee18a5a54da81646ac9a09f7f
Duplication Details
Repeated pattern
The following functions repeat the same sequence:
- Read top-level frontmatter key
- Type-assert
map[string]any - Check child field presence
- Invoke
applyFrontmatterLineTransform(...) - Log and return
Locations
pkg/cli/codemod_grep_tool.go:14pkg/cli/codemod_mcp_scripts.go:16pkg/cli/codemod_network_firewall.go:18
View representative duplicated block (semantic shape)
Apply: func(content string, frontmatter map[string]any) (string, bool, error) {
parentValue, hasParent := frontmatter["(parent)"]
if !hasParent {
return content, false, nil
}
parentMap, ok := parentValue.(map[string]any)
if !ok {
return content, false, nil
}
_, hasField := parentMap["(field)"]
if !hasField {
return content, false, nil
}
newContent, applied, err := applyFrontmatterLineTransform(content, func(lines []string) ([]string, bool) {
return removeFieldFromBlock(lines, "(field)", "(parent)")
})
if applied {
// log
}
return newContent, applied, err
}View evidence from two near-identical implementations
getGrepToolRemovalCodemod:pkg/cli/codemod_grep_tool.go:14getMCPScriptsModeCodemod:pkg/cli/codemod_mcp_scripts.go:16
These differ primarily by string literals (tools/grep vs mcp-scripts/mode) while control flow and transform wiring are equivalent.
Impact Analysis
- Maintainability: Any behavior change to guard/transform flow must be replicated across many codemods.
- Bug risk: Divergence risk is high when one codemod receives a safety fix and others do not.
- Code bloat: Repeated scaffolding obscures codemod-specific intent.
Refactoring Recommendations
- Introduce a generic field-removal codemod builder
- Suggested location:
pkg/cli/codemod_factory.go - Shape:
newFieldRemovalCodemod(meta..., parentKey, fieldKey, logMsg)returningCodemod - Benefit: Centralized guard logic and consistent behavior across codemods
- Allow optional post-transform hook for special cases
- Needed for extended behavior in
codemod_network_firewall.go - Benefit: Reuse common preconditions while preserving custom migration logic
- Add table-driven tests for the shared builder
- Validate parent missing / wrong type / field missing / successful removal
- Benefit: One test surface for common behavior
Implementation Checklist
- Add shared codemod builder for field-removal migrations
- Migrate
grepandmcp-scripts.modecodemods to builder - Evaluate partial migration of
network.firewallcodemod via hook-based extension - Add/adjust unit tests for shared path and migrated codemods
- Verify no behavior regressions in codemod output
Analysis Metadata
- Analyzed files: 562 changed non-test
pkg/**/*.gofiles in available commit snapshot - Detection method: Serena symbol analysis + structural comparison
- Workflow run: §23035479736
- Analysis date: 2026-03-13 UTC
References:
Generated by Duplicate Code Detector · ◷
Warning
⚠️ Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
ab.chatgpt.com
To allow these domains, add them to the network.allowed list in your workflow frontmatter:
network:
allowed:
- defaults
- "ab.chatgpt.com"See Network Configuration for more information.