audit follow-up PR-A: documentation pass (14 findings)

CLAUDE.md

@@ -91,6 +91,8 @@ Algorithmic packages. **No io, no syscalls, no `os/exec`** between them. Inputs
 
 These three packages are **gated at 100% statement coverage in CI** by `scripts/coverage-check.sh`. The gate skips packages with no coverage data ("N/A"), so empty pre-implementation stubs don't fire it; once a package gets code, the gate fires immediately.
 
+**Layout deviation note (PS-06).** The substrate's `assets/ai/go/guides/10-project-structure.md` flags `pkg/` as `SHOULD-AVOID` — a community convention rather than a Uber/Google rule, but one the Go standard library and most idiomatic projects don't follow. Cascade adopts `pkg/` deliberately: it makes the public OSS surface explicit, pairs cleanly with `internal/`'s language-enforced privacy boundary, and keeps the substrate of "what's importable by downstreams" obvious at a glance. The deviation is acknowledged in the M2-M5 audit (`docs/dev/0014-go-quality-audit.md` finding F-12) as **Acknowledged-with-rationale**. **Carry-forward:** revisit at v1.0 if a flatter layout becomes feasible without breaking downstream consumers' import paths.
+
 ### Private support — module-internal (`internal/project/`)
 
 Build-metadata package; carries cascade-specific Version / GitCommit / GitBranch / BuildDate values populated via `-ldflags` at link time, with a `runtime/debug.ReadBuildInfo` fallback for `go install`-built binaries. The pattern is generic but the values are cascade-specific; Go's `internal/` rule structurally enforces that downstream consumers cannot import this package. Also gated at 100% coverage by `scripts/coverage-check.sh`.

docs/dev/0014-go-quality-audit.md

@@ -340,6 +340,36 @@ None of this blocks M6. The audit's findings are quality-of-life improvements an
 
 Five-for-five on the CC self-check protocol catching zero softpedals across milestones; six-for-six now if you count this audit's discipline (the auditor independently re-derived findings from substrate, didn't re-litigate retro-disclosed deviations, surfaced cite drift back at the prompt). The methodology is paying compound interest.
 
+## Post-audit closure walk (per-finding disposition)
+
+Disposition for each finding per the work order in [`docs/dev/0015-audit-follow-up-plan-address-all-16-findings.md`](./0015-audit-follow-up-plan-address-all-16-findings.md). Each row names the resolution path and the post-fix file:line evidence so CDC can re-verify against the working tree.
+
+| ID       | Disposition                              | Resolution + post-fix evidence                                                                                                                                                                                                                                          |
+|----------|-------------------------------------------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
+| F-1      | **closed in PR-A**                        | `pkg/golist/errors.go:16` now reads `// against Error() output is forbidden (AP-13).` ✓                                                                                                                                                                                |
+| F-2      | **closed in PR-A**                        | `pkg/golist/golist.go:204-211` rewritten: cite changed from `EH-08` to `CC-08` and rationale strengthened to make the post-hoc-vs-propagation distinction explicit (also closes F-4 for this site). ✓                                                                  |
+| F-3      | **closed in PR-A**                        | `internal/cli/seam.go:62-69` rewritten: cite changed from `EH-08` to `CC-08` and rationale mirrors the strengthened pkg/golist version (also closes F-4 for this site). ✓                                                                                              |
+| F-4      | **closed in PR-A** (rationale-strengthen) | The "post-hoc consultation, not propagation" rationale now appears in both classifier comments with an explicit CC-08 cite and the cousin-shape lock-in note. PR-C (substrate-align by moving ctx to first param) declined per 0015 work-order recommendation.        |
+| F-5      | **deferred to PR-B**                      | Add `Dir string` field to `*GitDiffError`; populate from `cfg.root` at `internal/cli/cli.go:270`; update `internal/cli/seam_test.go` test to assert the new field.                                                                                                     |
+| F-6      | **closed in PR-A**                        | `// Parallel-unsafe:` comment block added at the top of `internal/cli/seam_test.go`, `pkg/golist/seam_test.go`, `pkg/changeset/seam_test.go`, and `internal/project/version_test.go` (each names which package-level vars the file mutates and points to S-2). ✓     |
+| F-7      | **closed in PR-A** (decline-with-cross-reference) | `internal/cli/cli_test.go:20-28` carries a 4-line comment cross-referencing `internal/project/version_test.go`'s `withMetadata` helper as the canonical pattern, plus a parallel-unsafe note. ✓                                                                       |
+| F-8      | **closed in PR-A**                        | `internal/cli/helpers_test.go` deleted; `writeFile` in `internal/cli/cli_test.go` inlines the body (`return os.WriteFile(path, []byte(content), 0o600)`); `os` import added to `cli_test.go`. ✓                                                                       |
+| F-9      | **closed in PR-A**                        | `pkg/golist/golist_test.go:99-122` now uses a `[]struct{ name string; patterns []string }` table with rows `{"nil", nil}` and `{"empty", []string{}}` — the inline-map subtest-name lookup is gone. ✓                                                                  |
+| F-10     | **closed in PR-A**                        | `pkg/changeset/changeset.go:65-75` doc rewritten to describe the actual post-bug-#12 behaviour: getCwd-error leaves `moduleRoot` empty at the fallback step, but `filepath.Abs("")` resolves it to the cwd anyway. The doc now matches the code. ✓                    |
+| F-11     | **closed in PR-A**                        | `pkg/changeset/changeset.go:113-118` internal comment rewritten to match the corrected doc. The "leave as-is" claim is replaced with "the empty value is replaced by the process cwd here, not 'left as-is.'" ✓                                                       |
+| F-12     | **closed in PR-A** (rationale + carry-forward) | `CLAUDE.md`'s Architecture / "Pure core" subsection grew a "Layout deviation note (PS-06)" paragraph stating the deviation is acknowledged-with-rationale and naming v1.0 as the carry-forward window. The deviation itself remains accepted. ✓                       |
+| F-13     | **closed in PR-A**                        | `pkg/golist/golist.go` block at former lines 238-241 collapsed: the `_ = io.EOF` placeholder and its justification comment are removed; the surviving block keeps the three useful compile-time assertions for `*ExitError`, `*ParseError`, and `Option`. ✓           |
+| F-14     | **deferred to PR-B**                      | Return argv from the git-diff seam (extend `gitDiffResult` with `argv []string`); drop the synthetic argv construction at `internal/cli/cli.go:269`; update tests.                                                                                                     |
+| F-15     | **closed in PR-A** (decline + document)   | `pkg/changeset/changeset.go:18-29` config struct's `moduleRootSet` field now carries a doc comment explaining why the flag exists post-bug-#12 (test-driven design choice for the os.Getwd-error fallback path). The flag itself stays. ✓                              |
+| F-16     | **closed in PR-A** (rationale-strengthen) | `internal/project/version.go:54-71` var-block doc grew an explicit "AP-07 deviation (acknowledged)" paragraph naming the link-time-vs-runtime distinction, the kubectl/hugo/cobra precedent, and the no-`t.Parallel()` test discipline (paired with F-6 / S-2). ✓     |
+| PROMPT-1 | **closed in PR-A**                        | `workbench/cc-audit-prompt-go-quality.md`'s "Receiver consistency" line now reads `IM-12 / TD-09` (was `TD-09 / IM-04`). The line carries a parenthetical pointing future-runners at the M3-retro / 0015 lineage so the drift's history is traceable. ✓               |
+
+**S-1 (citation drift): closed.** All three concrete instances are corrected (F-1, F-2, F-3) plus the meta-instance (PROMPT-1). The systemic recommendation (a write-time discipline or pre-commit hook) remains a future-work item; logged but unscheduled.
+
+**S-2 (seam pattern / no-`t.Parallel()` discipline): closed in PR-A** at the documentation layer per 0015's plan. The structural enforcement option (custom `go vet`-style linter) is logged as v1.0+ future work.
+
+**Net post-PR-A state:** 14 of 16 findings closed (plus PROMPT-1). 2 deferred to PR-B (F-5, F-14, both CONSIDER). 0 still open at exit. The `make check` bundle (build + lint + test, with `-race -count=1`) passes against the PR-A working tree.
+
 ## Closure
 
 Audit run complete. Total findings: 16. Open: 13. Acknowledged-with-rationale: 3 (F-4 ctx-as-last-param; F-12 `pkg/` layout; F-16 mutable build-info vars). CDC review **complete** — every spot-checked claim reproduces against the cited file:line; severity assignments calibrated; methodology adherence exemplary; closure recommendation: ready to consume, follow-up PRs grouped above.

internal/cli/cli_test.go

@@ -2,6 +2,7 @@ package cli_test
 
 import (
 	"bytes"
+	"os"
 	"path/filepath"
 	"strings"
 	"testing"
@@ -18,6 +19,11 @@ import (
 // prints the project metadata to stdout and exits 0. Mirrors the M1
 // in-process version test that previously lived in cmd/cascade/main_test.go.
 func TestRun_Version(t *testing.T) {
+	// The save-restore dance below mirrors internal/project/version_test.go's
+	// withMetadata helper; we reproduce it inline here because withMetadata
+	// is unexported and adding a public test-only API just for one consumer
+	// is not worth the surface. See F-7 in docs/dev/0014-go-quality-audit.md.
+	// Parallel-unsafe (mutates package-level project.* vars; cousin of S-2).
 	saveVersion, saveCommit, saveBranch, saveSummary, saveDate :=
 		project.Version, project.GitCommit, project.GitBranch, project.GitSummary, project.BuildDate
 	t.Cleanup(func() {
@@ -203,5 +209,5 @@ func TestRun_FileChangedFiles_OpenFailure(t *testing.T) {
 
 // writeFile is a tiny test helper that writes content to path.
 func writeFile(path, content string) error {
-	return writeAll(path, []byte(content))
+	return os.WriteFile(path, []byte(content), 0o600)
 }

internal/cli/seam.go

@@ -59,9 +59,13 @@ func defaultRunGitDiff(ctx context.Context, base, head, dir string) gitDiffResul
 //   - other exec errors (rare; e.g. permission denied on git binary, or git
 //     not on PATH): wrapped under ErrGitDiffFailed for category clarity.
 //
-// ctx is the last param (against EH-08 ordering convention) because it's only
-// used to detect cancellation post-hoc, not for propagation. Mirrors
-// pkg/golist's classifyRunError.
+// ctx is the last param, deliberately deviating from CC-08 (ctx is the
+// first parameter). The CC-08 MUST is calibrated to *propagation* — a
+// function that passes ctx forward into a blocking call. This classifier
+// consults ctx.Err() exactly once, post-hoc, to disambiguate "is this
+// error a cancellation?" and never propagates ctx anywhere else; that is
+// outside CC-08's intended scope. Mirrors pkg/golist's classifyRunError so
+// readers who learn one know the other.
 func classifyGitDiffError(err error, argv []string, _, stderr string, ctx context.Context) error {
 	if ctxErr := ctx.Err(); ctxErr != nil {
 		return fmt.Errorf("git diff: %w", ctxErr)

internal/cli/seam_test.go

@@ -15,6 +15,10 @@ import (
 	"github.com/geomyidia/cascade/pkg/golist"
 )
 
+// Parallel-unsafe: tests in this file mutate package-level seam variables
+// (runGitDiff, runGoListWrapper, signalContext). Do not call t.Parallel()
+// in any test that exercises them. See S-2 in docs/dev/0014-go-quality-audit.md.
+
 // withGitDiffSeam swaps the package-level runGitDiff for the duration of a
 // test, restoring the default afterward. Mirrors pkg/golist's withSeam.
 func withGitDiffSeam(t *testing.T, fn func(ctx context.Context, base, head, dir string) gitDiffResult) {

internal/project/version.go

@@ -51,6 +51,21 @@ var versionFile string
 // (GitCommit, BuildDate). GitBranch and GitSummary stay empty under the
 // fallback path — Go's build-info doesn't carry branch names, and a
 // synthesised git-describe could diverge from the Makefile representation.
+//
+// AP-07 deviation (acknowledged): these are exported package-level
+// **mutable** vars. The substrate's AP-07 ("Mutable Package-Level
+// Globals") is SHOULD-AVOID. Rationale for keeping them: cascade uses
+// them as **link-time injection targets** for `-ldflags -X`, populated
+// once before init() runs and never mutated thereafter in production.
+// They are the standard Go pattern for build-info embedding (also seen
+// in `kubectl`, `hugo`, every cobra-based CLI). AP-07's hazards
+// (order-dependent tests, multi-tenancy collisions) don't apply because
+// these aren't config or registries — they're constants whose value is
+// set at link time. Tests in this package and `internal/cli/cli_test.go`
+// do mutate them (via the withMetadata helper in version_test.go); the
+// "do not call t.Parallel() in any test that mutates these" discipline
+// is documented in S-2 of docs/dev/0014-go-quality-audit.md and at the
+// top of every seam-using *_test.go file.
 var (
 	// Version is the cascade module version, sourced from project/VERSION.
 	// Always populated (either via ldflags or the go:embed fallback).

internal/project/version_test.go

@@ -8,6 +8,11 @@ import (
 	"testing"
 )
 
+// Parallel-unsafe: tests in this file mutate the package-level build-info
+// vars (Version, GitCommit, GitBranch, GitSummary, BuildDate) and the
+// readBuildInfo seam. Do not call t.Parallel() in any test that uses
+// withMetadata. See S-2 in docs/dev/0014-go-quality-audit.md.
+
 // withMetadata sets the package-level build vars for the duration of the
 // test and restores them afterward. Tests use this to exercise both the
 // empty-default path and the populated-via-ldflags path without relying on

pkg/changeset/changeset.go

@@ -16,7 +16,15 @@ type Option func(*config)
 // config holds the resolved option state for a single Resolve call.
 // Unexported: callers compose configuration only via With* constructors.
 type config struct {
-	moduleRoot    string
+	moduleRoot string
+
+	// moduleRootSet distinguishes "user explicitly supplied WithModuleRoot"
+	// from "no option supplied." Post-bug-#12 the user-visible behaviour is
+	// identical for both (filepath.Abs("") and filepath.Abs(".") both
+	// resolve to the cwd), so the flag exists primarily so seam_test.go
+	// can drive the os.Getwd-error fallback branch deterministically.
+	// Documented as a deliberate test-driven design choice in F-15 of
+	// docs/dev/0014-go-quality-audit.md.
 	moduleRootSet bool
 }
 
@@ -64,8 +72,15 @@ var getCwd = os.Getwd
 //
 // opts configure the call. The only option in v0.x is WithModuleRoot; if
 // not supplied, Resolve falls back to os.Getwd. If os.Getwd itself returns
-// an error (rare), moduleRoot stays empty and relative paths silently fail
-// to resolve — same outcome as passing WithModuleRoot("").
+// an error (rare), moduleRoot is left as the zero value at the fallback
+// step; the subsequent filepath.Abs("") then resolves it to the process
+// cwd anyway, so relative changedFiles are interpreted against the cwd —
+// identical to passing WithModuleRoot("") or WithModuleRoot(".") (closes
+// bug #12). If the absolutized cwd doesn't share a prefix with any
+// pkg.Dir, the parent-directory lookup returns no matches and Resolve
+// produces an empty result — the same observable outcome the original
+// "stays empty" doc claimed, but reached through filepath.Abs's fallback
+// rather than the empty string flowing untouched.
 //
 // Mapping rules:
 //   - A file ending in ".go" whose parent directory exactly matches some
@@ -113,9 +128,11 @@ func Resolve(changedFiles []string, pkgs []golist.Package, opts ...Option) []str
 	// (which golist documents as absolute) succeeds even when the caller
 	// supplies a relative path. filepath.Abs("") and filepath.Abs(".")
 	// both resolve to the cwd, so this also handles the empty-string case
-	// gracefully. On error (rare; documented in os.Getwd), leave as-is —
-	// the lookup will fail consistently with the unfixed relative-input
-	// case rather than panic. Closes bug #12.
+	// (including the getCwd-error fallback at line 107) gracefully — the
+	// empty value is replaced by the process cwd here, not "left as-is."
+	// If filepath.Abs itself errors (rare; documented in path/filepath),
+	// the lookup falls back to the unmodified value and fails consistently
+	// rather than panicking. Closes bug #12.
 	if abs, err := filepath.Abs(cfg.moduleRoot); err == nil {
 		cfg.moduleRoot = abs
 	}

pkg/changeset/seam_test.go

@@ -7,6 +7,10 @@ import (
 	"github.com/geomyidia/cascade/pkg/golist"
 )
 
+// Parallel-unsafe: tests in this file mutate the package-level getCwd
+// seam. Do not call t.Parallel() in any test that exercises it. See S-2
+// in docs/dev/0014-go-quality-audit.md.
+
 // withCwdSeam swaps the package-level getCwd for the duration of a test,
 // restoring the default afterward. Mirrors pkg/golist's withSeam helper.
 func withCwdSeam(t *testing.T, fn func() (string, error)) {

pkg/golist/errors.go

@@ -13,7 +13,7 @@ import (
 //
 // These follow the EH-36 sentinel naming convention (ErrXxx) and are
 // the only mechanism callers should use to classify errors — string-match
-// against Error() output is forbidden (AP-02).
+// against Error() output is forbidden (AP-13).
 var (
 	// ErrGoNotFound is returned when the `go` binary cannot be found on
 	// $PATH (or at the path configured via WithGoBin). The wrapped error

pkg/golist/golist.go

@@ -200,8 +200,13 @@ func buildArgv(goBin string, tags []string, patterns []string) []string {
 //   - other exec errors (rare): returned wrapped with %w under
 //     ErrGoListFailed for category clarity.
 //
-// ctx is the last param (against EH-08 ordering convention) because it's
-// only used to detect cancellation post-hoc, not for propagation.
+// ctx is the last param, deliberately deviating from CC-08 (ctx is the
+// first parameter). The CC-08 MUST is calibrated to *propagation* — a
+// function that passes ctx forward into a blocking call. This classifier
+// consults ctx.Err() exactly once, post-hoc, to disambiguate "is this
+// error a cancellation?" and never propagates ctx anywhere else; that is
+// outside CC-08's intended scope. Cousin classifyGitDiffError keeps the
+// same shape so readers who learn one know the other.
 func classifyRunError(err error, argv []string, dir string, stderr string, ctx context.Context) error {
 	if ctxErr := ctx.Err(); ctxErr != nil {
 		return fmt.Errorf("go list: %w", ctxErr)
@@ -235,8 +240,4 @@ var (
 	_ error  = (*ExitError)(nil)
 	_ error  = (*ParseError)(nil)
 	_ Option = WithDir("")
-	// Reference io to keep the import alive for future use; the streaming
-	// decoder lives in parse.go and consumes this package's io types via
-	// the parseStream helper.
-	_ = io.EOF
 )