Several review findings recur across unrelated pull requests. They are not caught by CI, and AGENTS.md does not currently say anything that would prevent them, so each one is rediscovered by a reviewer and costs a round trip.
Host tool version floors. The clearest example. Zero shells out to git, and no minimum version is declared anywhere: not in README.md, CONTRIBUTING.md, AGENTS.md, or docs/. Adding a modern flag therefore raises the floor silently, and an older host fails with a usage error rather than a clear message. --end-of-options (Git 2.24) was raised on #671; --show-current (2.22) is already referenced in internal/agent/command_prefix.go; branch.autoSetupMerge=inherit (2.35) needed a version gate in a test. Nothing tells a contributor to check.
Tests that never reach the behavior they name. The recurring problem is not a missing test but one that passes with and without the fix, because an earlier guard rejects the input first. Recent cases: a staging-privacy test where the containment check fired first and returned a different error the assertion still accepted; an intermediate-symlink test that only exercised the outer containment pre-check and never the handle-relative walker it was named for. AGENTS.md requires a regression test but does not require proving it fails without the change.
Tests touching real user state. Several plan-mode tests read and wrote the developer's actual user config directory because the isolation helper was applied to some tests and not others reaching the same storage. On Windows this needs %AppData% set as well, since os.UserConfigDir ignores XDG_CONFIG_HOME there.
Platform-specific error codes. AGENTS.md covers path canonicalization across platforms but not error codes. A no-follow guard checked only ELOOP, while openat(O_DIRECTORY|O_NOFOLLOW) on a symlink returns ENOTDIR on Linux and Darwin, so the guard was inert on both and the refusal surfaced as an unrelated error. Windows has the same shape: \\?\ and \\.\ are not UNC prefixes, and NtCreateFile information classes must match the struct passed.
Proposed fix: extend AGENTS.md section 4 ("Common Review Blockers") with these four classes.
Separately, and worth a maintainer decision rather than a doc change: should the project declare a minimum supported Git version? The guidance above tells contributors to state and gate a new flag's floor, but there is no baseline to compare against. Declaring one (and noting it in README.md or CONTRIBUTING.md) would make the rule checkable instead of a judgment call.
Several review findings recur across unrelated pull requests. They are not caught by CI, and
AGENTS.mddoes not currently say anything that would prevent them, so each one is rediscovered by a reviewer and costs a round trip.Host tool version floors. The clearest example. Zero shells out to
git, and no minimum version is declared anywhere: not inREADME.md,CONTRIBUTING.md,AGENTS.md, ordocs/. Adding a modern flag therefore raises the floor silently, and an older host fails with a usage error rather than a clear message.--end-of-options(Git 2.24) was raised on #671;--show-current(2.22) is already referenced ininternal/agent/command_prefix.go;branch.autoSetupMerge=inherit(2.35) needed a version gate in a test. Nothing tells a contributor to check.Tests that never reach the behavior they name. The recurring problem is not a missing test but one that passes with and without the fix, because an earlier guard rejects the input first. Recent cases: a staging-privacy test where the containment check fired first and returned a different error the assertion still accepted; an intermediate-symlink test that only exercised the outer containment pre-check and never the handle-relative walker it was named for.
AGENTS.mdrequires a regression test but does not require proving it fails without the change.Tests touching real user state. Several plan-mode tests read and wrote the developer's actual user config directory because the isolation helper was applied to some tests and not others reaching the same storage. On Windows this needs
%AppData%set as well, sinceos.UserConfigDirignoresXDG_CONFIG_HOMEthere.Platform-specific error codes.
AGENTS.mdcovers path canonicalization across platforms but not error codes. A no-follow guard checked onlyELOOP, whileopenat(O_DIRECTORY|O_NOFOLLOW)on a symlink returnsENOTDIRon Linux and Darwin, so the guard was inert on both and the refusal surfaced as an unrelated error. Windows has the same shape:\\?\and\\.\are not UNC prefixes, andNtCreateFileinformation classes must match the struct passed.Proposed fix: extend
AGENTS.mdsection 4 ("Common Review Blockers") with these four classes.Separately, and worth a maintainer decision rather than a doc change: should the project declare a minimum supported Git version? The guidance above tells contributors to state and gate a new flag's floor, but there is no baseline to compare against. Declaring one (and noting it in
README.mdorCONTRIBUTING.md) would make the rule checkable instead of a judgment call.