Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 2 additions & 31 deletions pkg/cli/codemod_serena_import.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@ var serenaImportCodemodLog = logger.New("cli:codemod_serena_import")

// getSerenaToSharedImportCodemod creates a codemod that migrates removed tools.serena
// or engine.tools.serena configuration to an equivalent imports entry using
// shared/mcp/serena.md, and may normalize a pinned source ref to @main.
// shared/mcp/serena.md. The existing source: pin is preserved unchanged.
func getSerenaToSharedImportCodemod() Codemod {
return Codemod{
ID: "serena-tools-to-shared-import",
Name: "Migrate tools.serena or engine.tools.serena to shared Serena import",
Description: "Removes 'tools.serena' or 'engine.tools.serena', adds an equivalent 'imports' entry using shared/mcp/serena.md with languages, and may rewrite a pinned 'source:' ref to '@main'.",
Description: "Removes 'tools.serena' or 'engine.tools.serena' and adds an equivalent 'imports' entry using shared/mcp/serena.md with languages. The existing 'source:' pin is preserved.",
IntroducedIn: "1.0.0",
Apply: func(content string, frontmatter map[string]any) (string, bool, error) {
languages, ok := findSerenaLanguagesForMigration(frontmatter)
Expand All @@ -45,7 +45,6 @@ func getSerenaToSharedImportCodemod() Codemod {
return addSerenaImport(result, languages), true
})
if applied {
newContent = maybeUpdatePinnedSourceRef(newContent, frontmatter)
if alreadyImported {
serenaImportCodemodLog.Print("Removed tools.serena (shared/mcp/serena.md import already present)")
} else {
Expand Down Expand Up @@ -324,31 +323,3 @@ func hasNestedContent(lines []string, startIndex int, blockIndent string) (bool,

return false, len(lines)
}

func maybeUpdatePinnedSourceRef(content string, frontmatter map[string]any) string {
sourceAny, hasSource := frontmatter["source"]
if !hasSource {
return content
}

source, ok := sourceAny.(string)
if !ok || strings.TrimSpace(source) == "" {
return content
}

sourceSpec, err := parseSourceSpec(source)
if err != nil {
return content
}

if sourceSpec.Repo != "github/gh-aw" || !IsCommitSHA(sourceSpec.Ref) {
return content
}

updatedSource := sourceSpec.Repo + "/" + sourceSpec.Path + "@main"
updatedContent, err := UpdateFieldInFrontmatter(content, "source", updatedSource)
if err != nil {
return content
}
return updatedContent
}
60 changes: 58 additions & 2 deletions pkg/cli/codemod_serena_import_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ engine:
assert.False(t, hasEngineTools, "Engine tools should be removed after migration")
})

t.Run("updates github/gh-aw source pin from commit SHA to main when migrating serena", func(t *testing.T) {
t.Run("preserves github/gh-aw source pin when migrating serena (does not rewrite to @main)", func(t *testing.T) {
content := `---
source: github/gh-aw/.github/workflows/duplicate-code-detector.md@852cb06ad52958b402ed982b69957ffc57ca0619
engine: copilot
Expand All @@ -217,10 +217,66 @@ tools:
result, applied, err := codemod.Apply(content, frontmatter)
require.NoError(t, err, "Codemod should not return an error")
assert.True(t, applied, "Codemod should be applied when tools.serena is present")
assert.Contains(t, result, "source: github/gh-aw/.github/workflows/duplicate-code-detector.md@main", "Codemod should update pinned gh-aw source to main")
assert.Contains(t, result, "source: github/gh-aw/.github/workflows/duplicate-code-detector.md@852cb06ad52958b402ed982b69957ffc57ca0619", "Codemod should preserve the original pinned commit SHA in source")
assert.NotContains(t, result, "@main", "Codemod should not rewrite source pin to @main")
assert.Contains(t, result, "- uses: shared/mcp/serena.md", "Codemod should still add shared Serena import")
})

t.Run("storybookjs and FluidFramework: preserves pinned source and migrates tools.serena to imports", func(t *testing.T) {
// Simulates the duplicate-code-detector.md workflow used by storybookjs/storybook and
// microsoft/FluidFramework, both sourced from gh-aw at the same pinned commit.
// The codemod must migrate tools.serena without touching the source pin.
tests := []struct {
name string
}{
{name: "storybookjs/storybook"},
{name: "microsoft/FluidFramework"},
}

content := `---
source: github/gh-aw/.github/workflows/duplicate-code-detector.md@852cb06ad52958b402ed982b69957ffc57ca0619
name: Duplicate Code Detector
description: Identifies duplicate code patterns across the codebase
on:
workflow_dispatch:
schedule: daily
permissions:
contents: read
issues: read
pull-requests: read
engine: copilot
tools:
serena: ["typescript"]
imports:
- shared/mood.md
strict: true
---

# Duplicate Code Detection
`
frontmatter := map[string]any{
"source": "github/gh-aw/.github/workflows/duplicate-code-detector.md@852cb06ad52958b402ed982b69957ffc57ca0619",
"engine": "copilot",
"tools": map[string]any{
"serena": []any{"typescript"},
},
"imports": []any{"shared/mood.md"},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result, applied, err := codemod.Apply(content, frontmatter)
require.NoError(t, err, "Codemod should not return an error")
assert.True(t, applied, "Codemod should be applied when tools.serena is present")
assert.Contains(t, result, "source: github/gh-aw/.github/workflows/duplicate-code-detector.md@852cb06ad52958b402ed982b69957ffc57ca0619", "Source pin must be preserved unchanged")
assert.NotContains(t, result, "@main", "Source pin must not be rewritten to @main")
assert.Contains(t, result, "- uses: shared/mcp/serena.md", "Serena import must be added")
assert.Contains(t, result, `languages: ["typescript"]`, "Serena import must include languages")
assert.NotContains(t, result, " serena:", "tools.serena must be removed")
})
}
})

t.Run("falls back to engine.tools.serena when top-level tools.serena is invalid", func(t *testing.T) {
content := `---
engine:
Expand Down
Loading