From 5459cc8f1b4fae24e56d421e56fc2bb84660d1fd Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 23 Apr 2026 12:38:06 +0000 Subject: [PATCH 1/6] Initial plan From 100563a98e77e7da6a5d409ea318f91a4266f6a0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 23 Apr 2026 13:40:36 +0000 Subject: [PATCH 2/6] chore: outline plan for serena codemod edge cases Agent-Logs-Url: https://github.com/github/gh-aw/sessions/77db40ff-5e7f-49e1-85c8-2498e4f56230 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/spec_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/cli/spec_test.go b/pkg/cli/spec_test.go index c6b739b93fc..f46eabc88ec 100644 --- a/pkg/cli/spec_test.go +++ b/pkg/cli/spec_test.go @@ -1117,11 +1117,11 @@ func TestSpec_PublicAPI_ValidateWorkflowIntent(t *testing.T) { // Spec: "Sets a field in frontmatter YAML" func TestSpec_PublicAPI_UpdateFieldInFrontmatter(t *testing.T) { tests := []struct { - name string - content string - fieldName string - fieldValue string - wantErr bool + name string + content string + fieldName string + fieldValue string + wantErr bool checkContains string }{ { From b44c15f7395ee337f98941390888a9c6c5ab6eb9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 23 Apr 2026 13:59:08 +0000 Subject: [PATCH 3/6] fix: handle serena codemod edge cases for engine blocks and source pins Agent-Logs-Url: https://github.com/github/gh-aw/sessions/77db40ff-5e7f-49e1-85c8-2498e4f56230 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/codemod_serena_import.go | 171 +++++++++++++++++++------- pkg/cli/codemod_serena_import_test.go | 68 ++++++++++ 2 files changed, 195 insertions(+), 44 deletions(-) diff --git a/pkg/cli/codemod_serena_import.go b/pkg/cli/codemod_serena_import.go index 5e6409ee5f4..38230b4b30e 100644 --- a/pkg/cli/codemod_serena_import.go +++ b/pkg/cli/codemod_serena_import.go @@ -21,24 +21,8 @@ func getSerenaToSharedImportCodemod() Codemod { Description: "Removes 'tools.serena' and adds an equivalent 'imports' entry using shared/mcp/serena.md with languages.", IntroducedIn: "1.0.0", Apply: func(content string, frontmatter map[string]any) (string, bool, error) { - toolsAny, hasTools := frontmatter["tools"] - if !hasTools { - return content, false, nil - } - - toolsMap, ok := toolsAny.(map[string]any) - if !ok { - return content, false, nil - } - - serenaAny, hasSerena := toolsMap["serena"] - if !hasSerena { - return content, false, nil - } - - languages, ok := extractSerenaLanguages(serenaAny) + languages, ok := findSerenaLanguagesForMigration(frontmatter) if !ok || len(languages) == 0 { - serenaImportCodemodLog.Print("Found tools.serena but languages configuration is invalid or empty - skipping migration; verify tools.serena languages are set") return content, false, nil } @@ -50,7 +34,8 @@ func getSerenaToSharedImportCodemod() Codemod { return lines, false } - result = removeTopLevelBlockIfEmpty(result, "tools") + result = removeBlockIfEmpty(result, "tools") + result = removeBlockIfEmpty(result, "engine") if alreadyImported { return result, true @@ -59,6 +44,7 @@ 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 { @@ -70,6 +56,53 @@ func getSerenaToSharedImportCodemod() Codemod { } } +func findSerenaLanguagesForMigration(frontmatter map[string]any) ([]string, bool) { + toolsAny, hasTools := frontmatter["tools"] + if hasTools { + if toolsMap, ok := toolsAny.(map[string]any); ok { + if serenaAny, hasSerena := toolsMap["serena"]; hasSerena { + languages, ok := extractSerenaLanguages(serenaAny) + if ok && len(languages) > 0 { + return languages, true + } + return nil, false + } + } + } + + engineAny, hasEngine := frontmatter["engine"] + if !hasEngine { + return nil, false + } + + engineMap, ok := engineAny.(map[string]any) + if !ok { + return nil, false + } + + engineToolsAny, hasEngineTools := engineMap["tools"] + if !hasEngineTools { + return nil, false + } + + engineToolsMap, ok := engineToolsAny.(map[string]any) + if !ok { + return nil, false + } + + serenaAny, hasSerena := engineToolsMap["serena"] + if !hasSerena { + return nil, false + } + + languages, ok := extractSerenaLanguages(serenaAny) + if !ok || len(languages) == 0 { + return nil, false + } + + return languages, true +} + func extractSerenaLanguages(serenaAny any) ([]string, bool) { switch serena := serenaAny.(type) { case []string: @@ -205,6 +238,13 @@ func addSerenaImport(lines []string, languages []string) []string { for i, line := range lines { if isTopLevelKey(line) && strings.HasPrefix(strings.TrimSpace(line), "engine:") { insertAt = i + 1 + for j := i + 1; j < len(lines); j++ { + if isTopLevelKey(lines[j]) { + insertAt = j + break + } + insertAt = j + 1 + } break } } @@ -228,42 +268,85 @@ func formatStringArrayInline(values []string) string { return "[" + strings.Join(quoted, ", ") + "]" } -func removeTopLevelBlockIfEmpty(lines []string, blockName string) []string { - blockIdx := -1 - blockEnd := len(lines) - for i, line := range lines { - if isTopLevelKey(line) && strings.HasPrefix(strings.TrimSpace(line), blockName+":") { - blockIdx = i - for j := i + 1; j < len(lines); j++ { - if isTopLevelKey(lines[j]) { - blockEnd = j +func removeBlockIfEmpty(lines []string, blockName string) []string { + result := make([]string, 0, len(lines)) + for i := 0; i < len(lines); { + line := lines[i] + trimmed := strings.TrimSpace(line) + if !strings.HasPrefix(trimmed, blockName+":") { + result = append(result, line) + i++ + continue + } + + valuePart := strings.TrimSpace(strings.TrimPrefix(trimmed, blockName+":")) + if valuePart != "" && !strings.HasPrefix(valuePart, "#") { + result = append(result, line) + i++ + continue + } + + blockIndent := getIndentation(line) + blockEnd := i + 1 + hasMeaningfulNestedContent := false + for ; blockEnd < len(lines); blockEnd++ { + nestedLine := lines[blockEnd] + nestedTrimmed := strings.TrimSpace(nestedLine) + if nestedTrimmed == "" { + continue + } + + nestedIndent := getIndentation(nestedLine) + if strings.HasPrefix(nestedTrimmed, "#") { + if len(nestedIndent) <= len(blockIndent) { break } + continue } - break + + if len(nestedIndent) <= len(blockIndent) && strings.Contains(nestedLine, ":") { + break + } + + hasMeaningfulNestedContent = true } + + if hasMeaningfulNestedContent { + result = append(result, line) + i++ + continue + } + + i = blockEnd } - if blockIdx == -1 { - return lines + return result +} + +func maybeUpdatePinnedSourceRef(content string, frontmatter map[string]any) string { + sourceAny, hasSource := frontmatter["source"] + if !hasSource { + return content } - hasMeaningfulNestedContent := false - for _, line := range lines[blockIdx+1 : blockEnd] { - trimmed := strings.TrimSpace(line) - if trimmed == "" || strings.HasPrefix(trimmed, "#") { - continue - } - hasMeaningfulNestedContent = true - break + source, ok := sourceAny.(string) + if !ok || strings.TrimSpace(source) == "" { + return content } - if hasMeaningfulNestedContent { - return lines + sourceSpec, err := parseSourceSpec(source) + if err != nil { + return content } - result := make([]string, 0, len(lines)-(blockEnd-blockIdx)) - result = append(result, lines[:blockIdx]...) - result = append(result, lines[blockEnd:]...) - return result + 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 } diff --git a/pkg/cli/codemod_serena_import_test.go b/pkg/cli/codemod_serena_import_test.go index 927dae1f546..a3d0c9bb795 100644 --- a/pkg/cli/codemod_serena_import_test.go +++ b/pkg/cli/codemod_serena_import_test.go @@ -152,4 +152,72 @@ imports: assert.False(t, applied, "Codemod should not be applied when tools.serena is absent") assert.Equal(t, content, result, "Content should remain unchanged when no migration is needed") }) + + t.Run("migrates engine.tools.serena and preserves engine sibling fields", func(t *testing.T) { + content := `--- +engine: + tools: + serena: + languages: ["typescript"] + model: gpt-5.2 + id: copilot +--- + +# Test Workflow +` + frontmatter := map[string]any{ + "engine": map[string]any{ + "tools": map[string]any{ + "serena": map[string]any{ + "languages": []any{"typescript"}, + }, + }, + "model": "gpt-5.2", + "id": "copilot", + }, + } + + 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 engine.tools.serena is present") + assert.Contains(t, result, "imports:", "Codemod should add imports block") + assert.Contains(t, result, "languages: [\"typescript\"]", "Codemod should preserve engine.tools.serena languages") + assert.NotContains(t, result, "\n tools:", "Codemod should remove now-empty engine.tools block") + + parsed, parseErr := parser.ExtractFrontmatterFromContent(result) + require.NoError(t, parseErr, "Result should contain valid frontmatter") + engineAny, hasEngine := parsed.Frontmatter["engine"] + require.True(t, hasEngine, "Result should preserve engine block") + engine, ok := engineAny.(map[string]any) + require.True(t, ok, "Engine should remain an object when sibling fields remain") + assert.Equal(t, "gpt-5.2", engine["model"], "Engine model should remain under engine block") + assert.Equal(t, "copilot", engine["id"], "Engine id should remain under engine block") + _, hasEngineTools := engine["tools"] + 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) { + content := `--- +source: github/gh-aw/.github/workflows/duplicate-code-detector.md@852cb06ad52958b402ed982b69957ffc57ca0619 +engine: copilot +tools: + serena: ["typescript"] +--- + +# Test Workflow +` + frontmatter := map[string]any{ + "source": "github/gh-aw/.github/workflows/duplicate-code-detector.md@852cb06ad52958b402ed982b69957ffc57ca0619", + "engine": "copilot", + "tools": map[string]any{ + "serena": []any{"typescript"}, + }, + } + + 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, "- uses: shared/mcp/serena.md", "Codemod should still add shared Serena import") + }) } From ab5bd4e66637682da24afcd7fa2a5ba042fe2417 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 23 Apr 2026 14:06:51 +0000 Subject: [PATCH 4/6] refactor: simplify serena codemod block scanning and engine insert logic Agent-Logs-Url: https://github.com/github/gh-aw/sessions/77db40ff-5e7f-49e1-85c8-2498e4f56230 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/codemod_serena_import.go | 54 +++++++++++++++++--------------- 1 file changed, 28 insertions(+), 26 deletions(-) diff --git a/pkg/cli/codemod_serena_import.go b/pkg/cli/codemod_serena_import.go index 38230b4b30e..4a891ee1ee5 100644 --- a/pkg/cli/codemod_serena_import.go +++ b/pkg/cli/codemod_serena_import.go @@ -237,13 +237,12 @@ func addSerenaImport(lines []string, languages []string) []string { insertAt := 0 for i, line := range lines { if isTopLevelKey(line) && strings.HasPrefix(strings.TrimSpace(line), "engine:") { - insertAt = i + 1 + insertAt = len(lines) for j := i + 1; j < len(lines); j++ { if isTopLevelKey(lines[j]) { insertAt = j break } - insertAt = j + 1 } break } @@ -286,30 +285,7 @@ func removeBlockIfEmpty(lines []string, blockName string) []string { continue } - blockIndent := getIndentation(line) - blockEnd := i + 1 - hasMeaningfulNestedContent := false - for ; blockEnd < len(lines); blockEnd++ { - nestedLine := lines[blockEnd] - nestedTrimmed := strings.TrimSpace(nestedLine) - if nestedTrimmed == "" { - continue - } - - nestedIndent := getIndentation(nestedLine) - if strings.HasPrefix(nestedTrimmed, "#") { - if len(nestedIndent) <= len(blockIndent) { - break - } - continue - } - - if len(nestedIndent) <= len(blockIndent) && strings.Contains(nestedLine, ":") { - break - } - - hasMeaningfulNestedContent = true - } + hasMeaningfulNestedContent, blockEnd := hasNestedContent(lines, i+1, getIndentation(line)) if hasMeaningfulNestedContent { result = append(result, line) @@ -323,6 +299,32 @@ func removeBlockIfEmpty(lines []string, blockName string) []string { return result } +func hasNestedContent(lines []string, startIndex int, blockIndent string) (bool, int) { + for i := startIndex; i < len(lines); i++ { + nestedLine := lines[i] + nestedTrimmed := strings.TrimSpace(nestedLine) + if nestedTrimmed == "" { + continue + } + + nestedIndent := getIndentation(nestedLine) + if strings.HasPrefix(nestedTrimmed, "#") { + if len(nestedIndent) <= len(blockIndent) { + return false, i + } + continue + } + + if len(nestedIndent) <= len(blockIndent) && strings.Contains(nestedLine, ":") { + return false, i + } + + return true, i + } + + return false, len(lines) +} + func maybeUpdatePinnedSourceRef(content string, frontmatter map[string]any) string { sourceAny, hasSource := frontmatter["source"] if !hasSource { From a0a3471a8331c8b42cf3993d96810b35f753e0a0 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 23 Apr 2026 14:17:24 +0000 Subject: [PATCH 5/6] docs(adr): add draft ADR-28080 for multi-path serena codemod Records the architectural decisions behind extending the serena-tools-to-shared-import codemod to handle engine.tools.serena, fix structural YAML block insertion/removal, and rewrite pinned source SHA refs to @main during migration. Co-Authored-By: Claude Sonnet 4.6 --- ...a-codemod-with-structural-yaml-mutation.md | 81 +++++++++++++++++++ 1 file changed, 81 insertions(+) create mode 100644 docs/adr/28080-multi-path-serena-codemod-with-structural-yaml-mutation.md diff --git a/docs/adr/28080-multi-path-serena-codemod-with-structural-yaml-mutation.md b/docs/adr/28080-multi-path-serena-codemod-with-structural-yaml-mutation.md new file mode 100644 index 00000000000..68cb3591d10 --- /dev/null +++ b/docs/adr/28080-multi-path-serena-codemod-with-structural-yaml-mutation.md @@ -0,0 +1,81 @@ +# ADR-28080: Multi-Path Serena Config Discovery and Structural YAML Mutation in Codemod + +**Date**: 2026-04-23 +**Status**: Draft +**Deciders**: pelikhan + +--- + +## Part 1 — Narrative (Human-Friendly) + +### Context + +The `serena-tools-to-shared-import` codemod automates migration of inline Serena tool configuration to the shared `shared/mcp/serena.md` import. The original implementation only detected Serena config at the top-level `tools.serena` YAML path. However, workflows that specify a custom LLM engine can place the same config under `engine.tools.serena` instead. When such workflows are processed by the old codemod, the migration is silently skipped, leaving the workflow non-compilable after the shared import becomes mandatory. A second compounding issue: workflows pinned to a specific `github/gh-aw` commit SHA via `source:` reference an older version of the import chain that predates the required `with.languages` input, so even a successfully migrated workflow would fail validation at the pinned version. + +### Decision + +We will extend `findSerenaLanguagesForMigration` to probe both `tools.serena` (top-level) and `engine.tools.serena` (nested under an engine block), treating whichever location is populated as the migration source. We will fix YAML block insertion to scan forward to the end of the entire `engine` block before inserting the new `imports` entry, preserving sibling engine fields such as `model` and `id`. We will replace the simple top-level block removal with an indentation-aware `removeBlockIfEmpty` that avoids deleting engine blocks that retain meaningful sibling content. Finally, we will add `maybeUpdatePinnedSourceRef`, which rewrites any `source:` value pointing to `github/gh-aw` at a 40-character commit SHA to `@main` during the same migration pass, preventing stale-import validation failures. + +### Alternatives Considered + +#### Alternative 1: Separate Codemods per YAML Path + +Define a second, independent codemod that handles `engine.tools.serena` exclusively, leaving the original `tools.serena` codemod unchanged. This keeps each codemod small and single-purpose, but requires both to be applied in the correct order and makes it easy for a caller to apply only one — leaving workflows in a half-migrated state that is harder to debug than no migration at all. + +#### Alternative 2: Require Manual Migration for Engine-Scoped Serena Config + +Document that `engine.tools.serena` requires manual migration and skip automation entirely. This is low risk for the codemod itself but shifts toil onto every workflow author whose workflow uses this pattern, and provides no protection against the `source:` pin problem. Given the volume of affected workflows in the repository, manual migration was not a viable path. + +### Consequences + +#### Positive +- Workflows using `engine.tools.serena` are now migrated automatically, closing a silent failure mode. +- The `@main` source pin rewrite prevents post-migration validation failures against stale upstream import chains that lack the now-required `with.languages` input. +- Indentation-aware block removal preserves `engine` sibling fields (`id`, `model`) at the correct YAML depth, making the transformation structurally correct for the full range of engine block shapes. + +#### Negative +- `findSerenaLanguagesForMigration` now encodes a precedence rule (top-level `tools.serena` wins over `engine.tools.serena`); if a workflow has both, only the top-level value is used and the engine-scoped config is silently discarded. +- The `maybeUpdatePinnedSourceRef` rewrite is scoped to `github/gh-aw` sources pinned by commit SHA — workflows pinned to forks, tags, or other repos are unaffected, requiring separate handling if the same pin-staleness problem arises there. +- Indentation-aware YAML mutation increases the complexity of `removeBlockIfEmpty` and `hasNestedContent`, making edge cases harder to reason about without a property-based test suite. + +#### Neutral +- The change touches only `pkg/cli/codemod_serena_import.go`, keeping scope narrow and the diff reviewable in a single pass. +- Regression tests added for both `engine.tools.serena` migration with sibling preservation and the `source:` SHA rewrite path. + +--- + +## Part 2 — Normative Specification (RFC 2119) + +> The key words **MUST**, **MUST NOT**, **REQUIRED**, **SHALL**, **SHALL NOT**, **SHOULD**, **SHOULD NOT**, **RECOMMENDED**, **MAY**, and **OPTIONAL** in this section are to be interpreted as described in [RFC 2119](https://www.rfc-editor.org/rfc/rfc2119). + +### Serena Config Discovery + +1. Implementations **MUST** detect Serena configuration at `tools.serena` (top-level) before checking `engine.tools.serena`. +2. Implementations **MUST** fall back to `engine.tools.serena` when no `tools.serena` key is present in the frontmatter. +3. Implementations **MUST NOT** attempt migration when neither location contains a non-empty `languages` list. +4. Implementations **MUST NOT** merge languages from both paths; the first matching path **SHALL** be the sole source of the language list. + +### YAML Block Insertion + +1. When inserting the `imports` block adjacent to an `engine:` block, implementations **MUST** scan forward to the end of the full `engine` block (i.e., until the next top-level key or end-of-document) before computing the insertion point. +2. Implementations **MUST NOT** insert the `imports` block on the line immediately following the `engine:` key, as this would interleave the new block with the engine block's nested content. + +### Empty-Block Removal + +1. Implementations **MUST** remove a YAML block (`tools`, `engine`) only when it contains no meaningful nested content after migration — where "meaningful" means a non-empty, non-comment child line indented deeper than the block key. +2. Implementations **MUST** preserve the enclosing block (e.g., `engine:`) when sibling fields remain after the migrated sub-key (`tools`) is removed. +3. Implementations **MUST NOT** remove a block based solely on indentation depth; the check **MUST** inspect actual content. + +### Pinned Source Ref Rewrite + +1. When a migration is applied and `source:` is present in frontmatter, implementations **MUST** rewrite the ref to `@main` if and only if: (a) the source repo is `github/gh-aw`, and (b) the current ref is exactly a 40-character hexadecimal commit SHA. +2. Implementations **MUST NOT** rewrite `source:` values that point to a different repository, use a named branch, or use a tag. +3. Implementations **SHOULD** perform the source ref rewrite in the same migration pass as the `tools.serena` removal to keep the workflow in a consistent state after a single codemod run. + +### Conformance + +An implementation is considered conformant with this ADR if it satisfies all **MUST** and **MUST NOT** requirements above. Failure to meet any **MUST** or **MUST NOT** requirement constitutes non-conformance. + +--- + +*This is a DRAFT ADR generated by the [Design Decision Gate](https://github.com/github/gh-aw/actions/runs/24840177385) workflow. The PR author must review, complete, and finalize this document before the PR can merge.* From c6f0ecf487dccf02e6d7f79dec9555ba8ceb82f2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 23 Apr 2026 14:51:48 +0000 Subject: [PATCH 6/6] fix: address serena codemod review comments Agent-Logs-Url: https://github.com/github/gh-aw/sessions/b5e487ba-2124-47fd-8fcf-8e104de3e9ae Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/codemod_serena_import.go | 8 +++---- pkg/cli/codemod_serena_import_test.go | 34 +++++++++++++++++++++++++++ pkg/cli/fix_command.go | 15 +++++++++++- 3 files changed, 52 insertions(+), 5 deletions(-) diff --git a/pkg/cli/codemod_serena_import.go b/pkg/cli/codemod_serena_import.go index 4a891ee1ee5..a0ee93d8849 100644 --- a/pkg/cli/codemod_serena_import.go +++ b/pkg/cli/codemod_serena_import.go @@ -13,12 +13,13 @@ import ( var serenaImportCodemodLog = logger.New("cli:codemod_serena_import") // getSerenaToSharedImportCodemod creates a codemod that migrates removed tools.serena -// configuration to an equivalent imports entry using shared/mcp/serena.md. +// or engine.tools.serena configuration to an equivalent imports entry using +// shared/mcp/serena.md, and may normalize a pinned source ref to @main. func getSerenaToSharedImportCodemod() Codemod { return Codemod{ ID: "serena-tools-to-shared-import", - Name: "Migrate tools.serena to shared Serena import", - Description: "Removes 'tools.serena' and adds an equivalent 'imports' entry using shared/mcp/serena.md with languages.", + 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'.", IntroducedIn: "1.0.0", Apply: func(content string, frontmatter map[string]any) (string, bool, error) { languages, ok := findSerenaLanguagesForMigration(frontmatter) @@ -65,7 +66,6 @@ func findSerenaLanguagesForMigration(frontmatter map[string]any) ([]string, bool if ok && len(languages) > 0 { return languages, true } - return nil, false } } } diff --git a/pkg/cli/codemod_serena_import_test.go b/pkg/cli/codemod_serena_import_test.go index a3d0c9bb795..d3bd21d49f8 100644 --- a/pkg/cli/codemod_serena_import_test.go +++ b/pkg/cli/codemod_serena_import_test.go @@ -220,4 +220,38 @@ tools: 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, "- uses: shared/mcp/serena.md", "Codemod should still add shared Serena import") }) + + t.Run("falls back to engine.tools.serena when top-level tools.serena is invalid", func(t *testing.T) { + content := `--- +engine: + tools: + serena: + languages: ["typescript"] + id: copilot +tools: + serena: {} +--- + +# Test Workflow +` + frontmatter := map[string]any{ + "engine": map[string]any{ + "tools": map[string]any{ + "serena": map[string]any{ + "languages": []any{"typescript"}, + }, + }, + "id": "copilot", + }, + "tools": map[string]any{ + "serena": map[string]any{}, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + require.NoError(t, err, "Codemod should not return an error") + assert.True(t, applied, "Codemod should fall back to engine.tools.serena when top-level tools.serena is invalid") + assert.Contains(t, result, "- uses: shared/mcp/serena.md", "Codemod should add shared Serena import") + assert.Contains(t, result, "languages: [\"typescript\"]", "Codemod should use languages from engine.tools.serena") + }) } diff --git a/pkg/cli/fix_command.go b/pkg/cli/fix_command.go index e020c6daa0f..ac0cfb68615 100644 --- a/pkg/cli/fix_command.go +++ b/pkg/cli/fix_command.go @@ -345,7 +345,11 @@ imports: ` func scaffoldSerenaSharedWorkflowIfNeeded(filePath string, appliedCodemods []string, content string, verbose bool) error { - if !wasCodemodApplied(appliedCodemods, "Migrate tools.serena to shared Serena import") { + if !wasAnyCodemodApplied( + appliedCodemods, + "Migrate tools.serena to shared Serena import", + "Migrate tools.serena or engine.tools.serena to shared Serena import", + ) { return nil } if !strings.Contains(content, "shared/mcp/serena.md") { @@ -379,6 +383,15 @@ func wasCodemodApplied(appliedCodemods []string, codemodName string) bool { return slices.Contains(appliedCodemods, codemodName) } +func wasAnyCodemodApplied(appliedCodemods []string, codemodNames ...string) bool { + for _, codemodName := range codemodNames { + if wasCodemodApplied(appliedCodemods, codemodName) { + return true + } + } + return false +} + func resolveWorkflowRoot(filePath string) string { clean := filepath.Clean(filePath) needle := filepath.Join(".github", "workflows")