bindings/go/configuration/ocm/v1/runtime/convert_v1.go (1)

20-22: Consider using slog.Warn for aliases deprecation notice.

Since aliases are being silently dropped and this could lead to unexpected behavior for users relying on them, slog.Warn might be more appropriate than slog.Info to ensure visibility.

💡 Suggested change

 	if len(config.Aliases) > 0 {
-		slog.Info("aliases are not supported in ocm v2, ignoring")
+		slog.Warn("aliases are not supported in ocm v2, ignoring")
 	}

🤖 Prompt for AI Agents

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

In `@bindings/go/configuration/ocm/v1/runtime/convert_v1.go` around lines 20 - 22,
The current conversion path checks config.Aliases and logs with slog.Info when
aliases are dropped; change this to use slog.Warn so users see a
deprecation/ignored-aliases warning. Locate the check referencing config.Aliases
and the slog.Info call (in convert_v1.go), replace the Info call with slog.Warn
and keep the message content (or make it slightly more explicit) so dropped
aliases are more visible during runtime.

bindings/go/configuration/ocm/v1/runtime/convert_v1_test.go (2)

38-141: Good test coverage. Consider adding edge case tests for nil elements.

The tests comprehensively cover the main scenarios. However, given the potential nil pointer issues in convertResolvers, consider adding tests for:

• A resolver slice containing a nil element
• A resolver with a nil Repository field

These would help document expected behavior and catch regressions if nil handling is added.

💡 Suggested additional test cases

func TestConvertFromV1_NilResolverInSlice(t *testing.T) {
	input := &resolverv1.Config{
		Type: runtime.NewVersionedType(resolverv1.ConfigType, resolverv1.Version),
		Resolvers: []*resolverv1.Resolver{
			nil,
		},
	}
	_, err := ConvertFromV1(testScheme, input)
	assert.Error(t, err)
}

func TestConvertFromV1_NilRepositoryReturnsError(t *testing.T) {
	input := &resolverv1.Config{
		Type: runtime.NewVersionedType(resolverv1.ConfigType, resolverv1.Version),
		Resolvers: []*resolverv1.Resolver{
			{Repository: nil, Priority: ptr(10)},
		},
	}
	_, err := ConvertFromV1(testScheme, input)
	assert.Error(t, err)
}

🤖 Prompt for AI Agents

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

In `@bindings/go/configuration/ocm/v1/runtime/convert_v1_test.go` around lines 38
- 141, Add unit tests to cover nil-element edge cases in ConvertFromV1: create
two tests that call ConvertFromV1(testScheme, input) where input.Resolvers
contains (1) a nil element and (2) a resolver whose Repository is nil, and
assert that ConvertFromV1 returns an error in both cases; this exercises
convertResolvers/ConvertFromV1 error paths and prevents silent nil-pointer or
unexpected-success behavior (referencing ConvertFromV1 and the
resolverv1.Resolver Repository field to locate where to add the tests).

---

31-36: Minor: Raw JSON construction could break with special characters.

If baseURL contains characters like " or \, the inline JSON construction would produce invalid JSON. For test purposes this is likely acceptable since test inputs are controlled, but using json.Marshal would be more robust.

💡 More robust alternative

+import "encoding/json"

 func rawTestRepo(baseURL string) *runtime.Raw {
+	data, _ := json.Marshal(map[string]string{
+		"type":    "test-repo/v1",
+		"baseURL": baseURL,
+	})
 	return &runtime.Raw{
 		Type: runtime.NewVersionedType("test-repo", "v1"),
-		Data: []byte(`{"type":"test-repo/v1","baseURL":"` + baseURL + `"}`),
+		Data: data,
 	}
 }

🤖 Prompt for AI Agents

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

In `@bindings/go/configuration/ocm/v1/runtime/convert_v1_test.go` around lines 31
- 36, The rawTestRepo helper builds JSON by concatenating strings which breaks
if baseURL contains quotes or backslashes; update rawTestRepo to construct a
small struct (e.g., with fields Type and BaseURL) and use json.Marshal to
produce the Data byte slice, then set the Type field to
runtime.NewVersionedType("test-repo","v1") and Data to the marshaled bytes so
runtime.Raw.Data is valid JSON for any baseURL.

bindings/go/configuration/ocm/v1/runtime/convert_v1.go (1)

50-55: Wrap conversion errors with resolver index context.

Returning raw errors here makes triage harder; include resolver index/type so failures are actionable.

♻️ Proposed improvement

 		convertedRepo, err := repositoryScheme.NewObject(resolver.Repository.GetType())
 		if err != nil {
-			return nil, err
+			return nil, fmt.Errorf("resolver[%d]: create repository object for type %q: %w", i, resolver.Repository.GetType(), err)
 		}
 		if err := repositoryScheme.Convert(resolver.Repository, convertedRepo); err != nil {
-			return nil, err
+			return nil, fmt.Errorf("resolver[%d]: convert repository type %q: %w", i, resolver.Repository.GetType(), err)
 		}

🤖 Prompt for AI Agents

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

In `@bindings/go/configuration/ocm/v1/runtime/convert_v1.go` around lines 50 - 55,
When converting resolver.Repository using repositoryScheme.NewObject and
repositoryScheme.Convert, wrap any returned errors with contextual information
(resolver index and resolver.Repository.GetType()) instead of returning raw err;
capture the error, build a new error message like "failed to convert resolver %d
(type %s): %w" and return that (using error wrapping e.g. fmt.Errorf) so
failures from repositoryScheme.NewObject and repositoryScheme.Convert include
resolver index and type for easier triage.

bindings/go/configuration/ocm/v1/runtime/convert_v1_test.go (1)

38-161: Add a regression test for nil repositoryScheme.

Current coverage is strong, but it does not assert behavior when repositoryScheme is nil and Resolvers is non-empty—the remaining panic path from the converter.

🤖 Prompt for AI Agents

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

In `@bindings/go/configuration/ocm/v1/runtime/convert_v1_test.go` around lines 38
- 161, Add a regression test named
TestConvertFromV1_NilRepositorySchemeReturnsError that verifies ConvertFromV1
handles a nil repositoryScheme without panicking: save the current
repositoryScheme, set repositoryScheme = nil, defer restoring it, build an input
resolverv1.Config with one resolver (use rawTestRepo("registry.example.com")),
call ConvertFromV1(testScheme, input) and assert that it returns an error
(assert.Error); reference ConvertFromV1, repositoryScheme, rawTestRepo and the
new test function name to locate where to add this test.

bindings/go/configuration/ocm/v1/runtime/convert_v1.go (2)

53-59: Consider adding resolver index to scheme error messages for consistency.

Errors from NewObject and Convert (lines 55, 58) don't include the resolver index, unlike the nil check errors above (lines 48, 51). Adding the index would make debugging easier when conversions fail with multiple resolvers.

💡 Suggested change

 		convertedRepo, err := repositoryScheme.NewObject(resolver.Repository.GetType())
 		if err != nil {
-			return nil, err
+			return nil, fmt.Errorf("resolver at index %d: failed to create repository object: %w", i, err)
 		}
 		if err := repositoryScheme.Convert(resolver.Repository, convertedRepo); err != nil {
-			return nil, err
+			return nil, fmt.Errorf("resolver at index %d: failed to convert repository: %w", i, err)
 		}

🤖 Prompt for AI Agents

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

In `@bindings/go/configuration/ocm/v1/runtime/convert_v1.go` around lines 53 - 59,
The NewObject and Convert error returns in convert_v1.go do not include the
resolver index like the earlier nil-checks; update the error handling around
repositoryScheme.NewObject(resolver.Repository.GetType()) and
repositoryScheme.Convert(resolver.Repository, convertedRepo) to wrap or format
the returned error with the resolver index (e.g., include fmt.Errorf("resolver
%d: %w", idx, err) or equivalent) so the returned errors consistently identify
which resolver failed during conversion.

---

20-22: Consider using slog.Warn for dropped aliases.

Since aliases are silently ignored and data is effectively being dropped during conversion, slog.Warn would be more semantically appropriate than slog.Info to alert users that their configuration is not fully honored.

💡 Suggested change

 	if len(config.Aliases) > 0 {
-		slog.Info("aliases are not supported in ocm v2, ignoring")
+		slog.Warn("aliases are not supported in ocm v2, ignoring")
 	}

🤖 Prompt for AI Agents

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

In `@bindings/go/configuration/ocm/v1/runtime/convert_v1.go` around lines 20 - 22,
The code in convert_v1.go currently logs dropped aliases with slog.Info when
checking config.Aliases; change that call to slog.Warn (or slog.Logger.Warn if
using an instance) so dropped aliases are signaled as a warning rather than
info, and keep or slightly clarify the message like "aliases are not supported
in ocm v2, ignoring" to indicate data is being dropped; update the call site
that checks if len(config.Aliases) > 0 to use the warning-level logger.