internal/container/start.go (1)

298-311: Mutation of loop variable c.Tag is safe here but could be clearer.

On line 305, c.Tag = v mutates the loop variable. Since c is a value copy (not a pointer), this doesn't affect the original containers slice—which is correct behavior. However, this could be made more explicit for readability.

♻️ Optional: Use explicit copy for clarity

 func validateLicensesFromImages(ctx context.Context, rt runtime.Runtime, sink output.Sink, opts StartOptions, tel *telemetry.Client, containers []runtime.ContainerConfig, token string) error {
-	for _, c := range containers {
+	for i := range containers {
+		c := containers[i]
 		v, err := rt.GetImageVersion(ctx, c.Image)
 		if err != nil {
 			return fmt.Errorf("could not resolve version from image %s: %w", c.Image, err)
 		}
 		c.Tag = v
 		if err := validateLicense(ctx, sink, opts, tel, c, token); err != nil {
 			return err
 		}
 	}
 	return nil
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@internal/container/start.go` around lines 298 - 311, In
validateLicensesFromImages, avoid mutating the loop variable directly for
clarity: make an explicit copy (e.g., cfg := c), set cfg.Tag = v, and pass cfg
to validateLicense instead of c; this keeps behavior identical (since c is a
value copy) but makes the intent explicit and improves readability when working
with containers, c.Tag, and the validateLicense call.

test/integration/version_resolution_test.go (1)

19-53: Consider improving robustness of path parsing in mock server.

The path parsing on lines 29-30 extracts emulatorType by splitting the path and taking parts[len(parts)-2]. This assumes the path always ends with /version and the emulator type is the second-to-last segment.

This works for the current endpoint format (/v1/license/catalog/{emulator_type}/version), but could silently misbehave if the path format changes or if requests arrive malformed during test development.

♻️ Optional: Use more explicit path matching

 		case r.Method == http.MethodGet && strings.HasPrefix(r.URL.Path, "/v1/license/catalog/"):
 			if catalogVersion == "" {
 				w.WriteHeader(http.StatusServiceUnavailable)
 				return
 			}
-			parts := strings.Split(strings.Trim(r.URL.Path, "/"), "/")
-			emulatorType := parts[len(parts)-2]
+			// Expected: /v1/license/catalog/{emulator_type}/version
+			trimmed := strings.TrimPrefix(r.URL.Path, "/v1/license/catalog/")
+			trimmed = strings.TrimSuffix(trimmed, "/version")
+			emulatorType := trimmed
 			w.Header().Set("Content-Type", "application/json")
 			_, _ = fmt.Fprintf(w, `{"emulator_type":%q,"version":%q}`, emulatorType, catalogVersion)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/integration/version_resolution_test.go` around lines 19 - 53, In
createVersionResolutionMockServer, avoid fragile index-based extraction of
emulatorType: when handling the GET for catalog, explicitly verify the path
starts with "/v1/license/catalog/" and ends with "/version", then derive
emulatorType by trimming that prefix and suffix (or by splitting and validating
the expected segment names) and return a 404/400 if the extracted emulatorType
is empty or the path shape is unexpected; keep setting Content-Type and writing
the JSON response as before but only after validating the path so malformed
requests won't silently produce wrong emulatorType values.