diff --git a/docs/dev/0014-go-quality-audit.md b/docs/dev/0014-go-quality-audit.md index 5c15ae4..8314892 100644 --- a/docs/dev/0014-go-quality-audit.md +++ b/docs/dev/0014-go-quality-audit.md @@ -379,3 +379,18 @@ Total Go files audited: **27** (excluding `pkg/golist/testdata/sample-module/*.g Honest engagement note: per the methodology's flag-dissonance principle, the systemic citation-cleanup recommendation in S-1 includes the audit prompt's own `TD-09 / IM-04` mis-pair as evidence that the project has a recurring drift on substrate IDs. This is meta — the audit prompt recommended citing pattern IDs concretely, and I cited the prompt's own ID-drift back at it because the trend is the finding. CDC may rule that the prompt's drift is out of scope; I've surfaced it because the trending discipline asked me to. When CDC reviews: every per-row Verify is a `grep -n 'quoted-text' ` against the cited file:line, plus a re-read of the named substrate pattern ID. The protocol holds when CC's findings reproduce against an independent reading; my self-assessment of the report's verifiability is "high — every finding is anchored at file:line + verbatim quote." + +--- + +## Post-audit closure — PR-B (cousin-shape symmetry) + +Closes the two CONSIDER findings the 0015 work-order grouped as PR-B. Branch: `audit/follow-up-pr-b-cousin-shape`. Companion to PR-A (`audit/follow-up-pr-a-doc-pass`), which carries the closure walk for the other 14 findings + PROMPT-1. + +| ID | Disposition | Resolution + post-fix evidence | +|-------|---------------------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| F-5 | **closed in PR-B** | `internal/cli/errors.go`'s `*GitDiffError` now carries a `Dir string` field next to `Cmd []string`, mirroring `*golist.ExitError`'s shape exactly. `classifyGitDiffError`'s third parameter is `dir string` (was `_`); the caller at `internal/cli/cli.go` passes `cfg.root` which is now stored on `*GitDiffError.Dir` instead of being discarded. New assertion in `internal/cli/seam_test.go`'s `TestClassifyGitDiffError/exec_exit_error` verifies `ge.Dir == "/m"`. ✓ | +| F-14 | **closed in PR-B** | `gitDiffResult` (`internal/cli/seam.go`) grew an `argv []string` field; `defaultRunGitDiff` constructs argv once and uses it both for the actual `exec.CommandContext` invocation and as the returned diagnostic argv — they cannot drift. The synthetic argv reconstruction at `internal/cli/cli.go`'s `loadChangeSet` is removed; the call site now reads `classifyGitDiffError(res.err, res.argv, cfg.root, res.stderr, ctx)` and the comment names F-14. Tests that synthesize `gitDiffResult` without setting `argv` produce nil Cmd in the diagnostic — acceptable for the existing tests (none assert on Cmd); a future test that needs the argv sets it on the synthetic result. ✓ | + +`make check` (build + lint via `golangci-lint` + `go test -race -count=1 ./...`) passes against PR-B's working tree. + +When PR-A and PR-B both land on `main`, every audit finding is closed: 14 + PROMPT-1 via PR-A, F-5 + F-14 via PR-B. Net: 16 of 16 (plus the meta-finding) closed. No deferrals at exit. diff --git a/internal/cli/cli.go b/internal/cli/cli.go index f1c83c1..acb0019 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -266,8 +266,10 @@ func loadChangeSet(ctx context.Context, cfg *config, stdin io.Reader) ([]string, case "": res := runGitDiff(ctx, cfg.base, cfg.head, cfg.root) if res.err != nil { - argv := []string{"git", "diff", "--name-only", cfg.base + ".." + cfg.head} - return nil, classifyGitDiffError(res.err, argv, cfg.root, res.stderr, ctx) + // res.argv comes back from the seam itself, so the diagnostic + // chain reflects the actual exec invocation rather than a + // reconstruction (closes F-14 in 0014-go-quality-audit.md). + return nil, classifyGitDiffError(res.err, res.argv, cfg.root, res.stderr, ctx) } return scanLines(res.stdout) case "-": diff --git a/internal/cli/errors.go b/internal/cli/errors.go index b683e05..b0a7524 100644 --- a/internal/cli/errors.go +++ b/internal/cli/errors.go @@ -20,11 +20,16 @@ var ErrGitDiffFailed = errors.New("git diff failed") // Unwrap directly. // // The shape mirrors pkg/golist.ExitError so contributors who already know one -// know the other (EH-08 + EH-16). +// know the other (EH-08 + EH-16). The Dir field was added to close F-5 in +// docs/dev/0014-go-quality-audit.md, restoring full cousin-shape parity. type GitDiffError struct { // Cmd is the full argv as passed to exec, in order, for reproduction. Cmd []string + // Dir is the working directory the command was run in (typically the + // absolutized cfg.root from the cli layer). Mirrors *golist.ExitError.Dir. + Dir string + // ExitCode is the subprocess exit code (typically 1 or 128 for `git diff` // errors; may be other values). ExitCode int diff --git a/internal/cli/seam.go b/internal/cli/seam.go index 03a3d77..162e34d 100644 --- a/internal/cli/seam.go +++ b/internal/cli/seam.go @@ -12,12 +12,14 @@ import ( // gitDiffResult is the io seam's return shape: stdout (a Reader so tests can // supply bytes.Buffer / strings.Reader / etc.), the raw cmd.Run error (nil on // success; *exec.ExitError on non-zero; other errors for fork failures / -// cancellation / lookup failures), and the captured stderr text. Mirrors -// pkg/golist's runResult. +// cancellation / lookup failures), the captured stderr text, and the actual +// argv the seam used (so the cli layer's diagnostic doesn't drift from the +// real exec invocation; closes F-14). Mirrors pkg/golist's runResult. type gitDiffResult struct { stdout io.Reader err error stderr string + argv []string } // runGitDiff is the function-variable seam over the os/exec call. Production @@ -30,14 +32,18 @@ type gitDiffResult struct { var runGitDiff = defaultRunGitDiff // defaultRunGitDiff shells out to `git diff --name-only ..` in -// the configured working directory and captures stdout / stderr. +// the configured working directory and captures stdout / stderr. The argv +// it constructs is returned verbatim in the gitDiffResult so the cli layer's +// diagnostic chain (in *GitDiffError.Cmd) reflects the actual exec, not a +// reconstruction that could drift. // // gosec G204 (subprocess from variable) is suppressed: argv[0] is the literal // string "git" (a constant); argv[1:] is constructed from caller-supplied // refs that are documented as caller-controlled. The package contract makes // this explicit. func defaultRunGitDiff(ctx context.Context, base, head, dir string) gitDiffResult { - cmd := exec.CommandContext(ctx, "git", "diff", "--name-only", base+".."+head) //nolint:gosec + argv := []string{"git", "diff", "--name-only", base + ".." + head} + cmd := exec.CommandContext(ctx, argv[0], argv[1:]...) //nolint:gosec if dir != "" { cmd.Dir = dir } @@ -45,7 +51,7 @@ func defaultRunGitDiff(ctx context.Context, base, head, dir string) gitDiffResul cmd.Stdout = &stdout cmd.Stderr = &stderr err := cmd.Run() - return gitDiffResult{stdout: &stdout, err: err, stderr: stderr.String()} + return gitDiffResult{stdout: &stdout, err: err, stderr: stderr.String(), argv: argv} } // classifyGitDiffError converts an exec.Cmd.Run error into the appropriate @@ -66,7 +72,11 @@ func defaultRunGitDiff(ctx context.Context, base, head, dir string) gitDiffResul // 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 { +// +// dir is the working directory the seam ran the subprocess in (typically the +// absolutized cfg.root); stored on *GitDiffError.Dir for cousin-shape parity +// with *golist.ExitError (closes F-5 in docs/dev/0014-go-quality-audit.md). +func classifyGitDiffError(err error, argv []string, dir, stderr string, ctx context.Context) error { if ctxErr := ctx.Err(); ctxErr != nil { return fmt.Errorf("git diff: %w", ctxErr) } @@ -74,6 +84,7 @@ func classifyGitDiffError(err error, argv []string, _, stderr string, ctx contex if errors.As(err, &exitErr) { return &GitDiffError{ Cmd: append([]string(nil), argv...), + Dir: dir, ExitCode: exitErr.ExitCode(), Stderr: stderr, underlying: exitErr, diff --git a/internal/cli/seam_test.go b/internal/cli/seam_test.go index 45ee862..2c87264 100644 --- a/internal/cli/seam_test.go +++ b/internal/cli/seam_test.go @@ -264,6 +264,11 @@ func TestClassifyGitDiffError(t *testing.T) { if ge.Stderr != "fatal: bad ref\n" { t.Errorf("ge.Stderr = %q, want %q", ge.Stderr, "fatal: bad ref\n") } + // F-5: Dir is propagated onto *GitDiffError for cousin-shape parity + // with *golist.ExitError. + if ge.Dir != "/m" { + t.Errorf("ge.Dir = %q, want %q", ge.Dir, "/m") + } }) t.Run("other_exec_error", func(t *testing.T) {