fix(attestation-type): reconcile --summary flags with the empty-value rule

[FayeSGW]

What

The repo-wide empty-value rule refuses an empty value for a flag that was given on the command line. The --summary / --summary-json flags on create attestation-type were never reconciled with it, so their tests asserted errors the rule now produces earlier, and parseSummaryJSON treated whitespace as "flag not given".

Changes

• parseSummaryJSON only treats "" as no summary. An unset flag arrives as ""; the empty-value rule guarantees an explicitly empty one never reaches the parser. Whitespace was written on purpose, so it falls through and is rejected as invalid JSON rather than silently dropping the summary — which is what --summary-json "$VAR" with VAR=" " would otherwise do in CI.
• Tests reconciled with the rule. --summary '', --summary-json '', and the both-flags-given case now assert flag '<name>' was given an empty value, since parsing refuses the value before PreRunE (and before MuXRequiredFlags) runs. --summary-json '[]' is still accepted — a summary with no entries, not a missing value.
• New coverage for whitespace-only --summary-json, at both the command and unit level.
• Empty-flag audit spec + coverage extended to include both summary flags.

Testing

make test_integration_single TARGET=CreateAttestationTypeTestSuite

🤖 Generated with Claude Code

[claude]

Claude finished @FayeSGW's task in 2m 24s —— View job

---

Claude Code Review

• Gather context (read changed files & diff)
• Static analysis (go build, go vet, go mod tidy)
• Code review

Static analysis

Check	Result
go build ./...	✅ clean
go vet ./cmd/kosli/	✅ clean
go mod tidy + git diff go.mod go.sum	✅ no changes

Summary

This is a tightly-scoped, well-reasoned fix. The behaviour is correct against the actual empty-value machinery, and the tests + comments do a good job explaining the why. I traced each claim in the PR body to the code and it holds up. No blocking issues.

Verification of the core claim

The pivot of the PR is that --summary-json '' is refused during flag parsing (before PreRunE/MuXRequiredFlags), while --summary-json ' ' slips through to parseSummaryJSON. Confirmed:

• nonEmptyValue.Set (cmd/kosli/nonEmptyValue.go:70-75) rejects only value == "", not trimmed whitespace. So '' → errEmptyFlagValue → flag '--summary-json' was given an empty value, and ' ' passes through to the parser. This exactly matches the two golden strings.
• --summary '' is a stringArray (splitsOnCommas == false), so nonEmptySliceValue.Set delegates to nonEmptyValue.Set("") and also yields the empty-value error. Matches the updated golden.
• The both-flags case: flag values are set before PreRunE runs, so the empty --summary-json '' is refused before MuXRequiredFlags ever sees it. The golden change from the mux error to the empty-value error is correct.
• parseSummaryJSON(" ") → json.Unmarshal returns a *json.SyntaxError (unexpected end of JSON input), caught by the errors.As branch. Golden string matches.

The narrowing from strings.TrimSpace(value) == "" to value == "" in parseSummaryJSON (createAttestationType.go:108) is the right call and the accompanying comment explains the CI --summary-json "$VAR" motivation clearly.

Observations (non-blocking)

1. Coupling to a stdlib error string. The whitespace tests assert the literal unexpected end of JSON input. This is stable Go behaviour and it's already the convention here (the existing malformed-JSON test does the same), so it's consistent — just flagging that a future Go version wording change would ripple into goldens.
2. --summary ' ' (whitespace) has no explicit test. Symmetry with the new --summary-json ' ' coverage would be nice: it falls through to parseSummaryFlags, where strings.Cut finds no = and returns --summary entry 1 must be in the form NAME=EXPRESSION. Purely additive, optional.
3. --summary-json '[]' semantics. json.Unmarshal("[]", …) yields a non-nil empty slice, so run does not fall through to parseSummaryFlags, and omitempty drops the empty Summary from the payload — i.e. [] is effectively "no summary". This is intended and documented in the test comment; noting it so the distinction from a truly missing value is on record.

Nice work

• The audit spec + coverage manifest were updated in the same PR (hack/empty-flag-audit/spec.json, testdata/empty-flag-audit-coverage.json), so the two summary flags are now exercised by the empty-flag audit rather than silently drifting out of it.
• The unit test split (an unset flag is treated as no summary vs whitespace-only values are rejected) makes the two distinct contracts explicit instead of lumping them together.
• Comments were rewritten to describe the new reality (parse-time refusal) rather than left stale — easy to forget, done well here.
  · fix-empty-flag-rule-vs-summary-flags