diff --git a/pkg/cli/mcp_server_helpers.go b/pkg/cli/mcp_server_helpers.go index 2adcb6a0821..03a3f3c3b81 100644 --- a/pkg/cli/mcp_server_helpers.go +++ b/pkg/cli/mcp_server_helpers.go @@ -126,15 +126,17 @@ func hasWriteAccess(permission string) bool { } } -// validateWorkflowName validates that a workflow name exists in the repository. +// validateMCPWorkflowName validates that a workflow name exists in the repository. // Returns nil if the workflow exists, or an error with suggestions if not. -// Empty workflow names are considered valid (means all workflows). +// Empty workflow names are considered valid (means "all workflows"). // -// Note: This function checks whether a workflow exists/is accessible, not its format. -// For format-only validation (alphanumeric characters, hyphens, underscores), -// use validators.go:ValidateWorkflowName instead. -func validateWorkflowName(workflowName string) error { - // Empty workflow name means "all workflows" - this is valid +// Note: Unlike ValidateWorkflowName in validators.go (which enforces strict format +// rules and rejects empty names), this MCP-specific function accepts empty names +// because in the MCP context an empty workflow name is a valid wildcard meaning +// "apply to all workflows". It also performs existence checks rather than format +// checks, delegating to workflow.ResolveWorkflowName and the live workflow list. +func validateMCPWorkflowName(workflowName string) error { + // Empty workflow name means "all workflows" - this is valid in the MCP context if workflowName == "" { return nil } diff --git a/pkg/cli/mcp_server_helpers_test.go b/pkg/cli/mcp_server_helpers_test.go index 1de96d14bed..b35dc926bbc 100644 --- a/pkg/cli/mcp_server_helpers_test.go +++ b/pkg/cli/mcp_server_helpers_test.go @@ -109,8 +109,8 @@ func TestHasWriteAccess(t *testing.T) { func TestValidateWorkflowName_Empty(t *testing.T) { // Empty workflow name is always valid (means "all workflows") - if err := validateWorkflowName(""); err != nil { - t.Errorf("validateWorkflowName(\"\") returned error: %v", err) + if err := validateMCPWorkflowName(""); err != nil { + t.Errorf("validateMCPWorkflowName(\"\") returned error: %v", err) } } diff --git a/pkg/cli/mcp_server_workflow_validation_test.go b/pkg/cli/mcp_server_workflow_validation_test.go index c26051f1324..9dffd60693f 100644 --- a/pkg/cli/mcp_server_workflow_validation_test.go +++ b/pkg/cli/mcp_server_workflow_validation_test.go @@ -42,7 +42,7 @@ func TestMCPValidateWorkflowName(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - err := validateWorkflowName(tt.workflowName) + err := validateMCPWorkflowName(tt.workflowName) if tt.shouldSucceed { assert.NoError(t, err, "Validation should succeed for workflow: %s", tt.workflowName) diff --git a/pkg/cli/mcp_tools_privileged.go b/pkg/cli/mcp_tools_privileged.go index d0ac85b8e1f..3fee3ef196e 100644 --- a/pkg/cli/mcp_tools_privileged.go +++ b/pkg/cli/mcp_tools_privileged.go @@ -89,7 +89,7 @@ return a schema description instead of the full output. Adjust the 'max_tokens' } // Validate workflow name before executing command - if err := validateWorkflowName(args.WorkflowName); err != nil { + if err := validateMCPWorkflowName(args.WorkflowName); err != nil { mcpLog.Printf("Workflow name validation failed: %v", err) return nil, nil, newMCPError(jsonrpc.CodeInvalidParams, err.Error(), map[string]any{ "workflow_name": args.WorkflowName, diff --git a/pkg/cli/remote_workflow_test.go b/pkg/cli/remote_workflow_test.go index 6859bad0a3b..1456f062fd0 100644 --- a/pkg/cli/remote_workflow_test.go +++ b/pkg/cli/remote_workflow_test.go @@ -410,7 +410,6 @@ imports: assert.Empty(t, entries, "uses: form workflowspec imports should not be downloaded") } - func TestFetchAndSaveRemoteFrontmatterImports_NoImportsNoOpTracker(t *testing.T) { // Build a minimal FileTracker without calling NewFileTracker (which requires a real // git repository). We only need the tracking lists populated. diff --git a/pkg/cli/semver.go b/pkg/cli/semver.go index e823093abd8..17281f52fc2 100644 --- a/pkg/cli/semver.go +++ b/pkg/cli/semver.go @@ -1,12 +1,10 @@ package cli import ( - "strconv" "strings" - "golang.org/x/mod/semver" - "github.com/github/gh-aw/pkg/logger" + "github.com/github/gh-aw/pkg/semverutil" ) var semverLog = logger.New("cli:semver") @@ -23,59 +21,25 @@ type semanticVersion struct { // isSemanticVersionTag checks if a ref string looks like a semantic version tag // Uses golang.org/x/mod/semver for proper semantic version validation func isSemanticVersionTag(ref string) bool { - // Ensure ref has 'v' prefix for semver package - if !strings.HasPrefix(ref, "v") { - ref = "v" + ref - } - return semver.IsValid(ref) + return semverutil.IsValid(ref) } // parseVersion parses a semantic version string // Uses golang.org/x/mod/semver for proper semantic version parsing func parseVersion(v string) *semanticVersion { semverLog.Printf("Parsing semantic version: %s", v) - // Ensure version has 'v' prefix for semver package - if !strings.HasPrefix(v, "v") { - v = "v" + v - } - - // Check if valid semantic version - if !semver.IsValid(v) { + parsed := semverutil.ParseVersion(v) + if parsed == nil { semverLog.Printf("Invalid semantic version: %s", v) return nil } - - ver := &semanticVersion{raw: strings.TrimPrefix(v, "v")} - - // Use semver.Canonical to get normalized version - canonical := semver.Canonical(v) - - // Parse major, minor, patch from canonical form - // Strip prerelease and build metadata before splitting, since semver.Canonical - // preserves the prerelease suffix (e.g. "v1.2.3-beta.1" stays "v1.2.3-beta.1") - corePart := strings.TrimPrefix(canonical, "v") - if idx := strings.IndexAny(corePart, "-+"); idx >= 0 { - corePart = corePart[:idx] - } - parts := strings.Split(corePart, ".") - // Parse the numeric components; strconv.Atoi returns 0 on error, matching - // the previous behavior where non-numeric input produced 0. - if len(parts) >= 1 { - ver.major, _ = strconv.Atoi(parts[0]) - } - if len(parts) >= 2 { - ver.minor, _ = strconv.Atoi(parts[1]) + return &semanticVersion{ + major: parsed.Major, + minor: parsed.Minor, + patch: parsed.Patch, + pre: parsed.Pre, + raw: parsed.Raw, } - if len(parts) >= 3 { - ver.patch, _ = strconv.Atoi(parts[2]) - } - - // Get prerelease if any - prerelease := semver.Prerelease(v) - // semver.Prerelease includes the leading hyphen, strip it - ver.pre = strings.TrimPrefix(prerelease, "-") - - return ver } // isPreciseVersion returns true if this version has explicit minor and patch components @@ -94,14 +58,7 @@ func (v *semanticVersion) isPreciseVersion() bool { // isNewer returns true if this version is newer than the other // Uses golang.org/x/mod/semver.Compare for proper semantic version comparison func (v *semanticVersion) isNewer(other *semanticVersion) bool { - // Ensure versions have 'v' prefix for semver package - v1 := "v" + v.raw - v2 := "v" + other.raw - - // Use semver.Compare for comparison - result := semver.Compare(v1, v2) - - isNewer := result > 0 + isNewer := semverutil.Compare(v.raw, other.raw) > 0 semverLog.Printf("Version comparison: %s vs %s, isNewer=%v", v.raw, other.raw, isNewer) return isNewer } diff --git a/pkg/semverutil/semverutil.go b/pkg/semverutil/semverutil.go new file mode 100644 index 00000000000..7dcdb3ba531 --- /dev/null +++ b/pkg/semverutil/semverutil.go @@ -0,0 +1,157 @@ +// Package semverutil provides shared semantic versioning primitives used across +// the pkg/workflow and pkg/cli packages. Centralizing these helpers ensures that +// semver parsing, comparison, and compatibility logic is fixed in one place. +// +// Both workflow and cli packages previously duplicated the "ensure v-prefix" pattern +// and independently called golang.org/x/mod/semver. This package provides the +// canonical implementations so both packages can delegate here. +package semverutil + +import ( + "regexp" + "strconv" + "strings" + + "github.com/github/gh-aw/pkg/logger" + "golang.org/x/mod/semver" +) + +var log = logger.New("semverutil:semverutil") + +// actionVersionTagRegex matches version tags: vmajor, vmajor.minor, or vmajor.minor.patch. +// It intentionally excludes prerelease and build-metadata suffixes because GitHub Actions +// version pins use only these three forms. +var actionVersionTagRegex = regexp.MustCompile(`^v[0-9]+(\.[0-9]+(\.[0-9]+)?)?$`) + +// SemanticVersion represents a parsed semantic version. +type SemanticVersion struct { + Major int + Minor int + Patch int + Pre string + Raw string +} + +// EnsureVPrefix returns v with a leading "v" added if it is not already present. +// The golang.org/x/mod/semver package requires the "v" prefix; callers that may +// receive bare version strings (e.g. "1.2.3") should normalise via this helper. +func EnsureVPrefix(v string) string { + if !strings.HasPrefix(v, "v") { + return "v" + v + } + return v +} + +// IsActionVersionTag reports whether s is a valid GitHub Actions version tag. +// Accepted formats are vmajor, vmajor.minor, and vmajor.minor.patch; prerelease +// and build-metadata suffixes are not accepted. +func IsActionVersionTag(s string) bool { + return actionVersionTagRegex.MatchString(s) +} + +// IsValid reports whether ref is a valid semantic version string. +// It uses golang.org/x/mod/semver and accepts any valid semver, including +// prerelease and build-metadata suffixes. A bare version without a leading "v" +// is also accepted (the prefix is added internally). +func IsValid(ref string) bool { + return semver.IsValid(EnsureVPrefix(ref)) +} + +// ParseVersion parses v into a SemanticVersion. +// It returns nil if v is not a valid semantic version string. +func ParseVersion(v string) *SemanticVersion { + log.Printf("Parsing semantic version: %s", v) + v = EnsureVPrefix(v) + + if !semver.IsValid(v) { + log.Printf("Invalid semantic version: %s", v) + return nil + } + + ver := &SemanticVersion{Raw: strings.TrimPrefix(v, "v")} + + // Use semver.Canonical to get normalized version + canonical := semver.Canonical(v) + + // Strip prerelease and build metadata before splitting, since semver.Canonical + // preserves the prerelease suffix (e.g. "v1.2.3-beta.1" stays "v1.2.3-beta.1") + corePart := strings.TrimPrefix(canonical, "v") + if idx := strings.IndexAny(corePart, "-+"); idx >= 0 { + corePart = corePart[:idx] + } + parts := strings.Split(corePart, ".") + // Parse the numeric components; strconv.Atoi returns 0 on error, matching + // the previous behavior where non-numeric input produced 0. + if len(parts) >= 1 { + ver.Major, _ = strconv.Atoi(parts[0]) + } + if len(parts) >= 2 { + ver.Minor, _ = strconv.Atoi(parts[1]) + } + if len(parts) >= 3 { + ver.Patch, _ = strconv.Atoi(parts[2]) + } + + // Get prerelease if any; semver.Prerelease includes the leading hyphen, strip it + ver.Pre = strings.TrimPrefix(semver.Prerelease(v), "-") + + return ver +} + +// IsPrecise returns true if the version has explicit minor and patch components. +// For example, "v6.0.0" is precise, but "v6" is not. +func (v *SemanticVersion) IsPrecise() bool { + versionPart := strings.TrimPrefix(v.Raw, "v") + dotCount := strings.Count(versionPart, ".") + return dotCount >= 2 // Require at least major.minor.patch +} + +// IsNewer returns true if v is strictly newer than other. +// Uses golang.org/x/mod/semver.Compare for proper semantic version comparison. +func (v *SemanticVersion) IsNewer(other *SemanticVersion) bool { + v1 := EnsureVPrefix(v.Raw) + v2 := EnsureVPrefix(other.Raw) + isNewer := semver.Compare(v1, v2) > 0 + log.Printf("Version comparison: %s vs %s, isNewer=%v", v.Raw, other.Raw, isNewer) + return isNewer +} + +// Compare compares two semantic versions and returns 1 if v1 > v2, -1 if v1 < v2, +// or 0 if they are equal. A bare version without a leading "v" is accepted. +func Compare(v1, v2 string) int { + v1 = EnsureVPrefix(v1) + v2 = EnsureVPrefix(v2) + + result := semver.Compare(v1, v2) + + if result > 0 { + log.Printf("Version comparison result: %s > %s", v1, v2) + } else if result < 0 { + log.Printf("Version comparison result: %s < %s", v1, v2) + } else { + log.Printf("Version comparison result: %s == %s", v1, v2) + } + + return result +} + +// IsCompatible reports whether pinVersion is semver-compatible with requestedVersion. +// Semver compatibility is defined as both versions sharing the same major version. +// +// Examples: +// - IsCompatible("v5.0.0", "v5") → true +// - IsCompatible("v5.1.0", "v5.0.0") → true +// - IsCompatible("v6.0.0", "v5") → false +func IsCompatible(pinVersion, requestedVersion string) bool { + pinVersion = EnsureVPrefix(pinVersion) + requestedVersion = EnsureVPrefix(requestedVersion) + + pinMajor := semver.Major(pinVersion) + requestedMajor := semver.Major(requestedVersion) + + compatible := pinMajor == requestedMajor + log.Printf("Checking semver compatibility: pin=%s (major=%s), requested=%s (major=%s) -> %v", + pinVersion, pinMajor, requestedVersion, requestedMajor, compatible) + + return compatible +} diff --git a/pkg/semverutil/semverutil_test.go b/pkg/semverutil/semverutil_test.go new file mode 100644 index 00000000000..1c9952af1da --- /dev/null +++ b/pkg/semverutil/semverutil_test.go @@ -0,0 +1,264 @@ +//go:build !integration + +package semverutil + +import "testing" + +func TestEnsureVPrefix(t *testing.T) { + tests := []struct { + input string + want string + }{ + {"1.0.0", "v1.0.0"}, + {"v1.0.0", "v1.0.0"}, + {"1", "v1"}, + {"v1", "v1"}, + } + for _, tt := range tests { + t.Run(tt.input, func(t *testing.T) { + got := EnsureVPrefix(tt.input) + if got != tt.want { + t.Errorf("EnsureVPrefix(%q) = %q, want %q", tt.input, got, tt.want) + } + }) + } +} + +func TestIsActionVersionTag(t *testing.T) { + tests := []struct { + tag string + want bool + }{ + {"v0", true}, + {"v1", true}, + {"v10", true}, + {"v1.0", true}, + {"v1.2", true}, + {"v1.0.0", true}, + {"v1.2.3", true}, + {"v10.20.30", true}, + // invalid + {"v1.0.0.0", false}, + {"1.0.0", false}, + {"v", false}, + {"", false}, + {"latest", false}, + {"v1.0.0-beta", false}, // prerelease not accepted + {"abc123def456789012345678901234567890abcd", false}, + } + for _, tt := range tests { + t.Run(tt.tag, func(t *testing.T) { + got := IsActionVersionTag(tt.tag) + if got != tt.want { + t.Errorf("IsActionVersionTag(%q) = %v, want %v", tt.tag, got, tt.want) + } + }) + } +} + +func TestIsValid(t *testing.T) { + tests := []struct { + ref string + want bool + }{ + {"v1.0.0", true}, + {"1.0.0", true}, + {"v1.0", true}, + {"v1", true}, + {"v1.0.0-beta", true}, + {"v1.0.0+20230101", true}, + {"main", false}, + {"feature-branch", false}, + {"abc123def456", false}, + } + for _, tt := range tests { + t.Run(tt.ref, func(t *testing.T) { + got := IsValid(tt.ref) + if got != tt.want { + t.Errorf("IsValid(%q) = %v, want %v", tt.ref, got, tt.want) + } + }) + } +} + +func TestParseVersion(t *testing.T) { + tests := []struct { + name string + input string + wantMajor int + wantMinor int + wantPatch int + wantPre string + wantNil bool + }{ + { + name: "full version with v", + input: "v1.2.3", + wantMajor: 1, + wantMinor: 2, + wantPatch: 3, + }, + { + name: "full version without v", + input: "1.2.3", + wantMajor: 1, + wantMinor: 2, + wantPatch: 3, + }, + { + name: "version with prerelease", + input: "v1.2.3-beta.1", + wantMajor: 1, + wantMinor: 2, + wantPatch: 3, + wantPre: "beta.1", + }, + { + name: "two-part version", + input: "v1.2", + wantMajor: 1, + wantMinor: 2, + }, + { + name: "one-part version", + input: "v1", + wantMajor: 1, + }, + { + name: "invalid version", + input: "not-a-version", + wantNil: true, + }, + { + name: "branch name", + input: "main", + wantNil: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := ParseVersion(tt.input) + if tt.wantNil { + if got != nil { + t.Errorf("ParseVersion(%q) = %+v, want nil", tt.input, got) + } + return + } + if got == nil { + t.Errorf("ParseVersion(%q) = nil, want non-nil", tt.input) + return + } + if got.Major != tt.wantMajor { + t.Errorf("ParseVersion(%q).Major = %d, want %d", tt.input, got.Major, tt.wantMajor) + } + if got.Minor != tt.wantMinor { + t.Errorf("ParseVersion(%q).Minor = %d, want %d", tt.input, got.Minor, tt.wantMinor) + } + if got.Patch != tt.wantPatch { + t.Errorf("ParseVersion(%q).Patch = %d, want %d", tt.input, got.Patch, tt.wantPatch) + } + if got.Pre != tt.wantPre { + t.Errorf("ParseVersion(%q).Pre = %q, want %q", tt.input, got.Pre, tt.wantPre) + } + }) + } +} + +func TestSemanticVersionIsPrecise(t *testing.T) { + tests := []struct { + version string + want bool + }{ + {"v6", false}, + {"v6.0", false}, + {"v6.0.0", true}, + {"v1.2.3", true}, + {"6.0.0", true}, + } + for _, tt := range tests { + t.Run(tt.version, func(t *testing.T) { + v := ParseVersion(tt.version) + if v == nil { + t.Fatalf("failed to parse version: %s", tt.version) + } + got := v.IsPrecise() + if got != tt.want { + t.Errorf("ParseVersion(%q).IsPrecise() = %v, want %v", tt.version, got, tt.want) + } + }) + } +} + +func TestSemanticVersionIsNewer(t *testing.T) { + tests := []struct { + name string + version string + other string + want bool + }{ + {"newer major", "v2.0.0", "v1.0.0", true}, + {"older major", "v1.0.0", "v2.0.0", false}, + {"newer minor", "v1.2.0", "v1.1.0", true}, + {"same version", "v1.0.0", "v1.0.0", false}, + {"release vs prerelease", "v1.0.0", "v1.0.0-beta", true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + v := ParseVersion(tt.version) + other := ParseVersion(tt.other) + if v == nil || other == nil { + t.Fatalf("failed to parse versions %q or %q", tt.version, tt.other) + } + got := v.IsNewer(other) + if got != tt.want { + t.Errorf("(%q).IsNewer(%q) = %v, want %v", tt.version, tt.other, got, tt.want) + } + }) + } +} + +func TestCompare(t *testing.T) { + tests := []struct { + v1 string + v2 string + want int + }{ + {"1.0.0", "1.0.0", 0}, + {"1.0.1", "1.0.0", 1}, + {"1.0.0", "1.0.1", -1}, + {"2.0.0", "1.9.9", 1}, + {"24", "20", 1}, + {"20", "24", -1}, + } + for _, tt := range tests { + t.Run(tt.v1+"_vs_"+tt.v2, func(t *testing.T) { + got := Compare(tt.v1, tt.v2) + if got != tt.want { + t.Errorf("Compare(%q, %q) = %d, want %d", tt.v1, tt.v2, got, tt.want) + } + }) + } +} + +func TestIsCompatible(t *testing.T) { + tests := []struct { + pin string + requested string + want bool + }{ + {"v5.0.0", "v5", true}, + {"v5.1.0", "v5.0.0", true}, + {"v6.0.0", "v5", false}, + {"v4.6.2", "v4", true}, + {"v4.6.2", "v5", false}, + {"v10.2.3", "v10", true}, + } + for _, tt := range tests { + t.Run(tt.pin+"_"+tt.requested, func(t *testing.T) { + got := IsCompatible(tt.pin, tt.requested) + if got != tt.want { + t.Errorf("IsCompatible(%q, %q) = %v, want %v", tt.pin, tt.requested, got, tt.want) + } + }) + } +} diff --git a/pkg/workflow/semver.go b/pkg/workflow/semver.go index e782a891909..6d810eed337 100644 --- a/pkg/workflow/semver.go +++ b/pkg/workflow/semver.go @@ -1,48 +1,23 @@ package workflow import ( - "regexp" - "strings" - "github.com/github/gh-aw/pkg/logger" - "golang.org/x/mod/semver" + "github.com/github/gh-aw/pkg/semverutil" ) -// versionTagRegex matches version tags: vmajor, vmajor.minor, or vmajor.minor.patch -var versionTagRegex = regexp.MustCompile(`^v[0-9]+(\.[0-9]+(\.[0-9]+)?)?$`) - var semverLog = logger.New("workflow:semver") // isValidVersionTag checks if a string is a valid action version tag. // Supports vmajor, vmajor.minor, and vmajor.minor.patch formats only. func isValidVersionTag(s string) bool { - return versionTagRegex.MatchString(s) + return semverutil.IsActionVersionTag(s) } // compareVersions compares two semantic versions, returns 1 if v1 > v2, -1 if v1 < v2, 0 if equal // Uses golang.org/x/mod/semver for proper semantic version comparison func compareVersions(v1, v2 string) int { semverLog.Printf("Comparing versions: v1=%s, v2=%s", v1, v2) - - // Ensure versions have 'v' prefix for semver package - if !strings.HasPrefix(v1, "v") { - v1 = "v" + v1 - } - if !strings.HasPrefix(v2, "v") { - v2 = "v" + v2 - } - - result := semver.Compare(v1, v2) - - if result > 0 { - semverLog.Printf("Version comparison result: %s > %s", v1, v2) - } else if result < 0 { - semverLog.Printf("Version comparison result: %s < %s", v1, v2) - } else { - semverLog.Printf("Version comparison result: %s == %s", v1, v2) - } - - return result + return semverutil.Compare(v1, v2) } // isSemverCompatible checks if pinVersion is semver-compatible with requestedVersion @@ -52,21 +27,5 @@ func compareVersions(v1, v2 string) int { // - isSemverCompatible("v5.1.0", "v5.0.0") -> true // - isSemverCompatible("v6.0.0", "v5") -> false func isSemverCompatible(pinVersion, requestedVersion string) bool { - // Ensure versions have 'v' prefix for semver package - if !strings.HasPrefix(pinVersion, "v") { - pinVersion = "v" + pinVersion - } - if !strings.HasPrefix(requestedVersion, "v") { - requestedVersion = "v" + requestedVersion - } - - // Use semver.Major to get major version strings - pinMajor := semver.Major(pinVersion) - requestedMajor := semver.Major(requestedVersion) - - compatible := pinMajor == requestedMajor - semverLog.Printf("Checking semver compatibility: pin=%s (major=%s), requested=%s (major=%s) -> %v", - pinVersion, pinMajor, requestedVersion, requestedMajor, compatible) - - return compatible + return semverutil.IsCompatible(pinVersion, requestedVersion) } diff --git a/pkg/workflow/strings.go b/pkg/workflow/strings.go index c82b49d854e..ffbc08b49b1 100644 --- a/pkg/workflow/strings.go +++ b/pkg/workflow/strings.go @@ -16,7 +16,7 @@ // - SanitizeWorkflowIDForCacheKey: Sanitizes workflow ID for use in cache keys (removes hyphens) // - sanitizeJobName: Sanitizes workflow name to a valid GitHub Actions job name // - sanitizeRefForPath: Sanitizes a git ref for use in a file path -// - SanitizeIdentifier (workflow_name.go): Creates clean identifiers for user agents +// - SanitizeIdentifier: Creates clean identifiers for user agents // // Example: // @@ -391,6 +391,47 @@ func sanitizeRefForPath(ref string) string { return sanitized } +// SanitizeIdentifier sanitizes a workflow name to create a safe identifier +// suitable for use as a user agent string or similar context. +// +// This is a SANITIZE function (character validity pattern). Use this when creating +// identifiers that must be purely alphanumeric with hyphens, with no special characters +// preserved. Unlike SanitizeWorkflowName which preserves dots and underscores, this +// function removes ALL special characters except hyphens. +// +// The function: +// - Converts to lowercase +// - Replaces spaces and underscores with hyphens +// - Removes non-alphanumeric characters (except hyphens) +// - Consolidates multiple hyphens into a single hyphen +// - Trims leading and trailing hyphens +// - Returns "github-agentic-workflow" if the result would be empty +// +// Example inputs and outputs: +// +// SanitizeIdentifier("My Workflow") // returns "my-workflow" +// SanitizeIdentifier("test_workflow") // returns "test-workflow" +// SanitizeIdentifier("@@@") // returns "github-agentic-workflow" (default) +// SanitizeIdentifier("Weekly v2.0") // returns "weekly-v2-0" +// +// This function uses the unified SanitizeName function with options configured +// to trim leading/trailing hyphens and return a default value for empty results. +// Hyphens are preserved by default in SanitizeName, not via PreserveSpecialChars. +// +// See package documentation for guidance on when to use sanitize vs normalize patterns. +func SanitizeIdentifier(name string) string { + stringsLog.Printf("Sanitizing identifier: %s", name) + result := SanitizeName(name, &SanitizeOptions{ + PreserveSpecialChars: []rune{}, + TrimHyphens: true, + DefaultValue: "github-agentic-workflow", + }) + if result != name { + stringsLog.Printf("Sanitized identifier: %s -> %s", name, result) + } + return result +} + // formatList formats a list of strings as a comma-separated list with natural language conjunction func formatList(items []string) string { if len(items) == 0 { diff --git a/pkg/workflow/workflow_name.go b/pkg/workflow/workflow_name.go deleted file mode 100644 index 2a00408954c..00000000000 --- a/pkg/workflow/workflow_name.go +++ /dev/null @@ -1,46 +0,0 @@ -package workflow - -import "github.com/github/gh-aw/pkg/logger" - -var workflowNameLog = logger.New("workflow:workflow_name") - -// SanitizeIdentifier sanitizes a workflow name to create a safe identifier -// suitable for use as a user agent string or similar context. -// -// This is a SANITIZE function (character validity pattern). Use this when creating -// identifiers that must be purely alphanumeric with hyphens, with no special characters -// preserved. Unlike SanitizeWorkflowName which preserves dots and underscores, this -// function removes ALL special characters except hyphens. -// -// The function: -// - Converts to lowercase -// - Replaces spaces and underscores with hyphens -// - Removes non-alphanumeric characters (except hyphens) -// - Consolidates multiple hyphens into a single hyphen -// - Trims leading and trailing hyphens -// - Returns "github-agentic-workflow" if the result would be empty -// -// Example inputs and outputs: -// -// SanitizeIdentifier("My Workflow") // returns "my-workflow" -// SanitizeIdentifier("test_workflow") // returns "test-workflow" -// SanitizeIdentifier("@@@") // returns "github-agentic-workflow" (default) -// SanitizeIdentifier("Weekly v2.0") // returns "weekly-v2-0" -// -// This function uses the unified SanitizeName function with options configured -// to trim leading/trailing hyphens and return a default value for empty results. -// Hyphens are preserved by default in SanitizeName, not via PreserveSpecialChars. -// -// See package documentation for guidance on when to use sanitize vs normalize patterns. -func SanitizeIdentifier(name string) string { - workflowNameLog.Printf("Sanitizing workflow identifier: %s", name) - result := SanitizeName(name, &SanitizeOptions{ - PreserveSpecialChars: []rune{}, - TrimHyphens: true, - DefaultValue: "github-agentic-workflow", - }) - if result != name { - workflowNameLog.Printf("Sanitized identifier: %s -> %s", name, result) - } - return result -}