Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…nt files the unborn-HEAD change-set scoop (3781312, #11605) unions 'ls-files --others --exclude-standard' into changedFiles, but a fresh scaffold that runs npm install BEFORE writing .gitignore has thousands of untracked node_modules paths (--exclude-standard has nothing to honor yet). those flooded the 60-file cap, and because agent-written tool paths were spread LAST (Set dedupe keeps first occurrence), the flood evicted the agent's real files: 'what did you change' answered with node_modules noise and diffs rendered junk. - filter vendor/build dirs (node_modules, .venv, dist, ...) from the unborn-HEAD untracked scoop only; born-HEAD never scoops untracked and explicit tool-written paths are always kept - spread agentWritten first so explicit edit/write tool calls survive the MAX_CHANGED_FILES cap - drop the truncated garbage tail line when the ls-files listing was cut at maxBuffer (ENOBUFS)
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
2 participants
What
captureChangeSet's unborn-HEAD untracked scoop (introduced in 3781312, #11605 FIX C) could flood the 60-fileMAX_CHANGED_FILEScap and evict the agent's real files from the change set.Failure path (reproduced with a failing test before the fix): fresh repo (
git initthis session, zero commits), agent scaffolds an app and runsnpm installBEFORE writing.gitignore.ls-files --others --exclude-standardhas no.gitignoreto honor, so it returns thousands ofnode_modulespaths.changedFiles = [...tracked, ...untracked, ...agentWritten].slice(0, 60)spread the highest-signalagentWrittenentries LAST — andSetdedupe keeps first-occurrence position — so 60node_modulespaths evicted the agent'sindex.html/server.jsentirely.diffStatand per-file diffs rendered junk, and "what did you change" answered withnode_modulesnoise.Red run against the unfixed code (exact defect signature):
Fix (structural)
node_modules,.git,.venv,dist,build, etc. are dropped from thels-files --otherslisting. This is a fallback for the missing-.gitignorewindow only: the born-HEAD path never scoops untracked files (clutter invariant preserved), and explicit tool-written paths are always kept regardless (unioned separately viaagentWritten).agentWrittenspread FIRST into the changed-files union, so explicit edit/write tool calls can never be evicted by the cap. Set dedupe keeps first-occurrence order, so leading with them is the durable position.parseLsFiles— a listing cut atmaxBuffer(ENOBUFS on a huge untracked tree) ends mid-path instead of with a newline; the partial garbage line is dropped instead of surfacing as a phantom changed file.Tests
Three new tests in
__tests__/unit/workspace-diff.test.ts(real git repos, no mocks):filters vendor install output that predates any .gitignore— 120-packagenode_modules+ 3 real app files on an unborn HEAD; asserts all 3 real files survive and zeronode_modulespaths appear (failed before:index.html+ 59node_modules,package.json/server.jsevicted).agent-written files survive the cap ahead of a large scaffold— 70 untracked files + 1 tool-written file sorting last; asserts the tool-written file leads the list (failed before: evicted at position 71).parseLsFiles drops the truncated garbage tail of an over-maxBuffer listing.Verification (real output)
bunx vitest run __tests__/unit/workspace-diff.test.ts→ 24 passed (21 pre-existing + 3 new)bunx vitest run --config vitest.config.ts→ 150 files passed | 4 skipped, 1577 tests passed | 8 skipped, 0 failedbun run typecheck(tsgo) → cleanAll existing invariants pinned by the suite hold: born-HEAD clutter exclusion, baseline-dirty exclusion, gitignore honoring, tool-path relativization/escape rejection.
Refs 3781312 (#11605), #11578.