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
88 changes: 80 additions & 8 deletions pkg/workflow/apm_dependencies_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,8 @@ func TestExtractAPMDependenciesFromFrontmatter(t *testing.T) {

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := extractAPMDependenciesFromFrontmatter(tt.frontmatter)
result, err := extractAPMDependenciesFromFrontmatter(tt.frontmatter)
require.NoError(t, err, "Should not return an error for valid frontmatter")
if tt.expectedDeps == nil {
assert.Nil(t, result, "Should return nil for no dependencies")
} else {
Expand All @@ -147,7 +148,8 @@ func TestExtractAPMDependenciesGitHubApp(t *testing.T) {
},
},
}
result := extractAPMDependenciesFromFrontmatter(frontmatter)
result, err := extractAPMDependenciesFromFrontmatter(frontmatter)
require.NoError(t, err, "Should not return an error")
require.NotNil(t, result, "Should return non-nil APMDependenciesInfo")
assert.Equal(t, []string{"acme-org/acme-skills/plugins/dev-tools"}, result.Packages)
require.NotNil(t, result.GitHubApp, "GitHubApp should be set")
Expand All @@ -167,7 +169,8 @@ func TestExtractAPMDependenciesGitHubApp(t *testing.T) {
},
},
}
result := extractAPMDependenciesFromFrontmatter(frontmatter)
result, err := extractAPMDependenciesFromFrontmatter(frontmatter)
require.NoError(t, err, "Should not return an error")
require.NotNil(t, result, "Should return non-nil APMDependenciesInfo")
require.NotNil(t, result.GitHubApp, "GitHubApp should be set")
assert.Equal(t, "acme-org", result.GitHubApp.Owner)
Expand All @@ -183,7 +186,8 @@ func TestExtractAPMDependenciesGitHubApp(t *testing.T) {
},
},
}
result := extractAPMDependenciesFromFrontmatter(frontmatter)
result, err := extractAPMDependenciesFromFrontmatter(frontmatter)
require.NoError(t, err, "Should not return an error")
require.NotNil(t, result, "Packages should still be extracted")
assert.Nil(t, result.GitHubApp, "GitHubApp should be nil when app-id is missing")
})
Expand All @@ -192,7 +196,8 @@ func TestExtractAPMDependenciesGitHubApp(t *testing.T) {
frontmatter := map[string]any{
"dependencies": []any{"microsoft/apm-sample-package"},
}
result := extractAPMDependenciesFromFrontmatter(frontmatter)
result, err := extractAPMDependenciesFromFrontmatter(frontmatter)
require.NoError(t, err, "Should not return an error")
require.NotNil(t, result, "Should return non-nil APMDependenciesInfo")
assert.Nil(t, result.GitHubApp, "GitHubApp should be nil for array format")
})
Expand All @@ -206,7 +211,8 @@ func TestExtractAPMDependenciesVersion(t *testing.T) {
"version": "v1.0.0",
},
}
result := extractAPMDependenciesFromFrontmatter(frontmatter)
result, err := extractAPMDependenciesFromFrontmatter(frontmatter)
require.NoError(t, err, "Should not return an error for valid version")
require.NotNil(t, result, "Should return non-nil APMDependenciesInfo")
assert.Equal(t, "v1.0.0", result.Version, "Version should be extracted from object format")
})
Expand All @@ -215,7 +221,8 @@ func TestExtractAPMDependenciesVersion(t *testing.T) {
frontmatter := map[string]any{
"dependencies": []any{"microsoft/apm-sample-package"},
}
result := extractAPMDependenciesFromFrontmatter(frontmatter)
result, err := extractAPMDependenciesFromFrontmatter(frontmatter)
require.NoError(t, err, "Should not return an error")
require.NotNil(t, result, "Should return non-nil APMDependenciesInfo")
assert.Empty(t, result.Version, "Version should be empty for array format")
})
Expand All @@ -226,10 +233,75 @@ func TestExtractAPMDependenciesVersion(t *testing.T) {
"packages": []any{"microsoft/apm-sample-package"},
},
}
result := extractAPMDependenciesFromFrontmatter(frontmatter)
result, err := extractAPMDependenciesFromFrontmatter(frontmatter)
require.NoError(t, err, "Should not return an error")
require.NotNil(t, result, "Should return non-nil APMDependenciesInfo")
assert.Empty(t, result.Version, "Version should be empty when not specified")
})

t.Run("Invalid version with trailing quote produces error", func(t *testing.T) {
frontmatter := map[string]any{
"dependencies": map[string]any{
"packages": []any{"microsoft/apm-sample-package"},
"version": `v0.8.0"`,
},
}
result, err := extractAPMDependenciesFromFrontmatter(frontmatter)
require.Error(t, err, "Should return an error for invalid version tag")
assert.Nil(t, result, "Should return nil result on error")
assert.Contains(t, err.Error(), "dependencies.version", "Error should mention the field name")
})

t.Run("Invalid version without v prefix produces error", func(t *testing.T) {
frontmatter := map[string]any{
"dependencies": map[string]any{
"packages": []any{"microsoft/apm-sample-package"},
"version": "1.2.3",
},
}
result, err := extractAPMDependenciesFromFrontmatter(frontmatter)
require.Error(t, err, "Should return an error for version missing v prefix")
assert.Nil(t, result, "Should return nil result on error")
assert.Contains(t, err.Error(), "vX.Y.Z", "Error should describe expected format")
})

t.Run("Invalid version string 'latest' produces error", func(t *testing.T) {
frontmatter := map[string]any{
"dependencies": map[string]any{
"packages": []any{"microsoft/apm-sample-package"},
"version": "latest",
},
}
result, err := extractAPMDependenciesFromFrontmatter(frontmatter)
require.Error(t, err, "Should return an error for non-semver version string")
assert.Nil(t, result, "Should return nil result on error")
})

t.Run("Valid partial version v1 compiles without error", func(t *testing.T) {
frontmatter := map[string]any{
"dependencies": map[string]any{
"packages": []any{"microsoft/apm-sample-package"},
"version": "v1",
},
}
result, err := extractAPMDependenciesFromFrontmatter(frontmatter)
require.NoError(t, err, "Should not return an error for valid partial version")
require.NotNil(t, result, "Should return non-nil APMDependenciesInfo")
assert.Equal(t, "v1", result.Version, "Version should be extracted")
})

t.Run("Valid partial version v1.2 compiles without error", func(t *testing.T) {
frontmatter := map[string]any{
"dependencies": map[string]any{
"packages": []any{"microsoft/apm-sample-package"},
"version": "v1.2",
},
}
result, err := extractAPMDependenciesFromFrontmatter(frontmatter)
require.NoError(t, err, "Should not return an error for valid partial version")
require.NotNil(t, result, "Should return non-nil APMDependenciesInfo")
assert.Equal(t, "v1.2", result.Version, "Version should be extracted")
})
}

func TestEngineGetAPMTarget(t *testing.T) {
Expand Down
5 changes: 4 additions & 1 deletion pkg/workflow/compiler_orchestrator_tools.go
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,10 @@ func (c *Compiler) processToolsAndMarkdown(result *parser.FrontmatterResult, cle
}

// Extract APM dependencies from frontmatter
apmDependencies := extractAPMDependenciesFromFrontmatter(result.Frontmatter)
apmDependencies, err := extractAPMDependenciesFromFrontmatter(result.Frontmatter)
if err != nil {
return nil, err
}
if apmDependencies != nil {
orchestratorToolsLog.Printf("Extracted %d APM dependencies from frontmatter", len(apmDependencies.Packages))
}
Expand Down
13 changes: 8 additions & 5 deletions pkg/workflow/frontmatter_extraction_metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -351,10 +351,10 @@ func extractPluginsFromFrontmatter(frontmatter map[string]any) *PluginInfo {
// - Object format: {packages: ["org/pkg1", "org/pkg2"], isolated: true, github-app: {...}, version: "v0.8.0"}
//
// Returns nil if no dependencies field is present or if the field contains no packages.
func extractAPMDependenciesFromFrontmatter(frontmatter map[string]any) *APMDependenciesInfo {
func extractAPMDependenciesFromFrontmatter(frontmatter map[string]any) (*APMDependenciesInfo, error) {
value, exists := frontmatter["dependencies"]
if !exists {
return nil
return nil, nil
}

var packages []string
Expand Down Expand Up @@ -397,17 +397,20 @@ func extractAPMDependenciesFromFrontmatter(frontmatter map[string]any) *APMDepen
}
if versionAny, ok := v["version"]; ok {
if versionStr, ok := versionAny.(string); ok && versionStr != "" {
if !isValidVersionTag(versionStr) {
return nil, fmt.Errorf("dependencies.version %q is not a valid semver tag (expected format: vX.Y.Z)", versionStr)
}
version = versionStr
}
}
Comment on lines 398 to 405
default:
return nil
return nil, nil
}

if len(packages) == 0 {
return nil
return nil, nil
}

frontmatterMetadataLog.Printf("Extracted %d APM dependency packages from frontmatter (isolated=%v, github-app=%v, version=%s)", len(packages), isolated, githubApp != nil, version)
return &APMDependenciesInfo{Packages: packages, Isolated: isolated, GitHubApp: githubApp, Version: version}
return &APMDependenciesInfo{Packages: packages, Isolated: isolated, GitHubApp: githubApp, Version: version}, nil
}
Loading