[waveywaves]

Summary

• Stop auto-detecting further material kinds when AddMaterialContractFree returns a protovalidate.ValidationError.
• Surface invalid material-name validation errors directly instead of masking them behind a generic auto-discovery failure.

Fixes #2394

Assisted-by: pi

[javirln]

@waveywaves is this ready to be reviewed?

[cubic-dev-ai]

1 issue found across 2 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="pkg/attestation/crafter/crafter.go">

<violation number="1" location="pkg/attestation/crafter/crafter.go:670">
P2: DNS-1123 invalid-name errors are generic errors and are not treated as terminal in auto-detect, so they are retried and wrapped by the generic auto-discovery failure instead of being surfaced immediately.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.}

[waveywaves]

@jiparis Ready for review — all commits GPG-signed and rebased on main. This fixes a variable shadowing bug that silently masks DNS-1123 validation errors in AddMaterialContractFree (issue #2394).

[waveywaves]

@javirln Yes, this is ready for review! All commits are GPG-signed and rebased on latest main. Sorry for the delayed reply.

[waveywaves]

@migmartri Would you be able to take a look at this one? It fixes a variable shadowing bug where DNS-1123 validation errors get silently masked in AddMaterialContractFree (issue #2394). All commits GPG-signed, CI green, cubic-dev-ai's finding already addressed. Happy to answer any questions!

[jiparis]

Thanks @waveywaves . Check my comment for the actual fix. #2394 is not about DNS names, but just surfacing the error as you did in that line. Please check.

[waveywaves]

Thanks @jiparis — you're right, the DNS-1123 regex was out of scope. Stripped the PR back to just the two relevant changes:

1. Variable shadowing fix: m, err := → m, err = so the error propagates out of the loop
2. protovalidate.ValidationError early break so schema-level failures aren't masked by the kind-probing loop

All the unrelated changes (gracefulGitRepoHead refactor, policyEvalMatches removal, RunCollectors optimization) are gone. One file, 13 insertions, 7 deletions.

[waveywaves]

Addressed the latest feedback: dropped the outer m/err change, kept the early protovalidate.ValidationError return as the actual fix, added a regression test for invalid auto-detected material names, and refreshed the PR/commit AI disclosure to use Assisted-by: pi instead of the stale Claude Code wording.

Validated with:

go test ./pkg/attestation/crafter/...

[jiparis]

Thanks @waveywaves !