chore(agents): fix version normalization for lockfiles

Author: rektdeckard

pkg/agentfs/sdk_version_check.go

@@ -660,7 +660,7 @@ func isVersionSatisfied(version, minVersion string, sourceType SourceType) (bool
 	case SourceTypeLock:
 		// For lock files, we have the exact version that was installed
 		// Check if this exact version is >= the minimum version
-		normalizedVersion := normalizeVersion(version)
+		normalizedVersion := normalizeVersion(version, sourceType)
 		v, err := semver.NewVersion(normalizedVersion)
 		if err != nil {
 			return false, fmt.Errorf("invalid version format: %s", version)
@@ -687,37 +687,62 @@ func isVersionSatisfied(version, minVersion string, sourceType SourceType) (bool
 			return false, fmt.Errorf("invalid minimum version format: %s", minVersion)
 		}
 
-		// Check if the package constraint would allow installing a version >= minimum
-		// We do this by checking if there exists a version >= minimum that satisfies the package constraint
+		// The key insight: we need to check if the package constraint allows installing
+		// ANY version that is >= the minimum version, not if the minimum version itself
+		// satisfies the package constraint.
+		
+		// First, check if the minimum version itself satisfies the package constraint
 		if packageConstraint.Check(min) {
 			// The minimum version satisfies the package constraint, so it would be installable
 			return true, nil
 		}
 
+		// If the minimum version doesn't satisfy the package constraint, we need to check
+		// if the package constraint allows installing a version >= minimum.
+		// This handles cases like "^0.7.9" vs minimum "0.0.7" where the constraint
+		// requires >=0.7.9 but would still satisfy the minimum requirement of 0.0.7
+		// because 0.7.9 > 0.0.7.
+		
 		// Check if the package constraint allows any version >= minimum
-		// This handles cases like ">=1.5.0" where 1.0.0 doesn't satisfy it, but it would install 1.5.0+ which > 1.0.0
-		// We'll test a few strategic versions to see if any satisfy the package constraint and are >= minimum
+		// We'll test strategic versions to see if any satisfy both conditions
 		testVersions := []string{
-			minVersion, // The minimum version itself
-			fmt.Sprintf("%d.%d.%d", min.Major()+1, 0, 0),
-			fmt.Sprintf("%d.%d.%d", min.Major(), min.Minor()+1, 0),
+			// Test the minimum version itself (already checked above, but included for clarity)
+			minVersion,
+			// Test versions that are >= minimum and might satisfy the package constraint
 			fmt.Sprintf("%d.%d.%d", min.Major(), min.Minor(), min.Patch()+1),
+			fmt.Sprintf("%d.%d.%d", min.Major(), min.Minor()+1, 0),
+			fmt.Sprintf("%d.%d.%d", min.Major(), min.Minor()+2, 0),
+			fmt.Sprintf("%d.%d.%d", min.Major(), min.Minor()+5, 0),
+			fmt.Sprintf("%d.%d.%d", min.Major(), min.Minor()+10, 0),
+			fmt.Sprintf("%d.%d.%d", min.Major()+1, 0, 0),
+			fmt.Sprintf("%d.%d.%d", min.Major()+2, 0, 0),
 		}
 
-		// Add more versions to cover edge cases
+		// Add more specific versions that might be common in constraints
 		if min.Major() > 0 {
 			testVersions = append(testVersions, fmt.Sprintf("%d.0.0", min.Major()))
 		}
 		if min.Minor() > 0 {
 			testVersions = append(testVersions, fmt.Sprintf("%d.%d.0", min.Major(), min.Minor()))
 		}
 
-		// Add some specific versions that might be common in constraints
-		testVersions = append(testVersions,
-			fmt.Sprintf("%d.%d.0", min.Major(), min.Minor()+2),
-			fmt.Sprintf("%d.%d.0", min.Major(), min.Minor()+5),
-			fmt.Sprintf("%d.%d.0", min.Major(), min.Minor()+10),
-		)
+		// Add the actual version from the constraint if it's >= minimum
+		// This handles cases like "^0.7.9" where we need to test 0.7.9 specifically
+		// Use regex to extract the version number from constraints like ^0.7.9, ~1.2.0, >=1.5.0, etc.
+		versionRegex := regexp.MustCompile(`^[=~><!^]+\s*(\d+\.\d+\.\d+)`)
+		if matches := versionRegex.FindStringSubmatch(version); matches != nil {
+			constraintVersionStr := matches[1]
+			if constraintVersion, err := semver.NewVersion(constraintVersionStr); err == nil {
+				if !constraintVersion.LessThan(min) {
+					testVersions = append(testVersions, constraintVersion.String())
+					// Also add some versions around the constraint version
+					testVersions = append(testVersions, 
+						fmt.Sprintf("%d.%d.%d", constraintVersion.Major(), constraintVersion.Minor(), constraintVersion.Patch()+1),
+						fmt.Sprintf("%d.%d.%d", constraintVersion.Major(), constraintVersion.Minor()+1, 0),
+					)
+				}
+			}
+		}
 
 		for _, testVersion := range testVersions {
 			if v, err := semver.NewVersion(testVersion); err == nil {
@@ -735,16 +760,22 @@ func isVersionSatisfied(version, minVersion string, sourceType SourceType) (bool
 	}
 }
 
-// normalizeVersion normalizes version strings for semver parsing
-func normalizeVersion(version string) string {
-	// Remove common prefixes and suffixes
+// normalizeVersion cleans up version strings for parsing
+func normalizeVersion(version string, sourceType SourceType) string {
+	// Remove whitespace and quotes
 	version = strings.TrimSpace(version)
-	version = strings.Trim(version, " \"'")
+	version = strings.Trim(version, `"'`)
 
-	// Handle npm version ranges (^ and ~ are npm-specific, not semver constraints)
-	if strings.HasPrefix(version, "^") || strings.HasPrefix(version, "~") {
-		version = version[1:]
+	// For lock files, we want the exact version, so remove npm-style prefixes
+	// For package files, we want to preserve the constraint operators
+	if sourceType == SourceTypeLock {
+		// Handle npm version ranges (^ and ~ are npm-specific, not semver constraints)
+		if strings.HasPrefix(version, "^") || strings.HasPrefix(version, "~") {
+			version = version[1:]
+		}
 	}
+	// For SourceTypePackage, we preserve all operators including ^ and ~
+	// as they are valid semver constraints that the semver library can parse
 
 	return version
 }

pkg/agentfs/sdk_version_check_test.go

@@ -111,6 +111,19 @@ livekit-agents = ">=1.0.0"`,
 			expectError: true,
 			errorMsg:    "too old",
 		},
+		{
+			name:        "Node package.json with good version",
+			projectType: ProjectTypeNode,
+			setupFiles: map[string]string{
+				"package.json": `{
+  "dependencies": {
+    "@livekit/agents": "^1.1.1"
+  }
+}`,
+			},
+			expectError: false,
+		},
+
 		{
 			name:        "Node package-lock.json with valid version",
 			projectType: ProjectTypeNode,
@@ -207,19 +220,24 @@ func TestIsVersionSatisfied(t *testing.T) {
 		{"0.9.0", "1.0.0", SourceTypeLock, false, false},
 		{"1.5.0", "2.0.0", SourceTypeLock, false, false},
 		{"2.0.0", "2.0.0", SourceTypeLock, true, false},
-		
+
 		// Package file tests (constraint satisfaction)
 		{">=1.5.0", "1.0.0", SourceTypePackage, true, false},
 		{"<2.0.0", "1.0.0", SourceTypePackage, true, false},
 		{">=2.0.0", "1.0.0", SourceTypePackage, true, false},
 		{"~1.2.0", "1.0.0", SourceTypePackage, true, false},
 		{"^1.0.0", "1.0.0", SourceTypePackage, true, false},
-		
+		// Test the specific case that was failing: ^0.7.9 should satisfy minimum 0.0.7
+		{"^0.7.9", "0.0.7", SourceTypePackage, true, false},
+		// Test other caret scenarios
+		{"^0.5.0", "0.0.7", SourceTypePackage, true, false}, // ^0.5.0 allows 0.5.0+ which >= 0.0.7
+		{"^1.0.0", "0.0.7", SourceTypePackage, true, false}, // 1.0.0+ >= 0.0.7
+
 		// Special cases
 		{"latest", "1.0.0", SourceTypeLock, true, false},
 		{"*", "1.0.0", SourceTypeLock, true, false},
 		{"", "1.0.0", SourceTypeLock, true, false},
-		
+
 		// Error cases
 		{"invalid", "1.0.0", SourceTypeLock, false, true},
 		{"1.5.0", "invalid", SourceTypeLock, false, true},
@@ -251,25 +269,45 @@ func TestIsVersionSatisfied(t *testing.T) {
 
 func TestNormalizeVersion(t *testing.T) {
 	tests := []struct {
-		input    string
-		expected string
+		input       string
+		sourceType  SourceType
+		expected    string
+		description string
 	}{
-		{"1.5.0", "1.5.0"},
-		{"^1.5.0", "1.5.0"},
-		{"~1.5.0", "1.5.0"},
-		{">=1.5.0", ">=1.5.0"},
-		{"<2.0.0", "<2.0.0"},
-		{"==1.5.0", "==1.5.0"},
-		{" 1.5.0 ", "1.5.0"},
-		{`"1.5.0"`, "1.5.0"},
-		{`'1.5.0'`, "1.5.0"},
-		{"*", "*"},
-		{"latest", "latest"},
+		// Lock file tests (should remove ^ and ~)
+		{"1.5.0", SourceTypeLock, "1.5.0", "exact version"},
+		{"^1.5.0", SourceTypeLock, "1.5.0", "npm caret removed for lock file"},
+		{"~1.5.0", SourceTypeLock, "1.5.0", "npm tilde removed for lock file"},
+		{">=1.5.0", SourceTypeLock, ">=1.5.0", "semver operators preserved for lock file"},
+		{"<2.0.0", SourceTypeLock, "<2.0.0", "semver operators preserved for lock file"},
+		{"==1.5.0", SourceTypeLock, "==1.5.0", "semver operators preserved for lock file"},
+		{" 1.5.0 ", SourceTypeLock, "1.5.0", "whitespace removed"},
+		{`"1.5.0"`, SourceTypeLock, "1.5.0", "quotes removed"},
+		{`'1.5.0'`, SourceTypeLock, "1.5.0", "quotes removed"},
+		{"*", SourceTypeLock, "*", "wildcard preserved"},
+		{"latest", SourceTypeLock, "latest", "latest preserved"},
+
+		// Package file tests (should preserve ^ and ~)
+		{"1.5.0", SourceTypePackage, "1.5.0", "exact version"},
+		{"^1.5.0", SourceTypePackage, "^1.5.0", "npm caret preserved for package file"},
+		{"~1.5.0", SourceTypePackage, "~1.5.0", "npm tilde preserved for package file"},
+		{">=1.5.0", SourceTypePackage, ">=1.5.0", "semver operators preserved for package file"},
+		{"<2.0.0", SourceTypePackage, "<2.0.0", "semver operators preserved for package file"},
+		{"==1.5.0", SourceTypePackage, "==1.5.0", "semver operators preserved for package file"},
+		{" 1.5.0 ", SourceTypePackage, "1.5.0", "whitespace removed"},
+		{`"1.5.0"`, SourceTypePackage, "1.5.0", "quotes removed"},
+		{`'1.5.0'`, SourceTypePackage, "1.5.0", "quotes removed"},
+		{"*", SourceTypePackage, "*", "wildcard preserved"},
+		{"latest", SourceTypePackage, "latest", "latest preserved"},
 	}
 
 	for _, tt := range tests {
-		t.Run(tt.input, func(t *testing.T) {
-			result := normalizeVersion(tt.input)
+		sourceTypeStr := "Package"
+		if tt.sourceType == SourceTypeLock {
+			sourceTypeStr = "Lock"
+		}
+		t.Run(fmt.Sprintf("%s_%s_%s", tt.input, sourceTypeStr, tt.description), func(t *testing.T) {
+			result := normalizeVersion(tt.input, tt.sourceType)
 			if result != tt.expected {
 				t.Errorf("Expected %s but got %s", tt.expected, result)
 			}