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
15 changes: 15 additions & 0 deletions docs/dev/0014-go-quality-audit.md
Original file line number Diff line number Diff line change
Expand Up @@ -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' <file>` 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.
6 changes: 4 additions & 2 deletions internal/cli/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 "-":
Expand Down
7 changes: 6 additions & 1 deletion internal/cli/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
23 changes: 17 additions & 6 deletions internal/cli/seam.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -30,22 +32,26 @@ type gitDiffResult struct {
var runGitDiff = defaultRunGitDiff

// defaultRunGitDiff shells out to `git diff --name-only <base>..<head>` 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
}
var stdout, stderr bytes.Buffer
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
Expand All @@ -66,14 +72,19 @@ 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)
}
var exitErr *exec.ExitError
if errors.As(err, &exitErr) {
return &GitDiffError{
Cmd: append([]string(nil), argv...),
Dir: dir,
ExitCode: exitErr.ExitCode(),
Stderr: stderr,
underlying: exitErr,
Expand Down
5 changes: 5 additions & 0 deletions internal/cli/seam_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down