diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..b237134 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,43 @@ +name: CI + +on: + push: + branches: [main] + pull_request: + branches: [main] + +permissions: + contents: read + +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-go@v5 + with: + go-version-file: go.mod + + - name: Build + run: go build ./... + + - name: Vet + run: go vet ./... + + - name: Test + run: go test -race -count=1 -coverprofile=coverage.out ./... + + lint: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-go@v5 + with: + go-version-file: go.mod + + - name: golangci-lint + uses: golangci/golangci-lint-action@v7 + with: + version: latest diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000..cb6309e --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,33 @@ +version: "2" + +run: + timeout: 5m + +formatters: + enable: + - gofmt + - goimports + settings: + goimports: + local-prefixes: + - github.com/zero-dot-force/vibe-check + +linters: + enable: + - errcheck + - govet + - revive + - staticcheck + - unused + - ineffassign + settings: + revive: + rules: + - name: exported + severity: warning + - name: error-return + severity: warning + - name: error-strings + severity: warning + - name: unexported-return + severity: warning diff --git a/.opencode/uf/packs/go-custom.md b/.opencode/uf/packs/go-custom.md index 927ab3c..852b6ea 100644 --- a/.opencode/uf/packs/go-custom.md +++ b/.opencode/uf/packs/go-custom.md @@ -17,4 +17,19 @@ Use the `CR-NNN` prefix for all custom rules. Use `[MUST]`, ## Custom Rules - +### CR-001 [SHOULD] Functional Options for Multi-Parameter Constructors + +Constructor functions with 3 or more optional parameters MAY use +the functional options pattern (`type Option func(*T)`) instead +of the AP-001 Options struct pattern when the options are +independent and self-documenting. The functional options pattern +is preferred for adapter configuration where sensible defaults +are provided and each option modifies a single field. + +**Rationale**: The `metrics.ExternalAdapter` has 5 configurable +timeouts/limits with sensible defaults. Functional options are +more ergonomic for this use case — callers specify only the +overrides they need without constructing a full Options struct. + +**Scope**: `metrics` package adapter configuration. New packages +should prefer AP-001 unless the same conditions apply. diff --git a/.uf/dewey/learnings/universal-coupling-model-20260829T005320-jay-flowers.md b/.uf/dewey/learnings/universal-coupling-model-20260829T005320-jay-flowers.md new file mode 100644 index 0000000..e31e85e --- /dev/null +++ b/.uf/dewey/learnings/universal-coupling-model-20260829T005320-jay-flowers.md @@ -0,0 +1,10 @@ +--- +tag: universal-coupling-model +author: jay-flowers +category: pattern +created_at: 2026-08-29T00:53:20Z +identity: universal-coupling-model-20260829T005320-jay-flowers +tier: draft +--- + +When implementing a universal coupling metrics model for Go codebases (vibe-check project), the two-layer architecture pattern (Layer 1 = universal model, Layer 2 = language adapters) proved effective. The key design decisions that worked well: (1) flat metrics package rather than internal/ because the model IS the public API, (2) named metric types like Instability float64 with GoDoc documenting ranges and citations, (3) Module as the universal unit of analysis with language-neutral terminology, (4) JSON-RPC 2.0 over stdin/stdout for external adapters with newline-delimited framing. The ExternalAdapter's functional options pattern (WithAnalyzeTimeout, WithMaxResponseSize, etc.) provided clean configuration without breaking constructor signatures, though it deviates from the AP-001 Options struct convention — this deviation should be documented as a custom rule. diff --git a/.uf/dewey/learnings/universal-coupling-model-20260829T005332-jay-flowers.md b/.uf/dewey/learnings/universal-coupling-model-20260829T005332-jay-flowers.md new file mode 100644 index 0000000..0e758bd --- /dev/null +++ b/.uf/dewey/learnings/universal-coupling-model-20260829T005332-jay-flowers.md @@ -0,0 +1,10 @@ +--- +tag: universal-coupling-model +author: jay-flowers +category: gotcha +created_at: 2026-08-29T00:53:32Z +identity: universal-coupling-model-20260829T005332-jay-flowers +tier: draft +--- + +When testing Go subprocess communication (ExternalAdapter in vibe-check), the TestHelperProcess pattern (os.Args[0] with GO_WANT_HELPER_PROCESS env var) is the standard library-compatible approach that avoids external test binaries. Key gotchas: (1) the test function parameter should use _ *testing.T since it's not a real test, (2) tests using t.Setenv cannot use t.Parallel — Go 1.17+ enforces this, (3) limitedBuffer for stderr capture must always report the full write length to avoid breaking the subprocess pipe even on internal errors, (4) Gaze quality analysis revealed that constructor functions (NewExternalAdapter, NewRegistry) often have 0% contract coverage because tests call them but never directly assert on the return value — adding nil checks and default value assertions closes this gap easily. diff --git a/.uf/dewey/learnings/universal-coupling-model-20260829T005336-jay-flowers.md b/.uf/dewey/learnings/universal-coupling-model-20260829T005336-jay-flowers.md new file mode 100644 index 0000000..be47b69 --- /dev/null +++ b/.uf/dewey/learnings/universal-coupling-model-20260829T005336-jay-flowers.md @@ -0,0 +1,10 @@ +--- +tag: universal-coupling-model +author: jay-flowers +category: gotcha +created_at: 2026-08-29T00:53:36Z +identity: universal-coupling-model-20260829T005336-jay-flowers +tier: draft +--- + +For the vibe-check project's golangci-lint configuration, version 2 of golangci-lint treats gofmt and goimports as formatters not linters — they must go under formatters.enable, not linters.enable. Also, gosimple was merged into staticcheck in v2, so listing it separately causes an error. The .golangci.yml needs version: "2" at the top level. When implementing a hand-rolled JSON schema validator (metrics/validate.go) to avoid external dependencies, keep complexity low by extracting helper functions (validateModule, validateWarning) — Gaze flagged the main Validate function at complexity 16 which is the only Q4 Dangerous function in the codebase. diff --git a/.uf/dewey/learnings/universal-coupling-model-20260829T005344-jay-flowers.md b/.uf/dewey/learnings/universal-coupling-model-20260829T005344-jay-flowers.md new file mode 100644 index 0000000..c192131 --- /dev/null +++ b/.uf/dewey/learnings/universal-coupling-model-20260829T005344-jay-flowers.md @@ -0,0 +1,10 @@ +--- +tag: universal-coupling-model +author: jay-flowers +category: pattern +created_at: 2026-08-29T00:53:44Z +identity: universal-coupling-model-20260829T005344-jay-flowers +tier: draft +--- + +The review council consistently flagged zone classification threshold contradictions across spec artifacts — the design doc said thresholds are "configuration, not model" while the metrics-schema spec hardcoded them. The resolution was to accept thresholds as part of the model and update the design non-goal. This is a common spec review pattern: when a non-goal contradicts a spec requirement, the non-goal usually needs refinement rather than the spec. Another recurring theme was LCOM variant ambiguity — the constitution requires citing the measurement model, and LCOM4 (Hitz and Montazeri, 1995) was chosen for its connected-component semantics. Always specify WHICH variant of a well-known metric you're implementing. diff --git a/AGENTS.md b/AGENTS.md index 8d9d62e..3d965ec 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -234,8 +234,6 @@ hotfixes (retroactively documented). ## Build & Test Commands - - ```bash # Build go build ./... @@ -256,17 +254,29 @@ golangci-lint run ./... .opencode/ # OpenCode agent configuration, skills, packs .specify/ # Constitution and governance memory .uf/ # Unbound Force tooling configuration +metrics/ # Universal coupling metrics model (Layer 1) + adapter.go # Adapter interface and Capability type + compute.go # Metric computation functions + cycle.go # Cycle type for circular dependency representation + doc.go # Package-level GoDoc + external.go # ExternalAdapter (JSON-RPC subprocess) + graph.go # ModuleGraph and ModuleResult types + jsonrpc.go # JSON-RPC 2.0 protocol types + module.go # Module type (universal unit of analysis) + modulegraph.schema.json # JSON Schema for ModuleGraph validation + registry.go # Adapter registry (dependency-injected) + schema.go # Embedded JSON schema access + security.go # Path validation and environment sanitization + validate.go # JSON schema validation + values.go # Named metric types (Instability, Abstractness, etc.) + warning.go # Warning type for analysis caveats + zone.go # Zone and Status types openspec/ # OpenSpec change artifacts (proposals, specs, tasks) changes/ # Individual change directories schemas/ # Spec validation schemas specs/ # Spec templates ``` -> **Pre-implementation**: No Go source directories exist yet. -> Package layout will be established during the first spec -> workflow. Update this section when `go.mod` and source -> packages are created. - ## Architecture The planned architecture follows the RFC phasing: @@ -280,7 +290,8 @@ The planned architecture follows the RFC phasing: architectural drift tracking - **P3**: TS/JS adapter, SBOM integration, mutation testing hooks -Package layout will be established during the first spec workflow. +The `metrics` package implements the P0 universal model. Language +adapters (P1+) will be added as separate packages. ## Coding Conventions diff --git a/go.mod b/go.mod new file mode 100644 index 0000000..8e6a947 --- /dev/null +++ b/go.mod @@ -0,0 +1,3 @@ +module github.com/zero-dot-force/vibe-check + +go 1.25.7 diff --git a/metrics/adapter.go b/metrics/adapter.go new file mode 100644 index 0000000..b3f23fb --- /dev/null +++ b/metrics/adapter.go @@ -0,0 +1,37 @@ +package metrics + +import "context" + +// Adapter is the interface that all language-specific analyzers must implement. +// Adding a new language adapter requires only implementing this interface and +// registering with a [Registry]. +type Adapter interface { + // Analyze analyzes the project at projectPath and returns a complete ModuleGraph. + Analyze(ctx context.Context, projectPath string) (*ModuleGraph, error) + // Language returns the lowercase language identifier (e.g., "go", "python"). + Language() string + // Capabilities returns the list of metrics this adapter can compute. + // A nil or empty return may indicate either zero capabilities or a + // communication failure. Use Analyze for detailed error diagnostics. + Capabilities() []Capability +} + +// Capability represents a metric that an adapter can compute. +type Capability string + +const ( + // CapAfferentCoupling indicates the adapter can compute afferent coupling (Ca). + CapAfferentCoupling Capability = "ca" + // CapEfferentCoupling indicates the adapter can compute efferent coupling (Ce). + CapEfferentCoupling Capability = "ce" + // CapInstability indicates the adapter can compute instability (I). + CapInstability Capability = "instability" + // CapAbstractness indicates the adapter can compute abstractness (A). + CapAbstractness Capability = "abstractness" + // CapDistance indicates the adapter can compute distance from main sequence (D). + CapDistance Capability = "distance" + // CapLCOM indicates the adapter can compute Lack of Cohesion of Methods (LCOM4). + CapLCOM Capability = "lcom" + // CapCircularDeps indicates the adapter can detect circular dependencies. + CapCircularDeps Capability = "circular" +) diff --git a/metrics/compute.go b/metrics/compute.go new file mode 100644 index 0000000..53ab484 --- /dev/null +++ b/metrics/compute.go @@ -0,0 +1,52 @@ +package metrics + +import "math" + +// ComputeInstability computes I = Ce / (Ca + Ce). +// When both Ca and Ce are 0, returns 0.0 (maximally stable by convention). +// This follows the convention that an isolated module with no coupling +// relationships is treated as stable rather than unstable. +func ComputeInstability(ca, ce int) Instability { + denom := ca + ce + if denom == 0 { + return 0.0 + } + return Instability(float64(ce) / float64(denom)) +} + +// ComputeAbstractness computes A = abstractTypes / totalExported. +// When totalExported is 0, returns 0.0 (a module with no exported types +// is treated as fully concrete). +func ComputeAbstractness(abstractTypes, totalExported int) Abstractness { + if totalExported == 0 { + return 0.0 + } + return Abstractness(float64(abstractTypes) / float64(totalExported)) +} + +// ComputeDistance computes D = |A + I - 1|. +// The result measures how far a module is from the main sequence, where +// the main sequence represents the ideal balance between abstractness +// and instability. +func ComputeDistance(a Abstractness, i Instability) Distance { + return Distance(math.Abs(float64(a) + float64(i) - 1.0)) +} + +// ComputeZone classifies a module's position relative to the main sequence. +// Precedence (evaluated in order): +// 1. main-sequence: D < 0.2 +// 2. zone-of-pain: A < 0.2 AND I < 0.2 +// 3. zone-of-uselessness: A > 0.8 AND I > 0.8 +// 4. normal: all other cases +func ComputeZone(a Abstractness, i Instability, d Distance) Zone { + if d < 0.2 { + return ZoneMainSequence + } + if a < 0.2 && i < 0.2 { + return ZoneOfPain + } + if a > 0.8 && i > 0.8 { + return ZoneOfUselessness + } + return ZoneNormal +} diff --git a/metrics/compute_test.go b/metrics/compute_test.go new file mode 100644 index 0000000..957ea93 --- /dev/null +++ b/metrics/compute_test.go @@ -0,0 +1,181 @@ +package metrics + +import ( + "math" + "testing" +) + +const epsilon = 1e-10 + +// assertFloatEq compares two float64 values within an epsilon tolerance +// and reports a test failure with a descriptive message if they differ. +func assertFloatEq(t *testing.T, got, want float64) { + t.Helper() + if math.Abs(got-want) >= epsilon { + t.Errorf("got %v, want %v (within epsilon %v)", got, want, epsilon) + } +} + +func TestComputeInstability_HappyPath(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + ca int + ce int + want float64 + }{ + {name: "mixed coupling Ca=3 Ce=7", ca: 3, ce: 7, want: 0.7}, + {name: "equal coupling Ca=5 Ce=5", ca: 5, ce: 5, want: 0.5}, + {name: "Ca=1 Ce=3", ca: 1, ce: 3, want: 0.75}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got := ComputeInstability(tt.ca, tt.ce) + assertFloatEq(t, float64(got), tt.want) + }) + } +} + +func TestComputeInstability_ZeroDenominator(t *testing.T) { + t.Parallel() + + got := ComputeInstability(0, 0) + assertFloatEq(t, float64(got), 0.0) +} + +func TestComputeInstability_MaximallyStable(t *testing.T) { + t.Parallel() + + got := ComputeInstability(5, 0) + assertFloatEq(t, float64(got), 0.0) +} + +func TestComputeInstability_MaximallyUnstable(t *testing.T) { + t.Parallel() + + got := ComputeInstability(0, 5) + assertFloatEq(t, float64(got), 1.0) +} + +func TestComputeInstability_Determinism(t *testing.T) { + t.Parallel() + + // Same inputs must produce identical outputs across multiple calls. + // This verifies the constitutional requirement (Principle VI: Metric Fidelity) + // that metric computations are deterministic. + const iterations = 100 + first := ComputeInstability(3, 7) + for i := 0; i < iterations; i++ { + got := ComputeInstability(3, 7) + if got != first { + t.Fatalf("ComputeInstability(3, 7) produced non-deterministic result on iteration %d: got %v, want %v", i, got, first) + } + } +} + +func TestComputeAbstractness_FullyAbstract(t *testing.T) { + t.Parallel() + + got := ComputeAbstractness(3, 3) + assertFloatEq(t, float64(got), 1.0) +} + +func TestComputeAbstractness_FullyConcrete(t *testing.T) { + t.Parallel() + + got := ComputeAbstractness(0, 5) + assertFloatEq(t, float64(got), 0.0) +} + +func TestComputeAbstractness_Mixed(t *testing.T) { + t.Parallel() + + got := ComputeAbstractness(2, 5) + assertFloatEq(t, float64(got), 0.4) +} + +func TestComputeAbstractness_NoExports(t *testing.T) { + t.Parallel() + + got := ComputeAbstractness(0, 0) + assertFloatEq(t, float64(got), 0.0) +} + +func TestComputeAbstractness_Determinism(t *testing.T) { + t.Parallel() + + const iterations = 100 + first := ComputeAbstractness(2, 5) + for i := 0; i < iterations; i++ { + got := ComputeAbstractness(2, 5) + if got != first { + t.Fatalf("ComputeAbstractness(2, 5) produced non-deterministic result on iteration %d: got %v, want %v", i, got, first) + } + } +} + +func TestComputeDistance_OnMainSequence(t *testing.T) { + t.Parallel() + + got := ComputeDistance(0.5, 0.5) + assertFloatEq(t, float64(got), 0.0) +} + +func TestComputeDistance_ZoneOfPain(t *testing.T) { + t.Parallel() + + // Zone of pain: concrete (A=0.0) and stable (I=0.0). + // D = |0.0 + 0.0 - 1| = 1.0 + got := ComputeDistance(0.0, 0.0) + assertFloatEq(t, float64(got), 1.0) +} + +func TestComputeDistance_ZoneOfUselessness(t *testing.T) { + t.Parallel() + + // Zone of uselessness: abstract (A=1.0) and unstable (I=1.0). + // D = |1.0 + 1.0 - 1| = 1.0 + got := ComputeDistance(1.0, 1.0) + assertFloatEq(t, float64(got), 1.0) +} + +func TestComputeDistance_Determinism(t *testing.T) { + t.Parallel() + + const iterations = 100 + first := ComputeDistance(0.3, 0.4) + for i := 0; i < iterations; i++ { + got := ComputeDistance(0.3, 0.4) + if got != first { + t.Fatalf("ComputeDistance(0.3, 0.4) produced non-deterministic result on iteration %d: got %v, want %v", i, got, first) + } + } +} + +func TestComputeAbstractness_AbstractExceedsTotalExported(t *testing.T) { + t.Parallel() + + // When abstractTypes exceeds totalExported, the function trusts adapter + // input and may produce values > 1.0. This documents the behavior for + // invalid inputs — the adapter is responsible for providing valid data. + got := ComputeAbstractness(5, 3) + if float64(got) <= 1.0 { + t.Errorf("ComputeAbstractness(5, 3) = %v, expected > 1.0 for invalid input", got) + } +} + +func TestComputeInstability_NegativeInputs(t *testing.T) { + t.Parallel() + + // Negative coupling counts are semantically invalid. The function does + // not guard against them — callers are responsible for providing + // non-negative values. This test documents the unguarded behavior. + got := ComputeInstability(-1, 3) + if float64(got) < 0.0 || float64(got) > 1.0 { + // Document that negative inputs produce out-of-range values. + t.Logf("ComputeInstability(-1, 3) = %v (out of [0.0, 1.0] range for invalid input)", got) + } +} diff --git a/metrics/cycle.go b/metrics/cycle.go new file mode 100644 index 0000000..d39b660 --- /dev/null +++ b/metrics/cycle.go @@ -0,0 +1,13 @@ +package metrics + +// Cycle represents a circular dependency between modules as an ordered list +// of module paths. The cycle starts from the lexicographically smallest module +// path and does not repeat the start node. +// +// Example: if modules A→B→C→A form a cycle, it is represented as ["A", "B", "C"]. +// +// Canonical ordering: when a cycle is detected (e.g., C→A→B→C), it is rotated +// so that the lexicographically smallest module path appears first, yielding +// ["A", "B", "C"]. Multiple cycles in a result set are sorted lexicographically +// by their first element. +type Cycle []string diff --git a/metrics/cycle_test.go b/metrics/cycle_test.go new file mode 100644 index 0000000..b99db68 --- /dev/null +++ b/metrics/cycle_test.go @@ -0,0 +1,80 @@ +package metrics + +import "testing" + +func TestCycle_Construction(t *testing.T) { + t.Parallel() + + // Cycle is a named type over []string. Verify basic construction + // and element access work as expected. + cycle := Cycle{"A", "B", "C"} + + if len(cycle) != 3 { + t.Fatalf("len(cycle) = %d, want 3", len(cycle)) + } + if cycle[0] != "A" { + t.Errorf("cycle[0] = %q, want %q", cycle[0], "A") + } + if cycle[1] != "B" { + t.Errorf("cycle[1] = %q, want %q", cycle[1], "B") + } + if cycle[2] != "C" { + t.Errorf("cycle[2] = %q, want %q", cycle[2], "C") + } +} + +func TestCycle_CanonicalOrdering(t *testing.T) { + t.Parallel() + + // Per the spec, a cycle C→A→B→C is represented starting from the + // lexicographically smallest path: ["A", "B", "C"]. + // This test verifies that a correctly constructed Cycle follows + // the canonical ordering convention (smallest path first, no + // repeated start node). + // + // Note: Canonical ordering enforcement (rotation of detected cycles) + // is the responsibility of the cycle detection algorithm in language + // adapters, not the Cycle type itself. This test validates the + // convention on a pre-constructed cycle. + cycle := Cycle{"A", "B", "C"} + + // The first element must be the lexicographically smallest. + for i := 1; i < len(cycle); i++ { + if cycle[i] < cycle[0] { + t.Errorf("cycle[%d] = %q is lexicographically smaller than cycle[0] = %q; "+ + "canonical ordering requires the smallest path first", i, cycle[i], cycle[0]) + } + } + + // The start node must not be repeated at the end. + if len(cycle) > 1 && cycle[len(cycle)-1] == cycle[0] { + t.Errorf("cycle repeats start node %q at the end; canonical representation omits it", cycle[0]) + } +} + +func TestCycle_EmptyCycle(t *testing.T) { + t.Parallel() + + // An empty Cycle is valid — it represents "no cycle". + var cycle Cycle + if len(cycle) != 0 { + t.Errorf("len(empty cycle) = %d, want 0", len(cycle)) + } +} + +func TestCycle_TwoNodeCycle(t *testing.T) { + t.Parallel() + + // A two-node cycle A→B→A is represented as ["A", "B"]. + cycle := Cycle{"A", "B"} + + if len(cycle) != 2 { + t.Fatalf("len(cycle) = %d, want 2", len(cycle)) + } + if cycle[0] != "A" { + t.Errorf("cycle[0] = %q, want %q", cycle[0], "A") + } + if cycle[1] != "B" { + t.Errorf("cycle[1] = %q, want %q", cycle[1], "B") + } +} diff --git a/metrics/doc.go b/metrics/doc.go new file mode 100644 index 0000000..1ddffdf --- /dev/null +++ b/metrics/doc.go @@ -0,0 +1,58 @@ +// Package metrics defines the universal coupling and cohesion metrics model +// for the Vibe-Check toolkit. +// +// # Architecture +// +// The package follows a two-layer architecture: +// +// - Layer 1 (this package): A language-agnostic metrics model defining types +// for afferent coupling (Ca), efferent coupling (Ce), instability (I), +// abstractness (A), distance from main sequence (D), cohesion (LCOM4), +// and circular dependency detection. +// +// - Layer 2 (language adapters): Language-specific analyzers that implement +// the [Adapter] interface and populate the universal model with data from +// Go, Python, TypeScript, or other codebases. +// +// # Core Types +// +// [Module] represents the universal unit of analysis (a package in Go, a module +// in Python, a file/module in TypeScript). [ModuleResult] embeds Module and adds +// computed metrics. [ModuleGraph] contains the complete analysis result for a +// project, including all modules, detected cycles, warnings, and status. +// +// # Metrics +// +// Each metric is represented as a named type with documented value ranges: +// +// - [Instability]: I = Ce / (Ca + Ce), range [0.0, 1.0] +// - [Abstractness]: A = abstractTypes / exportedTypes, range [0.0, 1.0] +// - [Distance]: D = |A + I - 1|, range [0.0, 1.0] +// - [LCOM]: LCOM4 variant (Hitz & Montazeri, 1995), non-negative integer +// +// Metric computation functions ([ComputeInstability], [ComputeAbstractness], +// [ComputeDistance], [ComputeZone]) produce deterministic results for the same +// inputs. +// +// # Adapters +// +// Language-specific analyzers implement the [Adapter] interface and register +// with a [Registry]. The registry uses dependency injection (no global state). +// Adding a new language adapter requires only implementing [Adapter] and +// calling [Registry.Register] — no changes to this package are needed. +// +// External (out-of-process) adapters communicate via JSON-RPC 2.0 over +// stdin/stdout using the [ExternalAdapter] type. The protocol uses +// newline-delimited JSON framing. +// +// # JSON Schema +// +// The [ModuleGraph] type is serializable to JSON using the schema defined in +// modulegraph.schema.json (accessible via [SchemaJSON]). The [Validate] +// function checks JSON data against this schema. The schema includes a +// schemaVersion field for forward compatibility. +// +// Citations: +// - Robert C. Martin, "Agile Software Development" (2003) — Ca, Ce, I, A, D +// - Hitz & Montazeri, "Measuring Coupling and Cohesion in Object-Oriented Systems" (1995) — LCOM4 +package metrics diff --git a/metrics/external.go b/metrics/external.go new file mode 100644 index 0000000..15690ef --- /dev/null +++ b/metrics/external.go @@ -0,0 +1,420 @@ +package metrics + +import ( + "bufio" + "bytes" + "context" + "encoding/json" + "fmt" + "io" + "os/exec" + "sync" + "time" +) + +// Default limits for ExternalAdapter timeouts and buffer sizes. +const ( + defaultAnalyzeTimeout = 300 * time.Second + defaultCapabilitiesTimeout = 10 * time.Second + defaultShutdownTimeout = 5 * time.Second + defaultMaxResponseSize = 100 * 1024 * 1024 // 100 MB + defaultMaxStderrSize = 1 * 1024 * 1024 // 1 MB +) + +// ExternalAdapter wraps an external analyzer subprocess that communicates via +// JSON-RPC 2.0 over stdin/stdout. Each JSON-RPC message is framed as a single +// line of JSON terminated by a newline character. +// +// The adapter spawns a new subprocess for each Analyze call, sends a +// capabilities request followed by an analyze request, then shuts down the +// subprocess gracefully. If the subprocess does not exit within the shutdown +// timeout, it is killed with SIGKILL. +type ExternalAdapter struct { + binaryPath string + language string + env []string // sanitized environment + + // Configurable limits. + analyzeTimeout time.Duration + capabilitiesTimeout time.Duration + shutdownTimeout time.Duration + maxResponseSize int64 // bytes + maxStderrSize int64 // bytes +} + +// ExternalAdapterOption configures an ExternalAdapter. +type ExternalAdapterOption func(*ExternalAdapter) + +// WithAnalyzeTimeout sets the timeout for the analyze request. +func WithAnalyzeTimeout(d time.Duration) ExternalAdapterOption { + return func(a *ExternalAdapter) { + a.analyzeTimeout = d + } +} + +// WithCapabilitiesTimeout sets the timeout for the capabilities request. +func WithCapabilitiesTimeout(d time.Duration) ExternalAdapterOption { + return func(a *ExternalAdapter) { + a.capabilitiesTimeout = d + } +} + +// WithShutdownTimeout sets the grace period for subprocess shutdown before SIGKILL. +func WithShutdownTimeout(d time.Duration) ExternalAdapterOption { + return func(a *ExternalAdapter) { + a.shutdownTimeout = d + } +} + +// WithMaxResponseSize sets the maximum response size in bytes. +func WithMaxResponseSize(n int64) ExternalAdapterOption { + return func(a *ExternalAdapter) { + a.maxResponseSize = n + } +} + +// WithMaxStderrSize sets the maximum stderr capture size in bytes. +func WithMaxStderrSize(n int64) ExternalAdapterOption { + return func(a *ExternalAdapter) { + a.maxStderrSize = n + } +} + +// WithEnvironment sets additional environment variables to include beyond the +// default sanitized set (PATH, HOME, LANG). Credential-bearing variables are +// still excluded. +func WithEnvironment(allowlist []string) ExternalAdapterOption { + return func(a *ExternalAdapter) { + a.env = SanitizeEnvironment(allowlist) + } +} + +// NewExternalAdapter creates a new ExternalAdapter for the given binary path +// and language identifier. The binary path must refer to an executable that +// implements the vibe-check external analyzer JSON-RPC protocol. +func NewExternalAdapter(binaryPath, language string, opts ...ExternalAdapterOption) *ExternalAdapter { + a := &ExternalAdapter{ + binaryPath: binaryPath, + language: language, + env: SanitizeEnvironment(nil), + analyzeTimeout: defaultAnalyzeTimeout, + capabilitiesTimeout: defaultCapabilitiesTimeout, + shutdownTimeout: defaultShutdownTimeout, + maxResponseSize: defaultMaxResponseSize, + maxStderrSize: defaultMaxStderrSize, + } + for _, opt := range opts { + opt(a) + } + return a +} + +// Language returns the lowercase language identifier for this adapter. +func (a *ExternalAdapter) Language() string { + return a.language +} + +// Capabilities returns the list of metrics this adapter can compute. +// It spawns the subprocess, sends a capabilities request, and returns the +// result. The subprocess is shut down after the call. +// +// A nil or empty return value may indicate either that the adapter supports +// no capabilities or that a communication error occurred (spawn failure, +// protocol error, etc.). Use [ExternalAdapter.Analyze] for detailed error +// diagnostics when capabilities discovery fails. +func (a *ExternalAdapter) Capabilities() []Capability { + // Capabilities is defined without error return in the Adapter interface. + // On failure, return an empty slice — callers can use Analyze to get + // detailed error information. + ctx, cancel := context.WithTimeout(context.Background(), a.capabilitiesTimeout) + defer cancel() + + proc, err := a.spawnProcess(ctx) + if err != nil { + return nil + } + defer proc.shutdown(a.shutdownTimeout) + + id := 1 + req := JSONRPCRequest{ + JSONRPC: JSONRPCVersion, + Method: "capabilities", + ID: &id, + } + + resp, err := proc.call(req, a.maxResponseSize) + if err != nil { + return nil + } + + if resp.Error != nil { + return nil + } + + var result CapabilitiesResult + if err := json.Unmarshal(resp.Result, &result); err != nil { + return nil + } + + caps := make([]Capability, len(result.Metrics)) + for i, m := range result.Metrics { + caps[i] = Capability(m) + } + return caps +} + +// Analyze spawns the external analyzer subprocess, sends the capabilities and +// analyze requests, validates the response, and returns the resulting ModuleGraph. +func (a *ExternalAdapter) Analyze(ctx context.Context, projectPath string) (*ModuleGraph, error) { + if err := ValidateProjectPath(projectPath); err != nil { + return nil, fmt.Errorf("external analyze: %w", err) + } + + // Use the analyze timeout as the subprocess deadline, but respect the + // caller's context if it has an earlier deadline. + analyzeCtx, analyzeCancel := context.WithTimeout(ctx, a.analyzeTimeout) + defer analyzeCancel() + + proc, err := a.spawnProcess(analyzeCtx) + if err != nil { + return nil, fmt.Errorf("external analyze: spawn subprocess: %w", err) + } + defer proc.shutdown(a.shutdownTimeout) + + // Step 1: Send capabilities request. + capID := 1 + capReq := JSONRPCRequest{ + JSONRPC: JSONRPCVersion, + Method: "capabilities", + ID: &capID, + } + + capResp, err := proc.call(capReq, a.maxResponseSize) + if err != nil { + return nil, fmt.Errorf("external analyze: capabilities request: %w", err) + } + if capResp.Error != nil { + return nil, fmt.Errorf("external analyze: capabilities error: %s", capResp.Error.Message) + } + + // Step 2: Send analyze request. + analyzeID := 2 + analyzeReq := JSONRPCRequest{ + JSONRPC: JSONRPCVersion, + Method: "analyze", + Params: AnalyzeParams{ProjectPath: projectPath}, + ID: &analyzeID, + } + + analyzeResp, err := proc.call(analyzeReq, a.maxResponseSize) + if err != nil { + return nil, fmt.Errorf("external analyze: analyze request: %w", err) + } + if analyzeResp.Error != nil { + return nil, fmt.Errorf("external analyze: analyzer error: %s", analyzeResp.Error.Message) + } + + // Step 3: Validate response against schema. + if err := Validate(analyzeResp.Result); err != nil { + return nil, fmt.Errorf("external analyze: response validation: %w", err) + } + + // Step 4: Unmarshal into ModuleGraph. + var graph ModuleGraph + if err := json.Unmarshal(analyzeResp.Result, &graph); err != nil { + return nil, fmt.Errorf("external analyze: unmarshal response: %w", err) + } + + return &graph, nil +} + +// process represents a running analyzer subprocess with stdin/stdout pipes. +type process struct { + cmd *exec.Cmd + stdin io.WriteCloser + reader *bufio.Reader + stderr *limitedBuffer +} + +// spawnProcess starts the external analyzer binary as a subprocess with +// sanitized environment and connected stdin/stdout/stderr pipes. +func (a *ExternalAdapter) spawnProcess(ctx context.Context) (*process, error) { + cmd := exec.CommandContext(ctx, a.binaryPath) + cmd.Env = a.env + + stdinPipe, err := cmd.StdinPipe() + if err != nil { + return nil, fmt.Errorf("create stdin pipe: %w", err) + } + + stdoutPipe, err := cmd.StdoutPipe() + if err != nil { + return nil, fmt.Errorf("create stdout pipe: %w", err) + } + + stderrBuf := newLimitedBuffer(a.maxStderrSize) + cmd.Stderr = stderrBuf + + if err := cmd.Start(); err != nil { + return nil, fmt.Errorf("start subprocess: %w", err) + } + + return &process{ + cmd: cmd, + stdin: stdinPipe, + reader: bufio.NewReaderSize(stdoutPipe, 64*1024), + stderr: stderrBuf, + }, nil +} + +// call sends a JSON-RPC request and reads the newline-delimited response. +// The response size is limited to maxSize bytes. +func (p *process) call(req JSONRPCRequest, maxSize int64) (*JSONRPCResponse, error) { + // Marshal and send request with newline framing. + reqData, err := json.Marshal(req) + if err != nil { + return nil, fmt.Errorf("marshal request: %w", err) + } + reqData = append(reqData, '\n') + + if _, err := p.stdin.Write(reqData); err != nil { + return nil, fmt.Errorf("write request: %w", err) + } + + // Read one line of response, enforcing size limit. + // Use a LimitedReader wrapper around the buffered reader to prevent + // unbounded memory allocation from a malicious subprocess. + limited := io.LimitReader(p.reader, maxSize+1) // +1 to detect overflow + scanner := bufio.NewScanner(limited) + scanner.Buffer(make([]byte, 64*1024), int(maxSize)+1) + + if !scanner.Scan() { + if err := scanner.Err(); err != nil { + return nil, fmt.Errorf("read response: %w", err) + } + // Check if the process exited. + stderrMsg := p.stderr.String() + if stderrMsg != "" { + return nil, fmt.Errorf("read response: subprocess closed stdout (stderr: %s)", stderrMsg) + } + return nil, fmt.Errorf("read response: subprocess closed stdout without sending a response") + } + + line := scanner.Bytes() + if int64(len(line)) > maxSize { + return nil, fmt.Errorf("read response: response exceeds maximum size of %d bytes", maxSize) + } + + var resp JSONRPCResponse + if err := json.Unmarshal(line, &resp); err != nil { + return nil, fmt.Errorf("unmarshal response: %w", err) + } + + return &resp, nil +} + +// shutdown sends a shutdown notification to the subprocess and waits for it to +// exit gracefully. If the subprocess does not exit within the grace period, it +// is killed with SIGKILL. +func (p *process) shutdown(gracePeriod time.Duration) { + // Send shutdown notification (no ID = notification, no response expected). + shutdownReq := JSONRPCRequest{ + JSONRPC: JSONRPCVersion, + Method: "shutdown", + } + reqData, err := json.Marshal(shutdownReq) + if err == nil { + reqData = append(reqData, '\n') + // Best-effort write — the process may already be dead. + _, _ = p.stdin.Write(reqData) + } + _ = p.stdin.Close() + + // Wait for graceful exit with a timeout. + done := make(chan error, 1) + go func() { + done <- p.cmd.Wait() + }() + + select { + case <-done: + // Process exited gracefully. + case <-time.After(gracePeriod): + // Grace period expired — force kill. + if p.cmd.Process != nil { + _ = p.cmd.Process.Kill() + <-done // Wait for the kill to complete. + } + } +} + +// limitedBuffer is a bytes.Buffer that stops accepting writes after reaching +// a maximum size. It is used to capture subprocess stderr without unbounded +// memory growth. +type limitedBuffer struct { + mu sync.Mutex + buf bytes.Buffer + maxSize int64 + writeErr bool // set when an internal buffer write fails (e.g., OOM) +} + +// newLimitedBuffer creates a limitedBuffer with the given maximum size. +func newLimitedBuffer(maxSize int64) *limitedBuffer { + return &limitedBuffer{maxSize: maxSize} +} + +// Write implements io.Writer. Writes that would exceed the maximum size are +// silently truncated. The full input length is always reported as written to +// avoid breaking the subprocess's stderr pipe. +func (lb *limitedBuffer) Write(p []byte) (int, error) { + lb.mu.Lock() + defer lb.mu.Unlock() + + originalLen := len(p) + + remaining := lb.maxSize - int64(lb.buf.Len()) + if remaining <= 0 { + // Buffer is full — discard silently. + return originalLen, nil + } + + if int64(len(p)) > remaining { + p = p[:remaining] + } + + if _, err := lb.buf.Write(p); err != nil { + // Return the original length even on error to maintain the contract + // of never breaking the subprocess stderr pipe. Record the failure + // for observability in the String() output. + lb.writeErr = true + return originalLen, nil + } + return originalLen, nil +} + +// String returns the captured stderr content. If the buffer was truncated, +// a notice is appended. +func (lb *limitedBuffer) String() string { + lb.mu.Lock() + defer lb.mu.Unlock() + + s := lb.buf.String() + if int64(lb.buf.Len()) >= lb.maxSize { + s += "\n[stderr truncated at " + fmt.Sprintf("%d", lb.maxSize) + " bytes]" + } + if lb.writeErr { + s += "\n[stderr capture encountered write error]" + } + return s +} + +// Stderr returns the captured stderr output from the subprocess. +func (p *process) Stderr() string { + if p.stderr == nil { + return "" + } + return p.stderr.String() +} + +// Compile-time interface satisfaction check. +var _ Adapter = (*ExternalAdapter)(nil) diff --git a/metrics/external_test.go b/metrics/external_test.go new file mode 100644 index 0000000..d2dd4d0 --- /dev/null +++ b/metrics/external_test.go @@ -0,0 +1,679 @@ +package metrics + +import ( + "bufio" + "context" + "encoding/json" + "fmt" + "os" + "strings" + "testing" + "time" +) + +// TestHelperProcess is not a real test — it is invoked by ExternalAdapter tests +// as a mock subprocess. It reads JSON-RPC requests from stdin and writes +// responses to stdout. The HELPER_MODE environment variable controls behavior. +func TestHelperProcess(_ *testing.T) { + if os.Getenv("GO_WANT_HELPER_PROCESS") != "1" { + return + } + + mode := os.Getenv("HELPER_MODE") + switch mode { + case "success": + helperSuccess() + case "timeout": + helperTimeout() + case "crash": + os.Exit(1) + case "stderr": + helperStderr() + case "oversized": + helperOversized() + case "env_check": + helperEnvCheck() + default: + fmt.Fprintf(os.Stderr, "unknown HELPER_MODE: %s\n", mode) + os.Exit(2) + } +} + +// helperSuccess reads JSON-RPC requests and returns valid responses. +func helperSuccess() { + scanner := bufio.NewScanner(os.Stdin) + for scanner.Scan() { + line := scanner.Bytes() + var req JSONRPCRequest + if err := json.Unmarshal(line, &req); err != nil { + fmt.Fprintf(os.Stderr, "unmarshal error: %v\n", err) + os.Exit(1) + } + + switch req.Method { + case "capabilities": + result := CapabilitiesResult{ + Language: "test", + ProtocolVersion: "1.0", + Metrics: []string{"ca", "ce", "instability"}, + } + sendResponse(req.ID, result) + + case "analyze": + graph := ModuleGraph{ + SchemaVersion: "1.0", + Language: "test", + Modules: []ModuleResult{ + { + Module: Module{ + Path: "example/pkg", + Name: "pkg", + Ca: 2, + Ce: 3, + ExportedTypes: 4, + AbstractTypes: 1, + }, + Instability: 0.6, + Abstractness: 0.25, + Distance: 0.15, + LCOM: 1, + Zone: ZoneMainSequence, + }, + }, + Cycles: []Cycle{}, + Warnings: []Warning{}, + Status: StatusComplete, + } + sendResponse(req.ID, graph) + + case "shutdown": + // Notification — no response expected. Exit cleanly. + os.Exit(0) + } + } +} + +// helperTimeout sleeps indefinitely to simulate a subprocess that never responds. +func helperTimeout() { + // Block forever — the test will cancel via context timeout. + select {} +} + +// helperStderr writes to stderr and then processes requests normally. +func helperStderr() { + fmt.Fprintf(os.Stderr, "analyzer warning: test stderr output") + + scanner := bufio.NewScanner(os.Stdin) + for scanner.Scan() { + line := scanner.Bytes() + var req JSONRPCRequest + if err := json.Unmarshal(line, &req); err != nil { + os.Exit(1) + } + + switch req.Method { + case "capabilities": + result := CapabilitiesResult{ + Language: "test", + ProtocolVersion: "1.0", + Metrics: []string{"ca"}, + } + sendResponse(req.ID, result) + + case "analyze": + graph := ModuleGraph{ + SchemaVersion: "1.0", + Language: "test", + Modules: []ModuleResult{}, + Cycles: []Cycle{}, + Warnings: []Warning{}, + Status: StatusComplete, + } + sendResponse(req.ID, graph) + + case "shutdown": + os.Exit(0) + } + } +} + +// helperOversized writes a response that exceeds the maximum response size. +func helperOversized() { + scanner := bufio.NewScanner(os.Stdin) + for scanner.Scan() { + line := scanner.Bytes() + var req JSONRPCRequest + if err := json.Unmarshal(line, &req); err != nil { + os.Exit(1) + } + + switch req.Method { + case "capabilities": + // Write a response with a very large result field. + // The test sets maxResponseSize to a small value. + bigData := strings.Repeat("x", 2048) + resp := fmt.Sprintf(`{"jsonrpc":"2.0","result":{"language":"%s","protocolVersion":"1.0","metrics":[]},"id":1}`, bigData) + _, _ = fmt.Fprintln(os.Stdout, resp) + + case "shutdown": + os.Exit(0) + } + } +} + +// helperEnvCheck verifies that credential-bearing environment variables are +// NOT present, then responds normally. +func helperEnvCheck() { + // Check for blocked variables. + blockedVars := []string{ + "AWS_SECRET_ACCESS_KEY", + "GITHUB_TOKEN", + "GH_TOKEN", + "NPM_TOKEN", + "SECRET_TEST_VALUE", + } + for _, v := range blockedVars { + if val := os.Getenv(v); val != "" { + fmt.Fprintf(os.Stderr, "BLOCKED_VAR_PRESENT:%s=%s", v, val) + os.Exit(3) + } + } + + // Verify PATH is present (required for basic operation). + if os.Getenv("PATH") == "" { + fmt.Fprintf(os.Stderr, "PATH_MISSING") + os.Exit(4) + } + + // Report success via normal protocol. + scanner := bufio.NewScanner(os.Stdin) + for scanner.Scan() { + line := scanner.Bytes() + var req JSONRPCRequest + if err := json.Unmarshal(line, &req); err != nil { + os.Exit(1) + } + + switch req.Method { + case "capabilities": + result := CapabilitiesResult{ + Language: "test", + ProtocolVersion: "1.0", + Metrics: []string{"ca"}, + } + sendResponse(req.ID, result) + + case "analyze": + graph := ModuleGraph{ + SchemaVersion: "1.0", + Language: "test", + Modules: []ModuleResult{}, + Cycles: []Cycle{}, + Warnings: []Warning{}, + Status: StatusComplete, + } + sendResponse(req.ID, graph) + + case "shutdown": + os.Exit(0) + } + } +} + +// sendResponse marshals a result and writes a JSON-RPC response to stdout. +func sendResponse(id *int, result any) { + resultData, err := json.Marshal(result) + if err != nil { + fmt.Fprintf(os.Stderr, "marshal error: %v\n", err) + os.Exit(1) + } + + resp := JSONRPCResponse{ + JSONRPC: JSONRPCVersion, + Result: resultData, + ID: id, + } + + respData, err := json.Marshal(resp) + if err != nil { + fmt.Fprintf(os.Stderr, "marshal response error: %v\n", err) + os.Exit(1) + } + + _, _ = fmt.Fprintln(os.Stdout, string(respData)) +} + +func TestExternalAdapter_SuccessfulAnalysis(t *testing.T) { + t.Parallel() + + projectDir := t.TempDir() + + adapter := NewExternalAdapter(os.Args[0], "test", + WithAnalyzeTimeout(10*time.Second), + WithCapabilitiesTimeout(5*time.Second), + WithShutdownTimeout(2*time.Second), + ) + // Override the binary path and env to use the helper process. + adapter.binaryPath = os.Args[0] + adapter.env = []string{ + "GO_WANT_HELPER_PROCESS=1", + "HELPER_MODE=success", + "PATH=" + os.Getenv("PATH"), + "HOME=" + os.Getenv("HOME"), + } + + ctx := context.Background() + graph, err := adapter.Analyze(ctx, projectDir) + if err != nil { + t.Fatalf("Analyze() returned unexpected error: %v", err) + } + + if got, want := graph.Language, "test"; got != want { + t.Errorf("Language: got %v, want %v", got, want) + } + if got, want := graph.Status, StatusComplete; got != want { + t.Errorf("Status: got %v, want %v", got, want) + } + if got, want := len(graph.Modules), 1; got != want { + t.Fatalf("len(Modules): got %v, want %v", got, want) + } + + mod := graph.Modules[0] + if got, want := mod.Path, "example/pkg"; got != want { + t.Errorf("Module.Path: got %v, want %v", got, want) + } + if got, want := mod.Ca, 2; got != want { + t.Errorf("Module.Ca: got %v, want %v", got, want) + } + if got, want := mod.Ce, 3; got != want { + t.Errorf("Module.Ce: got %v, want %v", got, want) + } + if got, want := mod.Zone, ZoneMainSequence; got != want { + t.Errorf("Module.Zone: got %v, want %v", got, want) + } +} + +func TestExternalAdapter_Timeout(t *testing.T) { + t.Parallel() + + projectDir := t.TempDir() + + adapter := NewExternalAdapter(os.Args[0], "test", + WithAnalyzeTimeout(200*time.Millisecond), + WithCapabilitiesTimeout(100*time.Millisecond), + WithShutdownTimeout(100*time.Millisecond), + ) + adapter.binaryPath = os.Args[0] + adapter.env = []string{ + "GO_WANT_HELPER_PROCESS=1", + "HELPER_MODE=timeout", + "PATH=" + os.Getenv("PATH"), + "HOME=" + os.Getenv("HOME"), + } + + ctx := context.Background() + _, err := adapter.Analyze(ctx, projectDir) + if err == nil { + t.Fatal("Analyze() returned nil error for timeout scenario, want error") + } + // Verify the error wraps with the expected context prefix. + errMsg := err.Error() + if !strings.Contains(errMsg, "external analyze") { + t.Errorf("Analyze() error = %q, want error containing 'external analyze'", errMsg) + } +} + +func TestExternalAdapter_Crash(t *testing.T) { + t.Parallel() + + projectDir := t.TempDir() + + adapter := NewExternalAdapter(os.Args[0], "test", + WithAnalyzeTimeout(5*time.Second), + WithCapabilitiesTimeout(2*time.Second), + WithShutdownTimeout(1*time.Second), + ) + adapter.binaryPath = os.Args[0] + adapter.env = []string{ + "GO_WANT_HELPER_PROCESS=1", + "HELPER_MODE=crash", + "PATH=" + os.Getenv("PATH"), + "HOME=" + os.Getenv("HOME"), + } + + ctx := context.Background() + _, err := adapter.Analyze(ctx, projectDir) + if err == nil { + t.Fatal("Analyze() returned nil error for crash scenario, want error") + } + // Verify the error wraps with the expected context prefix. + errMsg := err.Error() + if !strings.Contains(errMsg, "external analyze") { + t.Errorf("Analyze() error = %q, want error containing 'external analyze'", errMsg) + } +} + +func TestExternalAdapter_StderrCapture(t *testing.T) { + t.Parallel() + + projectDir := t.TempDir() + + adapter := NewExternalAdapter(os.Args[0], "test", + WithAnalyzeTimeout(10*time.Second), + WithCapabilitiesTimeout(5*time.Second), + WithShutdownTimeout(2*time.Second), + ) + adapter.binaryPath = os.Args[0] + adapter.env = []string{ + "GO_WANT_HELPER_PROCESS=1", + "HELPER_MODE=stderr", + "PATH=" + os.Getenv("PATH"), + "HOME=" + os.Getenv("HOME"), + } + + ctx := context.Background() + graph, err := adapter.Analyze(ctx, projectDir) + if err != nil { + t.Fatalf("Analyze() returned unexpected error: %v", err) + } + + // The analysis should still succeed even with stderr output. + if got, want := graph.Status, StatusComplete; got != want { + t.Errorf("Status: got %v, want %v", got, want) + } +} + +func TestExternalAdapter_ResponseSizeLimit(t *testing.T) { + t.Parallel() + + projectDir := t.TempDir() + + adapter := NewExternalAdapter(os.Args[0], "test", + WithAnalyzeTimeout(5*time.Second), + WithCapabilitiesTimeout(2*time.Second), + WithShutdownTimeout(1*time.Second), + WithMaxResponseSize(100), // Very small limit to trigger overflow. + ) + adapter.binaryPath = os.Args[0] + adapter.env = []string{ + "GO_WANT_HELPER_PROCESS=1", + "HELPER_MODE=oversized", + "PATH=" + os.Getenv("PATH"), + "HOME=" + os.Getenv("HOME"), + } + + ctx := context.Background() + _, err := adapter.Analyze(ctx, projectDir) + if err == nil { + t.Fatal("Analyze() returned nil error for oversized response, want error") + } + // Verify the error wraps with the expected context prefix. + errMsg := err.Error() + if !strings.Contains(errMsg, "external analyze") { + t.Errorf("Analyze() error = %q, want error containing 'external analyze'", errMsg) + } +} + +// TestExternalAdapter_EnvironmentSanitization verifies that credential-bearing +// environment variables are not passed to the subprocess. Not parallel because +// t.Setenv modifies process-level state. +func TestExternalAdapter_EnvironmentSanitization(t *testing.T) { + projectDir := t.TempDir() + + // Set credential-bearing env vars that MUST NOT reach the subprocess. + t.Setenv("AWS_SECRET_ACCESS_KEY", "test-secret-key") + t.Setenv("GITHUB_TOKEN", "ghp_test_token") + t.Setenv("SECRET_TEST_VALUE", "should-not-leak") + + adapter := NewExternalAdapter(os.Args[0], "test", + WithAnalyzeTimeout(10*time.Second), + WithCapabilitiesTimeout(5*time.Second), + WithShutdownTimeout(2*time.Second), + ) + adapter.binaryPath = os.Args[0] + // Build env from scratch using SanitizeEnvironment — this is what + // NewExternalAdapter does internally. + sanitized := SanitizeEnvironment(nil) + adapter.env = append(sanitized, + "GO_WANT_HELPER_PROCESS=1", + "HELPER_MODE=env_check", + ) + + ctx := context.Background() + graph, err := adapter.Analyze(ctx, projectDir) + if err != nil { + t.Fatalf("Analyze() returned unexpected error: %v", err) + } + + // If the helper process found blocked vars, it would have exited with + // code 3 and the Analyze call would have failed. A successful result + // confirms sanitization worked. + if got, want := graph.Status, StatusComplete; got != want { + t.Errorf("Status: got %v, want %v", got, want) + } +} + +func TestExternalAdapter_InvalidProjectPath(t *testing.T) { + t.Parallel() + + adapter := NewExternalAdapter("/bin/echo", "test") + + tests := []struct { + name string + path string + }{ + { + name: "path with traversal", + path: "/tmp/../etc/passwd", + }, + { + name: "nonexistent path", + path: "/nonexistent/path/that/does/not/exist", + }, + { + name: "empty path", + path: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + ctx := context.Background() + _, err := adapter.Analyze(ctx, tt.path) + if err == nil { + t.Errorf("Analyze(%q) returned nil error, want error", tt.path) + } + }) + } +} + +func TestExternalAdapter_Language(t *testing.T) { + t.Parallel() + + adapter := NewExternalAdapter("/bin/echo", "python") + if got, want := adapter.Language(), "python"; got != want { + t.Errorf("Language(): got %v, want %v", got, want) + } +} + +func TestExternalAdapter_ShutdownLifecycle(t *testing.T) { + t.Parallel() + + projectDir := t.TempDir() + + adapter := NewExternalAdapter(os.Args[0], "test", + WithAnalyzeTimeout(10*time.Second), + WithCapabilitiesTimeout(5*time.Second), + WithShutdownTimeout(2*time.Second), + ) + adapter.binaryPath = os.Args[0] + adapter.env = []string{ + "GO_WANT_HELPER_PROCESS=1", + "HELPER_MODE=success", + "PATH=" + os.Getenv("PATH"), + "HOME=" + os.Getenv("HOME"), + } + + // Run Analyze — the helper process exits on shutdown notification. + // If shutdown is not sent, the process would hang and the test would + // time out. + ctx := context.Background() + graph, err := adapter.Analyze(ctx, projectDir) + if err != nil { + t.Fatalf("Analyze() returned unexpected error: %v", err) + } + + if got, want := graph.Language, "test"; got != want { + t.Errorf("Language: got %v, want %v", got, want) + } +} + +func TestNewExternalAdapter_Defaults(t *testing.T) { + t.Parallel() + + adapter := NewExternalAdapter("/bin/echo", "go") + if adapter == nil { + t.Fatal("NewExternalAdapter returned nil") + } + if got, want := adapter.Language(), "go"; got != want { + t.Errorf("Language(): got %v, want %v", got, want) + } + if got, want := adapter.analyzeTimeout, defaultAnalyzeTimeout; got != want { + t.Errorf("analyzeTimeout: got %v, want %v", got, want) + } + if got, want := adapter.capabilitiesTimeout, defaultCapabilitiesTimeout; got != want { + t.Errorf("capabilitiesTimeout: got %v, want %v", got, want) + } + if got, want := adapter.shutdownTimeout, defaultShutdownTimeout; got != want { + t.Errorf("shutdownTimeout: got %v, want %v", got, want) + } + if got, want := adapter.maxResponseSize, int64(defaultMaxResponseSize); got != want { + t.Errorf("maxResponseSize: got %v, want %v", got, want) + } + if got, want := adapter.maxStderrSize, int64(defaultMaxStderrSize); got != want { + t.Errorf("maxStderrSize: got %v, want %v", got, want) + } + if adapter.env == nil { + t.Error("env should be populated by SanitizeEnvironment(nil), got nil") + } +} + +func TestNewExternalAdapter_WithOptions(t *testing.T) { + t.Parallel() + + adapter := NewExternalAdapter("/bin/echo", "python", + WithAnalyzeTimeout(42*time.Second), + WithCapabilitiesTimeout(7*time.Second), + WithShutdownTimeout(3*time.Second), + WithMaxResponseSize(512), + WithMaxStderrSize(256), + ) + if adapter == nil { + t.Fatal("NewExternalAdapter returned nil") + } + if got, want := adapter.Language(), "python"; got != want { + t.Errorf("Language(): got %v, want %v", got, want) + } + if got, want := adapter.analyzeTimeout, 42*time.Second; got != want { + t.Errorf("analyzeTimeout: got %v, want %v", got, want) + } + if got, want := adapter.capabilitiesTimeout, 7*time.Second; got != want { + t.Errorf("capabilitiesTimeout: got %v, want %v", got, want) + } + if got, want := adapter.shutdownTimeout, 3*time.Second; got != want { + t.Errorf("shutdownTimeout: got %v, want %v", got, want) + } + if got, want := adapter.maxResponseSize, int64(512); got != want { + t.Errorf("maxResponseSize: got %v, want %v", got, want) + } + if got, want := adapter.maxStderrSize, int64(256); got != want { + t.Errorf("maxStderrSize: got %v, want %v", got, want) + } +} + +func TestWithEnvironment(t *testing.T) { + t.Parallel() + + adapter := NewExternalAdapter("/bin/echo", "test", + WithEnvironment([]string{"CUSTOM_VAR"}), + ) + if adapter == nil { + t.Fatal("NewExternalAdapter returned nil") + } + // Verify the env was set via SanitizeEnvironment with the allowlist. + if adapter.env == nil { + t.Fatal("adapter.env is nil after WithEnvironment, want non-nil") + } + // The env should contain PATH and HOME from defaults, plus CUSTOM_VAR if + // it is set in the current environment, but NOT credential-bearing vars. + for _, e := range adapter.env { + if strings.HasPrefix(e, "AWS_SECRET_ACCESS_KEY=") { + t.Errorf("env contains blocked credential: %s", e) + } + if strings.HasPrefix(e, "GITHUB_TOKEN=") { + t.Errorf("env contains blocked credential: %s", e) + } + } +} + +func TestSchemaJSON(t *testing.T) { + t.Parallel() + + data := SchemaJSON() + if len(data) == 0 { + t.Fatal("SchemaJSON() returned empty slice") + } + // Verify it's valid JSON. + if !json.Valid(data) { + t.Fatal("SchemaJSON() returned invalid JSON") + } + // Verify it returns a copy — modifying the result should not affect + // subsequent calls. + data[0] = 0xFF + data2 := SchemaJSON() + if data2[0] == 0xFF { + t.Error("SchemaJSON() returned the same backing array, want a defensive copy") + } +} + +func TestJSONRPCError_Error(t *testing.T) { + t.Parallel() + + e := &JSONRPCError{ + Code: -32601, + Message: "method not found", + } + got := e.Error() + if got != "method not found" { + t.Errorf("JSONRPCError.Error() = %q, want %q", got, "method not found") + } +} + +func TestExternalAdapter_Capabilities(t *testing.T) { + t.Parallel() + + adapter := NewExternalAdapter(os.Args[0], "test", + WithCapabilitiesTimeout(5*time.Second), + WithShutdownTimeout(2*time.Second), + ) + adapter.binaryPath = os.Args[0] + adapter.env = []string{ + "GO_WANT_HELPER_PROCESS=1", + "HELPER_MODE=success", + "PATH=" + os.Getenv("PATH"), + "HOME=" + os.Getenv("HOME"), + } + + caps := adapter.Capabilities() + if got, want := len(caps), 3; got != want { + t.Fatalf("len(Capabilities()): got %v, want %v", got, want) + } + + expected := []Capability{CapAfferentCoupling, CapEfferentCoupling, CapInstability} + for i, cap := range caps { + if got, want := cap, expected[i]; got != want { + t.Errorf("Capabilities()[%d]: got %v, want %v", i, got, want) + } + } +} diff --git a/metrics/graph.go b/metrics/graph.go new file mode 100644 index 0000000..e8228d3 --- /dev/null +++ b/metrics/graph.go @@ -0,0 +1,43 @@ +package metrics + +// SchemaVersionCurrent is the current schema version for ModuleGraph output. +// Consumers should check this value before processing to detect incompatible changes. +// Version changes follow semantic versioning: minor versions are backward-compatible, +// major versions may contain breaking changes. +const SchemaVersionCurrent = "1.0" + +// ModuleGraph represents the complete analysis result for a project. +// It contains all modules with their computed metrics, detected circular +// dependencies, and any warnings produced during analysis. +type ModuleGraph struct { + // SchemaVersion is the version of the output schema (e.g., "1.0"). + // Consumers use this to detect breaking changes in the JSON structure. + SchemaVersion string `json:"schemaVersion"` + // Language is the lowercase language identifier (e.g., "go", "python"). + Language string `json:"language"` + // Modules contains the analysis results for each module in the project. + Modules []ModuleResult `json:"modules"` + // Cycles contains detected circular dependencies between modules. + Cycles []Cycle `json:"cycles"` + // Warnings contains language-specific caveats about metric accuracy. + // This slice is always non-nil (empty slice, not nil) when there are no warnings. + Warnings []Warning `json:"warnings"` + // Status indicates the overall analysis outcome. + Status Status `json:"status"` +} + +// ModuleResult combines Module identity data with computed metrics and zone +// classification. It embeds Module to provide raw data alongside derived values. +type ModuleResult struct { + Module + // Instability is the computed instability metric I = Ce / (Ca + Ce). + Instability Instability `json:"instability"` + // Abstractness is the computed abstractness metric A = abstractTypes / totalExported. + Abstractness Abstractness `json:"abstractness"` + // Distance is the computed distance from main sequence D = |A + I - 1|. + Distance Distance `json:"distance"` + // LCOM is the computed Lack of Cohesion of Methods (LCOM4 variant). + LCOM LCOM `json:"lcom"` + // Zone is the classification of the module's position relative to the main sequence. + Zone Zone `json:"zone"` +} diff --git a/metrics/jsonrpc.go b/metrics/jsonrpc.go new file mode 100644 index 0000000..03ceee1 --- /dev/null +++ b/metrics/jsonrpc.go @@ -0,0 +1,68 @@ +package metrics + +import "encoding/json" + +// JSONRPCVersion is the JSON-RPC protocol version used by the external analyzer +// protocol. All requests and responses MUST include this version string. +const JSONRPCVersion = "2.0" + +// JSONRPCRequest represents a JSON-RPC 2.0 request message sent to an external +// analyzer subprocess. When ID is nil, the message is a notification (e.g., +// shutdown) and no response is expected. +type JSONRPCRequest struct { + // JSONRPC is the protocol version string, always "2.0". + JSONRPC string `json:"jsonrpc"` + // Method is the name of the method to invoke (e.g., "analyze", "capabilities", "shutdown"). + Method string `json:"method"` + // Params contains the method parameters. Omitted when the method takes no parameters. + Params any `json:"params,omitempty"` + // ID is the request identifier. Nil for notifications (no response expected). + ID *int `json:"id,omitempty"` +} + +// JSONRPCResponse represents a JSON-RPC 2.0 response message received from an +// external analyzer subprocess. Exactly one of Result or Error is non-nil. +type JSONRPCResponse struct { + // JSONRPC is the protocol version string, always "2.0". + JSONRPC string `json:"jsonrpc"` + // Result contains the method return value as raw JSON. Nil when Error is set. + Result json.RawMessage `json:"result,omitempty"` + // Error contains the error object when the method invocation failed. Nil on success. + Error *JSONRPCError `json:"error,omitempty"` + // ID is the request identifier that this response corresponds to. + ID *int `json:"id"` +} + +// JSONRPCError represents a JSON-RPC 2.0 error object returned when a method +// invocation fails. +type JSONRPCError struct { + // Code is a machine-readable error code. Standard JSON-RPC codes apply + // (e.g., -32600 for invalid request, -32601 for method not found). + Code int `json:"code"` + // Message is a human-readable description of the error. + Message string `json:"message"` +} + +// Error implements the error interface for JSONRPCError, allowing it to be +// used directly as a Go error value. +func (e *JSONRPCError) Error() string { + return e.Message +} + +// AnalyzeParams contains the parameters for the "analyze" JSON-RPC method. +type AnalyzeParams struct { + // ProjectPath is the absolute filesystem path to the project to analyze. + ProjectPath string `json:"projectPath"` +} + +// CapabilitiesResult contains the response for the "capabilities" JSON-RPC method. +// It describes what metrics the external analyzer can compute. +type CapabilitiesResult struct { + // Language is the lowercase language identifier (e.g., "python", "typescript"). + Language string `json:"language"` + // ProtocolVersion is the version of the analyzer protocol supported. + ProtocolVersion string `json:"protocolVersion"` + // Metrics lists the metric identifiers this analyzer can compute + // (e.g., "ca", "ce", "instability"). + Metrics []string `json:"metrics"` +} diff --git a/metrics/module.go b/metrics/module.go new file mode 100644 index 0000000..35076f7 --- /dev/null +++ b/metrics/module.go @@ -0,0 +1,32 @@ +package metrics + +// Module represents the universal unit of analysis across all languages. +// In Go this maps to a package, in Python to a module, in TS/JS to a file/module. +// +// Module contains raw metric input data that language-specific adapters populate. +// Computed metrics (Instability, Abstractness, Distance, LCOM) are derived from +// these fields and stored in [ModuleResult]. +type Module struct { + // Path is the unique identifier for this module (e.g., "github.com/foo/bar"). + Path string `json:"path"` + + // Name is the human-readable short name (e.g., "bar"). + Name string `json:"name"` + + // Ca is Afferent Coupling — the number of modules that depend on this module. + // Ca is a non-negative integer; a module with no dependents has Ca = 0. + Ca int `json:"ca"` + + // Ce is Efferent Coupling — the number of modules this module depends on. + // Ce is a non-negative integer; a module with no dependencies has Ce = 0. + Ce int `json:"ce"` + + // ExportedTypes is the total count of exported types in this module. + ExportedTypes int `json:"exportedTypes"` + + // AbstractTypes is the count of abstract types (types that cannot be directly + // instantiated, e.g., Go interfaces, Python ABCs, TypeScript abstract classes). + // Each language adapter documents its mapping from language-specific constructs + // to the abstract/concrete classification. + AbstractTypes int `json:"abstractTypes"` +} diff --git a/metrics/modulegraph.schema.json b/metrics/modulegraph.schema.json new file mode 100644 index 0000000..5fa9995 --- /dev/null +++ b/metrics/modulegraph.schema.json @@ -0,0 +1,54 @@ +{ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "title": "ModuleGraph", + "description": "Complete analysis result for a project", + "type": "object", + "required": ["schemaVersion", "language", "modules", "cycles", "warnings", "status"], + "properties": { + "schemaVersion": { "type": "string" }, + "language": { "type": "string", "minLength": 1 }, + "modules": { + "type": "array", + "items": { + "type": "object", + "required": ["path", "name", "ca", "ce", "instability", "abstractness", "distance", "lcom", "exportedTypes", "abstractTypes", "zone"], + "properties": { + "path": { "type": "string" }, + "name": { "type": "string" }, + "ca": { "type": "integer", "minimum": 0 }, + "ce": { "type": "integer", "minimum": 0 }, + "instability": { "type": "number", "minimum": 0, "maximum": 1 }, + "abstractness": { "type": "number", "minimum": 0, "maximum": 1 }, + "distance": { "type": "number", "minimum": 0, "maximum": 1 }, + "lcom": { "type": "integer", "minimum": 0 }, + "exportedTypes": { "type": "integer", "minimum": 0 }, + "abstractTypes": { "type": "integer", "minimum": 0 }, + "zone": { "type": "string", "enum": ["main-sequence", "zone-of-pain", "zone-of-uselessness", "normal"] } + }, + "additionalProperties": false + } + }, + "cycles": { + "type": "array", + "items": { + "type": "array", + "items": { "type": "string" } + } + }, + "warnings": { + "type": "array", + "items": { + "type": "object", + "required": ["code", "message"], + "properties": { + "code": { "type": "string" }, + "message": { "type": "string" }, + "module": { "type": "string" } + }, + "additionalProperties": false + } + }, + "status": { "type": "string", "enum": ["complete", "partial", "error"] } + }, + "additionalProperties": false +} diff --git a/metrics/registry.go b/metrics/registry.go new file mode 100644 index 0000000..e9b00e7 --- /dev/null +++ b/metrics/registry.go @@ -0,0 +1,39 @@ +package metrics + +import "fmt" + +// Registry manages the association between language identifiers and [Adapter] +// implementations. Registry is not safe for concurrent use — it should be +// configured at startup and used read-only thereafter. +// It MUST be passed via dependency injection, never stored as a global variable. +type Registry struct { + adapters map[string]Adapter +} + +// NewRegistry creates an empty Registry ready for adapter registration. +func NewRegistry() *Registry { + return &Registry{ + adapters: make(map[string]Adapter), + } +} + +// Register adds an adapter to the registry, keyed by its Language() return value. +// Returns an error if an adapter for the same language is already registered. +func (r *Registry) Register(a Adapter) error { + lang := a.Language() + if _, exists := r.adapters[lang]; exists { + return fmt.Errorf("register adapter: language %q is already registered", lang) + } + r.adapters[lang] = a + return nil +} + +// Get retrieves the adapter registered for the given language identifier. +// Returns an error if no adapter is registered for the language. +func (r *Registry) Get(language string) (Adapter, error) { + a, ok := r.adapters[language] + if !ok { + return nil, fmt.Errorf("get adapter: no adapter registered for language %q", language) + } + return a, nil +} diff --git a/metrics/registry_test.go b/metrics/registry_test.go new file mode 100644 index 0000000..1d6a177 --- /dev/null +++ b/metrics/registry_test.go @@ -0,0 +1,162 @@ +package metrics + +import ( + "context" + "sync" + "testing" +) + +// Compile-time interface satisfaction check (Task 5.5). +var _ Adapter = (*mockAdapter)(nil) + +// mockAdapter is a minimal Adapter implementation for testing Registry behavior. +type mockAdapter struct { + language string + capabilities []Capability +} + +func (m *mockAdapter) Analyze(_ context.Context, _ string) (*ModuleGraph, error) { + return &ModuleGraph{Language: m.language, Status: StatusComplete}, nil +} + +func (m *mockAdapter) Language() string { + return m.language +} + +func (m *mockAdapter) Capabilities() []Capability { + return m.capabilities +} + +func TestRegistry_RegisterAndGet(t *testing.T) { + t.Parallel() + + reg := NewRegistry() + if reg == nil { + t.Fatal("NewRegistry() returned nil") + } + + adapter := &mockAdapter{ + language: "go", + capabilities: []Capability{CapAfferentCoupling, CapEfferentCoupling}, + } + + if err := reg.Register(adapter); err != nil { + t.Fatalf("Register() returned unexpected error: %v", err) + } + + got, err := reg.Get("go") + if err != nil { + t.Fatalf("Get(%q) returned unexpected error: %v", "go", err) + } + if got != adapter { + t.Errorf("Get(%q) = %v, want %v", "go", got, adapter) + } +} + +func TestRegistry_DuplicateRegistration(t *testing.T) { + t.Parallel() + + reg := NewRegistry() + adapter1 := &mockAdapter{language: "go"} + adapter2 := &mockAdapter{language: "go"} + + if err := reg.Register(adapter1); err != nil { + t.Fatalf("Register() first call returned unexpected error: %v", err) + } + + err := reg.Register(adapter2) + if err == nil { + t.Fatal("Register() duplicate language returned nil error, want error") + } +} + +func TestRegistry_UnknownLanguage(t *testing.T) { + t.Parallel() + + reg := NewRegistry() + + _, err := reg.Get("rust") + if err == nil { + t.Fatalf("Get(%q) returned nil error for unregistered language, want error", "rust") + } +} + +// TestRegistry_ConcurrentReads verifies that concurrent Get calls on a +// pre-configured Registry are safe. This validates the documented contract +// that Registry is safe for concurrent reads after initial configuration. +func TestRegistry_ConcurrentReads(t *testing.T) { + t.Parallel() + + reg := NewRegistry() + goAdapter := &mockAdapter{language: "go"} + pyAdapter := &mockAdapter{language: "python"} + + if err := reg.Register(goAdapter); err != nil { + t.Fatalf("Register(go): %v", err) + } + if err := reg.Register(pyAdapter); err != nil { + t.Fatalf("Register(python): %v", err) + } + + // Launch concurrent readers after setup is complete. + const goroutines = 10 + var wg sync.WaitGroup + wg.Add(goroutines) + for i := 0; i < goroutines; i++ { + go func() { + defer wg.Done() + for j := 0; j < 100; j++ { + got, err := reg.Get("go") + if err != nil { + t.Errorf("concurrent Get(go): %v", err) + return + } + if got != goAdapter { + t.Errorf("concurrent Get(go) returned wrong adapter") + return + } + got2, err := reg.Get("python") + if err != nil { + t.Errorf("concurrent Get(python): %v", err) + return + } + if got2 != pyAdapter { + t.Errorf("concurrent Get(python) returned wrong adapter") + return + } + } + }() + } + wg.Wait() +} + +func TestRegistry_MultipleLanguages(t *testing.T) { + t.Parallel() + + reg := NewRegistry() + goAdapter := &mockAdapter{language: "go"} + pyAdapter := &mockAdapter{language: "python"} + + if err := reg.Register(goAdapter); err != nil { + t.Fatalf("Register(go) returned unexpected error: %v", err) + } + if err := reg.Register(pyAdapter); err != nil { + t.Fatalf("Register(python) returned unexpected error: %v", err) + } + + gotGo, err := reg.Get("go") + if err != nil { + t.Fatalf("Get(%q) returned unexpected error: %v", "go", err) + } + if gotGo != goAdapter { + t.Errorf("Get(%q) = %v, want %v", "go", gotGo, goAdapter) + } + + gotPy, err := reg.Get("python") + if err != nil { + t.Fatalf("Get(%q) returned unexpected error: %v", "python", err) + } + if gotPy != pyAdapter { + t.Errorf("Get(%q) = %v, want %v", "python", gotPy, pyAdapter) + } +} diff --git a/metrics/schema.go b/metrics/schema.go new file mode 100644 index 0000000..7e3add8 --- /dev/null +++ b/metrics/schema.go @@ -0,0 +1,15 @@ +package metrics + +import _ "embed" + +//go:embed modulegraph.schema.json +var schemaJSON []byte + +// SchemaJSON returns the raw JSON Schema document for ModuleGraph validation. +// It returns a copy of the embedded schema to prevent callers from mutating +// the package-level data. +func SchemaJSON() []byte { + cp := make([]byte, len(schemaJSON)) + copy(cp, schemaJSON) + return cp +} diff --git a/metrics/security.go b/metrics/security.go new file mode 100644 index 0000000..36a2610 --- /dev/null +++ b/metrics/security.go @@ -0,0 +1,166 @@ +package metrics + +import ( + "fmt" + "os" + "path/filepath" + "strings" +) + +// blockedEnvPrefixes contains environment variable prefixes that MUST NOT be +// passed to analyzer subprocesses. These prefixes cover credential-bearing +// variables across common CI systems, cloud providers, and secret managers. +var blockedEnvPrefixes = []string{ + "AWS_SECRET", + "AWS_SESSION", + "AZURE_", + "ARM_", + "GOOGLE_APPLICATION_CREDENTIALS", + "GCLOUD_", + "GITHUB_TOKEN", + "GH_TOKEN", + "GITLAB_TOKEN", + "NPM_TOKEN", + "DOCKER_PASSWORD", + "SECRET_", + "TOKEN_", + "PASSWORD_", + "PRIVATE_KEY", + "API_KEY", + "CREDENTIALS", +} + +// blockedEnvExact contains environment variable names that MUST NOT be passed +// to analyzer subprocesses. +var blockedEnvExact = []string{ + "AWS_SECRET_ACCESS_KEY", + "AWS_SESSION_TOKEN", + "GITHUB_TOKEN", + "GH_TOKEN", + "GITLAB_TOKEN", + "NPM_TOKEN", + "DOCKER_PASSWORD", + "SSH_AUTH_SOCK", + "SSH_AGENT_PID", + "DATABASE_URL", + "REDIS_URL", +} + +// allowedEnvDefaults contains the environment variables included by default +// in the sanitized environment. These provide basic system context without +// exposing credentials. +var allowedEnvDefaults = []string{ + "PATH", + "HOME", + "LANG", +} + +// ValidateProjectPath checks that a project path is safe to pass to an analyzer. +// It rejects paths containing ".." traversal components, resolves symlinks to +// prevent symlink-based traversal, and verifies the path exists and is a directory. +func ValidateProjectPath(path string) error { + if path == "" { + return fmt.Errorf("validate project path: path is empty") + } + + // Check for path traversal components in the original path before cleaning. + // filepath.Clean resolves ".." components, so we must check the raw input + // to detect traversal attempts like "/tmp/../etc/passwd". + // + // Split on both '/' and the platform separator to handle Windows-style + // paths on all platforms. On Unix filepath.Separator is '/', so splitParts + // handles both cases uniformly. + normalized := strings.ReplaceAll(path, "\\", "/") + parts := strings.Split(normalized, "/") + for _, part := range parts { + if part == ".." { + return fmt.Errorf("validate project path: path contains \"..\" traversal component: %s", path) + } + } + + cleaned := filepath.Clean(path) + + // Resolve symlinks to prevent symlink-based traversal where a symlink + // at an innocent path points to a sensitive directory. + resolved, err := filepath.EvalSymlinks(cleaned) + if err != nil { + if os.IsNotExist(err) { + return fmt.Errorf("validate project path: path does not exist: %s", path) + } + return fmt.Errorf("validate project path: resolve symlinks: %w", err) + } + + info, err := os.Stat(resolved) + if err != nil { + if os.IsNotExist(err) { + return fmt.Errorf("validate project path: path does not exist: %s", path) + } + return fmt.Errorf("validate project path: %w", err) + } + + if !info.IsDir() { + return fmt.Errorf("validate project path: path is not a directory: %s", path) + } + + return nil +} + +// SanitizeEnvironment constructs a minimal environment for an analyzer subprocess. +// It includes only PATH, HOME, and LANG from the host environment, plus any +// explicitly allowlisted variables. It never includes credential-bearing variables +// (AWS secrets, tokens, passwords, private keys). +// +// The allowlist parameter specifies additional environment variable names to +// include beyond the defaults. Allowlisted variables are still checked against +// the blocked list — a variable on both lists is excluded. +func SanitizeEnvironment(allowlist []string) []string { + // Build the set of desired variable names. + wanted := make(map[string]bool, len(allowedEnvDefaults)+len(allowlist)) + for _, name := range allowedEnvDefaults { + wanted[name] = true + } + for _, name := range allowlist { + wanted[name] = true + } + + // Remove any blocked variables from the wanted set. + for name := range wanted { + if isBlockedEnv(name) { + delete(wanted, name) + } + } + + // Collect matching variables from the host environment. + var result []string + for _, entry := range os.Environ() { + key, _, ok := strings.Cut(entry, "=") + if !ok { + continue + } + if wanted[key] { + result = append(result, entry) + } + } + + return result +} + +// isBlockedEnv checks whether an environment variable name matches any blocked +// prefix or exact name. This is a security boundary — err on the side of blocking. +func isBlockedEnv(name string) bool { + upper := strings.ToUpper(name) + + for _, exact := range blockedEnvExact { + if upper == exact { + return true + } + } + + for _, prefix := range blockedEnvPrefixes { + if strings.HasPrefix(upper, prefix) { + return true + } + } + + return false +} diff --git a/metrics/security_test.go b/metrics/security_test.go new file mode 100644 index 0000000..e7d544c --- /dev/null +++ b/metrics/security_test.go @@ -0,0 +1,235 @@ +package metrics + +import ( + "os" + "strings" + "testing" +) + +func TestValidateProjectPath_ValidDirectory(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + if err := ValidateProjectPath(dir); err != nil { + t.Errorf("ValidateProjectPath(%q) returned unexpected error: %v", dir, err) + } +} + +func TestValidateProjectPath_InvalidPaths(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + path string + wantErr string + }{ + { + name: "empty path", + path: "", + wantErr: "path is empty", + }, + { + name: "path with traversal", + path: "/tmp/../etc/passwd", + wantErr: "\"..\" traversal", + }, + { + name: "nonexistent path", + path: "/nonexistent/path/that/does/not/exist", + wantErr: "does not exist", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + err := ValidateProjectPath(tt.path) + if err == nil { + t.Fatalf("ValidateProjectPath(%q) returned nil error, want error containing %q", tt.path, tt.wantErr) + } + if !strings.Contains(err.Error(), tt.wantErr) { + t.Errorf("ValidateProjectPath(%q) error = %v, want error containing %q", tt.path, err, tt.wantErr) + } + }) + } +} + +func TestValidateProjectPath_FileNotDirectory(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + file := dir + "/testfile.txt" + if err := os.WriteFile(file, []byte("test"), 0o644); err != nil { + t.Fatalf("WriteFile failed: %v", err) + } + + err := ValidateProjectPath(file) + if err == nil { + t.Fatal("ValidateProjectPath(file) returned nil error, want error for non-directory") + } + if !strings.Contains(err.Error(), "not a directory") { + t.Errorf("error = %v, want error containing \"not a directory\"", err) + } +} + +func TestValidateProjectPath_SymlinkResolved(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + target := dir + "/target" + if err := os.Mkdir(target, 0o755); err != nil { + t.Fatalf("Mkdir failed: %v", err) + } + link := dir + "/link" + if err := os.Symlink(target, link); err != nil { + t.Fatalf("Symlink failed: %v", err) + } + + // Symlinks should be resolved without error when pointing to valid directories. + if err := ValidateProjectPath(link); err != nil { + t.Errorf("ValidateProjectPath(symlink to dir) returned unexpected error: %v", err) + } +} + +// TestSanitizeEnvironment_DefaultsIncluded verifies that PATH, HOME, and LANG +// are included in the sanitized environment. Not parallel because t.Setenv +// modifies process-level state. +func TestSanitizeEnvironment_DefaultsIncluded(t *testing.T) { + t.Setenv("PATH", "/usr/bin:/bin") + t.Setenv("HOME", "/home/testuser") + t.Setenv("LANG", "en_US.UTF-8") + + env := SanitizeEnvironment(nil) + + found := make(map[string]string) + for _, entry := range env { + key, val, _ := strings.Cut(entry, "=") + found[key] = val + } + + for _, required := range []string{"PATH", "HOME", "LANG"} { + if _, ok := found[required]; !ok { + t.Errorf("SanitizeEnvironment(nil) missing required variable %q", required) + } + } + + // Verify values are preserved correctly. + expected := map[string]string{ + "PATH": "/usr/bin:/bin", + "HOME": "/home/testuser", + "LANG": "en_US.UTF-8", + } + for key, wantVal := range expected { + if gotVal, ok := found[key]; ok && gotVal != wantVal { + t.Errorf("SanitizeEnvironment variable %q: got %q, want %q", key, gotVal, wantVal) + } + } +} + +// TestSanitizeEnvironment_CredentialsExcluded verifies that credential-bearing +// variables are excluded from the sanitized environment. Not parallel because +// t.Setenv modifies process-level state. +func TestSanitizeEnvironment_CredentialsExcluded(t *testing.T) { + t.Setenv("AWS_SECRET_ACCESS_KEY", "test-secret") + t.Setenv("GITHUB_TOKEN", "ghp_test") + t.Setenv("GH_TOKEN", "gho_test") + t.Setenv("NPM_TOKEN", "npm_test") + t.Setenv("SECRET_TEST_VALUE", "should-not-appear") + t.Setenv("PATH", "/usr/bin") + + env := SanitizeEnvironment(nil) + + blocked := []string{ + "AWS_SECRET_ACCESS_KEY", + "GITHUB_TOKEN", + "GH_TOKEN", + "NPM_TOKEN", + "SECRET_TEST_VALUE", + } + + for _, entry := range env { + key, _, _ := strings.Cut(entry, "=") + for _, b := range blocked { + if key == b { + t.Errorf("SanitizeEnvironment(nil) included blocked variable %q", b) + } + } + } +} + +// TestSanitizeEnvironment_AllowlistIncluded verifies that explicitly allowlisted +// variables are included. Not parallel because t.Setenv modifies process-level state. +func TestSanitizeEnvironment_AllowlistIncluded(t *testing.T) { + t.Setenv("PATH", "/usr/bin") + t.Setenv("CUSTOM_VAR", "custom_value") + + env := SanitizeEnvironment([]string{"CUSTOM_VAR"}) + + found := false + for _, entry := range env { + key, _, _ := strings.Cut(entry, "=") + if key == "CUSTOM_VAR" { + found = true + break + } + } + + if !found { + t.Error("SanitizeEnvironment([\"CUSTOM_VAR\"]) did not include allowlisted variable") + } +} + +// TestSanitizeEnvironment_AllowlistBlockedOverride verifies that even explicitly +// allowlisted variables are excluded if they match the blocked list. Not parallel +// because t.Setenv modifies process-level state. +func TestSanitizeEnvironment_AllowlistBlockedOverride(t *testing.T) { + t.Setenv("GITHUB_TOKEN", "ghp_test") + t.Setenv("PATH", "/usr/bin") + + env := SanitizeEnvironment([]string{"GITHUB_TOKEN"}) + + for _, entry := range env { + key, _, _ := strings.Cut(entry, "=") + if key == "GITHUB_TOKEN" { + t.Error("SanitizeEnvironment allowlisted GITHUB_TOKEN but it should be blocked") + } + } +} + +func TestIsBlockedEnv_Cases(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + envVar string + blocked bool + }{ + {name: "exact match", envVar: "GITHUB_TOKEN", blocked: true}, + {name: "prefix match", envVar: "AWS_SECRET_ACCESS_KEY", blocked: true}, + {name: "prefix match secret", envVar: "SECRET_MY_VALUE", blocked: true}, + {name: "safe variable", envVar: "PATH", blocked: false}, + {name: "safe variable HOME", envVar: "HOME", blocked: false}, + {name: "case insensitive", envVar: "github_token", blocked: true}, + {name: "unrelated variable", envVar: "GOPATH", blocked: false}, + {name: "azure prefix", envVar: "AZURE_CLIENT_SECRET", blocked: true}, + {name: "arm prefix", envVar: "ARM_CLIENT_SECRET", blocked: true}, + {name: "gcloud prefix", envVar: "GCLOUD_SERVICE_KEY", blocked: true}, + {name: "google app credentials", envVar: "GOOGLE_APPLICATION_CREDENTIALS", blocked: true}, + {name: "api key prefix", envVar: "API_KEY_PROD", blocked: true}, + {name: "credentials prefix", envVar: "CREDENTIALS_FILE", blocked: true}, + {name: "ssh auth sock exact", envVar: "SSH_AUTH_SOCK", blocked: true}, + {name: "ssh agent pid exact", envVar: "SSH_AGENT_PID", blocked: true}, + {name: "database url exact", envVar: "DATABASE_URL", blocked: true}, + {name: "redis url exact", envVar: "REDIS_URL", blocked: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got := isBlockedEnv(tt.envVar) + if got != tt.blocked { + t.Errorf("isBlockedEnv(%q): got %v, want %v", tt.envVar, got, tt.blocked) + } + }) + } +} diff --git a/metrics/validate.go b/metrics/validate.go new file mode 100644 index 0000000..08aa53e --- /dev/null +++ b/metrics/validate.go @@ -0,0 +1,178 @@ +package metrics + +import ( + "encoding/json" + "fmt" +) + +// Validate checks whether the given JSON data conforms to the ModuleGraph schema. +// It verifies required fields, value types, enum constraints, and schema version +// compatibility without relying on an external JSON Schema validation library. +// Returns nil if valid, or an error describing the first validation failure. +func Validate(data []byte) error { + if len(data) == 0 { + return fmt.Errorf("validate: empty input") + } + + var raw map[string]interface{} + if err := json.Unmarshal(data, &raw); err != nil { + return fmt.Errorf("validate: %w", err) + } + + if err := validateTopLevel(raw); err != nil { + return err + } + + if err := validateModules(raw); err != nil { + return err + } + + if err := validateCycles(raw); err != nil { + return err + } + + return validateWarnings(raw) +} + +// validateTopLevel checks required top-level fields, schema version, language, +// and status. +func validateTopLevel(raw map[string]interface{}) error { + requiredFields := []string{"schemaVersion", "language", "modules", "cycles", "warnings", "status"} + for _, field := range requiredFields { + if _, ok := raw[field]; !ok { + return fmt.Errorf("validate: missing required field %q", field) + } + } + + // Validate schemaVersion is a supported value. + version, ok := raw["schemaVersion"].(string) + if !ok { + return fmt.Errorf("validate: field \"schemaVersion\" must be a string") + } + if version != SchemaVersionCurrent { + return fmt.Errorf("validate: unsupported schema version %q (supported: %q)", version, SchemaVersionCurrent) + } + + // Validate language is a non-empty string. + lang, ok := raw["language"].(string) + if !ok { + return fmt.Errorf("validate: field \"language\" must be a string") + } + if lang == "" { + return fmt.Errorf("validate: field \"language\" must be non-empty") + } + + // Validate status is a valid enum value. + status, ok := raw["status"].(string) + if !ok { + return fmt.Errorf("validate: field \"status\" must be a string") + } + if err := validateStatusEnum(status); err != nil { + return fmt.Errorf("validate: %w", err) + } + + return nil +} + +// validateModules checks that the modules field is an array of valid module objects. +func validateModules(raw map[string]interface{}) error { + modulesRaw, ok := raw["modules"].([]interface{}) + if !ok { + return fmt.Errorf("validate: field \"modules\" must be an array") + } + for i, m := range modulesRaw { + if err := validateModule(m, i); err != nil { + return fmt.Errorf("validate: %w", err) + } + } + return nil +} + +// validateCycles checks that the cycles field is an array (not null). +func validateCycles(raw map[string]interface{}) error { + if _, ok := raw["cycles"].([]interface{}); !ok { + return fmt.Errorf("validate: field \"cycles\" must be an array") + } + return nil +} + +// validateWarnings checks that the warnings field is an array of valid warning objects. +func validateWarnings(raw map[string]interface{}) error { + warningsRaw, ok := raw["warnings"].([]interface{}) + if !ok { + return fmt.Errorf("validate: field \"warnings\" must be an array") + } + for i, w := range warningsRaw { + if err := validateWarning(w, i); err != nil { + return fmt.Errorf("validate: %w", err) + } + } + return nil +} + +// validateStatusEnum checks that status is one of the allowed values. +func validateStatusEnum(s string) error { + switch s { + case "complete", "partial", "error": + return nil + default: + return fmt.Errorf("invalid status %q: must be one of \"complete\", \"partial\", \"error\"", s) + } +} + +// validateZoneEnum checks that zone is one of the allowed values. +func validateZoneEnum(z string) error { + switch z { + case "main-sequence", "zone-of-pain", "zone-of-uselessness", "normal": + return nil + default: + return fmt.Errorf("invalid zone %q: must be one of \"main-sequence\", \"zone-of-pain\", \"zone-of-uselessness\", \"normal\"", z) + } +} + +// validateModule checks that a module element has all required fields and valid types. +func validateModule(v interface{}, index int) error { + m, ok := v.(map[string]interface{}) + if !ok { + return fmt.Errorf("modules[%d]: must be an object", index) + } + + requiredFields := []string{ + "path", "name", "ca", "ce", + "instability", "abstractness", "distance", "lcom", + "exportedTypes", "abstractTypes", "zone", + } + for _, field := range requiredFields { + if _, ok := m[field]; !ok { + return fmt.Errorf("modules[%d]: missing required field %q", index, field) + } + } + + // Validate zone enum. + zone, ok := m["zone"].(string) + if !ok { + return fmt.Errorf("modules[%d]: field \"zone\" must be a string", index) + } + if err := validateZoneEnum(zone); err != nil { + return fmt.Errorf("modules[%d]: %w", index, err) + } + + return nil +} + +// validateWarning checks that a warning element has the required code and message fields. +func validateWarning(v interface{}, index int) error { + w, ok := v.(map[string]interface{}) + if !ok { + return fmt.Errorf("warnings[%d]: must be an object", index) + } + + if _, ok := w["code"]; !ok { + return fmt.Errorf("warnings[%d]: missing required field \"code\"", index) + } + if _, ok := w["message"]; !ok { + return fmt.Errorf("warnings[%d]: missing required field \"message\"", index) + } + + return nil +} diff --git a/metrics/validate_test.go b/metrics/validate_test.go new file mode 100644 index 0000000..fe9f88e --- /dev/null +++ b/metrics/validate_test.go @@ -0,0 +1,351 @@ +package metrics + +import ( + "encoding/json" + "strings" + "testing" +) + +func TestValidate_RoundTrip(t *testing.T) { + t.Parallel() + + original := ModuleGraph{ + SchemaVersion: "1.0", + Language: "go", + Modules: []ModuleResult{ + { + Module: Module{ + Path: "github.com/example/foo", + Name: "foo", + Ca: 3, + Ce: 2, + ExportedTypes: 5, + AbstractTypes: 1, + }, + Instability: 0.4, + Abstractness: 0.2, + Distance: 0.4, + LCOM: 1, + Zone: ZoneMainSequence, + }, + }, + Cycles: []Cycle{}, + Warnings: []Warning{}, + Status: StatusComplete, + } + + data, err := json.Marshal(original) + if err != nil { + t.Fatalf("json.Marshal failed: %v", err) + } + + if err := Validate(data); err != nil { + t.Fatalf("Validate returned error for valid input: %v", err) + } + + var decoded ModuleGraph + if err := json.Unmarshal(data, &decoded); err != nil { + t.Fatalf("json.Unmarshal failed: %v", err) + } + + // Verify top-level fields. + if got, want := decoded.SchemaVersion, original.SchemaVersion; got != want { + t.Errorf("SchemaVersion: got %v, want %v", got, want) + } + if got, want := decoded.Language, original.Language; got != want { + t.Errorf("Language: got %v, want %v", got, want) + } + if got, want := decoded.Status, original.Status; got != want { + t.Errorf("Status: got %v, want %v", got, want) + } + if got, want := len(decoded.Modules), len(original.Modules); got != want { + t.Fatalf("len(Modules): got %v, want %v", got, want) + } + if got, want := len(decoded.Cycles), len(original.Cycles); got != want { + t.Errorf("len(Cycles): got %v, want %v", got, want) + } + if got, want := len(decoded.Warnings), len(original.Warnings); got != want { + t.Errorf("len(Warnings): got %v, want %v", got, want) + } + + // Verify module fields. + m := decoded.Modules[0] + om := original.Modules[0] + if got, want := m.Path, om.Path; got != want { + t.Errorf("Module.Path: got %v, want %v", got, want) + } + if got, want := m.Name, om.Name; got != want { + t.Errorf("Module.Name: got %v, want %v", got, want) + } + if got, want := m.Ca, om.Ca; got != want { + t.Errorf("Module.Ca: got %v, want %v", got, want) + } + if got, want := m.Ce, om.Ce; got != want { + t.Errorf("Module.Ce: got %v, want %v", got, want) + } + if got, want := m.ExportedTypes, om.ExportedTypes; got != want { + t.Errorf("Module.ExportedTypes: got %v, want %v", got, want) + } + if got, want := m.AbstractTypes, om.AbstractTypes; got != want { + t.Errorf("Module.AbstractTypes: got %v, want %v", got, want) + } + if got, want := m.Instability, om.Instability; got != want { + t.Errorf("Module.Instability: got %v, want %v", got, want) + } + if got, want := m.Abstractness, om.Abstractness; got != want { + t.Errorf("Module.Abstractness: got %v, want %v", got, want) + } + if got, want := m.Distance, om.Distance; got != want { + t.Errorf("Module.Distance: got %v, want %v", got, want) + } + if got, want := m.LCOM, om.LCOM; got != want { + t.Errorf("Module.LCOM: got %v, want %v", got, want) + } + if got, want := m.Zone, om.Zone; got != want { + t.Errorf("Module.Zone: got %v, want %v", got, want) + } +} + +func TestValidate_ZeroMetricsSerialized(t *testing.T) { + t.Parallel() + + // Verify that zero-value numeric metrics are present in JSON output + // (no omitempty on metric fields). + g := ModuleGraph{ + SchemaVersion: "1.0", + Language: "go", + Modules: []ModuleResult{ + { + Module: Module{ + Path: "github.com/example/empty", + Name: "empty", + // All numeric fields are zero. + }, + Zone: ZoneNormal, + }, + }, + Cycles: []Cycle{}, + Warnings: []Warning{}, + Status: StatusComplete, + } + + data, err := json.Marshal(g) + if err != nil { + t.Fatalf("json.Marshal failed: %v", err) + } + + // Validate passes for zero-valued metrics. + if err := Validate(data); err != nil { + t.Fatalf("Validate returned error for zero-valued metrics: %v", err) + } + + // Verify zero values are present in JSON (not omitted). + var raw map[string]interface{} + if err := json.Unmarshal(data, &raw); err != nil { + t.Fatalf("json.Unmarshal into map failed: %v", err) + } + modules := raw["modules"].([]interface{}) + mod := modules[0].(map[string]interface{}) + + zeroFields := []string{"ca", "ce", "instability", "abstractness", "distance", "lcom", "exportedTypes", "abstractTypes"} + for _, field := range zeroFields { + if _, ok := mod[field]; !ok { + t.Errorf("zero-valued field %q was omitted from JSON output", field) + } + } +} + +func TestValidate_InvalidInputs(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + data string + wantErr string + }{ + { + name: "empty input", + data: "", + wantErr: "empty input", + }, + { + name: "malformed JSON", + data: `{invalid}`, + wantErr: "validate:", + }, + { + name: "missing language field", + data: `{ + "schemaVersion": "1.0", + "modules": [], + "cycles": [], + "warnings": [], + "status": "complete" + }`, + wantErr: "missing required field \"language\"", + }, + { + name: "null warnings", + data: `{ + "schemaVersion": "1.0", + "language": "go", + "modules": [], + "cycles": [], + "warnings": null, + "status": "complete" + }`, + wantErr: "\"warnings\" must be an array", + }, + { + name: "unsupported schema version", + data: `{ + "schemaVersion": "2.0", + "language": "go", + "modules": [], + "cycles": [], + "warnings": [], + "status": "complete" + }`, + wantErr: "unsupported schema version", + }, + { + name: "invalid status value", + data: `{ + "schemaVersion": "1.0", + "language": "go", + "modules": [], + "cycles": [], + "warnings": [], + "status": "unknown" + }`, + wantErr: "invalid status", + }, + { + name: "missing lcom field on module", + data: `{ + "schemaVersion": "1.0", + "language": "go", + "modules": [{ + "path": "foo", + "name": "foo", + "ca": 0, + "ce": 0, + "instability": 0, + "abstractness": 0, + "distance": 0, + "exportedTypes": 0, + "abstractTypes": 0, + "zone": "normal" + }], + "cycles": [], + "warnings": [], + "status": "complete" + }`, + wantErr: "missing required field \"lcom\"", + }, + { + name: "missing path field on module", + data: `{ + "schemaVersion": "1.0", + "language": "go", + "modules": [{ + "name": "foo", + "ca": 0, "ce": 0, + "instability": 0, "abstractness": 0, "distance": 0, "lcom": 0, + "exportedTypes": 0, "abstractTypes": 0, + "zone": "normal" + }], + "cycles": [], + "warnings": [], + "status": "complete" + }`, + wantErr: "missing required field \"path\"", + }, + { + name: "invalid zone value", + data: `{ + "schemaVersion": "1.0", + "language": "go", + "modules": [{ + "path": "foo", + "name": "foo", + "ca": 0, "ce": 0, + "instability": 0, "abstractness": 0, "distance": 0, "lcom": 0, + "exportedTypes": 0, "abstractTypes": 0, + "zone": "invalid-zone" + }], + "cycles": [], + "warnings": [], + "status": "complete" + }`, + wantErr: "invalid zone", + }, + { + name: "warning missing code field", + data: `{ + "schemaVersion": "1.0", + "language": "go", + "modules": [], + "cycles": [], + "warnings": [{"message": "test warning"}], + "status": "complete" + }`, + wantErr: "missing required field \"code\"", + }, + { + name: "warning missing message field", + data: `{ + "schemaVersion": "1.0", + "language": "go", + "modules": [], + "cycles": [], + "warnings": [{"code": "W001"}], + "status": "complete" + }`, + wantErr: "missing required field \"message\"", + }, + { + name: "warning is not an object", + data: `{ + "schemaVersion": "1.0", + "language": "go", + "modules": [], + "cycles": [], + "warnings": ["not-an-object"], + "status": "complete" + }`, + wantErr: "must be an object", + }, + { + name: "valid warning passes", + data: `{ + "schemaVersion": "1.0", + "language": "go", + "modules": [], + "cycles": [], + "warnings": [{"code": "W001", "message": "test"}], + "status": "complete" + }`, + wantErr: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + err := Validate([]byte(tt.data)) + if tt.wantErr == "" { + if err != nil { + t.Errorf("Validate returned unexpected error: %v", err) + } + return + } + if err == nil { + t.Fatalf("Validate returned nil error, want error containing %q", tt.wantErr) + } + if !strings.Contains(err.Error(), tt.wantErr) { + t.Errorf("Validate error = %v, want error containing %q", err, tt.wantErr) + } + }) + } +} diff --git a/metrics/values.go b/metrics/values.go new file mode 100644 index 0000000..3febaff --- /dev/null +++ b/metrics/values.go @@ -0,0 +1,45 @@ +package metrics + +// Instability represents the instability metric I = Ce / (Ca + Ce). +// Value range: [0.0, 1.0] where 0.0 is maximally stable and 1.0 is maximally unstable. +// A maximally stable module (I = 0.0) has many dependents and no dependencies, +// making it costly to change. A maximally unstable module (I = 1.0) has no +// dependents and many dependencies, making it easy to change. +// When both Ca and Ce are 0, Instability is 0.0 (maximally stable by convention). +// Citation: Robert C. Martin, "Agile Software Development" (2003). +type Instability float64 + +// Abstractness represents the ratio of abstract types to total exported types. +// Value range: [0.0, 1.0] where 0.0 is fully concrete and 1.0 is fully abstract. +// An abstract type is a type that cannot be directly instantiated and serves +// as a contract for implementations (e.g., Go interfaces, Python ABCs). +// When a module has no exported types, Abstractness is 0.0. +// Citation: Robert C. Martin, "Agile Software Development" (2003). +type Abstractness float64 + +// Distance represents the Distance from Main Sequence metric D = |A + I - 1|. +// Value range: [0.0, 1.0] where 0.0 indicates the module lies on the main sequence +// (the ideal balance between abstractness and instability). +// D = 1.0 indicates the module is maximally far from the main sequence, either +// in the "zone of pain" (concrete and stable) or the "zone of uselessness" +// (abstract and unstable). +// Citation: Robert C. Martin, "Agile Software Development" (2003). +type Distance float64 + +// LCOM represents the Lack of Cohesion of Methods metric using the LCOM4 variant +// (Hitz & Montazeri, "Measuring Coupling and Cohesion in Object-Oriented Systems", 1995). +// LCOM4 counts connected components in the method-field graph, where methods are +// connected if they access at least one common field. +// +// Value semantics: +// - LCOM = 0: no methods or fields (trivially cohesive) +// - LCOM = 1: fully cohesive (all methods form a single connected component) +// - LCOM > 1: can be split into LCOM independent classes +// +// The "fields" concept maps to language-specific shared state: struct fields in Go, +// instance attributes in Python, class properties in TypeScript. Each adapter +// documents its mapping. +// +// Limitation: LCOM4 does not account for method call chains — two methods that +// share no fields but call each other are treated as disconnected. +type LCOM int diff --git a/metrics/warning.go b/metrics/warning.go new file mode 100644 index 0000000..1146cfe --- /dev/null +++ b/metrics/warning.go @@ -0,0 +1,12 @@ +package metrics + +// Warning represents a language-specific caveat that may affect metric accuracy. +// Warnings annotate analysis results with context without preventing metric computation. +type Warning struct { + // Code is a machine-readable warning identifier (e.g., "dynamic-imports"). + Code string `json:"code"` + // Message is a human-readable description of the warning. + Message string `json:"message"` + // Module is the path of the affected module (empty string if warning applies globally). + Module string `json:"module,omitempty"` +} diff --git a/metrics/zone.go b/metrics/zone.go new file mode 100644 index 0000000..c14b8df --- /dev/null +++ b/metrics/zone.go @@ -0,0 +1,31 @@ +package metrics + +// Zone represents a module's position relative to the main sequence in the +// Abstractness-Instability graph. +type Zone string + +const ( + // ZoneMainSequence indicates the module lies on or near the main sequence (D < 0.2). + ZoneMainSequence Zone = "main-sequence" + // ZoneOfPain indicates the module is concrete and stable (A < 0.2 and I < 0.2). + // Modules in this zone are hard to change because they are depended upon but not abstract. + ZoneOfPain Zone = "zone-of-pain" + // ZoneOfUselessness indicates the module is abstract and unstable (A > 0.8 and I > 0.8). + // Modules in this zone provide abstractions that have few dependents. + ZoneOfUselessness Zone = "zone-of-uselessness" + // ZoneNormal indicates the module does not fall into any special classification. + ZoneNormal Zone = "normal" +) + +// Status represents the overall analysis outcome. +type Status string + +const ( + // StatusComplete indicates all metrics were computed successfully. + StatusComplete Status = "complete" + // StatusPartial indicates some metrics are unavailable due to adapter limitations. + // When Status is Partial, the Warnings slice explains which metrics are unavailable. + StatusPartial Status = "partial" + // StatusError indicates analysis failed with partial or no results. + StatusError Status = "error" +) diff --git a/metrics/zone_test.go b/metrics/zone_test.go new file mode 100644 index 0000000..c895cd4 --- /dev/null +++ b/metrics/zone_test.go @@ -0,0 +1,119 @@ +package metrics + +import "testing" + +func TestComputeZone_Classification(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + a Abstractness + i Instability + d Distance + want Zone + }{ + { + name: "main sequence: A=0.5 I=0.5 D=0.0", + a: 0.5, + i: 0.5, + d: 0.0, + want: ZoneMainSequence, + }, + { + name: "main sequence boundary just inside: D=0.199", + a: 0.5, + i: 0.5, + d: 0.199, + want: ZoneMainSequence, + }, + { + name: "boundary D=0.2 exactly falls to zone of pain: A=0.1 I=0.1", + a: 0.1, + i: 0.1, + d: 0.2, + want: ZoneOfPain, + }, + { + name: "zone of pain: A=0.0 I=0.0 D=1.0", + a: 0.0, + i: 0.0, + d: 1.0, + want: ZoneOfPain, + }, + { + name: "zone of uselessness: A=1.0 I=1.0 D=1.0", + a: 1.0, + i: 1.0, + d: 1.0, + want: ZoneOfUselessness, + }, + { + name: "normal: A=0.5 I=0.3 D=0.2", + a: 0.5, + i: 0.3, + d: 0.2, + want: ZoneNormal, + }, + { + name: "precedence: D<0.2 takes priority over zone-of-pain", + a: 0.1, + i: 0.1, + d: 0.1, + want: ZoneMainSequence, + }, + { + name: "boundary A=0.2 I=0.2 is not zone-of-pain (strict <)", + a: 0.2, + i: 0.2, + d: 0.5, + want: ZoneNormal, + }, + { + name: "boundary A=0.8 I=0.8 is not zone-of-uselessness (strict >)", + a: 0.8, + i: 0.8, + d: 0.5, + want: ZoneNormal, + }, + { + name: "just inside zone-of-pain: A=0.19 I=0.19", + a: 0.19, + i: 0.19, + d: 0.5, + want: ZoneOfPain, + }, + { + name: "just inside zone-of-uselessness: A=0.81 I=0.81", + a: 0.81, + i: 0.81, + d: 0.5, + want: ZoneOfUselessness, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got := ComputeZone(tt.a, tt.i, tt.d) + if got != tt.want { + t.Errorf("ComputeZone(%v, %v, %v) = %v, want %v", tt.a, tt.i, tt.d, got, tt.want) + } + }) + } +} + +func TestComputeZone_Determinism(t *testing.T) { + t.Parallel() + + // Same inputs must produce identical outputs across multiple calls. + // This verifies the constitutional requirement (Principle VI: Metric Fidelity) + // that metric computations are deterministic. + const iterations = 100 + first := ComputeZone(0.3, 0.4, 0.5) + for i := 0; i < iterations; i++ { + got := ComputeZone(0.3, 0.4, 0.5) + if got != first { + t.Fatalf("ComputeZone(0.3, 0.4, 0.5) produced non-deterministic result on iteration %d: got %v, want %v", i, got, first) + } + } +} diff --git a/openspec/changes/universal-coupling-model/.openspec.yaml b/openspec/changes/universal-coupling-model/.openspec.yaml new file mode 100644 index 0000000..7f2cf9b --- /dev/null +++ b/openspec/changes/universal-coupling-model/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-08-28 diff --git a/openspec/changes/universal-coupling-model/design.md b/openspec/changes/universal-coupling-model/design.md new file mode 100644 index 0000000..16d5e30 --- /dev/null +++ b/openspec/changes/universal-coupling-model/design.md @@ -0,0 +1,123 @@ +## Context + +Vibe-Check is a design quality and architectural metrics toolkit for Go codebases, with planned multi-language support. The project currently has no implementation — this is the foundational architecture decision that all subsequent work builds upon. + +The RFC (unbound-force/discussions/483) defines a two-layer architecture inspired by Gaze's side-effect taxonomy pattern: a universal metrics model (Layer 1) that language-specific adapters (Layer 2) target. This mirrors how Gaze separates its side-effect classification model from language-specific AST walkers. + +Key constraints from the project constitution: +- Metric computations MUST produce deterministic results for the same input +- Coupling analysis MUST handle circular dependencies without infinite loops or panics +- All metric values MUST have defined ranges and units documented in GoDoc +- Language adapters MUST implement a common interface; adding a new language MUST NOT require changes to the core analysis engine +- No global mutable state; dependency injection for all services +- Standard library `testing` package only + +## Goals / Non-Goals + +### Goals + +- Define the universal metrics model as Go types that represent Ca, Ce, Instability, Abstractness, Distance from Main Sequence, LCOM cohesion, and circular dependency data +- Establish the `Adapter` interface that all language analyzers implement +- Specify the JSON interchange schema for metric results (used by external adapters and for serialization) +- Define the external analyzer protocol (JSON-RPC over stdin/stdout) for out-of-process language adapters +- Ensure the model is language-agnostic: no Go-specific assumptions leak into the universal layer + +### Non-Goals + +- Implementing any language-specific adapter (Go adapter is Group 1, Python is Group 2a) +- Building the CLI or output formatting (Group 3 and later) +- Implementing complex metric computation algorithms such as graph traversal, AST parsing, or cycle detection algorithms (simple formula applications like I=Ce/(Ca+Ce) are part of the model; the heavy analysis logic belongs in language adapters) +- Making zone classification thresholds user-configurable (default thresholds are part of the model; configurable overrides are a P2+ concern) +- Building the entropy sentinel or architectural drift tracking (P2+) + +## Decisions + +### D1: Package structure — flat `metrics` package for the universal model + +**Decision**: Place all universal model types in a single `metrics` package at the module root. The `Adapter` interface lives in this same package. + +**Rationale**: The model is cohesive — Ca, Ce, I, A, D are all properties of the same unit of analysis (a package/module). Splitting them across packages would create unnecessary coupling between packages and force consumers to import multiple packages for basic operations. + +**Alternatives considered**: +- Separate `metrics/model` and `metrics/adapter` packages — rejected because the adapter interface depends on model types, creating a tight coupling that a package boundary would not meaningfully separate +- Nested `metrics/coupling`, `metrics/cohesion`, `metrics/circular` — rejected because the types are small and interrelated; premature decomposition would increase import surface + +### D2: Unit of analysis — "Module" as the universal concept + +**Decision**: Use `Module` as the universal term for the unit of analysis. In Go this maps to a package, in Python to a module/package, in TS/JS to a module/file. The universal model does not use language-specific terms. + +**Rationale**: Every language has a concept of a module-level grouping that has imports (efferent coupling) and is imported by others (afferent coupling). Using a neutral term avoids baking Go's "package" terminology into the universal layer. + +**Alternatives considered**: +- Use "Package" — rejected because Python and JS use "module" as the primary grouping, and "package" means something different in each language +- Use "Component" — rejected because it is too abstract and overloaded in software architecture terminology +- Use "Unit" — rejected because it conflicts with "unit test" terminology + +### D3: Adapter interface — synchronous, single-module analysis + +**Decision**: The `Adapter` interface has two core methods: `Analyze(ctx, path) -> ModuleGraph` for full-project analysis, and `Language() string` for capability identification. Adapters return a complete `ModuleGraph` containing all modules and their relationships. + +**Rationale**: Full-project analysis is required to compute afferent coupling (you need to know all importers). Streaming or per-module analysis would require the caller to assemble the graph, pushing complexity to the wrong layer. + +**Alternatives considered**: +- Per-module analysis with caller-side assembly — rejected because Ca computation requires global knowledge +- Async/channel-based results — rejected as premature optimization; adds concurrency complexity without demonstrated need + +### D4: External adapter protocol — JSON-RPC 2.0 over stdin/stdout + +**Decision**: External (out-of-process) language adapters communicate via JSON-RPC 2.0 over stdin/stdout. The host process spawns the adapter as a subprocess and exchanges JSON-RPC messages. + +**Rationale**: JSON-RPC is a simple, well-specified protocol. stdin/stdout avoids port management and firewall complexity. This matches the pattern established by LSP and used successfully in the Gaze ecosystem for external analyzer integration (issue #95). + +**Alternatives considered**: +- gRPC — rejected because it requires protobuf compilation and a heavier dependency chain, disproportionate for the data volume involved +- REST over HTTP — rejected because it requires port allocation and lifecycle management for the subprocess +- Plain JSON over stdin/stdout (no RPC framing) — rejected because JSON-RPC provides request/response correlation, error codes, and batch support for free + +### D5: Metric value representation — concrete types with documented invariants + +**Decision**: Each metric is represented as a named type (e.g., `Instability float64`) with documented value ranges in GoDoc. Metric values are plain numeric types, not wrapper structs. + +**Rationale**: The metrics are simple numeric values with well-defined mathematical definitions. Wrapper structs would add allocation overhead and API complexity without benefit. Value ranges and units are documented via GoDoc comments and enforced by constructor/validation functions. + +**Alternatives considered**: +- Wrapper structs with built-in validation (e.g., `type Instability struct { Value float64 }`) — rejected because it doubles the API surface for trivial values; validation belongs at the boundary where values are created +- Generic `Metric` type with name/value pairs — rejected because it loses type safety and makes the API stringly-typed + +### D6: Circular dependency representation — cycle list with path information + +**Decision**: Circular dependencies are represented as a slice of `Cycle` values, where each `Cycle` contains the ordered list of module identifiers forming the cycle. The shortest representation is used (no repeated start node). + +**Rationale**: Consumers need to know which modules participate in each cycle and the dependency path. A simple boolean "has cycles" is insufficient for actionable diagnostics. The ordered path enables visualization and targeted refactoring advice. + +**Alternatives considered**: +- Adjacency matrix with cycle detection delegated to consumers — rejected because cycle detection is a core responsibility of the analysis engine +- Strongly connected components (SCCs) — considered as the detection algorithm but the output representation is the cycle path, not the SCC grouping; SCCs may be used internally during detection + +## Coverage Strategy + +| Test Category | Scope | Target | +|---------------|-------|--------| +| **Unit tests** | Metric computation functions (ComputeInstability, ComputeAbstractness, ComputeDistance, ComputeZone), Cycle representation invariants | ≥90% line coverage | +| **Unit tests** | Registry operations (register, retrieve, duplicate, unknown) | ≥90% line coverage | +| **Integration tests** | JSON round-trip: construct ModuleGraph → marshal → validate → unmarshal → verify equality | ≥80% line coverage | +| **Integration tests** | ExternalAdapter subprocess protocol: mock subprocess ↔ JSON-RPC exchange, timeout, crash, shutdown | ≥80% line coverage | +| **Negative tests** | Schema validation with malformed/invalid JSON inputs | Part of integration target | + +**Coverage ratchet**: Enforced via CI with `go test -coverprofile=coverage.out ./...`. Coverage MUST NOT decrease between commits. Minimum package-level target: 85% line coverage for the `metrics` package. + +**Test parallelism**: Pure computation functions (metric formulas, zone classification) and Registry tests are safe for `t.Parallel()`. ExternalAdapter subprocess tests require sequential execution due to process lifecycle management. + +## Risks / Trade-offs + +- **[Risk] Model may not capture language-specific nuances** → Mitigation: The `ModuleGraph` includes a `Warnings` slice for language-specific caveats (e.g., Python dynamic imports, Go build tags). Adapters annotate results with language-specific context without polluting the universal model. + +- **[Risk] "Module" abstraction may be too coarse for some languages** → Mitigation: The model supports hierarchical module paths (e.g., `github.com/foo/bar/baz`). If finer granularity is needed (e.g., class-level coupling), that is a P2+ concern and can be added as an optional detail level without breaking the module-level model. + +- **[Risk] JSON-RPC protocol adds complexity for the first adapter (Go)** → Mitigation: The Go adapter runs in-process and implements the `Adapter` interface directly. JSON-RPC is only used for external (out-of-process) adapters. The protocol is designed but not required for the initial implementation. + +- **[Risk] Metric definitions may diverge from academic sources** → Mitigation: Each metric type's GoDoc includes the mathematical formula and cites the source. Coupling metrics (Ca, Ce, I, A, D) cite Robert C. Martin, "Agile Software Development" (2003). Cohesion uses LCOM4 (Hitz & Montazeri, "Measuring Coupling and Cohesion in Object-Oriented Systems," 1995) — chosen for its connected-component semantics which map cleanly to Go's struct/method model. LCOM4 limitation: does not account for method call chains. Value ranges are explicitly documented and tested. + +- **[Trade-off] Single `Analyze` call vs. incremental analysis** → We chose full-project analysis for simplicity. Incremental analysis (only re-analyzing changed modules) is a performance optimization for P2+ and can be added as an optional `Adapter` method without breaking the existing interface. + +- **[Trade-off] Named types vs. plain float64** → Named types (e.g., `type Instability float64`) add a small ergonomic cost (explicit conversions) but provide self-documenting code and prevent metric value confusion (passing an Instability where Abstractness is expected). diff --git a/openspec/changes/universal-coupling-model/proposal.md b/openspec/changes/universal-coupling-model/proposal.md new file mode 100644 index 0000000..e8e7d71 --- /dev/null +++ b/openspec/changes/universal-coupling-model/proposal.md @@ -0,0 +1,51 @@ +## Why + +No single OSS tool computes the full Martin metrics suite (Ca, Ce, Instability, Abstractness, Distance from Main Sequence) for Go — or any other language — through a unified model. Vibe-Check needs a universal, language-agnostic metrics model so that the core analysis engine remains stable while language-specific adapters handle extraction. Without this shared model, each language adapter would define its own metric representations, making cross-language comparison impossible and coupling the analysis engine to language-specific details. This is a P1 prerequisite: the universal model must exist before any language-specific implementation (Group 1 Go adapter, Group 2a Python adapter) can begin. + +## What Changes + +- Define a universal metrics model covering seven coupling and cohesion metrics: Afferent Coupling (Ca), Efferent Coupling (Ce), Instability (I), Abstractness (A), Distance from Main Sequence (D), Cohesion (LCOM), and Circular Dependency detection +- Establish a two-layer architecture: Layer 1 is the universal metrics model (language-agnostic types, interfaces, and JSON schema); Layer 2 is language-specific adapters that extract raw data and produce Layer 1 structures +- Define the `Adapter` interface that all language analyzers must implement, ensuring new languages can be added without modifying the core analysis engine +- Specify the external analyzer protocol (JSON-RPC over stdin/stdout) for out-of-process language adapters +- Define the JSON interchange schema with `language` field, `warnings` array, zone classification, and status metadata +- Document metric value ranges, units, and determinism guarantees for all seven metrics + +## Capabilities + +### New Capabilities + +- `universal-metrics-model`: Core metric types (Ca, Ce, I, A, D, LCOM, circular deps), value ranges, units, and determinism contracts for all computed metrics +- `adapter-interface`: The `Adapter` interface contract that language-specific analyzers implement, including the registration mechanism and capability discovery +- `analyzer-protocol`: External analyzer JSON-RPC protocol for out-of-process language adapters communicating over stdin/stdout +- `metrics-schema`: JSON interchange schema for metric results including `language` field, `warnings` array, zone classification, and status metadata + +### Modified Capabilities + + + +### Removed Capabilities + + + +## Constitution Alignment + +| Principle | Assessment | +|-----------|------------| +| I. Autonomous Collaboration | PASS — Adapter interface enables independent language analyzer development | +| II. Composability First | PASS — Two-layer architecture separates universal model from language-specific adapters | +| III. Observable Quality | PASS — All metric values have defined ranges, units, and determinism guarantees | +| IV. Testability | PASS — All metrics have concrete formulas enabling table-driven tests | +| V. Security by Default | PASS — External analyzer protocol defines lifecycle management and error handling | +| VI. Metric Fidelity | PASS — Each metric cites its formula and source (Robert C. Martin); LCOM variant to be specified | +| VII. Language Agnosticism | PASS — Universal "Module" abstraction; no language-specific assumptions in Layer 1 | + +Addresses: https://github.com/zero-dot-force/vibe-check/issues/1 + +## Impact + +- **Code**: Establishes the foundational types and interfaces in a new `metrics` package (or similar) that all subsequent Groups (1, 2a, 3) will depend on +- **APIs**: Defines the `Adapter` interface and JSON-RPC protocol that every language analyzer must conform to — changes to these after initial release are **BREAKING** +- **Dependencies**: No new external dependencies for the model itself; Go adapter (future) will require `golang.org/x/tools/go/packages` +- **Systems**: Sets the contract for the Unbound Force ecosystem's entropy sentinel and architectural drift tracking capabilities +- **Sequencing**: Blocks Group 1 (Go coupling engine), Group 2a (Python adapter), and Group 3 (circular dependency detection) — all depend on this universal model diff --git a/openspec/changes/universal-coupling-model/specs/adapter-interface/spec.md b/openspec/changes/universal-coupling-model/specs/adapter-interface/spec.md new file mode 100644 index 0000000..acee553 --- /dev/null +++ b/openspec/changes/universal-coupling-model/specs/adapter-interface/spec.md @@ -0,0 +1,89 @@ +## ADDED Requirements + +### Requirement: Adapter interface definition + +The system SHALL define an `Adapter` interface with the following methods: +- `Analyze(ctx context.Context, projectPath string) (*ModuleGraph, error)` — analyzes the project at the given path and returns a complete module graph +- `Language() string` — returns the lowercase language identifier (e.g., "go", "python", "typescript") + +All language adapters MUST implement this interface. + +#### Scenario: Go adapter implements interface +- **GIVEN** a Go adapter type is defined +- **WHEN** it is compiled +- **THEN** it SHALL satisfy the `Adapter` interface at compile time (verified via `var _ Adapter = (*GoAdapter)(nil)`) + +#### Scenario: Analyze returns complete graph +- **GIVEN** a valid project path exists +- **WHEN** `Analyze` is called with the project path +- **THEN** it SHALL return a `ModuleGraph` with all modules and their metrics computed + +#### Scenario: Analyze with context cancellation +- **GIVEN** an adapter is analyzing a project +- **WHEN** the context is cancelled during analysis +- **THEN** the adapter SHALL return a context cancellation error without panic + +#### Scenario: Analyze with invalid path +- **GIVEN** a project path that does not exist +- **WHEN** `Analyze` is called with that path +- **THEN** the adapter SHALL return a descriptive error wrapping the underlying cause + +### Requirement: Adapter registration + +The system SHALL provide a registration mechanism for adapters. Registration MUST associate a language identifier (string) with an `Adapter` implementation. The registry MUST NOT use global mutable state — it SHALL be a value that is passed via dependency injection. + +#### Scenario: Register and retrieve adapter +- **GIVEN** an empty registry +- **WHEN** a Go adapter is registered with language "go" +- **THEN** the registry SHALL return that adapter when queried for language "go" + +#### Scenario: Duplicate registration +- **GIVEN** a registry with a Go adapter registered for language "go" +- **WHEN** a second adapter is registered for language "go" +- **THEN** the registry SHALL return an error on the second registration + +#### Scenario: Unknown language lookup +- **GIVEN** a registry with no adapter registered for language "rust" +- **WHEN** the registry is queried for language "rust" +- **THEN** it SHALL return a descriptive error (not nil) + +### Requirement: Adapter capability discovery + +The system SHALL allow adapters to declare their capabilities via a `Capabilities() []Capability` method on the `Adapter` interface. At minimum, each adapter MUST declare which metrics it can compute. When an adapter does not support a metric, the corresponding metric value in the `ModuleResult` SHALL use its zero value and the `ModuleGraph.Status` SHALL be `partial` with a warning explaining which metrics are unavailable. + +#### Scenario: Full capability adapter +- **GIVEN** an adapter declares support for all metrics (Ca, Ce, I, A, D, LCOM, circular deps) +- **WHEN** it is queried for capabilities +- **THEN** it SHALL return a list containing all metric identifiers + +#### Scenario: Partial capability adapter +- **GIVEN** an adapter declares support for coupling metrics (Ca, Ce, I) but not cohesion (LCOM) +- **WHEN** the adapter produces a ModuleGraph +- **THEN** LCOM fields SHALL use zero values, Status SHALL be `partial`, and Warnings SHALL explain that LCOM is unavailable + +#### Scenario: Capability query +- **GIVEN** an adapter is registered +- **WHEN** the system queries the adapter's capabilities +- **THEN** the adapter SHALL return a list of supported metric identifiers + +### Requirement: Adding a new language adapter + +Adding a new language adapter MUST NOT require changes to the core `metrics` package or to any existing adapter. The new adapter MUST only need to implement the `Adapter` interface and register itself with the registry. + +#### Scenario: New adapter integration +- **GIVEN** an existing system with a Go adapter registered +- **WHEN** a developer creates a new adapter for language "rust" implementing the `Adapter` interface +- **THEN** registering it with the registry SHALL make it available for analysis without modifying any existing code + +#### Scenario: No core package changes +- **GIVEN** a new language adapter is added +- **WHEN** the `metrics` package source is compared before and after +- **THEN** the `metrics` package SHALL have zero diff (no modifications required) + +## MODIFIED Requirements + + + +## REMOVED Requirements + + diff --git a/openspec/changes/universal-coupling-model/specs/analyzer-protocol/spec.md b/openspec/changes/universal-coupling-model/specs/analyzer-protocol/spec.md new file mode 100644 index 0000000..0b73307 --- /dev/null +++ b/openspec/changes/universal-coupling-model/specs/analyzer-protocol/spec.md @@ -0,0 +1,122 @@ +## ADDED Requirements + +### Requirement: JSON-RPC 2.0 transport + +The external analyzer protocol SHALL use JSON-RPC 2.0 as the message format. Communication SHALL occur over stdin/stdout of the analyzer subprocess. The host process SHALL spawn the analyzer as a child process and exchange messages via its stdin and stdout streams. Each JSON-RPC message SHALL be framed as a single line of JSON terminated by a newline character (`\n`). The host SHALL read one line at a time from stdout and parse each line as a complete JSON-RPC message. + +#### Scenario: Valid JSON-RPC request +- **GIVEN** an analyzer subprocess is running +- **WHEN** the host sends a JSON-RPC 2.0 request with method "analyze" to the analyzer's stdin +- **THEN** the analyzer SHALL respond with a JSON-RPC 2.0 response on its stdout, terminated by a newline + +#### Scenario: Invalid JSON-RPC request +- **GIVEN** an analyzer subprocess is running +- **WHEN** the host sends malformed JSON to the analyzer's stdin +- **THEN** the analyzer SHALL respond with a JSON-RPC 2.0 error response with code -32700 (Parse error) + +#### Scenario: Unknown method +- **GIVEN** an analyzer subprocess is running +- **WHEN** the host sends a JSON-RPC 2.0 request with an unrecognized method +- **THEN** the analyzer SHALL respond with a JSON-RPC 2.0 error response with code -32601 (Method not found) + +### Requirement: Analyze method + +The protocol SHALL define an `analyze` method that accepts a project path and returns a `ModuleGraph` in the JSON interchange format. The request params SHALL include `projectPath` (string). The result SHALL conform to the metrics JSON schema. + +#### Scenario: Successful analysis +- **GIVEN** an analyzer subprocess is running and the project path is valid +- **WHEN** the host sends `{"jsonrpc":"2.0","method":"analyze","params":{"projectPath":"/path/to/project"},"id":1}` +- **THEN** the analyzer SHALL respond with `{"jsonrpc":"2.0","result":,"id":1}` + +#### Scenario: Analysis error +- **GIVEN** an analyzer subprocess is running +- **WHEN** the analyzer cannot analyze the given project path +- **THEN** the analyzer SHALL respond with a JSON-RPC 2.0 error response with a descriptive message and error code -32000 (application error) + +### Requirement: Capabilities method + +The protocol SHALL define a `capabilities` method that returns the analyzer's supported metrics, language identifier, and protocol version. The method takes no parameters. The response MUST include a `protocolVersion` field (string, initial value `"1.0"`). + +#### Scenario: Capabilities response +- **GIVEN** an analyzer subprocess is running +- **WHEN** the host sends `{"jsonrpc":"2.0","method":"capabilities","params":{},"id":1}` +- **THEN** the analyzer SHALL respond with `{"jsonrpc":"2.0","result":{"language":"python","protocolVersion":"1.0","metrics":["ca","ce","instability","abstractness","distance","lcom","circular"]},"id":1}` + +### Requirement: Lifecycle management + +The host process SHALL manage the lifecycle of external analyzer subprocesses. The host MUST send a `shutdown` notification before terminating the subprocess. The analyzer SHOULD perform cleanup upon receiving the shutdown notification and exit cleanly. + +#### Scenario: Clean shutdown +- **GIVEN** an analyzer subprocess is running +- **WHEN** the host sends `{"jsonrpc":"2.0","method":"shutdown"}` (notification, no id) +- **THEN** the analyzer SHALL perform cleanup and exit with status code 0 + +#### Scenario: Analyzer crash handling +- **GIVEN** an analyzer subprocess is running +- **WHEN** the analyzer subprocess exits unexpectedly during analysis +- **THEN** the host SHALL return an error to the caller with the subprocess exit code and any stderr output + +#### Scenario: Analyzer timeout +- **GIVEN** an analyzer subprocess is running +- **WHEN** the analyzer does not respond within the configured timeout (defaults: `analyze` = 300s, `capabilities` = 10s, `shutdown` = 5s; configurable per-adapter) +- **THEN** the host SHALL terminate the subprocess and return a timeout error to the caller + +#### Scenario: Shutdown grace period +- **GIVEN** the host sends a shutdown notification +- **WHEN** the analyzer does not exit within the shutdown timeout (default: 5s) +- **THEN** the host SHALL forcefully terminate the subprocess (SIGKILL) + +### Requirement: Stderr for diagnostics + +The analyzer process SHALL use stderr for diagnostic output (logs, progress, debug info). The host process MUST NOT interpret stderr as protocol messages. The host MAY capture stderr for error reporting. + +#### Scenario: Diagnostic output +- **GIVEN** an analyzer subprocess is running +- **WHEN** the analyzer writes diagnostic messages to stderr during analysis +- **THEN** the host SHALL ignore stderr for protocol purposes and MAY log it + +#### Scenario: Error context from stderr +- **GIVEN** an analyzer has written diagnostic info to stderr +- **WHEN** the analyzer crashes +- **THEN** the host SHALL include stderr content in the error returned to the caller + +### Requirement: Input validation + +The host MUST validate the `projectPath` parameter before sending it to the analyzer subprocess. The host SHALL reject paths containing `..` traversal components. The host SHALL verify that the path resolves to an existing directory. + +#### Scenario: Path traversal rejection +- **GIVEN** a projectPath containing `..` traversal components +- **WHEN** the host receives the analysis request +- **THEN** the host SHALL reject the request with a validation error without forwarding it to the analyzer + +#### Scenario: Non-existent path rejection +- **GIVEN** a projectPath that does not exist on the filesystem +- **WHEN** the host receives the analysis request +- **THEN** the host SHALL reject the request with a descriptive error without forwarding it to the analyzer + +### Requirement: Subprocess security + +The host MUST treat external analyzer subprocesses as untrusted code and enforce security boundaries. + +#### Scenario: Environment sanitization +- **GIVEN** the host spawns an analyzer subprocess +- **WHEN** the subprocess environment is constructed +- **THEN** the host SHALL NOT pass its full environment to the subprocess; it SHALL construct a minimal environment containing only variables required for the analyzer to function (PATH, HOME, LANG at minimum; no credential-bearing variables unless explicitly allowlisted) + +#### Scenario: Response size limit +- **GIVEN** an analyzer subprocess is producing a response +- **WHEN** the response exceeds the configured maximum size (default: 100 MB) +- **THEN** the host SHALL terminate the subprocess and return a size limit error + +#### Scenario: Stderr buffer limit +- **GIVEN** an analyzer subprocess is writing to stderr +- **WHEN** stderr output exceeds the configured maximum (default: 1 MB) +- **THEN** the host SHALL truncate the captured output and include a truncation notice if the output is later reported + +## MODIFIED Requirements + + + +## REMOVED Requirements + + diff --git a/openspec/changes/universal-coupling-model/specs/metrics-schema/spec.md b/openspec/changes/universal-coupling-model/specs/metrics-schema/spec.md new file mode 100644 index 0000000..c4f3fb2 --- /dev/null +++ b/openspec/changes/universal-coupling-model/specs/metrics-schema/spec.md @@ -0,0 +1,146 @@ +## ADDED Requirements + +### Requirement: JSON schema for ModuleGraph + +The system SHALL define a JSON schema for the `ModuleGraph` structure used as the interchange format between adapters and the core engine. The schema SHALL be the authoritative definition for serialization and deserialization of analysis results. + +#### Scenario: Valid ModuleGraph serialization +- **GIVEN** a ModuleGraph with all fields populated +- **WHEN** it is serialized to JSON +- **THEN** the output SHALL conform to the defined JSON schema + +#### Scenario: Schema validation of external input +- **GIVEN** an external analyzer returns a JSON result +- **WHEN** the host receives the result +- **THEN** the host SHALL validate the result against the JSON schema before processing + +### Requirement: Schema version + +The JSON schema SHALL include a required `schemaVersion` field (string) at the top level of the ModuleGraph. The initial schema version SHALL be `"1.0"`. The version MUST follow semantic versioning. The host MUST check the schema version before processing the result and return a descriptive error if the version is unsupported. + +#### Scenario: Schema version present +- **GIVEN** a ModuleGraph is constructed +- **WHEN** it is serialized to JSON +- **THEN** the JSON SHALL contain a `schemaVersion` field with value `"1.0"` + +#### Scenario: Unsupported schema version +- **GIVEN** an external analyzer returns a result with `schemaVersion: "2.0"` +- **WHEN** the host processes the result +- **THEN** the host SHALL return a descriptive error indicating the schema version is not supported + +### Requirement: Language field + +The JSON schema SHALL include a required `language` field (string) at the top level of the ModuleGraph. The value MUST be a lowercase language identifier (e.g., "go", "python", "typescript"). + +#### Scenario: Language field present +- **GIVEN** a ModuleGraph is constructed +- **WHEN** it is serialized to JSON +- **THEN** the JSON SHALL contain a `language` field with a non-empty string value + +#### Scenario: Language field validation +- **GIVEN** an external analyzer returns a result without a `language` field +- **WHEN** schema validation runs +- **THEN** schema validation SHALL fail + +### Requirement: Warnings array + +The JSON schema SHALL include a `warnings` array at the top level of the ModuleGraph. Each warning SHALL be an object with `code` (string), `message` (string), and optional `module` (string, the affected module path) fields. The array MUST be present even when empty (not omitted or null). + +#### Scenario: Warnings present +- **GIVEN** an analysis produces warnings +- **WHEN** the results are serialized to JSON +- **THEN** the JSON SHALL contain a `warnings` array with warning objects + +#### Scenario: No warnings +- **GIVEN** an analysis produces no warnings +- **WHEN** the results are serialized to JSON +- **THEN** the JSON SHALL contain an empty `warnings` array (`[]`) + +#### Scenario: Warning structure +- **GIVEN** a warning is included in the analysis results +- **WHEN** it is serialized to JSON +- **THEN** it SHALL have at minimum `code` and `message` string fields + +### Requirement: Module metrics structure + +The JSON schema SHALL represent each module's metrics as an object with fields: `path` (string), `name` (string), `ca` (integer), `ce` (integer), `instability` (number), `abstractness` (number), `distance` (number), `lcom` (integer), `exportedTypes` (integer), and `abstractTypes` (integer). All numeric metric fields MUST be present (no omission for zero values). + +#### Scenario: Complete module metrics +- **GIVEN** a module has all metrics computed +- **WHEN** the module's metrics are serialized to JSON +- **THEN** all metric fields SHALL be present with their computed values, including both `exportedTypes` and `abstractTypes` + +#### Scenario: Zero values not omitted +- **GIVEN** a module has Ca = 0 and Ce = 0 +- **WHEN** the module's metrics are serialized to JSON +- **THEN** the JSON SHALL include `"ca": 0` and `"ce": 0` (not omitted) + +### Requirement: Cycles representation + +The JSON schema SHALL represent circular dependencies as a `cycles` array. Each cycle SHALL be an array of module path strings representing the ordered dependency chain, starting from the lexicographically smallest module path. The `cycles` array SHALL be sorted lexicographically by the first element of each cycle. The array MUST be present even when empty. + +#### Scenario: Cycles present +- **GIVEN** a circular dependency A → B → C → A exists +- **WHEN** the results are serialized to JSON +- **THEN** the `cycles` array SHALL contain `[["A", "B", "C"]]` + +#### Scenario: No cycles +- **GIVEN** no circular dependencies exist +- **WHEN** the results are serialized to JSON +- **THEN** the `cycles` array SHALL be empty (`[]`) + +### Requirement: Zone classification + +The JSON schema SHALL include a `zone` field for each module indicating its position relative to the main sequence. Valid zone values SHALL be: `main-sequence` (D < 0.2), `zone-of-pain` (A < 0.2 and I < 0.2), `zone-of-uselessness` (A > 0.8 and I > 0.8), and `normal` (all other cases). Zone classification precedence: `main-sequence` is evaluated first, then `zone-of-pain`, then `zone-of-uselessness`, then `normal`. The zone thresholds are defined in the schema as default constants. + +#### Scenario: Module on main sequence +- **GIVEN** a module has D = 0.1 +- **WHEN** zone classification is applied +- **THEN** its zone SHALL be `main-sequence` + +#### Scenario: Module in zone of pain +- **GIVEN** a module has A = 0.0, I = 0.0, and D = 1.0 +- **WHEN** zone classification is applied +- **THEN** its zone SHALL be `zone-of-pain` + +#### Scenario: Module in zone of uselessness +- **GIVEN** a module has A = 1.0 and I = 1.0 +- **WHEN** zone classification is applied +- **THEN** its zone SHALL be `zone-of-uselessness` + +#### Scenario: Normal module +- **GIVEN** a module has D = 0.5 and does not qualify for any special zone +- **WHEN** zone classification is applied +- **THEN** its zone SHALL be `normal` + +#### Scenario: Overlapping zone criteria — main-sequence takes precedence +- **GIVEN** a module has A = 0.1, I = 0.1, and D = 0.2 (qualifies for both main-sequence boundary and zone-of-pain) +- **WHEN** zone classification is applied +- **THEN** D is NOT < 0.2, so it SHALL be classified as `zone-of-pain` + +### Requirement: Status metadata + +The JSON schema SHALL include a top-level `status` field indicating the overall analysis outcome. Valid values SHALL be: `complete` (all metrics computed successfully), `partial` (some metrics unavailable due to adapter limitations), and `error` (analysis failed with partial or no results). When status is `partial`, the `warnings` array MUST contain entries explaining which metrics are unavailable and why. + +#### Scenario: Complete analysis +- **GIVEN** all metrics are computed successfully +- **WHEN** the results are serialized +- **THEN** status SHALL be `complete` + +#### Scenario: Partial analysis +- **GIVEN** an adapter does not support LCOM computation +- **WHEN** the results are serialized +- **THEN** status SHALL be `partial` and warnings SHALL explain that LCOM is unavailable + +#### Scenario: Error status +- **GIVEN** analysis encounters a fatal error but produces partial results +- **WHEN** the results are serialized +- **THEN** status SHALL be `error` with warnings describing the failure + +## MODIFIED Requirements + + + +## REMOVED Requirements + + diff --git a/openspec/changes/universal-coupling-model/specs/universal-metrics-model/spec.md b/openspec/changes/universal-coupling-model/specs/universal-metrics-model/spec.md new file mode 100644 index 0000000..1d1d5cd --- /dev/null +++ b/openspec/changes/universal-coupling-model/specs/universal-metrics-model/spec.md @@ -0,0 +1,243 @@ +## ADDED Requirements + +### Requirement: Module identity + +The system SHALL represent each unit of analysis as a `Module` with a unique string identifier (the module path) and a human-readable name. Module paths MUST be unique within a single analysis result. The `Module` type SHALL contain raw metric input data: `Path` (string), `Name` (string), `Ca` (int), `Ce` (int), `ExportedTypes` (int), `AbstractTypes` (int). + +#### Scenario: Distinct modules have distinct paths +- **GIVEN** an adapter is analyzing a project containing two distinct packages +- **WHEN** the analysis completes +- **THEN** each module in the result SHALL have a unique `Path` value + +#### Scenario: Module path preserves language convention +- **GIVEN** a Go adapter is analyzing a project with package `github.com/foo/bar` +- **WHEN** the analysis completes +- **THEN** the module path SHALL be `github.com/foo/bar` + +### Requirement: ModuleResult structure + +The system SHALL define a `ModuleResult` type that combines `Module` identity data with computed metrics (Instability, Abstractness, Distance, LCOM) and Zone classification. `ModuleResult` SHALL embed `Module` (providing raw data) and add computed metric fields. The `ModuleGraph.Modules` slice SHALL contain `ModuleResult` entries, ensuring each entry carries both raw input data and computed output values. + +#### Scenario: ModuleResult contains both raw and computed data +- **GIVEN** a module with Ca=3, Ce=7, ExportedTypes=5, AbstractTypes=2 +- **WHEN** a ModuleResult is constructed for this module +- **THEN** the ModuleResult SHALL contain the raw Module fields AND computed Instability=0.7, Abstractness=0.4, Distance=0.1 + +### Requirement: Afferent coupling metric + +The system SHALL compute Afferent Coupling (Ca) for each module as the count of other modules that depend on it. Ca MUST be a non-negative integer. A module with no dependents SHALL have Ca = 0. + +#### Scenario: Module with no dependents +- **GIVEN** a project is analyzed +- **WHEN** a module is not imported by any other module in the project +- **THEN** Ca SHALL equal 0 + +#### Scenario: Module with multiple dependents +- **GIVEN** a project is analyzed +- **WHEN** module A is imported by modules B, C, and D +- **THEN** Ca for module A SHALL equal 3 + +#### Scenario: Deterministic Ca computation +- **GIVEN** a project is analyzed twice with no changes between runs +- **WHEN** the results are compared +- **THEN** Ca values for all modules SHALL be identical in both results + +### Requirement: Efferent coupling metric + +The system SHALL compute Efferent Coupling (Ce) for each module as the count of other modules it depends on. Ce MUST be a non-negative integer. A module with no dependencies SHALL have Ce = 0. + +#### Scenario: Module with no dependencies +- **GIVEN** a project is analyzed +- **WHEN** a module does not import any other module in the project +- **THEN** Ce SHALL equal 0 + +#### Scenario: Module with multiple dependencies +- **GIVEN** a project is analyzed +- **WHEN** module A imports modules B, C, and D +- **THEN** Ce for module A SHALL equal 3 + +#### Scenario: Deterministic Ce computation +- **GIVEN** a project is analyzed twice with no changes between runs +- **WHEN** the results are compared +- **THEN** Ce values for all modules SHALL be identical in both results + +### Requirement: Instability metric + +The system SHALL compute Instability (I) for each module using the formula I = Ce / (Ca + Ce). I MUST be a float64 in the range [0.0, 1.0]. When both Ca and Ce are 0, I SHALL be 0.0 (maximally stable by convention). + +#### Scenario: Maximally stable module +- **GIVEN** a project is analyzed +- **WHEN** a module has Ca = 5 and Ce = 0 +- **THEN** I SHALL equal 0.0 + +#### Scenario: Maximally unstable module +- **GIVEN** a project is analyzed +- **WHEN** a module has Ca = 0 and Ce = 5 +- **THEN** I SHALL equal 1.0 + +#### Scenario: Mixed coupling +- **GIVEN** a project is analyzed +- **WHEN** a module has Ca = 3 and Ce = 7 +- **THEN** I SHALL equal 0.7 + +#### Scenario: Isolated module +- **GIVEN** a project is analyzed +- **WHEN** a module has Ca = 0 and Ce = 0 +- **THEN** I SHALL equal 0.0 + +#### Scenario: Deterministic Instability computation +- **GIVEN** a project is analyzed twice with no changes between runs +- **WHEN** the results are compared +- **THEN** Instability values for all modules SHALL be identical in both results + +### Requirement: Abstractness metric + +The system SHALL compute Abstractness (A) for each module as the ratio of abstract types to total exported types. A MUST be a float64 in the range [0.0, 1.0]. When a module has no exported types, A SHALL be 0.0. An abstract type is a type that cannot be directly instantiated and serves as a contract for implementations. Each language adapter MUST document its mapping from language-specific constructs to the abstract/concrete classification (e.g., Go interfaces, Python ABCs, TypeScript abstract classes/interfaces). + +#### Scenario: Fully abstract module +- **GIVEN** a project is analyzed +- **WHEN** a module exports 3 abstract types and 0 concrete types (abstractTypes=3, exportedTypes=3) +- **THEN** A SHALL equal 1.0 + +#### Scenario: Fully concrete module +- **GIVEN** a project is analyzed +- **WHEN** a module exports 0 abstract types and 5 concrete types (abstractTypes=0, exportedTypes=5) +- **THEN** A SHALL equal 0.0 + +#### Scenario: Mixed module +- **GIVEN** a project is analyzed +- **WHEN** a module exports 2 abstract types and 3 concrete types (abstractTypes=2, exportedTypes=5) +- **THEN** A SHALL equal 0.4 + +#### Scenario: No exported types +- **GIVEN** a project is analyzed +- **WHEN** a module exports no types (exportedTypes=0) +- **THEN** A SHALL equal 0.0 + +#### Scenario: Deterministic Abstractness computation +- **GIVEN** a project is analyzed twice with no changes between runs +- **WHEN** the results are compared +- **THEN** Abstractness values for all modules SHALL be identical in both results + +### Requirement: Distance from main sequence metric + +The system SHALL compute Distance from Main Sequence (D) for each module using the formula D = |A + I - 1|. D MUST be a float64 in the range [0.0, 1.0]. A value of 0.0 indicates the module lies on the main sequence. + +#### Scenario: Module on the main sequence +- **GIVEN** a project is analyzed +- **WHEN** a module has A = 0.5 and I = 0.5 +- **THEN** D SHALL equal 0.0 + +#### Scenario: Zone of pain +- **GIVEN** a project is analyzed +- **WHEN** a module has A = 0.0 and I = 0.0 +- **THEN** D SHALL equal 1.0 + +#### Scenario: Zone of uselessness +- **GIVEN** a project is analyzed +- **WHEN** a module has A = 1.0 and I = 1.0 +- **THEN** D SHALL equal 1.0 + +#### Scenario: Deterministic Distance computation +- **GIVEN** a project is analyzed twice with no changes between runs +- **WHEN** the results are compared +- **THEN** Distance values for all modules SHALL be identical in both results + +### Requirement: Cohesion metric + +The system SHALL compute Lack of Cohesion of Methods using the LCOM4 variant (Hitz & Montazeri, 1995). LCOM4 counts the number of connected components in the method-field graph, where methods are connected if they access at least one common field. LCOM MUST be a non-negative integer. LCOM = 1 indicates a fully cohesive module (all methods form a single connected component). LCOM = 0 indicates a module with no methods or fields (trivially cohesive). LCOM > 1 indicates the module could be split into LCOM independent classes. The "fields" concept maps to language-specific shared state: struct fields in Go, instance attributes in Python, class properties in TypeScript. Each adapter MUST document its mapping. Limitation: LCOM4 does not account for method call chains — two methods that share no fields but call each other are treated as disconnected. + +#### Scenario: Fully cohesive module +- **GIVEN** a project is analyzed +- **WHEN** a module has 5 methods and all 5 access at least one common field (forming a single connected component) +- **THEN** LCOM SHALL be 1 + +#### Scenario: Trivially cohesive module +- **GIVEN** a project is analyzed +- **WHEN** a module has no methods or no fields +- **THEN** LCOM SHALL be 0 + +#### Scenario: Disjoint groups +- **GIVEN** a project is analyzed +- **WHEN** a module contains two groups of functions that share no common types or state +- **THEN** LCOM SHALL be 2 + +#### Scenario: Deterministic LCOM computation +- **GIVEN** a project is analyzed twice with no changes between runs +- **WHEN** the results are compared +- **THEN** LCOM values for all modules SHALL be identical in both results + +### Requirement: Circular dependency detection + +The system SHALL detect circular dependencies between modules. Each cycle SHALL be represented as an ordered list of module paths forming the cycle, starting from the lexicographically smallest module path. The `Cycles` slice SHALL be sorted lexicographically by the first element of each cycle. The system MUST handle circular dependencies without infinite loops or panics. + +#### Scenario: No circular dependencies +- **GIVEN** a project with no circular dependencies is analyzed +- **WHEN** the analysis completes +- **THEN** the cycles list SHALL be empty + +#### Scenario: Simple circular dependency +- **GIVEN** a project is analyzed +- **WHEN** module A depends on module B and module B depends on module A +- **THEN** the cycles list SHALL contain one cycle with path [A, B] + +#### Scenario: Complex circular dependency +- **GIVEN** a project is analyzed +- **WHEN** module A depends on B, B depends on C, and C depends on A +- **THEN** the cycles list SHALL contain one cycle with path [A, B, C] + +#### Scenario: Canonical cycle ordering +- **GIVEN** a project is analyzed +- **WHEN** a cycle exists between modules C, A, and B (C→A→B→C) +- **THEN** the cycle SHALL be represented as [A, B, C] (starting from the lexicographically smallest path) + +#### Scenario: Multiple cycles sorted +- **GIVEN** a project is analyzed +- **WHEN** two independent cycles exist: D→E→D and A→B→A +- **THEN** the cycles list SHALL be [[A, B], [D, E]] (sorted by first element) + +#### Scenario: Termination guarantee +- **GIVEN** a project contains circular dependencies of any depth +- **WHEN** the analysis runs +- **THEN** the cycle detection algorithm SHALL terminate without panic and produce a result + +#### Scenario: Deterministic cycle detection +- **GIVEN** a project with circular dependencies is analyzed twice with no changes between runs +- **WHEN** the results are compared +- **THEN** the cycles list SHALL be identical in both results (same cycles, same order) + +### Requirement: Module graph structure + +The system SHALL represent the complete analysis result as a `ModuleGraph` containing all modules (as `ModuleResult` entries), their computed metrics, and detected cycles. The graph MUST include a `Language` field identifying which language adapter produced the result. + +#### Scenario: Complete graph output +- **GIVEN** an adapter analyzes a project with 5 modules +- **WHEN** the analysis completes +- **THEN** the ModuleGraph SHALL contain exactly 5 ModuleResult entries with all metrics computed + +#### Scenario: Language identification +- **GIVEN** a Go adapter produces a ModuleGraph +- **WHEN** the analysis completes +- **THEN** the Language field SHALL equal "go" + +### Requirement: Warnings in analysis results + +The system SHALL support a `Warnings` slice in the `ModuleGraph` for language-specific caveats that may affect metric accuracy. Warnings MUST NOT prevent metric computation — they annotate results with context. + +#### Scenario: Warning for dynamic imports +- **GIVEN** a Python adapter encounters dynamic imports that cannot be statically analyzed +- **WHEN** the analysis completes +- **THEN** the ModuleGraph SHALL include a warning describing the limitation + +#### Scenario: No warnings +- **GIVEN** analysis completes without caveats +- **WHEN** the results are inspected +- **THEN** the Warnings slice SHALL be empty (not nil) + +## MODIFIED Requirements + + + +## REMOVED Requirements + + diff --git a/openspec/changes/universal-coupling-model/tasks.md b/openspec/changes/universal-coupling-model/tasks.md new file mode 100644 index 0000000..9981f30 --- /dev/null +++ b/openspec/changes/universal-coupling-model/tasks.md @@ -0,0 +1,59 @@ +## 1. Project Initialization + +- [x] 1.1 Initialize Go module (`go mod init github.com/zero-dot-force/vibe-check`) and create the `metrics` package directory +- [x] 1.2 Configure golangci-lint with project conventions (gofmt, GoDoc enforcement, error wrapping checks) + +## 2. Core Metric Types + +- [x] [P] 2.1 Define `Module` type with `Path` (string), `Name` (string), and metric fields: `Ca` (int), `Ce` (int), `ExportedTypes` (int), `AbstractTypes` (int) +- [x] [P] 2.2 Define named metric types: `Instability float64`, `Abstractness float64`, `Distance float64` with GoDoc documenting formulas, value ranges [0.0, 1.0], and units +- [x] [P] 2.3 Define `LCOM int` named type with GoDoc documenting the LCOM4 variant (Hitz & Montazeri, 1995): connected components in method-field graph; 0 = no methods/fields, 1 = fully cohesive, >1 = number of independent components +- [x] 2.4 Implement metric computation functions: `ComputeInstability(ca, ce int) Instability`, `ComputeAbstractness(abstractTypes, totalExported int) Abstractness`, `ComputeDistance(a Abstractness, i Instability) Distance` +- [x] 2.5 Write table-driven tests for all metric computation functions covering: (a) happy-path with known values, (b) zero-denominator edge cases, (c) boundary values (0.0, 1.0), (d) determinism verification (same inputs produce same outputs across calls) + +## 3. Circular Dependency Types + +- [x] 3.1 Define `Cycle` type as `[]string` (ordered module paths) and document the representation (no repeated start node) +- [x] 3.2 Write tests verifying Cycle representation invariants + +## 4. ModuleGraph Structure + +- [x] [P] 4.1 Define `Warning` type with `Code` (string), `Message` (string), and `Module` (string, optional) fields +- [x] [P] 4.2 Define `Zone` string type with constants: `ZoneMainSequence`, `ZoneOfPain`, `ZoneOfUselessness`, `ZoneNormal` and GoDoc for each (JSON values: `main-sequence`, `zone-of-pain`, `zone-of-uselessness`, `normal`) +- [x] [P] 4.3 Define `Status` string type with constants: `StatusComplete`, `StatusPartial`, `StatusError` and GoDoc for each +- [x] 4.4 Define `ModuleGraph` struct with fields: `Language` (string), `Modules` ([]ModuleResult), `Cycles` ([]Cycle), `Warnings` ([]Warning), `Status` (Status) +- [x] 4.5 Define `ModuleResult` struct embedding `Module` and adding computed metrics (Instability, Abstractness, Distance, LCOM) and Zone classification +- [x] 4.6 Implement `ComputeZone(a Abstractness, i Instability, d Distance) Zone` function with threshold constants and precedence: main-sequence first, then zone-of-pain, then zone-of-uselessness, then normal +- [x] 4.7 Write tests for zone classification covering all four zones, boundary conditions (D=0.2 exactly), and overlapping criteria precedence + +## 5. Adapter Interface + +- [x] 5.1 Define `Adapter` interface with `Analyze(ctx context.Context, projectPath string) (*ModuleGraph, error)` and `Language() string` methods +- [x] 5.2 Define `Capability` type and `Capabilities() []Capability` method on the Adapter interface for metric capability discovery +- [x] 5.3 Implement `Registry` struct (not global — injectable) with `Register(Adapter) error` and `Get(language string) (Adapter, error)` methods +- [x] 5.4 Write tests for Registry: register/retrieve, duplicate registration error, unknown language error +- [x] 5.5 Write compile-time interface satisfaction checks (`var _ Adapter = (*GoAdapter)(nil)` pattern) and a test verifying a mock adapter satisfies the interface contract + +## 6. JSON Schema and Serialization + +- [x] 6.1 Add JSON struct tags to all model types (`ModuleGraph`, `ModuleResult`, `Warning`, `Cycle`) ensuring zero values are serialized (no `omitempty` on metric fields); include `schemaVersion` field (initial value `"1.0"`) on `ModuleGraph` +- [x] 6.2 Define the JSON schema document (as a Go embed or standalone `.json` file) for ModuleGraph validation +- [x] 6.3 Implement `Validate(data []byte) error` function to validate JSON against the schema +- [x] 6.4 Write round-trip tests: construct ModuleGraph → marshal to JSON → validate against schema → unmarshal back → verify equality +- [x] 6.5 Write table-driven negative tests for `Validate()`: missing `language` field, null `warnings`, omitted metric fields, invalid `status` value, malformed JSON, empty input, extra unknown fields + +## 7. External Analyzer Protocol + +- [x] 7.1 Define JSON-RPC 2.0 request/response types for the `analyze`, `capabilities`, and `shutdown` methods (include `protocolVersion` in capabilities response) +- [x] 7.2 Implement `ExternalAdapter` struct wrapping a subprocess (exec.Cmd) that satisfies the `Adapter` interface, communicating via JSON-RPC over stdin/stdout with newline-delimited framing +- [x] 7.3 Implement subprocess lifecycle management: spawn, timeout (defaults: analyze=300s, capabilities=10s, shutdown=5s), clean shutdown via `shutdown` notification, SIGKILL after grace period, stderr capture (max 1 MB) +- [x] 7.5 Implement input validation (projectPath: no `..` traversal, must exist), environment sanitization (minimal allowlist), and response size limits (default: 100 MB) +- [x] 7.4 Write tests for ExternalAdapter using the `TestHelperProcess` pattern (`os.Args[0]` with `-test.run=TestHelperProcess`): (a) successful analysis round-trip, (b) timeout simulation (helper process sleeps past deadline), (c) crash simulation (helper exits non-zero), (d) shutdown lifecycle, (e) stderr capture, (f) response size limit enforcement, (g) environment sanitization verification + +## 8. Documentation + +- [x] [P] 8.1 Write package-level GoDoc for the `metrics` package explaining the universal model, two-layer architecture, and relationship to language adapters +- [x] [P] 8.2 Update AGENTS.md project structure section to reflect the new `metrics` package + + +