fix(glob): allow retrieving matches beyond the default page - #3658
Conversation
🦋 Changeset detectedLatest commit: 21c93d1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
commit: |
![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: 2ad35c63bc
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| const offset = args.offset ?? 0; | ||
| const headLimit = args.head_limit ?? DEFAULT_HEAD_LIMIT; | ||
| const limited = headLimit === 0 ? kept.slice(offset) : kept.slice(offset, offset + headLimit); |
There was a problem hiding this comment.
When head_limit=0 produces roughly 10.0–10.5 million ASCII characters, this returns the entire collected list, but the downstream truncation pipeline saves only the first 10,000,000 characters (DEFAULT_TOOL_RESULT_MAX_RETAINED_CHARS), while runRgOnce can collect up to 10 * 1024 * 1024 bytes. Consequently, the spill file advertised for retrieving all collected matches loses its tail, potentially in the middle of a path, and Read cannot recover those matches; either keep this result within the retention cap or provide a complete tool-owned spill.
Useful? React with 👍 / 👎.
| lines.push(`Showing matches ${String(offset + 1)}–${String(offset + limited.length)} of ${total}.`); | ||
| } | ||
| if (truncated) { | ||
| lines.push(`[Truncated at ${String(MAX_MATCHES)} matches — use a more specific pattern]`); | ||
| lines.push(`Only the first ${String(MAX_MATCHES)} matches are returned.`); | ||
| lines.push( | ||
| `Continue with the same search arguments and offset=${String(offset + limited.length)}.`, |
There was a problem hiding this comment.
Whenever Glob paginates, these new status lines are passed to parseGlobOutput, but the NOTICE expression in apps/kimi-code/src/tui/components/messages/tool-renderers/grep-output.ts:55-56 does not recognize Showing matches, Continue with, To retrieve, No more matches, or No matches collected. The TUI therefore treats them as file paths: for example, a default 100-file page is displayed as 103 files and its collapsed glance shows notice text instead of actual paths, while an exhausted page is displayed as one file. Update the parser alongside the output format.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 21c93d15b6
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const { lines, footer } = pageNotices(candidates.length, characterLimited); | ||
| return [...lines, ...footer].join('\n').length + 2; | ||
| })); | ||
| let remaining = DEFAULT_TOOL_RESULT_MAX_RETAINED_CHARS - noticeChars; |
There was a problem hiding this comment.
When a broad search returns at least one match plus close to the 10 MiB stderr cap of ASCII traversal errors, formatGlobWarning places that entire stderr buffer in warnings, so noticeChars can exceed the 10,000,000-character retention limit. This makes remaining negative and causes the subsequent zero-path check to return an error, discarding every valid match instead of returning a partial page; truncate or separately spill the diagnostics while reserving enough room for complete paths.
Useful? React with 👍 / 👎.
|
@codex review |
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Related Issue
No linked issue. This fixes Glob searches that expose only the first 100 matching paths with no way to request the remaining results.
Problem
A search matching 347 files currently returns 100 paths and asks the agent to narrow the query. Repeating the same query cannot reach the remaining 247 paths. The tool discards them before generic output persistence, so a spill file cannot recover them either.
What changed
offsetandhead_limit, following the existing Grep parameter convention. The default remains 100 paths; results provide the next offset. Filtering happens before pagination.head_limit: 0to remove the match-count limit. Each page includes only complete paths and stays within the existing character retention budget, including notices. If more paths remain, the result gives the next offset; large pages use the existing spill-to-file / Read pipeline.Each page re-runs the search against the current filesystem with the existing modification-time ordering. This does not add snapshot guarantees, change ignore or sensitive-file policies, or remove the 20-second / 10 MiB capture limits. Removing the match-count limit does not bypass the character retention budget or recover paths omitted by underlying capture limits.
Validation
CI follow-up: refresh the agent-loop and registered-tool snapshots for the new Glob schema and description, assert numeric telemetry rather than an unrelated fixed tool-schema token count, and use the existing fixed tool subset for the full-history compaction budget test. All three suites containing Glob tool snapshots plus the compaction suite pass (291 tests); scoped lint has no errors. Production behavior is unchanged by this follow-up.
Review follow-up validation: 609 core tests and 116 TUI renderer tests passed, along with core and CLI typechecks, repository lint (existing warnings only), and the documentation build. An executor integration test recovers 60,000 expanded paths across complete saved pages through spill and Read.
Checklist
gen-changesetsskill.gen-docsskill.