fix(foreman): scope-overlap check catches Go files in new directories#962
Open
Defilan wants to merge 2 commits into
Open
fix(foreman): scope-overlap check catches Go files in new directories#962Defilan wants to merge 2 commits into
Defilan wants to merge 2 commits into
Conversation
The scope-overlap check (defilantech#782) called `dirtyPathSet` which used `git status --porcelain` to enumerate changed files. However, at gate time the coder's edits are uncommitted and any new file is untracked, so `git status --porcelain` misses them. The gate runs as VerifyTerminal before the executor commits, so the check was blind to new untracked files. Mirror the fix applied to the reference-grounding check in defilantech#906: stage everything with `git add -A` and diff against HEAD with `git diff --name-only --cached`, which surfaces both staged and unstaged changes including untracked new files. `--src-prefix=a/` and `--dst-prefix=b/` are forced so the output is a clean path list, not unified-diff path-prefix output (the `c/`/`i/` gotcha). A new regression test (`TestCheckScopeOverlap_CatchesUntrackedNewFile`) confirms the scope-overlap check flags drift when the coder adds a new file outside the relevant set. Existing tests are updated to use the new `diffRunner` seam. Fixes defilantech#907 Signed-off-by: Foreman Bot <chris@mahercode.io>
… diff flags Review polish. The prior scope-overlap path (git status --porcelain) already saw untracked files in tracked dirs; the real gap it missed was a Go file in a brand-new untracked directory, which porcelain collapses to a single "newdir/" entry that fails the .go suffix filter. Point the regression test at that actual case and correct its comment. Also drop --src-prefix/--dst-prefix from the diff: they are inert with --name-only (the c/i prefix gotcha only affects unified-diff parsers). Refs defilantech#907 Signed-off-by: Christopher Maher <chris@mahercode.io>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
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
1 participant
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.
What
Fixes the coder gate's scope-overlap check so it sees a Go file created in a brand-new untracked directory.
Why
Fixes #907
The scope-overlap check enumerated the coder's changed Go files via
git status --porcelain. Porcelain lists untracked files in already-tracked directories fine, but it collapses a brand-new untracked directory to a singlenewdir/entry — which fails the.gosuffix filter, so an out-of-scope Go file created in a new directory slipped past the check.(Note: the original issue described this as the check "diffing committed history" / being "blind pre-commit." On inspection the pre-change code was
git status --porcelain, not abase...HEADdiff, so it did see most uncommitted changes — the real, narrower gap is the new-directory collapse above. The fix is the same either way.)How
Replace the porcelain scan with
git add -Afollowed bygit diff --name-only --cached HEAD, which lists each new file individually (including files in new directories). This mirrors the established working-tree-diff approach used by the reference-grounding check (#906).Safety of
git add -Ainside the gate: it runs pre-commit, but the executor's commit path (repo.Commit) runs its owngit add -Aimmediately beforegit commit -s, so the committed file set is identical regardless of what the gate staged — no junk inclusion, no interference. This is the same documented-safe pattern as #906.The regression test models the actual fixed case (a Go file in a new directory) and the diff drops two flags (
--src-prefix/--dst-prefix) that are inert with--name-only.Authored by Foreman, LLMKube's agentic coder harness (local model on the lab fleet); the maintainer adversarially reviewed the git-side-effect safety, corrected the diagnosis/test, and takes responsibility per the AI-assisted contribution policy.
Checklist
make testpasses locallymake lintpasses locally (+GOOS=linux golangci-lint, 0 issues)git commit -s) per DCO