Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
163 changes: 163 additions & 0 deletions docs/superpowers/plans/2026-08-29-v1.8.0-repairs.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,163 @@
# CodeGuard v1.8.0 Repairs Implementation Plan

> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.

**Goal:** Repair workspace resolution, baseline identity/accounting, installed version reporting, mutable-global precision, and contextual boolean naming for the v1.8.x line.

**Architecture:** Parse Go workspace/module manifests into per-file resolver metadata, replace many-to-many baseline lookup with a consuming tiered matcher, and separate detector declaration roles from reporting evidence. Keep public schemas and rule IDs stable while testing release metadata through the real command boundary.

**Tech Stack:** Go, Go AST/parser/token packages, `golang.org/x/mod/modfile`, table-driven tests, CLI integration tests.

**Spec:** `docs/superpowers/specs/2026-08-29-v1.8.0-repairs-design.md`

## Global Constraints

- Do not invoke `go list` or require network/module-cache state for import resolution.
- Preserve existing baseline serialization, CLI flags, report fields, and rule IDs.
- Context/content fingerprints are fallback match signals, never duplicate identity.
- Do not add a strict reassignable-global rule or configuration.
- Unknown helper calls alone are not mutation evidence.
- Imperative boolean vocabulary is grammatical and applies only to parameters.
- Version tests use synthetic `v9.8.7`; release automation validates the real tag.

---

### Task 1: Go workspace/module resolver

**Files:**
- Create: `internal/codeguard/checks/quality/quality_go_modules.go`
- Modify: `internal/codeguard/checks/quality/quality_ai_target_go.go`
- Test: `tests/checks/quality_ai_additional_test.go`

**Interfaces:**
- Produces: `newGoModuleResolver(root string) goModuleResolver`
- Produces: `(goModuleResolver).metadataForFile(rel string) goModuleMetadata`
- Produces: `(goModuleMetadata).resolvesImport(importPath string) bool`

- [ ] Write a failing fixture test with `go.work`, sibling/nested modules, direct and indirect requirements, module/workspace replacements, standard-library imports, a cross-workspace import, and one unresolved dotted import. Assert only the unresolved import produces `quality.ai.hallucinated-import`.
- [ ] Run `szr go test ./tests/checks -run 'TestGo.*Workspace.*Import' -count=1` and confirm valid imports are falsely reported.
- [ ] Implement manifest parsing with `modfile.Parse`/`modfile.ParseWork`, nearest-ancestor module ownership, module-prefix matching, and replacement/workspace module paths.
- [ ] Pass one resolver through `goAITargetFindings` and select metadata per file.
- [ ] Re-run the focused test and `szr go test ./tests/checks -run 'QualityAI' -count=1`.
- [ ] Commit with `fix: resolve Go imports by owning workspace module`.

### Task 2: Governance scan parity

**Files:**
- Modify: `internal/cli/baseline.go`
- Test: `internal/cli/baseline_go_workspace_test.go`

**Interfaces:**
- Modify: `governanceInputs` to retain scan mode/base-ref/target-path inputs needed by `service.RunWithOptions`.
- Consumes: the shared resolver through the normal runner.

- [ ] Write failing CLI tests that create a workspace baseline and exercise `baseline audit`, `baseline prune -write`, then `baseline prune -check`; assert valid module-specific imports do not become findings and write output immediately passes check.
- [ ] Run `szr go test ./internal/cli -run 'TestBaseline.*GoWorkspace' -count=1` and confirm the governance scan diverges.
- [ ] Route governance commands through the same normalized full-scan options/target configuration as ordinary scans.
- [ ] Re-run the focused CLI tests.
- [ ] Commit with `fix: preserve scan resolution in baseline governance`.

### Task 3: Deterministic one-to-one baseline matching

**Files:**
- Modify: `internal/cli/baseline_audit.go`
- Modify: `internal/cli/baseline.go`
- Test: `internal/cli/baseline_audit_test.go`
- Test: `internal/cli/baseline_io_test.go`

**Interfaces:**
- Create internal `baselineMatch` containing one entry index, one finding index, tier, and collision state.
- Create internal `matchBaselineEntries(entries []core.BaselineEntry, findings []core.Finding) baselineMatchResult`.

- [ ] Write failing tests proving exact matches consume first, context/content fallbacks use only unmatched records, one finding cannot activate two entries, and ambiguous matches are stable across input order.
- [ ] Write failing tests proving shared weak fingerprints are collisions but not duplicates and do not fail prune check semantics.
- [ ] Run `szr go test ./internal/cli -run 'TestAudit|TestPrune' -count=1` and record the expected count/duplicate failures.
- [ ] Implement sorted tiered matching and exact-only duplicate detection; retain separate weak collision diagnostics.
- [ ] Change prune check to reject only stale, invalid, and exact duplicate records.
- [ ] Re-run focused tests.
- [ ] Commit with `fix: consume baseline matches deterministically`.

### Task 4: Audit distribution invariants

**Files:**
- Modify: `internal/cli/baseline_audit.go`
- Test: `internal/cli/baseline_audit_test.go`

**Interfaces:**
- Consumes: canonical pairs from `matchBaselineEntries`.
- Produces: `Group` values whose count, confidence total, and language total share one input set.

- [ ] Add failing duplicate/collision-heavy tests that literally sum each `by_rule` confidence and language distribution and compare both with `Group.Count`.
- [ ] Run `szr go test ./internal/cli -run 'TestAudit.*Distribution' -count=1` and confirm the existing many-match samples overcount.
- [ ] Build groups from one canonical finding per active entry; apply sample limits only after distributions are calculated.
- [ ] Re-run focused audit tests.
- [ ] Commit with `fix: align baseline audit distributions`.

### Task 5: Installed/build version reporting

**Files:**
- Modify: `internal/version/version.go`
- Create: `internal/version/version_test.go`
- Test: `tests/cli/cli_test.go`
- Inspect: `.github/workflows/release.yml`

**Interfaces:**
- Preserve: `Resolve(current string, info *debug.BuildInfo) string`.
- Define: linker value precedence, main-module `v9.8.7` resolution, and `devel` fallback.

- [ ] Add failing table tests for injected `v9.8.7`, build-info `v9.8.7`, precedence, and absent metadata returning `devel` rather than `0.1.0`.
- [ ] Add a CLI integration test that runs `go build -ldflags '-X github.com/devr-tools/codeguard/internal/version.Number=v9.8.7'` on `./cmd/codeguard`, executes `version`, and expects exactly `v9.8.7`.
- [ ] Run `szr go test ./internal/version ./tests/cli -run 'Version' -count=1` and confirm the fallback failure.
- [ ] Replace the legacy fallback with `devel` while preserving linker and valid main-module precedence.
- [ ] Re-run focused version tests and inspect release workflow tag propagation.
- [ ] Commit with `fix: report installed module version`.

### Task 6: Evidence-based mutable globals

**Files:**
- Create: `internal/codeguard/checks/quality/quality_mutable_globals.go`
- Modify: `internal/codeguard/checks/quality/quality_precision.go`
- Test: `tests/checks/function_precision_test.go`

**Interfaces:**
- Define internal declaration classification and evidence kinds for mutable contents, reassignment, known mutation, and mutation-capable escape.
- Preserve public rule ID `quality.mutable-global-state`.

- [ ] Add failing Go fixture tests for `regexp.MustCompile` exemption, safe unknown read-only calls, binding reassignment, maps/slices, known mutating operations, writable-address escape, shared-storage assignment, returned/exported mutable state, and alias mutation.
- [ ] Run `szr go test ./tests/checks -run 'TestGoMutableGlobal' -count=1` and confirm immutable constructor false positives.
- [ ] Classify declarations independently from package-level evidence; include a narrow immutable-constructor registry and technical-reassignability metadata.
- [ ] Emit only for mutable shapes or meaningful mutation/escape evidence, with evidence-specific messages and calibrated confidence.
- [ ] Re-run focused tests and existing mutable-global precision tests.
- [ ] Commit with `fix: require mutation evidence for Go globals`.

### Task 7: Contextual imperative boolean names

**Files:**
- Modify: `internal/codeguard/checks/quality/quality_precision_workstreams_cd.go`
- Modify: `internal/codeguard/checks/quality/quality_precision.go`
- Test: `tests/checks/function_precision_test.go`
- Test: `tests/checks/quality_precision_retune_false_positive_test.go`

**Interfaces:**
- Create internal `isImperativeBooleanParameterName(name string) bool` based on word boundaries and prefixes `include`, `require`, `allow`, `skip`, `enable`, `disable`, `force`, and `use`.

- [ ] Add failing Go/C++ tests accepting imperative boolean parameters and rejecting the same grammar for fields, locals, getters, and boolean-returning functions.
- [ ] Run `szr go test ./tests/checks -run 'Test.*ImperativeBoolean' -count=1` and confirm parameters are reported.
- [ ] Apply imperative grammar only while iterating typed parameters; do not add the vocabulary to general predicate names.
- [ ] Re-run focused naming tests and all existing naming precision tests.
- [ ] Commit with `fix: recognize imperative boolean parameters`.

### Task 8: Full verification

**Files:**
- Modify only files needed to correct failures caused by Tasks 1-7.

**Interfaces:**
- Produces: a buildable branch satisfying the design spec without schema or policy expansion.

- [ ] Run `szr go test ./internal/cli ./internal/version ./tests/cli ./tests/checks -count=1`.
- [ ] Run `szr go test ./... -count=1`.
- [ ] Run the repository lint/static-check target discovered from `Makefile` or CI.
- [ ] Run `szr go build ./cmd/codeguard`.
- [ ] Review `szr git diff --check`, `szr git diff --stat`, and the final diff for scope and compatibility.
- [ ] Commit any verification-only corrections with a narrowly scoped message.
126 changes: 126 additions & 0 deletions docs/superpowers/specs/2026-08-29-v1.8.0-repairs-design.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
# CodeGuard v1.8.0 Repairs Design

## Scope

This repair addresses six v1.8.0 defects without adding a new policy surface:

1. Make Go import resolution workspace- and module-aware in normal scans, baseline audit, and baseline prune.
2. Make baseline matching deterministic and one-to-one while distinguishing exact duplicates from weaker-fingerprint collisions.
3. Report the installed module version from `codeguard version`, with release linker metadata taking precedence.
4. Improve `quality.mutable-global-state` precision through evidence-based mutation analysis.
5. Permit grammatical imperative names for boolean parameters while retaining predicate guidance in state and result contexts.
6. Keep audit `by_rule` totals consistent with confidence and language distributions after collision handling.

The baseline schema, CLI flags, report schema, and existing rule IDs remain compatible. A strict rule for technically reassignable globals is explicitly out of scope.

## Go Workspace and Module Resolution

CodeGuard will build module-resolution metadata once for a scan target and resolve it per Go source file. A file belongs to the nearest ancestor `go.mod`, so nested modules override parent modules. A containing `go.work` contributes all `use` modules and workspace-level `replace` directives.

The resolver will parse:

- each owning module's `module`, direct and indirect `require`, and `replace` directives;
- `go.work` `use` and `replace` directives;
- all workspace module paths; and
- standard-library import paths.

Resolution is based on module-prefix ownership. A required or replaced module `example.com/dependency` resolves imports beneath that prefix such as `example.com/dependency/client`; the full package import need not appear literally in `go.mod`. Imports belonging to another workspace module also resolve. Local-module imports resolve beneath the owning module path. Imports with no dotted first path segment are treated as standard-library imports.

The hallucinated-import detector consumes the metadata for the file's owning module instead of reading only `<target>/go.mod`. Normal scans and governance scans use the same runner and resolver. Baseline audit and prune must preserve the same scan options and target configuration used by ordinary full scans rather than creating a reduced dependency-resolution path.

No `go list` subprocess is introduced. This keeps scans deterministic, fast, and independent of network access, downloaded module caches, and command trust settings.

## Baseline Identity, Matching, and Diagnostics

An entry's exact fingerprint is its identity. Context and content fingerprints are weaker fallback signals only. Two records are true duplicates only when they have the same exact identity. Distinct exact fingerprints sharing context or content fingerprints are collisions, are preserved, and do not make `baseline prune -check` fail.

Matching is deterministic and one-to-one:

1. Match exact fingerprints first and consume each matched entry/finding pair.
2. Match context fingerprints only among unmatched entries and unmatched findings.
3. Match content fingerprints last, again only among unmatched entries and findings.
4. Sort entries and findings by stable keys before resolving ambiguous groups.
5. Pair ambiguous fallback groups deterministically and report the ambiguity as a collision diagnostic.

A current finding cannot activate more than one baseline entry. A baseline entry cannot consume more than one current finding for accounting. Collision diagnostics remain separate for audit visibility and describe baseline/current cardinality at the relevant matching tier.

Prune check fails only for stale entries, invalid entries, or true exact duplicates. Prune write preserves active distinct entries even when their weaker fingerprints collide. If a written baseline has no stale, invalid, or exact-duplicate entries, an immediate prune check must pass.

## Audit Accounting

Each active baseline entry receives at most one canonical matched finding from the one-to-one matcher. Group counts, samples, confidence distributions, and language distributions all derive from those same entry/finding pairs.

For every `by_rule` group:

- `count` equals the number of active matched entries in the group;
- the sum of confidence distribution counts equals `count`; and
- the sum of language distribution counts equals `count`.

Duplicate and collision-heavy inputs must preserve these invariants. Sample limits affect only displayed samples, never distribution calculation.

## Version Resolution

Version resolution uses this precedence:

1. a linker-injected release version;
2. `debug.ReadBuildInfo()` main-module version embedded by `go install module/cmd@version`; and
3. the explicit development fallback `devel`.

The historical hardcoded `0.1.0` value must not override valid linker or build metadata. Tests use a synthetic version such as `v9.8.7` so they remain independent of a particular release. The release pipeline separately verifies propagation of the actual release tag.

The integration test builds and executes the real `cmd/codeguard` command with synthetic linker metadata. Unit tests exercise module build information and precedence. Together they verify both supported release paths without network access.

## Mutable Global Precision

The existing `quality.mutable-global-state` rule becomes a two-phase analysis.

Declaration classification records:

- value shape: scalar, map, slice, pointer, immutable-by-convention object, or unknown;
- constructor origin where known; and
- whether the binding is technically reassignable.

Evidence analysis then looks for demonstrated harmful mutability:

- reassignment of the package-level binding;
- mutation of map, slice, pointer, or object contents;
- passing an address to a parameter capable of writing through it;
- assignment into mutable shared storage;
- returning or exporting mutable state;
- passing the value to a known mutating API; or
- later mutation through an alias.

Known immutable-by-convention constructor results, including `regexp.MustCompile`, are exempt by default unless meaningful mutation evidence exists. Go's required `var` syntax is not evidence by itself. Unknown helper calls and ordinary argument passing do not automatically count as mutation-capable escape. Uncertain evidence may be retained internally but does not produce a high-confidence finding.

Maps, slices, mutation-capable pointers, and demonstrated reassignment continue to produce findings. Internal evidence kinds distinguish mutable contents, reassigned bindings, and mutation-capable escapes so messages can be precise and the implementation can support a future strict rule without adding one now.

## Boolean Naming by Role

Boolean naming analysis will classify the symbol's role before applying vocabulary guidance.

Boolean parameters may use grammatical imperative prefixes such as `include`, `require`, `allow`, `skip`, `enable`, `disable`, `force`, and `use`, followed by a meaningful object or condition. Examples such as `includeInactive`, `requireCompleteMetadata`, `allowPartial`, and `skipCache` are representative rather than hardcoded exceptions.

The same names remain subject to predicate guidance when used as fields, stored local state, getters, or boolean-returning function names. Existing accepted predicate vocabulary continues to apply. Go and C++ fixtures cover both accepted imperative parameters and rejected state/result uses.

## Test Strategy

Implementation follows red-green-refactor cycles. Each behavior first receives a focused failing test that is run to confirm the expected failure before production code changes.

Workspace fixtures cover a root `go.work`, sibling and nested modules, module-specific direct and indirect requirements, module and workspace replacements, imports across workspace modules, standard-library imports, and a genuinely unresolved import. The fixtures run through normal scan, baseline audit, prune write, and prune check.

Baseline tests cover exact duplicates, shared context/content fingerprints, ambiguous fallback matches, stable one-to-one consumption, collision reporting, write-then-check idempotence, and report distribution invariants.

Detector tests cover immutable constructor exemptions, reassignment, mutable collections, known mutation, meaningful escape, unknown read-only calls, alias mutation, imperative boolean parameters, and non-parameter negative cases in both Go and C++.

Version tests cover linker precedence, synthetic main-module build information, development fallback, and execution of a built command. Final verification runs focused packages throughout development, followed by the complete Go test suite, repository static checks, and a clean command build.

## Compatibility and Non-Goals

This repair does not:

- add a strict reassignable-global rule;
- add configuration for such a rule;
- change baseline serialization or public report fields;
- treat weak-fingerprint collisions as duplicates;
- invoke Go dependency-resolution subprocesses; or
- weaken predicate guidance outside imperative parameter contexts.
Loading
Loading