feat: implement universal coupling metrics model (Layer 1) - #17
Conversation
OpenSpec proposal, design, specs, and tasks for the universal multi-language coupling metrics model (Layer 1). Capabilities: universal-metrics-model, adapter-interface, analyzer-protocol, metrics-schema. Addresses: #1
Core metrics package implementing the universal multi-language coupling metrics model: - Module type and named metric types (Instability, Abstractness, Distance, LCOM) with GoDoc formulas and value ranges - Metric computation functions with zero-denominator handling - Cycle type for circular dependency representation - ModuleGraph and ModuleResult with zone classification - Adapter interface and dependency-injected Registry - JSON Schema (draft 2020-12) with embedded validation - ExternalAdapter (JSON-RPC 2.0 subprocess protocol) with security controls (path validation, env sanitization, response size limits) - 46 tests passing with -race Implements: openspec/changes/universal-coupling-model Addresses: #1
- Add validateWarning test coverage (CRAP score 20 → covered)
- Add wantErr assertions to all Validate error test cases
- Add NewExternalAdapter constructor contract tests
- Add NewRegistry nil check assertion
- Add SanitizeEnvironment value verification
- Add zone boundary condition tests (A=0.2, I=0.2, etc.)
- Add SchemaVersionCurrent constant
- Fix limitedBuffer.Write to maintain never-break-pipe contract
- Simplify validate.go error wrapping
- Modernize interface{} → any
- Architecture patterns for two-layer metrics model - Spec review insights (zone thresholds, LCOM variant) - Testing gotchas (TestHelperProcess, t.Setenv, limitedBuffer) - golangci-lint v2 configuration gotchas Assisted-by: claude-opus-4 Generated with AI assistance (claude-opus-4)
jflowers
left a comment
There was a problem hiding this comment.
Council Verdict: APPROVE
Reviewers: Adversary, Architect, Curator, Envoy, Guard, Herald, Scribe, SRE, Testing
Iterations: 1 (3 REQUEST CHANGES resolved, all 9 now APPROVE)
Adversary (APPROVE)
- [MEDIUM] Symlink-based path traversal bypass in
ValidateProjectPath—filepath.EvalSymlinksnot called; limited blast radius (analyzer only reads, sanitized env) - [MEDIUM] Blocked env var list gaps (AZURE_, GOOGLE_, API_KEY, SSH_AUTH_SOCK) — mitigated by allowlist-first design
- [MEDIUM] Windows-style path traversal not guarded — darwin-only currently
- 3 LOW findings omitted
Architect (APPROVE)
- [MEDIUM] Functional options pattern deviates from AP-001 Options struct convention — needs custom rule in go-custom.md
- [MEDIUM]
Validatefunction complexity CRAP 16.8 — decomposition opportunity (SHOULD-level) - 5 LOW findings omitted
Guard (APPROVE)
No MEDIUM+ findings. All tasks traced to implementation, all constitution principles satisfied.
- 3 LOW findings omitted
Testing (APPROVE)
All HIGH findings resolved in iteration 1 (floatEq refactored to assertFloatEq with t.Helper(), 3 untested functions now tested). All addressed MEDIUM findings resolved (edge case tests added, error assertions strengthened).
- 3 MEDIUM findings deferred (design.md docs, cycle enforcement, Registry concurrency) — acceptable for model layer scope
- 3 LOW findings omitted
SRE (APPROVE)
- [MEDIUM]
limitedBuffer.Writeswallows errors silently — could add writeErr field for observability - [MEDIUM] No CI workflow files — greenfield project, track as follow-up
- [MEDIUM]
go.modspecifies Go 1.25.7 withouttoolchaindirective - 5 LOW findings omitted
Curator (APPROVE)
All findings resolved — issue #18 filed for README/CHANGELOG, issue #19 for blog, AGENTS.md stale content fixed.
- 1 LOW finding omitted
Scribe (APPROVE)
- [MEDIUM]
Validatedoesn't checkschemaVersionvalue — spec-to-code gap - 7 LOW findings omitted
Envoy (APPROVE)
- [MEDIUM]
Capabilities()returns nil on all error paths — interface constraint, needs GoDoc clarification - 9 LOW findings omitted
Herald (APPROVE)
All findings resolved — issue #18 satisfies CHANGELOG documentation gate, issue #19 tracks blog.
- 3 LOW findings omitted
This review was generated by /review-council (AI-assisted).
- Refactor floatEq to assertFloatEq with t.Helper() for mechanical assertion tracing (Testing HIGH) - Add tests for WithEnvironment, SchemaJSON, JSONRPCError.Error() (Testing HIGH) - Add edge case tests for invalid metric inputs (Testing MEDIUM) - Strengthen error assertions in timeout/crash/size tests (Testing MEDIUM) - Remove stale placeholder comment from AGENTS.md (Curator MEDIUM) - Update architecture sentence in AGENTS.md (Scribe MEDIUM)
Security (Adversary): - Add filepath.EvalSymlinks to ValidateProjectPath for symlink traversal defense - Normalize path separators for cross-platform traversal detection - Expand blocked env var lists: AZURE_, ARM_, GCLOUD_, GOOGLE_APPLICATION_CREDENTIALS, API_KEY, CREDENTIALS, SSH_AUTH_SOCK, SSH_AGENT_PID, DATABASE_URL, REDIS_URL Architecture (Architect): - Document functional options AP-001 deviation as CR-001 in go-custom.md - Decompose Validate into validateTopLevel, validateModules, validateCycles, validateWarnings helpers to reduce cyclomatic complexity Documentation (Scribe/Envoy): - Add schemaVersion value validation against SchemaVersionCurrent - Add GoDoc clarifying Capabilities() nil return semantics on Adapter interface and ExternalAdapter Observability (SRE): - Add writeErr field to limitedBuffer for internal write failure tracking - Surface write errors in limitedBuffer.String() output - Add toolchain directive to go.mod for reproducible builds - Add CI workflow (.github/workflows/ci.yml) with build, vet, test, lint jobs Testing: - Add Registry concurrent read safety test - Add symlink resolution test for ValidateProjectPath - Add unsupported schema version test - Expand isBlockedEnv test table with new blocked patterns - Add canonical ordering enforcement comment to cycle test
The golangci-lint-action@v6 only supports golangci-lint v1.x, which was built with Go 1.24 and cannot lint Go 1.25.7 code. Action v7+ supports golangci-lint v2 (which is built with Go 1.25.x and matches our .golangci.yml version: "2" config).
golangci-lint v2 renamed the top-level 'linters-settings' key to 'linters.settings' (nested under linters). The previous config used the v1 key name despite declaring version: "2", causing config verification to fail with 'additional properties linters-settings not allowed'.
Summary
Implements the universal, language-agnostic coupling metrics model
(Layer 1) for vibe-check, addressing GitHub issue #1. This provides
the foundation that all language-specific analyzers will target.
Core capabilities:
LCOM4, circular dependency detection
support
security hardening (path validation, environment sanitization,
response size limits)
with structural validation
Key design decisions:
metricspackage (public API, not internal/)Moduleas universal unit of analysis (language-neutral)How to Test
Key acceptance scenarios verified by tests:
How to Demo
This is a model/interface implementation — there is no CLI or
end-to-end demo yet. The Go adapter (P0) and CLI (P1) will
consume these types.
Key Files Changed
metrics/openspec/changes/universal-coupling-model/.go.mod,.golangci.yml,AGENTS.md(project structure update).uf/dewey/learnings/Known Issues
The following findings from the review council were acknowledged
but deferred as follow-up work (greenfield project):
.github/workflows/) — automatedquality gate not yet configured
README.mdorCHANGELOG.md— initialproject documentation not yet created
This PR was generated by /uf.finale (AI-assisted).