From 61f42c218447cd7712aaff490a90941b63e1973c Mon Sep 17 00:00:00 2001 From: Yvonne Devlin Date: Mon, 31 Aug 2026 12:09:29 +0000 Subject: [PATCH] refactor: consolidate safeSSABuild into shared internal/ssaguard package Extract the byte-identical safeSSABuild recover-guard (duplicated in internal/analysis/mutation.go and internal/quality/pairing.go) into a new stdlib-only package internal/ssaguard with exported SafeSSABuild. Reverses spec-021 R3 (duplication-for-dependency-lightness) since the guard has zero external dependencies. - Add internal/ssaguard with GoDoc documenting the ssa.BuildSerially caller precondition and goroutine-scoped recover() rationale (spec 033) - Update BuildSSA and BuildTestSSA to call ssaguard.SafeSSABuild, retaining caller-side log.Warn/log.Debug (preserves pkg-path context) - Remove duplicated TestSafeSSABuild_* triads and export_test.go shims; delete now-empty internal/quality/pairing_test.go - Consolidate to 3 canonical tests (100% branch coverage); net test -3, behavioral coverage neutral - Add OpenSpec artifacts and AGENTS.md/docs updates Closes #238 Assisted-by: claude-opus Generated with AI assistance (claude-opus) --- ...ssa-guard-20260831T112125-yvonne-devlin.md | 10 ++ .../ssaguard-20260831T112119-yvonne-devlin.md | 10 ++ AGENTS.md | 2 + docs/concepts/analysis-pipeline.md | 2 +- internal/analysis/export_test.go | 5 - internal/analysis/mutation.go | 15 +- internal/analysis/mutation_test.go | 55 +------ internal/quality/export_test.go | 5 - internal/quality/pairing.go | 21 +-- internal/quality/pairing_test.go | 67 -------- internal/ssaguard/ssaguard.go | 34 ++++ internal/ssaguard/ssaguard_test.go | 55 +++++++ .../consolidate-ssa-guard/.openspec.yaml | 2 + .../changes/consolidate-ssa-guard/design.md | 121 ++++++++++++++ .../changes/consolidate-ssa-guard/proposal.md | 152 ++++++++++++++++++ .../specs/ssaguard/spec.md | 101 ++++++++++++ .../changes/consolidate-ssa-guard/tasks.md | 88 ++++++++++ 17 files changed, 585 insertions(+), 160 deletions(-) create mode 100644 .uf/dewey/learnings/consolidate-ssa-guard-20260831T112125-yvonne-devlin.md create mode 100644 .uf/dewey/learnings/ssaguard-20260831T112119-yvonne-devlin.md delete mode 100644 internal/quality/pairing_test.go create mode 100644 internal/ssaguard/ssaguard.go create mode 100644 internal/ssaguard/ssaguard_test.go create mode 100644 openspec/changes/consolidate-ssa-guard/.openspec.yaml create mode 100644 openspec/changes/consolidate-ssa-guard/design.md create mode 100644 openspec/changes/consolidate-ssa-guard/proposal.md create mode 100644 openspec/changes/consolidate-ssa-guard/specs/ssaguard/spec.md create mode 100644 openspec/changes/consolidate-ssa-guard/tasks.md diff --git a/.uf/dewey/learnings/consolidate-ssa-guard-20260831T112125-yvonne-devlin.md b/.uf/dewey/learnings/consolidate-ssa-guard-20260831T112125-yvonne-devlin.md new file mode 100644 index 00000000..ee68bf9b --- /dev/null +++ b/.uf/dewey/learnings/consolidate-ssa-guard-20260831T112125-yvonne-devlin.md @@ -0,0 +1,10 @@ +--- +tag: consolidate-ssa-guard +author: yvonne-devlin +category: gotcha +created_at: 2026-08-31T11:21:25Z +identity: consolidate-ssa-guard-20260831T112125-yvonne-devlin +tier: draft +--- + +When deduplicating tests during a DRY consolidation, deleting a whole *_test.go file can be the correct Zero-Waste outcome: internal/quality/pairing_test.go held ONLY the 3 duplicated TestSafeSSABuild_* tests plus a stale SC-001 comment block with no test functions. After removing the duplicated triad it would not compile (unused testing/quality imports), so deleting the entire file was cleaner than leaving a stub. Behavioral coverage was preserved because the 3 canonical tests moved to internal/ssaguard/ssaguard_test.go. Reviewers accept a net test-count decrease (-3 here: 6 duplicates -> 3 shared) as long as the acceptance criterion is framed as 'behavioral coverage is neutral-to-positive' NOT 'net test count does not decrease' — the latter wording is a self-contradicting acceptance criterion that a spec reviewer (divisor-testing) will flag HIGH. Also satisfy Constitution IV by stating the coverage strategy explicitly (unit only, 100% branch coverage, enumerate the branches) in proposal/design/tasks. diff --git a/.uf/dewey/learnings/ssaguard-20260831T112119-yvonne-devlin.md b/.uf/dewey/learnings/ssaguard-20260831T112119-yvonne-devlin.md new file mode 100644 index 00000000..41ae89d1 --- /dev/null +++ b/.uf/dewey/learnings/ssaguard-20260831T112119-yvonne-devlin.md @@ -0,0 +1,10 @@ +--- +tag: ssaguard +author: yvonne-devlin +category: pattern +created_at: 2026-08-31T11:21:19Z +identity: ssaguard-20260831T112119-yvonne-devlin +tier: draft +--- + +Consolidating a duplicated recover()-guard helper (safeSSABuild) into a shared package (internal/ssaguard) is safe and coverage-neutral when the guard has ZERO external dependencies — it takes a func() and returns any. This reverses spec-021 R3's 'keep packages dependency-light' rationale, which only applied when a helper might drag in dependencies. Key design decisions that made review pass 5/5: (1) Name the package `ssaguard` NOT `ssautil` to avoid shadowing golang.org/x/tools/go/ssa/ssautil already imported at both call sites. (2) Export the function (SafeSSABuild) so both callers' export_test.go shims are eliminated — under internal/ this adds no API surface concern. (3) Keep log.Warn/log.Debug at the CALLER recovery site, do NOT move logging into the guard — the guard has no visibility into the package being built (loses pkg.PkgPath diagnostic context) and importing a logger would violate the stdlib-only constraint. (4) Document the ssa.BuildSerially caller precondition in GoDoc as documentation-only (NOT runtime validation) — mode flags are set by callers before ssautil.AllPackages, outside the guard's scope; recover() is goroutine-scoped so omitting BuildSerially causes silent panic-escape/process crash (spec 033). diff --git a/AGENTS.md b/AGENTS.md index ad4a4df0..84c4a48d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -324,6 +324,7 @@ internal/ aireport/ AI-powered CI quality report pipeline (gaze report) protocol/ JSON-RPC 2.0 client for external analyzer communication adapter/ External analyzer provider adapters (protocol → crap interfaces) + ssaguard/ Shared SSA panic-recovery guard (BuildSerially precondition) provider/ goprovider/ Go-specific provider implementations (gocyclo, go test, SSA) mockprovider/ Mock providers for unit testing the scoring core @@ -485,6 +486,7 @@ Formatters: gofmt, goimports. ## Recent Changes +- consolidate-ssa-guard: Consolidated the byte-identical `safeSSABuild` recover-guard (previously duplicated in `internal/analysis/mutation.go` and `internal/quality/pairing.go`) into a new shared stdlib-only package `internal/ssaguard` with exported `SafeSSABuild(buildFn func()) (panicVal any)`. Reverses spec-021 R3 (which chose duplication to keep packages dependency-light) — justified because the guard has zero external dependencies. GoDoc documents the `ssa.BuildSerially` caller precondition and the goroutine-scoped `recover()` rationale (spec 033); the guard does NOT validate build mode at runtime. Both callers (`BuildSSA`, `BuildTestSSA`) now call `ssaguard.SafeSSABuild(prog.Build)` and retain their `log.Warn`/`log.Debug` recovery-site calls (logging not moved into the guard, preserving pkg-path context). Removed duplicated `TestSafeSSABuild_*` triads from `mutation_test.go`, deleted `internal/quality/pairing_test.go` (held only the duplicated triad plus a stale no-test comment block), and removed the `SafeSSABuild` shim from both `export_test.go` files. Three canonical tests live in `internal/ssaguard/ssaguard_test.go` (100% branch coverage). Net test count -3, behavioral coverage neutral. Closes #238. - decompose-high-complexity-functions: Decomposed 5 high-complexity functions to reduce cyclomatic complexity (CC) and CRAPload (issue #200, continuation of #166). `isPointerArgStore` (CC 13→3): removed structurally unreachable branches — `tracesToParam` already walks FieldAddr/IndexAddr/UnOp chains. `detectASTReceiverMutations` (CC 24→11): extracted 3 per-node-type handlers (`handleReceiverAssignStmt`, `handleReceiverIncDecStmt`, `handleReceiverCallExpr`) from `ast.Inspect` closure. `runCrap` (CC 19→14): extracted `resolveBaselineAndCompare`, `writeCrapOutputAndSummary`, `evaluateCrapGates` preserving D7 gate ordering (baseline before threshold). `runQuality` (CC 32→14): extracted `loadQualityConfig`, `setupQualityDeps`, `writeQualityReport`, `handleQualityEmptyResults`, plus previously extracted `runQualityPerPackage` and `writeQualityEmptyResults`. `writeOneResult` (CC 32→4): extracted `buildEffectsTable`, `writeTierSummary`, plus previously extracted `writeEffectRows` and `writeVerboseSignals`. Added ~35 new unit tests across 3 packages using synthetic AST, DI structs, and `testdata/src/` fixtures. No behavioral changes — all existing tests pass without modification. - adapter-call-unmarshal-helper: Extracted a generic `callAndUnmarshal[T any](ctx context.Context, client *protocol.Client, method string, params any) (T, error)` helper (`internal/adapter/call.go`, unexported) that centralizes the repeated JSON-RPC `Call → transport-error check → protocol-error check → json.Unmarshal[T]` pattern. Migrated 3 hard-error batch call sites: `ExternalComplexityProvider.Analyze` (`complexity.go`, `callAndUnmarshal[protocol.ComplexityResult]`), `ExternalLineCoverageProvider.Coverage` (`coverage.go`, `[protocol.CoverageResult]`), and `ExternalSideEffectAnalyzer.loadBatch` (`sideeffect.go`, `[protocol.AnalyzeResult]`). Because the method constants are literally `"complexity"`/`"coverage"`/`"analyze"`, the helper's D2 error templates (`"%s protocol call: %w"`, `"%s protocol error: %s (code %d)"`, `"parsing %s result: %w"`) reproduce the exact legacy error strings. Error-chain contract (design D6): transport and unmarshal errors wrap with `%w` (`errors.Is`/`errors.As` preserved); protocol errors format with `%s` (structured JSON-RPC error object, not a Go error chain). `Session.Initialize` (`session.go`) intentionally retains its inline per-branch error handling (design D2 sanctioned option) because the collapsed generic helper cannot reproduce its three distinct legacy strings (`"initialize handshake"`/`"initialize error"`/`"parsing initialize result"`); it also keeps `s.client.Close()` cleanup on each error branch. Explicitly excluded from migration: `CallStream`/streaming side-effect path, `fetchTestMappings` `p.warn()` graceful-degradation path (design D3), and `internal/protocol/` (transport layer stays usage-agnostic). Added table-driven `TestCallAndUnmarshal` (`call_test.go`, `package adapter` internal) with 5 subtests (success, transport error, protocol error, unmarshal failure, generic instantiation across a second result type) driven through the fake analyzer binary; the internal test builds its own fake-analyzer copy lazily via `sync.Once` because only one `TestMain` is allowed per test binary (the existing one lives in `package adapter_test`). Split from #201; closes #237. - quality-empty-results-gate: Fixed silent exit-0 bug (#103) when `gaze quality` encounters Ginkgo/BDD suites or packages where no test functions can be resolved to targets. Added `SkippedTests int` and `SkippedTestNames []string` fields to `taxonomy.PackageSummary` (`internal/taxonomy/types.go`). `quality.Assess` (`internal/quality/quality.go`) now counts and names skipped test functions at the `len(targets) == 0` continue site. `runQuality` (`cmd/gaze/main.go`) now prints structured stdout summary when `allReports` is empty (total test count, skipped names truncated at 20, `--target` hint), returns error when `--min-contract-coverage` or `--max-over-specification` thresholds are set (quality gate failure), and produces valid JSON for `--format=json`. Added `writeSkippedTests` section to `quality.WriteText` (`internal/quality/report.go`) with 20-name truncation. Updated `mergeSummaries` to aggregate skipped test data. Updated JSON Schema (`internal/report/schema.go`). Propagated through report pipeline: `runQualityForPackage` now returns 3 values, `qualityStepResult.SkippedTests`, `ReportSummary.SkippedTests`, `compactSummary.SkippedTests`, `compactPackageSummary.SkippedTests`/`SkippedTestNames`. Added `testdata/src/bddstyle/` fixture. 13 new tests across 3 packages. Closes #103. diff --git a/docs/concepts/analysis-pipeline.md b/docs/concepts/analysis-pipeline.md index 48a808db..ded471a6 100644 --- a/docs/concepts/analysis-pipeline.md +++ b/docs/concepts/analysis-pipeline.md @@ -142,7 +142,7 @@ The pipeline is designed to produce useful results even when parts fail: | Failure | Impact | Mitigation | |---|---|---| -| SSA build panics | No mutation detection via SSA | `safeSSABuild` recovers the panic; AST fallback detects common mutation patterns | +| SSA build panics | No mutation detection via SSA | `ssaguard.SafeSSABuild` recovers the panic; AST fallback detects common mutation patterns | | SSA build returns nil | Same as panic | AST fallback activates automatically | | Type info unavailable | Reduced precision for global detection, import resolution | Fallback to AST name matching (may produce false positives) | | Package load errors | No analysis for that package | Error returned to caller; other packages unaffected | diff --git a/internal/analysis/export_test.go b/internal/analysis/export_test.go index 9499ce81..0f37838e 100644 --- a/internal/analysis/export_test.go +++ b/internal/analysis/export_test.go @@ -29,11 +29,6 @@ func BaseTypeName(expr ast.Expr) string { return baseTypeName(expr) } -// SafeSSABuild is exported for testing. See safeSSABuild. -func SafeSSABuild(buildFn func()) any { - return safeSSABuild(buildFn) -} - // ExprRootIdent is exported for testing. See exprRootIdent. func ExprRootIdent(expr ast.Expr) *ast.Ident { return exprRootIdent(expr) diff --git a/internal/analysis/mutation.go b/internal/analysis/mutation.go index 8a9104af..05e40e54 100644 --- a/internal/analysis/mutation.go +++ b/internal/analysis/mutation.go @@ -12,21 +12,10 @@ import ( "golang.org/x/tools/go/ssa" "golang.org/x/tools/go/ssa/ssautil" + "github.com/unbound-force/gaze/internal/ssaguard" "github.com/unbound-force/gaze/internal/taxonomy" ) -// safeSSABuild calls buildFn and recovers from any panic it produces. -// Returns the recovered panic value, or nil if buildFn completed -// without panicking. This isolates the recover() pattern so it can -// be tested independently of the SSA builder. -func safeSSABuild(buildFn func()) (panicVal any) { - defer func() { - panicVal = recover() - }() - buildFn() - return nil -} - // BuildSSA constructs the SSA representation for a loaded package. // The result is reusable across multiple function analyses within // the same package, avoiding the cost of rebuilding SSA per function. @@ -46,7 +35,7 @@ func BuildSSA(pkg *packages.Package) (ssaPkg *ssa.Package) { ssa.InstantiateGenerics|ssa.BuildSerially, ) - if r := safeSSABuild(prog.Build); r != nil { + if r := ssaguard.SafeSSABuild(prog.Build); r != nil { log.Warn("SSA build skipped: internal panic recovered", "pkg", pkg.PkgPath) log.Debug("SSA panic value", "pkg", pkg.PkgPath, "panic", r) return nil diff --git a/internal/analysis/mutation_test.go b/internal/analysis/mutation_test.go index 82021115..3b7667b0 100644 --- a/internal/analysis/mutation_test.go +++ b/internal/analysis/mutation_test.go @@ -1,7 +1,6 @@ package analysis_test import ( - "errors" "go/ast" "go/parser" "go/token" @@ -15,62 +14,16 @@ import ( "golang.org/x/tools/go/ssa" ) -// --------------------------------------------------------------------------- -// safeSSABuild tests -// --------------------------------------------------------------------------- - -// TestSafeSSABuild_NoPanic verifies that safeSSABuild returns nil -// when the build function completes without panicking. -func TestSafeSSABuild_NoPanic(t *testing.T) { - result := analysis.SafeSSABuild(func() { - // no panic - }) - if result != nil { - t.Errorf("safeSSABuild returned %v, want nil for non-panicking function", result) - } -} - -// TestSafeSSABuild_PanicString verifies that safeSSABuild recovers -// a panic with a string value and returns it. -func TestSafeSSABuild_PanicString(t *testing.T) { - result := analysis.SafeSSABuild(func() { - panic("test panic message") - }) - s, ok := result.(string) - if !ok { - t.Fatalf("safeSSABuild returned %T, want string", result) - } - if s != "test panic message" { - t.Errorf("safeSSABuild returned %q, want %q", s, "test panic message") - } -} - -// TestSafeSSABuild_PanicError verifies that safeSSABuild recovers -// a panic with an error value and returns it. -func TestSafeSSABuild_PanicError(t *testing.T) { - errPanic := errors.New("SSA builder error") - result := analysis.SafeSSABuild(func() { - panic(errPanic) - }) - e, ok := result.(error) - if !ok { - t.Fatalf("safeSSABuild returned %T, want error", result) - } - if e != errPanic { - t.Errorf("safeSSABuild returned error %v, want %v", e, errPanic) - } -} - // --------------------------------------------------------------------------- // SC-001 / SC-002: panic recovery contract tests // // Note: BuildSSA's panic recovery cannot be tested end-to-end because // prog.Build() is a concrete method on *ssa.Program that cannot be // mocked or injected. The recovery pattern is verified through the -// safeSSABuild helper tests above (which exercise the identical -// defer/recover logic). BuildSSA's logging behavior is verified by -// code inspection — the log.Warn/log.Debug calls are co-located with the -// safeSSABuild call in the same if-block. +// ssaguard.SafeSSABuild tests in internal/ssaguard/ssaguard_test.go +// (which exercise the identical defer/recover logic). BuildSSA's logging +// behavior is verified by code inspection — the log.Warn/log.Debug calls +// are co-located with the ssaguard.SafeSSABuild call in the same if-block. // --------------------------------------------------------------------------- // TestSC001_BuildSSANoPanicReturnsPackage verifies that BuildSSA diff --git a/internal/quality/export_test.go b/internal/quality/export_test.go index c3cd6915..329de4d2 100644 --- a/internal/quality/export_test.go +++ b/internal/quality/export_test.go @@ -3,11 +3,6 @@ package quality // ResolveExprRoot exports resolveExprRoot for testing. var ResolveExprRoot = resolveExprRoot -// SafeSSABuild is exported for testing. See safeSSABuild. -func SafeSSABuild(buildFn func()) any { - return safeSSABuild(buildFn) -} - // MapAssertionsToEffectsWithStderr exports mapAssertionsToEffectsImpl // for testing AI mapper with stderr capture. var MapAssertionsToEffectsWithStderr = mapAssertionsToEffectsImpl diff --git a/internal/quality/pairing.go b/internal/quality/pairing.go index ec9e43f8..bf732bcd 100644 --- a/internal/quality/pairing.go +++ b/internal/quality/pairing.go @@ -11,24 +11,9 @@ import ( "golang.org/x/tools/go/packages" "golang.org/x/tools/go/ssa" "golang.org/x/tools/go/ssa/ssautil" -) -// safeSSABuild calls buildFn and recovers from any panic it produces. -// Returns the recovered panic value, or nil if buildFn completed -// without panicking. This isolates the recover() pattern so it can -// be tested independently of the SSA builder. -// -// Duplicated from internal/analysis/mutation.go because Go's package -// system does not allow sharing unexported symbols across internal -// packages. A shared package was rejected to keep both packages -// dependency-light — see specs/021-ssa-panic-recovery/research.md R3. -func safeSSABuild(buildFn func()) (panicVal any) { - defer func() { - panicVal = recover() - }() - buildFn() - return nil -} + "github.com/unbound-force/gaze/internal/ssaguard" +) // TestFunc represents a test function found in a test package. type TestFunc struct { @@ -132,7 +117,7 @@ func BuildTestSSA(pkg *packages.Package) (program *ssa.Program, ssaPkg *ssa.Pack ssa.InstantiateGenerics|ssa.BuildSerially, ) - if r := safeSSABuild(prog.Build); r != nil { + if r := ssaguard.SafeSSABuild(prog.Build); r != nil { log.Warn("SSA build skipped: internal panic recovered", "pkg", pkg.PkgPath) log.Debug("SSA panic value", "pkg", pkg.PkgPath, "panic", r) return nil, nil, fmt.Errorf("SSA build panicked for package %s: internal panic recovered", pkg.PkgPath) diff --git a/internal/quality/pairing_test.go b/internal/quality/pairing_test.go deleted file mode 100644 index e8c47fcf..00000000 --- a/internal/quality/pairing_test.go +++ /dev/null @@ -1,67 +0,0 @@ -package quality_test - -import ( - "errors" - "testing" - - "github.com/unbound-force/gaze/internal/quality" -) - -// --------------------------------------------------------------------------- -// safeSSABuild tests (quality package copy) -// -// safeSSABuild is duplicated in internal/quality because Go's package -// system does not allow importing unexported symbols across internal -// packages. The duplication is intentional and bounded to this one -// 6-line function — see specs/021-ssa-panic-recovery/research.md R3. -// --------------------------------------------------------------------------- - -// TestSafeSSABuild_NoPanic verifies the quality package's -// safeSSABuild returns nil for non-panicking functions. -func TestSafeSSABuild_NoPanic(t *testing.T) { - result := quality.SafeSSABuild(func() {}) - if result != nil { - t.Errorf("safeSSABuild returned %v, want nil", result) - } -} - -// TestSafeSSABuild_PanicString verifies the quality package's -// safeSSABuild recovers a string panic. -func TestSafeSSABuild_PanicString(t *testing.T) { - result := quality.SafeSSABuild(func() { - panic("test panic") - }) - s, ok := result.(string) - if !ok { - t.Fatalf("safeSSABuild returned %T, want string", result) - } - if s != "test panic" { - t.Errorf("safeSSABuild returned %q, want %q", s, "test panic") - } -} - -// TestSafeSSABuild_PanicError verifies the quality package's -// safeSSABuild recovers an error-typed panic. -func TestSafeSSABuild_PanicError(t *testing.T) { - errPanic := errors.New("SSA builder error") - result := quality.SafeSSABuild(func() { - panic(errPanic) - }) - e, ok := result.(error) - if !ok { - t.Fatalf("safeSSABuild returned %T, want error", result) - } - if e != errPanic { - t.Errorf("safeSSABuild returned error %v, want %v", e, errPanic) - } -} - -// --------------------------------------------------------------------------- -// SC-001: BuildTestSSA panic recovery contract -// -// Note: BuildTestSSA's panic recovery cannot be tested end-to-end -// because prog.Build() is a concrete method on *ssa.Program that -// cannot be mocked. The recovery pattern is verified through the -// safeSSABuild helper tests above. BuildTestSSA's logging behavior -// is verified by code inspection. -// --------------------------------------------------------------------------- diff --git a/internal/ssaguard/ssaguard.go b/internal/ssaguard/ssaguard.go new file mode 100644 index 00000000..8ecaa9f7 --- /dev/null +++ b/internal/ssaguard/ssaguard.go @@ -0,0 +1,34 @@ +// Package ssaguard provides a shared panic-recovery guard for SSA +// construction. SSA builds via golang.org/x/tools can panic on certain +// upstream bugs (e.g., generic type substitution under Go 1.25); this +// package isolates the recover() pattern so callers degrade gracefully +// instead of crashing. +package ssaguard + +// SafeSSABuild calls buildFn and recovers from any panic it produces. +// It returns the recovered panic value, or nil if buildFn completed +// without panicking. Isolating the recover() pattern here lets it be +// tested independently of the SSA builder and shared by every SSA +// build site. +// +// Caller precondition — ssa.BuildSerially: callers MUST construct the +// SSA program with the ssa.BuildSerially mode flag (alongside +// ssa.InstantiateGenerics) before invoking SafeSSABuild(prog.Build). +// Go's recover() is goroutine-scoped and cannot catch panics raised in +// child goroutines. Without ssa.BuildSerially, prog.Build() spawns a +// child goroutine per package and any panic there escapes this guard, +// crashing the process. BuildSerially forces all construction onto the +// calling goroutine so the deferred recover() below can catch it. See +// specs/033-ssa-goroutine-panic for the invariant. +// +// SafeSSABuild does NOT validate the build mode at runtime: the mode +// flags are set by callers before ssautil.AllPackages, which is outside +// this guard's scope. The precondition is documented and enforced by +// convention, not by a runtime check. +func SafeSSABuild(buildFn func()) (panicVal any) { + defer func() { + panicVal = recover() + }() + buildFn() + return nil +} diff --git a/internal/ssaguard/ssaguard_test.go b/internal/ssaguard/ssaguard_test.go new file mode 100644 index 00000000..52521379 --- /dev/null +++ b/internal/ssaguard/ssaguard_test.go @@ -0,0 +1,55 @@ +package ssaguard_test + +import ( + "errors" + "testing" + + "github.com/unbound-force/gaze/internal/ssaguard" +) + +// Coverage strategy: unit tests only. These three cases achieve 100% +// branch coverage of SafeSSABuild — the deferred recover() path via the +// two panic cases, and the normal return nil path via the no-panic +// case. This is the complete set of reachable branches. + +// TestSafeSSABuild_NoPanic verifies that SafeSSABuild returns nil +// when the build function completes without panicking. +func TestSafeSSABuild_NoPanic(t *testing.T) { + result := ssaguard.SafeSSABuild(func() { + // no panic + }) + if result != nil { + t.Errorf("SafeSSABuild returned %v, want nil for non-panicking function", result) + } +} + +// TestSafeSSABuild_PanicString verifies that SafeSSABuild recovers +// a panic with a string value and returns it. +func TestSafeSSABuild_PanicString(t *testing.T) { + result := ssaguard.SafeSSABuild(func() { + panic("test panic message") + }) + s, ok := result.(string) + if !ok { + t.Fatalf("SafeSSABuild returned %T, want string", result) + } + if s != "test panic message" { + t.Errorf("SafeSSABuild returned %q, want %q", s, "test panic message") + } +} + +// TestSafeSSABuild_PanicError verifies that SafeSSABuild recovers +// a panic with an error value and returns it. +func TestSafeSSABuild_PanicError(t *testing.T) { + errPanic := errors.New("SSA builder error") + result := ssaguard.SafeSSABuild(func() { + panic(errPanic) + }) + e, ok := result.(error) + if !ok { + t.Fatalf("SafeSSABuild returned %T, want error", result) + } + if e != errPanic { + t.Errorf("SafeSSABuild returned error %v, want %v", e, errPanic) + } +} diff --git a/openspec/changes/consolidate-ssa-guard/.openspec.yaml b/openspec/changes/consolidate-ssa-guard/.openspec.yaml new file mode 100644 index 00000000..0d971659 --- /dev/null +++ b/openspec/changes/consolidate-ssa-guard/.openspec.yaml @@ -0,0 +1,2 @@ +schema: unbound-force +created: 2026-08-31 diff --git a/openspec/changes/consolidate-ssa-guard/design.md b/openspec/changes/consolidate-ssa-guard/design.md new file mode 100644 index 00000000..37a38ac8 --- /dev/null +++ b/openspec/changes/consolidate-ssa-guard/design.md @@ -0,0 +1,121 @@ +## Context + +`safeSSABuild` is a 7-line `recover()` guard around `prog.Build()` that +is byte-for-byte duplicated in two packages: + +- `internal/analysis/mutation.go` — used by `BuildSSA`, which builds the + SSA program via + `ssautil.AllPackages([]*packages.Package{pkg}, + ssa.InstantiateGenerics|ssa.BuildSerially)` and then calls + `safeSSABuild(prog.Build)`. +- `internal/quality/pairing.go` — used by `BuildTestSSA`, identical shape, + plus a duplication-lineage comment citing spec 021 research decision R3. + +The duplication also spans: +- `export_test.go` shims exposing the unexported guard to external-package + tests (`internal/analysis/export_test.go`, + `internal/quality/export_test.go`). +- Three-case test triads in both `mutation_test.go` and `pairing_test.go` + (`TestSafeSSABuild_NoPanic` / `_PanicString` / `_PanicError`). + +The guard encodes a safety invariant established by spec 033 (issue #33): +`recover()` is goroutine-scoped, so it only catches `prog.Build()` panics +when the caller sets `ssa.BuildSerially` (forcing serial construction on +the calling goroutine). Without that flag, `Build()` spawns child +goroutines whose panics escape the guard and crash the process. The +current design relies on this invariant living implicitly next to each +caller — a fragile arrangement that already required a synchronized +two-site fix during spec 033. + +## Goals / Non-Goals + +### Goals +- Establish a single source of truth for the SSA panic guard in a new + `internal/ssaguard` leaf package. +- Export `SafeSSABuild(buildFn func()) (panicVal any)` so it is directly + testable without `export_test.go` shim indirection. +- Document the `ssa.BuildSerially` precondition authoritatively in one + GoDoc comment. +- Update both callers to delegate; remove all duplicated definitions, + shims, and tests. +- Preserve exact runtime behavior and keep race tests green. + +### Non-Goals +- Runtime validation of the SSA build mode. `SafeSSABuild` receives only a + `func()`; it has no visibility into how the program was constructed. The + `BuildSerially` flag is set by callers before `ssautil.AllPackages`, + outside this function's scope. Enforcement stays documentation-based. +- Changing the callers' SSA construction (mode flags, package loading) + beyond swapping the local guard for the shared one. +- Any change to Gaze's exported API, CLI, output formats, or config. + +## Decisions + +**D1 — New package name `ssaguard`, not `ssautil`.** Both `mutation.go` +and `pairing.go` already import `golang.org/x/tools/go/ssa/ssautil`. +Naming the new package `ssautil` would shadow that import at call sites +and invite confusion/misuse. `ssaguard` is descriptive (it guards SSA +builds) and follows the project's flat `internal/` naming convention +(`loader`, `taxonomy`, `classify`, `docscan`). + +**D2 — Export the function.** Making `SafeSSABuild` exported removes the +need for `export_test.go` shims in both consumers. This improves +testability (Constitution IV) by allowing direct in-package unit tests in +`internal/ssaguard/ssaguard_test.go`, and eliminates two lines of shim +indirection. + +**D3 — Reverse spec 021 R3.** R3 duplicated the guard to keep packages +dependency-light. Since the guard has zero external dependencies (input +`func()`, output `any`, body uses only `recover()`), a shared leaf +package adds no transitive coupling. Both callers already depend on +`x/tools/go/ssa`; the new internal edge is strictly lighter. The reversal +is recorded in the delta spec's REMOVED section. + +**D4 — Documentation-only invariant.** The `BuildSerially` requirement is +captured in the `SafeSSABuild` GoDoc as a mandatory caller precondition, +with the goroutine-scoped `recover()` rationale. This centralizes the +knowledge that spec 033 previously spread across two files, reducing the +risk that a future third build site omits the flag. + +**D5 — Consolidate tests, preserve behavioral coverage.** The three +canonical cases (no-panic → nil; string panic → recovered string; error +panic → recovered error) move into `internal/ssaguard/ssaguard_test.go`. +The six duplicated tests are removed. Net test count is -3, but the same +logic is now exercised once in its canonical home; behavioral coverage is +neutral-to-positive. + +**Coverage strategy (Constitution IV)**: Unit tests only. The three test +cases achieve 100% branch coverage of the 7-line `SafeSSABuild` function: +the deferred `recover()` path is exercised by the two panic scenarios +(string and error), and the normal `return nil` path is exercised by the +no-panic scenario. These three branches are the complete set — the +function has no other reachable paths. No integration or e2e tests are +required because the guard is a pure recovery wrapper with zero +dependencies. Target: 100% branch coverage of the new package. + +**Constitution ties** (Gaze project constitution, +`.specify/memory/constitution.md`): This is an internal refactor — +Accuracy (I) is N/A (no analysis behavior/output change). Minimal +Assumptions (II) holds: `ssaguard` is a standalone zero-dependency leaf +that adds no new host-project assumptions. Actionable Output (III) is N/A +(no human- or machine-readable output change); the documented invariant +improves internal maintainability of a safety property. Testability (IV) +improves via D2 and D5. + +## Risks / Trade-offs + +- **R1 — Loss of caller proximity for the invariant.** Moving the guard + away from its callers means the `BuildSerially` requirement is no longer + physically adjacent to `ssautil.AllPackages`. Mitigation: D4's mandatory + GoDoc precondition makes the invariant explicit and discoverable at the + guard's definition, which is stronger than implicit proximity. +- **R2 — Net test count decreases by 3.** Accepted trade-off: the removed + tests were duplicates of the same 7-line logic. The acceptance criterion + "net behavioral coverage does not decrease" is satisfied because the + three canonical cases remain, now tested once. Race tests + (`-race -count=1`) must still pass. +- **R3 — Mechanical churn across six files.** Low risk: the guard is pure + (no state, no side effects) and both callers use it identically + (`safeSSABuild(prog.Build)` → `ssaguard.SafeSSABuild(prog.Build)`). + Verified by building and running the full `-race -count=1` suite plus + `golangci-lint run` per the CI Parity Gate. diff --git a/openspec/changes/consolidate-ssa-guard/proposal.md b/openspec/changes/consolidate-ssa-guard/proposal.md new file mode 100644 index 00000000..124c0014 --- /dev/null +++ b/openspec/changes/consolidate-ssa-guard/proposal.md @@ -0,0 +1,152 @@ +## Why + +The `safeSSABuild` helper — a `recover()` guard wrapped around +`prog.Build()` — is byte-for-byte duplicated across two packages: + +- `internal/analysis/mutation.go` (called by `BuildSSA`) +- `internal/quality/pairing.go` (called by `BuildTestSSA`) + +The duplication extends beyond the 7-line function body into parallel +`export_test.go` shims and three near-identical test triads +(`TestSafeSSABuild_NoPanic` / `_PanicString` / `_PanicError`) in each +package. This is a direct violation of the **Zero-Waste Mandate** (no +duplicated code, no maintenance drift). + +The original decision to duplicate (spec 021, research decision R3) chose +to keep both packages "dependency-light." That rationale is now stale: +`safeSSABuild` takes a `func()` and returns `any` — it has **zero +external dependencies**. A shared leaf package introduces no transitive +coupling into either consumer. + +More importantly, `safeSSABuild` encodes a **safety-critical invariant**: +`recover()` is goroutine-scoped and only catches panics from +`prog.Build()` because the callers set `ssa.BuildSerially` (established +by spec 033, issue #33). This invariant currently lives implicitly via +caller proximity in two separate files. Spec 033 already demonstrated +the dual-maintenance hazard — both copies had to be corrected in lock +step. Centralizing the helper and documenting the `BuildSerially` +requirement in one authoritative place removes the drift risk and the +chance a future third SSA build site silently omits `BuildSerially` +(which would cause an unrecoverable panic → process crash). + +This change was triaged (issue #238, split from #201) and received a +unanimous VALID / enhancement verdict from all five Divisor reviewers. + +## What Changes + +- Create a new leaf package `internal/ssaguard/` exporting + `SafeSSABuild(buildFn func()) (panicVal any)`. +- Document the `ssa.BuildSerially` requirement in the exported function's + GoDoc comment as a mandatory caller precondition (documentation only — + not runtime validation, since the mode flags are set by callers before + `ssautil.AllPackages`, outside this function's scope). +- Update `internal/analysis/mutation.go` (`BuildSSA`) to call + `ssaguard.SafeSSABuild(prog.Build)`; remove the local `safeSSABuild`. +- Update `internal/quality/pairing.go` (`BuildTestSSA`) to call + `ssaguard.SafeSSABuild(prog.Build)`; remove the local `safeSSABuild` + and its duplication-lineage comment. +- Consolidate the three test cases into `internal/ssaguard/`; remove the + six duplicated tests from `mutation_test.go` and `pairing_test.go`. +- Remove the `SafeSSABuild` shim line from both + `internal/analysis/export_test.go` and `internal/quality/export_test.go` + (retaining all other shims in each file). + +The package is named `ssaguard` (not `ssautil`) deliberately, to avoid +shadowing `golang.org/x/tools/go/ssa/ssautil`, which is already imported +in both `mutation.go` and `pairing.go`. + +## Capabilities + +### New Capabilities +- `internal/ssaguard.SafeSSABuild`: Single-source-of-truth panic guard + for `ssa.Program.Build()`, with the `BuildSerially` invariant + documented in its GoDoc. + +### Modified Capabilities +- `internal/analysis.BuildSSA`: now delegates panic recovery to + `ssaguard.SafeSSABuild`; behavior unchanged. +- `internal/quality.BuildTestSSA`: now delegates panic recovery to + `ssaguard.SafeSSABuild`; behavior unchanged. + +### Removed Capabilities +- `internal/analysis.safeSSABuild` (unexported): removed; folded into + `internal/ssaguard`. +- `internal/quality.safeSSABuild` (unexported): removed; folded into + `internal/ssaguard`. + +## Impact + +- **Files changed**: `internal/analysis/mutation.go`, + `internal/analysis/mutation_test.go`, + `internal/analysis/export_test.go`, + `internal/quality/pairing.go`, `internal/quality/pairing_test.go`, + `internal/quality/export_test.go`. +- **Files added**: `internal/ssaguard/ssaguard.go`, + `internal/ssaguard/ssaguard_test.go`. +- **Behavior**: No behavioral change. Pure internal refactor. No exported + API surface (all symbols under `internal/`), no CLI change, no config + change, no release-pipeline change. +- **Tests**: Net test count decreases by 3 (six duplicates → three + shared), but behavioral coverage is neutral-to-positive — the same + logic is now tested once in its canonical home instead of twice. +- **Dependency graph**: adds two leaf edges + (`analysis → ssaguard`, `quality → ssaguard`); `ssaguard` itself + imports nothing beyond the standard library. + +## Constitution Alignment + +Assessed against the Gaze project constitution +(`.specify/memory/constitution.md`), the highest-authority document for +this repository. + +### I. Accuracy + +**Assessment**: N/A + +This is an internal implementation refactor with no behavioral change. +Side-effect detection, CRAP scoring, and every analysis result are +byte-for-byte identical before and after. No false positives or false +negatives are introduced or removed. + +### II. Minimal Assumptions + +**Assessment**: PASS + +No new assumptions about the host project's language, test framework, or +coding style are introduced. The new `internal/ssaguard` package is a +self-contained, zero-dependency leaf. Both consumers already depend on +`golang.org/x/tools/go/ssa`; the new internal edge is strictly lighter +than their existing dependency graph. + +### III. Actionable Output + +**Assessment**: N/A + +No change to Gaze's human-readable or machine-parseable output. The +change improves internal maintainability of a safety invariant by +documenting the `ssa.BuildSerially` precondition in a single +authoritative GoDoc comment, reducing the chance of silent panic-escape +regressions. + +### IV. Testability + +**Assessment**: PASS + +Testability improves. Exporting `SafeSSABuild` as a first-class function +eliminates the `export_test.go` shim indirection required to test the +unexported copies — the function becomes directly testable in isolation. +The consolidated test triad verifies the observable behavior (returned +panic value for no-panic, string-panic, and error-panic cases) rather +than implementation details. + +**Coverage strategy**: Unit tests only. The three canonical test cases +achieve 100% branch coverage of `SafeSSABuild` — the deferred `recover()` +path (the two panic scenarios) and the normal `return nil` path (the +no-panic scenario). No integration or e2e tests are required; the guard +is a pure recovery wrapper with no dependencies. The net test count +decreases by 3 (six duplicates removed, three canonical tests added), but +behavioral coverage is neutral-to-positive: the same logic is now tested +once in its canonical home instead of twice. Race tests +(`-race -count=1`) continue to pass. The measurable acceptance criterion +is: all three canonical test cases exist in +`internal/ssaguard/ssaguard_test.go` and pass under `-race -count=1`. diff --git a/openspec/changes/consolidate-ssa-guard/specs/ssaguard/spec.md b/openspec/changes/consolidate-ssa-guard/specs/ssaguard/spec.md new file mode 100644 index 00000000..157c358c --- /dev/null +++ b/openspec/changes/consolidate-ssa-guard/specs/ssaguard/spec.md @@ -0,0 +1,101 @@ +## ADDED Requirements + +### Requirement: Shared SSA panic guard + +The system MUST provide a single, shared panic-recovery guard for +`ssa.Program.Build()` in a dedicated leaf package `internal/ssaguard`. +The exported function `SafeSSABuild(buildFn func()) (panicVal any)` MUST +execute `buildFn` under a deferred `recover()` and MUST return the +recovered panic value, or `nil` when no panic occurred. The package MUST +import nothing beyond the Go standard library. + +#### Scenario: No panic returns nil + +- **GIVEN** a `buildFn` that completes without panicking +- **WHEN** `SafeSSABuild(buildFn)` is called +- **THEN** it returns `nil` + +#### Scenario: String panic is recovered + +- **GIVEN** a `buildFn` that calls `panic("boom")` +- **WHEN** `SafeSSABuild(buildFn)` is called +- **THEN** it returns the recovered value `"boom"` and does not + propagate the panic to the caller + +#### Scenario: Error panic is recovered + +- **GIVEN** a `buildFn` that calls `panic(err)` with an `error` value +- **WHEN** `SafeSSABuild(buildFn)` is called +- **THEN** it returns the recovered `error` value and does not propagate + the panic to the caller + +These three scenarios exercise all reachable branches of `SafeSSABuild`: +the deferred `recover()` capturing a non-nil panic value (scenarios 2-3) +and the normal `return nil` path when `recover()` returns nil (scenario +1). No additional branches exist, so the three cases constitute complete +branch coverage. + +### Requirement: BuildSerially invariant documentation + +The `SafeSSABuild` GoDoc comment MUST document that callers MUST build +the SSA program with the `ssa.BuildSerially` mode flag set. The +documentation MUST explain that `recover()` is goroutine-scoped and that +without `ssa.BuildSerially`, `prog.Build()` spawns child goroutines whose +panics escape the guard, causing an unrecoverable process crash. This is +a documentation-only precondition; `SafeSSABuild` MUST NOT attempt +runtime validation of the SSA build mode (the mode flags are set by +callers before `ssautil.AllPackages`, outside this function's scope). + +#### Scenario: Guard documents the serial-build precondition + +- **GIVEN** the exported `SafeSSABuild` function +- **WHEN** a developer reads its GoDoc comment +- **THEN** the comment states the `ssa.BuildSerially` requirement and the + goroutine-scoped `recover()` rationale + +### Requirement: Both SSA build sites use the shared guard + +Both `internal/analysis.BuildSSA` and `internal/quality.BuildTestSSA` +MUST call `internal/ssaguard.SafeSSABuild` for panic recovery around +`prog.Build()`. Neither package MUST retain a local copy of the guard +function. Both packages MUST continue to set the `ssa.BuildSerially` mode +flag when constructing the SSA program. + +Both callers MUST retain their existing `log.Warn` and `log.Debug` calls +at the panic-recovery site. Logging MUST NOT be moved into `SafeSSABuild` +because: (a) the guard receives only a `func()` and has no visibility +into the package being built, so moving the logs would lose the +package-path diagnostic context; and (b) adding a logger dependency would +violate the standard-library-only constraint on `internal/ssaguard`. The +package-path-annotated warning is the only runtime observability signal +for degraded SSA builds and MUST be preserved in the callers. + +#### Scenario: analysis package delegates to shared guard + +- **GIVEN** `internal/analysis.BuildSSA` constructing an SSA program with + `ssa.BuildSerially` set +- **WHEN** it builds the program +- **THEN** it invokes `ssaguard.SafeSSABuild(prog.Build)`, no local + `safeSSABuild` function exists in the package, and the existing + `log.Warn`/`log.Debug` recovery-site calls remain in `BuildSSA` + +#### Scenario: quality package delegates to shared guard + +- **GIVEN** `internal/quality.BuildTestSSA` constructing an SSA program + with `ssa.BuildSerially` set +- **WHEN** it builds the program +- **THEN** it invokes `ssaguard.SafeSSABuild(prog.Build)`, no local + `safeSSABuild` function exists in the package, and the existing + `log.Warn`/`log.Debug` recovery-site calls remain in `BuildTestSSA` + +## REMOVED Requirements + +### Requirement: Duplicated per-package SSA panic guard + +Removed. The previous design (spec 021, research decision R3) duplicated +the `safeSSABuild` guard in both `internal/analysis` and +`internal/quality` to keep each package dependency-light. This rationale +no longer holds: the guard has zero external dependencies, so a shared +leaf package introduces no coupling. The duplicated function definitions, +their `export_test.go` shims, and their duplicated test triads are +removed in favor of the shared `internal/ssaguard` package. diff --git a/openspec/changes/consolidate-ssa-guard/tasks.md b/openspec/changes/consolidate-ssa-guard/tasks.md new file mode 100644 index 00000000..a9777f08 --- /dev/null +++ b/openspec/changes/consolidate-ssa-guard/tasks.md @@ -0,0 +1,88 @@ + + +## 1. Create the shared ssaguard package + +- [x] 1.1 Create `internal/ssaguard/ssaguard.go` with a package doc + comment and the exported `SafeSSABuild(buildFn func()) (panicVal any)` + function. Body: `defer func() { panicVal = recover() }()`, call + `buildFn()`, return `nil`. Import nothing beyond the standard library + (D1, D3). +- [x] 1.2 Write the `SafeSSABuild` GoDoc comment documenting the mandatory + `ssa.BuildSerially` caller precondition and the goroutine-scoped + `recover()` rationale (D4; spec 033 invariant). State explicitly that + the guard does NOT validate the build mode at runtime. +- [x] 1.3 Create `internal/ssaguard/ssaguard_test.go` with the three + canonical test cases: `TestSafeSSABuild_NoPanic` (returns nil), + `TestSafeSSABuild_PanicString` (returns recovered string), + `TestSafeSSABuild_PanicError` (returns recovered error) (D5). Use the + standard library `testing` package only; assert with `t.Errorf` / + `t.Fatalf`. Coverage strategy: unit tests only; these three cases + achieve 100% branch coverage of `SafeSSABuild` (deferred `recover()` + path via the two panic cases, normal `return nil` path via the no-panic + case) — the complete set of reachable branches. + +## 2. Update callers to use the shared guard + +- [x] 2.1 [P] In `internal/analysis/mutation.go`: import + `github.com/unbound-force/gaze/internal/ssaguard`, replace the + `safeSSABuild(prog.Build)` call in `BuildSSA` with + `ssaguard.SafeSSABuild(prog.Build)`, and delete the local + `safeSSABuild` function and its doc comment. Keep the + `ssa.BuildSerially` mode flag in the `ssautil.AllPackages` call. Retain + the existing `log.Warn`/`log.Debug` recovery-site calls in `BuildSSA` — + do NOT move logging into `ssaguard.SafeSSABuild`. +- [x] 2.2 [P] In `internal/quality/pairing.go`: import + `github.com/unbound-force/gaze/internal/ssaguard`, replace the + `safeSSABuild(prog.Build)` call in `BuildTestSSA` with + `ssaguard.SafeSSABuild(prog.Build)`, and delete the local + `safeSSABuild` function, its doc comment, and its duplication-lineage + comment (the R3 reference). Keep the `ssa.BuildSerially` mode flag. + Retain the existing `log.Warn`/`log.Debug` recovery-site calls in + `BuildTestSSA` — do NOT move logging into `ssaguard.SafeSSABuild`. + +## 3. Remove duplicated tests and shims + +- [x] 3.1 [P] In `internal/analysis/mutation_test.go`: remove the three + duplicated `TestSafeSSABuild_*` tests. +- [x] 3.2 [P] In `internal/analysis/export_test.go`: remove the + `SafeSSABuild` shim line only; retain all other shims in the file. +- [x] 3.3 [P] In `internal/quality/pairing_test.go`: remove the three + duplicated `TestSafeSSABuild_*` tests. +- [x] 3.4 [P] In `internal/quality/export_test.go`: remove the + `SafeSSABuild` shim line only; retain all other shims in the file. + +## 4. Verify (CI Parity Gate) + +- [x] 4.1 `go build ./cmd/gaze` succeeds. +- [x] 4.2 `go test -race -count=1 -short ./...` passes (all packages, + including the new `internal/ssaguard`). +- [x] 4.3 `golangci-lint run` is clean (no new findings; confirm no + `ssautil` shadow and correct import grouping). +- [x] 4.4 Confirm no remaining `safeSSABuild` definition exists in + `internal/analysis` or `internal/quality` (e.g., `grep -rn + "func safeSSABuild"` returns nothing) and that both call sites now + reference `ssaguard.SafeSSABuild`. + +## 5. Constitution alignment check + +- [x] 5.1 Verify Composability (II): `internal/ssaguard` imports only the + standard library; no new external dependency introduced. +- [x] 5.2 Verify Testability (IV): `SafeSSABuild` is directly tested in + `internal/ssaguard` without `export_test.go` shims; behavioral coverage + of the three cases is preserved; race tests pass. + +## 6. Documentation validation gate + +- [x] 6.1 Assess documentation impact. No user-facing behavior, CLI, + flag, or output-format change — README and website issue are NOT + required (internal refactor). Add `ssaguard/` with a one-line + description to the AGENTS.md "Architecture" internal-package list (which + does enumerate every internal package) at an appropriate position. + + + +