Test config helper contracts

.agents/sow/current/SOW-0102-20260603-quality-complexity-duplication-coverage.md

@@ -4,7 +4,7 @@
 
 Status: in-progress
 
-Sub-state: twenty-seventh implementation slice validated; downloader canonical coverage PR pending
+Sub-state: twenty-eighth implementation slice validated; config helper coverage PR pending
 
 ## Requirements
 
@@ -2151,6 +2151,53 @@ Open decisions:
 
 - No new user design decision is required because the slice is behavior-preserving test coverage under the previously approved quality plan.
 
+## Slice 28 Results
+
+Changes made:
+
+- Added behavior tests for config helper contracts in `pkg/config/helper_contract_test.go`.
+- Covered category lookup, default-public category behavior, explicit private category behavior, and category ordering.
+- Covered nil-config category helper behavior.
+- Covered sorted artifact names and artifact child lookup respecting `SourceOrder`.
+- Covered retention-window URL parser success and error cases.
+- Covered `ParseMergeURL` additive-only compatibility behavior.
+- Production code was unchanged.
+
+Measured result:
+
+- Baseline: `pkg/config` coverage was `71.2%` by `go test` output.
+- After tests: `pkg/config` coverage is `75.4%` by `go test` output.
+- `CategoryByName`, `CategoriesOrdered`, `PublicCategoriesOrdered`, `CategoryIsPublic`, `CategoryDefinition.IsPublic`, and `categoriesOrdered` moved from `0.0%` to `100.0%`.
+- `ArtifactChildren` moved from `0.0%` to `88.9%`.
+- `SortedArtifactNames` moved from `0.0%` to `100.0%`.
+- `ParseRetentionWindowURL` moved from `0.0%` to `94.4%`.
+- `ParseMergeURL` moved from `0.0%` to `100.0%`.
+- Root coverage by `go tool cover -func=coverage.out` moved from `73.2%` to `73.4%`.
+- `tools/archposture` after this slice: source files `635`, source lines `127725`, large files `49`, large functions `25`, and production large functions `0`.
+
+Tests or equivalent validation:
+
+- `go test ./pkg/config`: passed.
+- `go test -coverprofile=/tmp/update-ipsets-config-slice28.cover -covermode=atomic ./pkg/config`: passed, `75.4%`.
+- `go tool cover -func=/tmp/update-ipsets-config-slice28.cover`: passed; targeted helper coverage listed above.
+- `go run ./tools/archposture -root . > /tmp/update-ipsets-archposture-slice28.json`: passed.
+- `make lint`: passed.
+- `make staticcheck`: passed.
+- `make golangci-lint`: passed with `0 issues`.
+- `CI=true make coverage`: passed, root total `73.4%`.
+- `make test-strict`: passed.
+- `git diff --check`: passed.
+- Durable-artifact forbidden-name scan over added diff lines found no newly added personal name, authorship, tool, or vendor-attribution text.
+
+Artifact maintenance gate:
+
+- AGENTS.md: no update needed.
+- Runtime project skills: no update needed; no new durable process rule was found.
+- Specs: no update needed; config helper behavior is unchanged.
+- End-user/operator docs: no update needed.
+- End-user/operator skills: no update needed.
+- SOW lifecycle: remains in `.agents/sow/current/`; Slice 28 is validated and pending PR merge.
+
 ## Slice 27 Results
 
 Changes made:
@@ -2199,7 +2246,83 @@ Artifact maintenance gate:
 - Specs: no update needed; canonical downloader behavior is unchanged.
 - End-user/operator docs: no update needed.
 - End-user/operator skills: no update needed.
-- SOW lifecycle: remains in `.agents/sow/current/`; Slice 27 is validated and pending PR merge.
+- SOW lifecycle: remains in `.agents/sow/current/`; Slice 27 merged through PR #31 as merge commit `dec6b9395e80094cb1c663b0fbb531946249c133`.
+
+## Pre-Implementation Gate - Slice 28
+
+Status: ready.
+
+Problem / root-cause model:
+
+- Facts: after the Slice 27 merge, local `tools/archposture` still reports zero production large functions.
+- Facts: `go test -coverprofile=/tmp/update-ipsets-config-slice28-baseline.cover -covermode=atomic ./pkg/config` reports several clear config helper gaps: `CategoryByName`, `CategoriesOrdered`, `PublicCategoriesOrdered`, `CategoryIsPublic`, `CategoryDefinition.IsPublic`, and `categoriesOrdered` at `0.0%`; `ArtifactChildren` and `SortedArtifactNames` at `0.0%`; `ParseRetentionWindowURL` and `ParseMergeURL` at `0.0%`.
+- Working theory: config helper behavior is a low-risk coverage slice because these helpers expose deterministic ordering, public/private category semantics, artifact child lookup, and URL parsing contracts without requiring engine, network, or filesystem side effects beyond temporary test inputs.
+
+Evidence reviewed:
+
+- `pkg/config/category_registry.go`
+- `pkg/config/artifacts.go`
+- `pkg/config/expand.go`
+- `pkg/config/config_test.go`
+- `pkg/config/catalog_verify_test.go`
+- `/tmp/update-ipsets-config-slice28-baseline.cover`
+- `/tmp/update-ipsets-archposture-after-pr30.json`
+- Project coding, testing, hygiene, Go best-practices, Go behavioral-testing, and content-surface skills.
+
+Affected contracts and surfaces:
+
+- Category lookup, default-public category behavior, explicit private category behavior, and category ordering.
+- Artifact child lookup order and sorted artifact names.
+- Retention-window URL parsing and merge URL compatibility wrapper behavior.
+- SOW and tests only; no production code, docs, or specs are expected to change.
+
+Existing patterns to reuse:
+
+- Existing `pkg/config` same-package tests.
+- Existing table-driven test style and synthetic `Config` values.
+- Existing `SourceOrder` behavior for deterministic source ordering.
+- Existing URL scheme constants and parsing helpers.
+
+Risk and blast radius:
+
+- This slice should be test-only.
+- Tests must assert documented helper behavior, not implementation details such as map iteration.
+- Category ordering tests must include tie-breaks so future regressions are observable.
+- No runtime config semantics, catalog files, downloader behavior, scheduler behavior, public serving, install behavior, or UI behavior should change.
+
+Sensitive data handling plan:
+
+- This slice uses only synthetic category, artifact, and source names.
+- No secrets, tokens, cookies, private endpoints, customer data, or personal data are needed.
+- Durable artifacts will record only file paths, metrics, validation outcomes, and sanitized command evidence.
+
+Implementation plan:
+
+1. Add category registry behavior tests for lookup, default-public behavior, explicit private behavior, and ordering tie-breaks.
+2. Add artifact helper tests for sorted artifact names and `ArtifactChildren` respecting `SourceOrder`.
+3. Add URL parser tests for `ParseRetentionWindowURL` success/failure cases and `ParseMergeURL` additive-only compatibility behavior.
+4. Keep production code unchanged.
+
+Validation plan:
+
+- Run `go test ./pkg/config`.
+- Run `go test -coverprofile=/tmp/update-ipsets-config-slice28.cover -covermode=atomic ./pkg/config` and inspect `go tool cover -func`.
+- Run `go run ./tools/archposture -root . > /tmp/update-ipsets-archposture-slice28.json`.
+- Run `make lint`, `make staticcheck`, `make golangci-lint`, `CI=true make coverage`, and `make test-strict`.
+- Run whitespace and durable-artifact forbidden-name scans over the changed files before commit.
+
+Artifact impact plan:
+
+- AGENTS.md: no update expected.
+- Runtime project skills: no update needed unless a repeatable config-helper testing lesson is found.
+- Specs: no update expected because config helper behavior is unchanged.
+- End-user/operator docs: no update expected.
+- End-user/operator skills: no update expected.
+- SOW lifecycle: this SOW remains in `.agents/sow/current/`; Slice 28 results will be recorded after validation.
+
+Open decisions:
+
+- No new user design decision is required because the slice is behavior-preserving test coverage under the previously approved quality plan.
 
 ## Slice 25 Results
 
@@ -3524,7 +3647,7 @@ Open decisions:
 
 ## Outcome
 
-First through twenty-sixth implementation slices are complete, validated locally, and merged. The twenty-seventh implementation slice is complete and validated locally. The SOW remains open for the next focused coverage, complexity, or duplication slice.
+First through twenty-seventh implementation slices are complete, validated locally, and merged. The twenty-eighth implementation slice is complete and validated locally. The SOW remains open for the next focused coverage, complexity, or duplication slice.
 
 ## Lessons Extracted
 

pkg/config/helper_contract_test.go

@@ -0,0 +1,169 @@
+package config
+
+import (
+	"reflect"
+	"testing"
+)
+
+func TestCategoryRegistryHelpers(t *testing.T) {
+	private := false
+	public := true
+	cfg := New()
+	cfg.Categories = map[string]CategoryDefinition{
+		"hidden": {
+			Label:       "Hidden",
+			Description: "private category",
+			SortOrder:   1,
+			Public:      &private,
+		},
+		"alpha_a": {
+			Label:       "Alpha",
+			Description: "first alpha category",
+			SortOrder:   1,
+			Public:      &public,
+		},
+		"alpha_b": {
+			Label:       "Alpha",
+			Description: "second alpha category",
+			SortOrder:   1,
+		},
+		"beta": {
+			Label:       "Beta",
+			Description: "beta category",
+			SortOrder:   2,
+		},
+	}
+
+	def, ok := cfg.CategoryByName("hidden")
+	if !ok || def.Label != "Hidden" {
+		t.Fatalf("CategoryByName(hidden) = %+v, %v", def, ok)
+	}
+	if _, ok := cfg.CategoryByName("missing"); ok {
+		t.Fatal("CategoryByName(missing) ok = true, want false")
+	}
+	if (&Config{}).CategoryIsPublic("") {
+		t.Fatal("blank category should not be public")
+	}
+	if !cfg.CategoryIsPublic("unknown") {
+		t.Fatal("unknown categories should default to public")
+	}
+	if cfg.CategoryIsPublic("hidden") {
+		t.Fatal("explicit private category should not be public")
+	}
+	if !cfg.CategoryIsPublic("alpha_b") {
+		t.Fatal("category with nil Public should be public")
+	}
+
+	if got, want := categoryNames(cfg.CategoriesOrdered()), []string{"alpha_a", "alpha_b", "hidden", "beta"}; !reflect.DeepEqual(got, want) {
+		t.Fatalf("CategoriesOrdered names = %v, want %v", got, want)
+	}
+	if got, want := categoryNames(cfg.PublicCategoriesOrdered()), []string{"alpha_a", "alpha_b", "beta"}; !reflect.DeepEqual(got, want) {
+		t.Fatalf("PublicCategoriesOrdered names = %v, want %v", got, want)
+	}
+}
+
+func TestCategoryRegistryNilConfig(t *testing.T) {
+	var cfg *Config
+	if _, ok := cfg.CategoryByName("anything"); ok {
+		t.Fatal("nil CategoryByName ok = true, want false")
+	}
+	if cfg.CategoriesOrdered() != nil {
+		t.Fatal("nil CategoriesOrdered should return nil")
+	}
+	if cfg.PublicCategoriesOrdered() != nil {
+		t.Fatal("nil PublicCategoriesOrdered should return nil")
+	}
+	if !cfg.CategoryIsPublic("anything") {
+		t.Fatal("nil config should treat unknown named category as public")
+	}
+	if (CategoryDefinition{}).IsPublic() != true {
+		t.Fatal("zero CategoryDefinition should default to public")
+	}
+}
+
+func TestArtifactHelpers(t *testing.T) {
+	cfg := New()
+	cfg.Artifacts = map[string]*Artifact{
+		"zeta":  {Name: "zeta"},
+		"alpha": {Name: "alpha"},
+		"mid":   {Name: "mid"},
+	}
+	cfg.Sources = map[string]*Source{
+		"first":  {Name: "first", ArtifactParent: "alpha"},
+		"second": {Name: "second", ArtifactParent: "alpha"},
+		"other":  {Name: "other", ArtifactParent: "zeta"},
+		"plain":  {Name: "plain"},
+	}
+	cfg.SourceOrder = []string{"second", "plain", "first", "other"}
+
+	if got, want := SortedArtifactNames(cfg), []string{"alpha", "mid", "zeta"}; !reflect.DeepEqual(got, want) {
+		t.Fatalf("SortedArtifactNames = %v, want %v", got, want)
+	}
+	if SortedArtifactNames(nil) != nil {
+		t.Fatal("SortedArtifactNames(nil) should return nil")
+	}
+	children := cfg.ArtifactChildren("alpha")
+	if got, want := sourceNames(children), []string{"second", "first"}; !reflect.DeepEqual(got, want) {
+		t.Fatalf("ArtifactChildren(alpha) = %v, want %v", got, want)
+	}
+	if got := cfg.ArtifactChildren("missing"); len(got) != 0 {
+		t.Fatalf("ArtifactChildren(missing) len = %d, want 0", len(got))
+	}
+}
+
+func TestParseRetentionWindowURL(t *testing.T) {
+	parent, minutes, err := ParseRetentionWindowURL(InternalRetentionWindowScheme + "?parent=base&minutes=1440")
+	if err != nil {
+		t.Fatalf("ParseRetentionWindowURL(valid): %v", err)
+	}
+	if parent != "base" || minutes != 1440 {
+		t.Fatalf("ParseRetentionWindowURL(valid) = %q, %d", parent, minutes)
+	}
+
+	tests := []string{
+		"internal://other?parent=base&minutes=1440",
+		InternalRetentionWindowScheme + "?minutes=1440",
+		InternalRetentionWindowScheme + "?parent=base",
+		InternalRetentionWindowScheme + "?parent=base&minutes=not-an-int",
+		InternalRetentionWindowScheme + "?parent=base&minutes=0",
+	}
+	for _, rawURL := range tests {
+		t.Run(rawURL, func(t *testing.T) {
+			if _, _, err := ParseRetentionWindowURL(rawURL); err == nil {
+				t.Fatal("ParseRetentionWindowURL error = nil, want error")
+			}
+		})
+	}
+}
+
+func TestParseMergeURLCompatibility(t *testing.T) {
+	inputs, err := ParseMergeURL(InternalMergeScheme + "?inputs=alpha,,beta&exclude=private")
+	if err != nil {
+		t.Fatalf("ParseMergeURL(valid): %v", err)
+	}
+	if got, want := inputs, []string{"alpha", "beta"}; !reflect.DeepEqual(got, want) {
+		t.Fatalf("ParseMergeURL inputs = %v, want %v", got, want)
+	}
+	if _, err := ParseMergeURL("internal://other?inputs=alpha"); err == nil {
+		t.Fatal("ParseMergeURL(wrong scheme) error = nil, want error")
+	}
+	if _, err := ParseMergeURL(InternalMergeScheme + "?exclude=private"); err == nil {
+		t.Fatal("ParseMergeURL(missing inputs) error = nil, want error")
+	}
+}
+
+func categoryNames(categories []NamedCategory) []string {
+	names := make([]string, 0, len(categories))
+	for _, category := range categories {
+		names = append(names, category.Name)
+	}
+	return names
+}
+
+func sourceNames(sources []*Source) []string {
+	names := make([]string, 0, len(sources))
+	for _, source := range sources {
+		names = append(names, source.Name)
+	}
+	return names
+}