fix: add directory guard in formatUntrackedFile to prevent EISDIR crash#48
fix: add directory guard in formatUntrackedFile to prevent EISDIR crash#48stoneHee99 wants to merge 3 commits intoopenai:mainfrom
Conversation
…penai#14) Add a stat.isDirectory() guard before reading untracked file contents, so working-tree reviews no longer crash when untracked directories exist.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 407b3ab2c7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
tests/git.test.mjs
Outdated
| fs.mkdirSync(path.join(cwd, "untracked-dir")); | ||
| fs.writeFileSync(path.join(cwd, "untracked-dir", "nested.js"), "// nested\n"); |
There was a problem hiding this comment.
This setup does not exercise the new stat.isDirectory() branch: with git ls-files --others --exclude-standard, a normal untracked folder containing files is reported as file paths (e.g. untracked-dir/nested.js), not as a directory entry (git ls-files -h documents --directory as the flag that changes this behavior). Because the test creates untracked-dir/nested.js, it would pass even before this fix and cannot catch a regression of the original EISDIR crash path.
Useful? React with 👍 / 👎.
…y guard Address review feedback — the previous integration test did not exercise the isDirectory() guard because git ls-files returns individual file paths. Export formatUntrackedFile and test it directly with a directory path.
1 participant
Summary
stat.isDirectory()check informatUntrackedFile()to skip directories instead of attempting to read them as filesProblem
When a repository contains untracked directories,
git ls-files --others --exclude-standardincludes directory entries. TheformatUntrackedFile()function inscripts/lib/git.mjsattempts to read these paths withfs.readFileSync(), which throws anEISDIR: illegal operation on a directory, readerror and crashes the review.Fix
Added a directory check right after
fs.statSync()informatUntrackedFile()(plugins/codex/scripts/lib/git.mjs:139). When the path is a directory, it returns a(skipped: directory)message instead of trying to read it.Test Plan
tests/git.test.mjsthat creates an untracked directory with a nested file, runscollectReviewContextwithworking-treescope, and verifies it completes without crashingnode --test tests/git.test.mjsFixes #14