fix(core): handle ENAMETOOLONG in robustRealpath - #26741
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical issue where the CLI would crash due to unhandled Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
|
Heads up — I should have checked the open PR queue before opening this. There are at least three other open PRs proposing essentially the same fix:
There is also #25980, which takes a different approach (try/catch at the I'll leave this PR open for visibility, but I have no preference — please feel free to close this in favor of any of the above. The crash is reproducible and the fix is straightforward; the main blocker seems to be reviewer bandwidth rather than the change itself. Happy to help review or test whichever PR a maintainer prefers. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the robustRealpath utility to handle ENAMETOOLONG errors, ensuring the application can recover when file paths exceed system limits. Feedback was provided regarding a redundant test case that is ineffective due to existing global mocks, suggesting its removal to maintain test suite clarity.
| it('should not throw when input exceeds PATH_MAX (ENAMETOOLONG)', () => { | ||
| const longInput = 'a'.repeat(5000); | ||
| expect(() => resolveToRealPath(longInput)).not.toThrow(); | ||
| }); |
There was a problem hiding this comment.
This test is ineffective as a regression test because fs.realpathSync is globally mocked at line 28 to return the input string without throwing. As a result, the catch block in robustRealpath is never reached during this test, and it passes even without the bug fix. Since the subsequent test at line 570 correctly uses vi.spyOn to simulate the ENAMETOOLONG error and verify the recovery logic, this test is redundant and should be removed to avoid confusion.
gemini-cli
Bot
added
priority/p2
When a long string (e.g., a pasted multi-line stack trace) is interpreted as an @path command, it is forwarded to robustRealpath. The underlying fs.realpathSync / fs.lstatSync calls then throw ENAMETOOLONG (the input exceeds the system's PATH_MAX), which was unhandled and surfaced as an unhandled rejection that crashed the CLI. Treat ENAMETOOLONG the same as ENOENT/EISDIR so the value is gracefully treated as "not a real path" instead of a fatal error. PR google-gemini#25009 added a length guard at the CLI layer, but the @-command processor bypasses that guard, so the safeguard is needed at the core layer as well. Fixes google-gemini#26368
a99fd79 to
0cd6585
Compare
2 participants
Summary
@pathcommand, it reachesrobustRealpathinpackages/core/src/utils/paths.ts.fs.realpathSync/fs.lstatSyncthen throwENAMETOOLONG, which was unhandled and crashed the CLI.ENAMETOOLONGthe same asENOENT/EISDIRso the value is gracefully classified as "not a real path".Why a CLI-layer fix isn't enough
PR #25009 added a length guard in the
clipackage, but the@-command processor bypasses it. The crash reported in #26368 and #26627 happens after that bypass, insidecore. The safeguard therefore needs to live incoreto be effective for every input vector.Test plan
resolveToRealPath(would previously crash withENAMETOOLONG)realpathSynctest assertingENAMETOOLONGis handled likeENOENT/EISDIRnpx vitest run packages/core/src/utils/paths.test.ts— 106 passed