audit follow-up PR-B: cousin-shape symmetry (F-5, F-14)#17
Merged
oubiwann merged 1 commit intoMay 8, 2026
Merged
Conversation
Per docs/dev/0015-audit-follow-up-plan-address-all-16-findings.md. F-5: *GitDiffError grows a Dir field, mirroring *golist.ExitError's shape. classifyGitDiffError's third parameter changes from `_` to `dir string`; the caller at internal/cli/cli.go passes cfg.root which is now stored on the diagnostic instead of discarded. Assertion added in TestClassifyGitDiffError/exec_exit_error verifies ge.Dir == "/m". F-14: gitDiffResult gains argv []string. defaultRunGitDiff constructs argv once and uses it both for exec.CommandContext and as the returned diagnostic argv — the two paths can no longer drift. The synthetic argv reconstruction at cli.go's loadChangeSet (the dead-letter that the audit flagged) is removed; the call site now reads res.argv. Tests that synthesize gitDiffResult without setting argv produce nil Cmd in the diagnostic; acceptable for the existing tests (none assert on Cmd) and a future test that needs argv sets it on the synthetic. Audit doc 0014 grows a "Post-audit closure — PR-B" section at the bottom (separate from PR-A's closure walk to avoid merge conflict). make check passes: build + lint (golangci-lint clean) + test (go test -race -count=1 ./...) all green. Closes the last 2 of 16 audit findings + 1 meta-finding (PROMPT-1) when both PR-A and PR-B land on main.
6a2ef77 to
5e2d07c
Compare
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes the two CONSIDER findings the 0015 plan grouped as PR-B — both about the diagnostic chain matching the actual
git diffexec invocation. Single commit per the plan's PR-B scope.Findings closed
F-5 —
*GitDiffErrorcousin-shape parity*GitDiffError(internal/cli/errors.go) now carries aDir stringfield next toCmd []string, mirroring*golist.ExitError's shape exactly.classifyGitDiffError's third parameter isdir string(was_); the caller atinternal/cli/cli.gopassescfg.rootwhich is now stored on*GitDiffError.Dirinstead of being discarded.Test addition:
TestClassifyGitDiffError/exec_exit_errorininternal/cli/seam_test.gonow assertsge.Dir == "/m".F-14 — return argv from the git-diff seam
gitDiffResult(internal/cli/seam.go) grew anargv []stringfield.defaultRunGitDiffconstructs argv once and uses it both for the actualexec.CommandContextinvocation and as the returned diagnostic argv — the two paths can no longer drift. The synthetic argv reconstruction atinternal/cli/cli.go'sloadChangeSetis removed; the call site now readsclassifyGitDiffError(res.err, res.argv, cfg.root, res.stderr, ctx).Tests that synthesize
gitDiffResultwithout settingargvproducenilCmd in the diagnostic — acceptable for the existing tests (none assert on Cmd); a future test that needs argv just sets it on the synthetic result.Diff scope
5 files modified, 47 insertions / 9 deletions. Plan estimated ~30 lines.
internal/cli/errors.go+ Dir stringfield on*GitDiffError+ docinternal/cli/seam.go+ argv []stringongitDiffResult;defaultRunGitDiffreturns argv;classifyGitDiffErrortakesdir stringand stores itinternal/cli/cli.gores.argvinternal/cli/seam_test.goge.Dir == "/m"assertiondocs/dev/0014-go-quality-audit.mdTest plan
make check(build + lint viagolangci-lint+go test -race -count=1 ./...)*GitDiffErrorshape matches*golist.ExitError(grep -n 'Dir ' internal/cli/errors.go pkg/golist/errors.go)cli.go:269is gone (grep -n 'git diff --name-only' internal/cli/cli.go— should only show the doc / help text, not a constructed argv)docs/dev/0014-go-quality-audit.mdStacking
Base:
audit/0014-go-quality-audit(PR #15). After PR #15 merges, this PR's base can be retargeted tomain.Parallel to PR-A (PR #16, doc pass closing 14 findings + PROMPT-1). PR-A and PR-B do not conflict in code (different files / different lines); each updates
docs/dev/0014-go-quality-audit.mdat a different anchor (PR-A above the existing## Closureheading; PR-B at the file end) so the doc edits don't conflict either.🤖 Generated with Claude Code