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
2 changes: 2 additions & 0 deletions pkg/parser/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,8 @@ The package is designed for use both in the main CLI binary and in WebAssembly c
| `GetSafeOutputTypeKeys` | `func() ([]string, error)` | Returns valid safe-output type keys from the schema |
| `GetMainWorkflowDeprecatedFields` | `func() ([]DeprecatedField, error)` | Returns deprecated frontmatter fields with migration notes |
| `FindDeprecatedFieldsInFrontmatter` | `func(map[string]any, []DeprecatedField) []DeprecatedField` | Finds deprecated fields present in a parsed frontmatter map |
| `GetMainWorkflowDeprecatedFieldsDeep` | `func() ([]DeprecatedField, error)` | Returns deprecated fields at any schema nesting level (e.g. `tools.grep`) with dot-separated paths |
| `FindDeprecatedFieldsInFrontmatterDeep` | `func(map[string]any, []DeprecatedField) []DeprecatedField` | Finds deprecated fields at any nesting depth in frontmatter using dot-separated paths |
| `FindClosestMatches` | `func(target string, candidates []string, maxResults int) []string` | Finds the closest string matches (for typo suggestions) |
| `LevenshteinDistance` | `func(a, b string) int` | Computes edit distance between two strings |

Expand Down
213 changes: 213 additions & 0 deletions pkg/parser/schema_deprecated_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,3 +140,216 @@ func TestExtractReplacementFromDescription(t *testing.T) {
})
}
}

// --- Tests for deep walker -------------------------------------------------------

func TestGetMainWorkflowDeprecatedFieldsDeep(t *testing.T) {
fields, err := GetMainWorkflowDeprecatedFieldsDeep()
if err != nil {
t.Fatalf("GetMainWorkflowDeprecatedFieldsDeep() error = %v", err)
}

// Build path→field map for easy lookup.
byPath := make(map[string]DeprecatedField, len(fields))
for _, f := range fields {
byPath[f.Path] = f
}

// tools.grep must be detected with its x-deprecation-message.
grep, ok := byPath["tools.grep"]
if !ok {
t.Error("expected 'tools.grep' in deep deprecated fields, not found")
} else {
if grep.DeprecationMessage == "" {
t.Error("tools.grep: DeprecationMessage should not be empty")
}
}

// tools.serena must be detected.
if _, ok := byPath["tools.serena"]; !ok {
t.Error("expected 'tools.serena' in deep deprecated fields, not found")
}

// tools.github.repos must be detected with its x-deprecation-message.
repos, ok := byPath["tools.github.repos"]
if !ok {
t.Error("expected 'tools.github.repos' in deep deprecated fields, not found")
} else {
if repos.DeprecationMessage == "" {
t.Error("tools.github.repos: DeprecationMessage should not be empty")
}
}

// tools.github.toolset must be detected.
if _, ok := byPath["tools.github.toolset"]; !ok {
t.Error("expected 'tools.github.toolset' in deep deprecated fields, not found")
}

t.Logf("Found %d deep deprecated fields in schema", len(fields))
}

func TestCollectDeprecatedDeep(t *testing.T) {
// Build a minimal schema that exercises nesting and oneOf traversal.
schema := map[string]any{
"properties": map[string]any{
"tools": map[string]any{
"properties": map[string]any{
"grep": map[string]any{
"deprecated": true,
"description": "DEPRECATED: grep is always available.",
"x-deprecation-message": "Use bash instead.",
},
"github": map[string]any{
"oneOf": []any{
map[string]any{"type": "boolean"},
map[string]any{
"type": "object",
"properties": map[string]any{
"repos": map[string]any{
"deprecated": true,
"description": "Deprecated. Use 'allowed-repos' instead.",
"x-deprecation-message": "'tools.github.repos' is deprecated. Use 'tools.github.allowed-repos' instead.",
},
},
},
},
},
},
},
},
}

var results []DeprecatedField
collectDeprecatedDeep(schema, "", &results)

byPath := make(map[string]DeprecatedField)
for _, f := range results {
byPath[f.Path] = f
}

if _, ok := byPath["tools.grep"]; !ok {
t.Error("expected tools.grep to be found")
}
if _, ok := byPath["tools.github.repos"]; !ok {
t.Error("expected tools.github.repos to be found")
}
// Non-deprecated fields must not appear.
if _, ok := byPath["tools"]; ok {
t.Error("tools (non-deprecated) should not appear")
}
if _, ok := byPath["tools.github"]; ok {
t.Error("tools.github (non-deprecated) should not appear")
}
}

func TestFindDeprecatedFieldsInFrontmatterDeep(t *testing.T) {
fields := []DeprecatedField{
{Name: "grep", Path: "tools.grep", DeprecationMessage: "Use bash instead."},
{Name: "repos", Path: "tools.github.repos", DeprecationMessage: "Use allowed-repos."},
{Name: "old", Path: "old", DeprecationMessage: "Field removed."},
}

tests := []struct {
name string
frontmatter map[string]any
wantPaths []string
}{
{
name: "no deprecated fields used",
frontmatter: map[string]any{
"engine": "copilot",
"tools": map[string]any{"bash": true},
},
wantPaths: nil,
},
{
name: "tools.grep present",
frontmatter: map[string]any{
"tools": map[string]any{"grep": true},
},
wantPaths: []string{"tools.grep"},
},
{
name: "nested tools.github.repos present",
frontmatter: map[string]any{
"tools": map[string]any{
"github": map[string]any{"repos": []any{"owner/repo"}},
},
},
wantPaths: []string{"tools.github.repos"},
},
{
name: "top-level deprecated field present",
frontmatter: map[string]any{
"old": "value",
},
wantPaths: []string{"old"},
},
{
name: "multiple deprecated fields",
frontmatter: map[string]any{
"old": "value",
"tools": map[string]any{"grep": true},
},
wantPaths: []string{"old", "tools.grep"},
},
{
name: "tools key present but not grep",
frontmatter: map[string]any{
"tools": map[string]any{"bash": true},
},
wantPaths: nil,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
found := FindDeprecatedFieldsInFrontmatterDeep(tt.frontmatter, fields)

foundPaths := make(map[string]bool)
for _, f := range found {
foundPaths[f.Path] = true
}

if len(found) != len(tt.wantPaths) {
t.Errorf("FindDeprecatedFieldsInFrontmatterDeep() found %d, want %d (%v vs %v)",
len(found), len(tt.wantPaths), foundPaths, tt.wantPaths)
}
for _, p := range tt.wantPaths {
if !foundPaths[p] {
t.Errorf("expected path %q to be found, got %v", p, foundPaths)
}
}
})
}
}

func TestFieldExistsAtPath(t *testing.T) {
m := map[string]any{
"tools": map[string]any{
"grep": true,
"github": map[string]any{"repos": []any{"owner/repo"}},
},
}

tests := []struct {
segments []string
want bool
}{
{[]string{"tools"}, true},
{[]string{"tools", "grep"}, true},
{[]string{"tools", "github", "repos"}, true},
{[]string{"tools", "bash"}, false},
{[]string{"engine"}, false},
{[]string{}, false},
// tools is not a scalar — deeper navigation not possible from non-map
{[]string{"tools", "grep", "nested"}, false},
}

for _, tt := range tests {
got := fieldExistsAtPath(m, tt.segments)
if got != tt.want {
t.Errorf("fieldExistsAtPath(%v) = %v, want %v", tt.segments, got, tt.want)
}
}
}
152 changes: 149 additions & 3 deletions pkg/parser/schema_deprecation.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
"fmt"
"regexp"
"sort"
"strings"
"sync"

"github.com/github/gh-aw/pkg/logger"
Expand All @@ -14,9 +15,11 @@

// DeprecatedField represents a deprecated field with its replacement information
type DeprecatedField struct {
Name string // The deprecated field name
Replacement string // The recommended replacement field name
Description string // Description from the schema
Name string // The deprecated field name (leaf key only)
Path string // Full dot-separated path, e.g. "tools.grep" (empty for top-level fields)
Replacement string // The recommended replacement field name
Description string // Description from the schema
DeprecationMessage string // x-deprecation-message from the schema (preferred user-facing text)
}

// deprecatedFieldsCache caches the result of parsing the main workflow schema so that
Expand All @@ -38,7 +41,7 @@
schemaDeprecationLog.Print("Getting deprecated fields from main workflow schema")
var schemaDoc map[string]any
if err := json.Unmarshal([]byte(mainWorkflowSchema), &schemaDoc); err != nil {
deprecatedFieldsErr = fmt.Errorf("failed to parse main workflow schema: %w", err)

Check failure on line 44 in pkg/parser/schema_deprecation.go

View workflow job for this annotation

GitHub Actions / error-message-lint

avoid generic 'failed to ...: %w' wrapping; add specific recovery guidance

Check failure on line 44 in pkg/parser/schema_deprecation.go

View workflow job for this annotation

GitHub Actions / error-message-lint

error message uses negative language without constructive guidance; include expected/requires/should/example details
return
}
fields, err := extractDeprecatedFields(schemaDoc)
Expand Down Expand Up @@ -135,3 +138,146 @@
schemaDeprecationLog.Printf("Deprecated field check complete: found %d of %d fields in frontmatter", len(found), len(deprecatedFields))
return found
}

// ---- Deep (nested) schema walker ------------------------------------------------

// deprecatedFieldsDeepCache caches the result of the deep schema walk so the
// expensive 414 KB JSON unmarshal is only performed once per process lifetime.
var (
deprecatedFieldsDeepOnce sync.Once
deprecatedFieldsDeepCache []DeprecatedField
deprecatedFieldsDeepErr error
)

// GetMainWorkflowDeprecatedFieldsDeep returns deprecated fields from the entire
// schema hierarchy (nested properties and oneOf variants) as dot-separated paths
// (e.g. "tools.grep", "tools.github.repos").
// The result is cached after the first call so the schema is only parsed once.
// Callers must not modify the returned slice.
func GetMainWorkflowDeprecatedFieldsDeep() ([]DeprecatedField, error) {
deprecatedFieldsDeepOnce.Do(func() {
schemaDeprecationLog.Print("Getting deep deprecated fields from main workflow schema")
var schemaDoc map[string]any
if err := json.Unmarshal([]byte(mainWorkflowSchema), &schemaDoc); err != nil {
deprecatedFieldsDeepErr = fmt.Errorf("failed to parse main workflow schema: %w", err)

Check failure on line 162 in pkg/parser/schema_deprecation.go

View workflow job for this annotation

GitHub Actions / error-message-lint

avoid generic 'failed to ...: %w' wrapping; add specific recovery guidance

Check failure on line 162 in pkg/parser/schema_deprecation.go

View workflow job for this annotation

GitHub Actions / error-message-lint

error message uses negative language without constructive guidance; include expected/requires/should/example details
return
}
var fields []DeprecatedField
collectDeprecatedDeep(schemaDoc, "", &fields)
sort.Slice(fields, func(i, j int) bool {
return fields[i].Path < fields[j].Path
})
deprecatedFieldsDeepCache = fields
schemaDeprecationLog.Printf("Found %d deprecated fields (deep) in main workflow schema", len(fields))
})
return deprecatedFieldsDeepCache, deprecatedFieldsDeepErr
}

// collectDeprecatedDeep recursively walks a schema node and appends any
// deprecated leaf properties to results.
//
// parentPath is the dot-joined path to the current schema node's parent
// (empty for the root schema). The function looks at the node's "properties"
// map (and "oneOf" / "anyOf" / "allOf" variants that may add more properties
// at the same level) to find fields marked deprecated:true.
func collectDeprecatedDeep(schemaNode map[string]any, parentPath string, results *[]DeprecatedField) {
properties, ok := schemaNode["properties"].(map[string]any)
if !ok {
return
}
Comment on lines +183 to +187

for fieldName, fieldSchema := range properties {
fieldSchemaMap, ok := fieldSchema.(map[string]any)
if !ok {
continue
}

path := fieldName
if parentPath != "" {
path = parentPath + "." + fieldName
}

if isDeprecated, ok := fieldSchemaMap["deprecated"].(bool); ok && isDeprecated {
description := ""
if desc, ok := fieldSchemaMap["description"].(string); ok {
description = desc
}
deprecationMsg := ""
if msg, ok := fieldSchemaMap["x-deprecation-message"].(string); ok {
deprecationMsg = msg
}
replacement := extractReplacementFromDescription(description)

*results = append(*results, DeprecatedField{
Name: fieldName,
Path: path,
Replacement: replacement,
Description: description,
DeprecationMessage: deprecationMsg,
})
// Do not recurse further into a deprecated field — its children
// are implicitly deprecated through the parent.
continue
}

// Recurse into nested properties.
collectDeprecatedDeep(fieldSchemaMap, path, results)

// Also recurse into oneOf / anyOf / allOf variants: these can introduce
// additional properties at the same level (e.g. tools.github has an
// oneOf with an object variant that owns toolset, repos, etc.).
for _, keyword := range []string{"oneOf", "anyOf", "allOf"} {
if variants, ok := fieldSchemaMap[keyword].([]any); ok {
for _, v := range variants {
if vm, ok := v.(map[string]any); ok {
collectDeprecatedDeep(vm, path, results)
}
}
}
}
}
}

// FindDeprecatedFieldsInFrontmatterDeep checks the full (possibly nested)
// frontmatter map for deprecated fields identified by their dot-separated paths.
// It returns every DeprecatedField whose path resolves to an existing key in the
// frontmatter (e.g. "tools.grep" matches frontmatter["tools"]["grep"]).
func FindDeprecatedFieldsInFrontmatterDeep(frontmatter map[string]any, deprecatedFields []DeprecatedField) []DeprecatedField {
schemaDeprecationLog.Printf("Deep-checking frontmatter for deprecated fields: %d fields to check", len(deprecatedFields))
var found []DeprecatedField

for _, f := range deprecatedFields {
lookupPath := f.Path
if lookupPath == "" {
lookupPath = f.Name
}
segments := strings.Split(lookupPath, ".")
if fieldExistsAtPath(frontmatter, segments) {
schemaDeprecationLog.Printf("Found deprecated field at path %q", lookupPath)
found = append(found, f)
}
}

schemaDeprecationLog.Printf("Deep deprecated field check complete: found %d", len(found))
return found
}

// fieldExistsAtPath reports whether the nested key path described by segments
// exists in m. An empty segments slice always returns false.
func fieldExistsAtPath(m map[string]any, segments []string) bool {
if len(segments) == 0 {
return false
}
value, exists := m[segments[0]]
if !exists {
return false
}
if len(segments) == 1 {
return true
}
nested, ok := value.(map[string]any)
if !ok {
return false
}
return fieldExistsAtPath(nested, segments[1:])
}
Loading
Loading