Skip to content

fix(step): stop dropping bracket-prefixed paths in narrowed find_files searches - #279

Open
li-xiu-qi wants to merge 1 commit into
stepfun-ai:mainfrom
li-xiu-qi:fix/find-files-bracket-path
Open

li-xiu-qi wants to merge 1 commit into
stepfun-ai:mainfrom
li-xiu-qi:fix/find-files-bracket-path

Conversation

@li-xiu-qi

Copy link
Copy Markdown
Collaborator

Problem

find_files loses valid matches whenever the first character of a returned path is [. Next.js App Router names dynamic segments that way ([id], [...slug], [[...slug]]), so with routes/[id]/page.tsx present:

{ "pattern": "*.tsx", "path": "routes" }

returns (no matches) even though the native find backend resolved the file. search_files on the same directory returns it correctly, which is the clearest way to see that this is one tool's gap rather than a missing capability.

Trigger is a narrowed search. Paths come back relative to the search root, so path: "routes" yields [id]/page.tsx with the bracket in first position. Searching from the repository root yields routes/[id]/page.tsx, and the bug does not appear — which is why it survives casual use.

Root cause. stripNativeFindNotices (packages/coding-agent/src/step/tool-profile.ts) dropped every line starting with [ or WARNING: to remove the notices the find tool appends. Those notices are themselves a bracketed block appended at the end of the output (find.ts::finalizeOutput appends "\n\n[" + notices.join(". ") + "]"). A path and a notice therefore share the same first character, and a per-line prefix test cannot separate them.

Two distinct losses. When the directory also holds ordinary files, the bracket routes vanish silently and matchedFiles under-reports — 1 instead of 3 in the repro below — so the model is told the directory has fewer files than it does. When every match is bracket-prefixed, the result collapses to (no matches), and the match is lost outright rather than shown wrong.

Changes

The find tool already knows the resolved paths; the adapter now reads them instead of inferring them from rendered text.

packages/coding-agent/src/core/tools/find.ts

  • FindToolDetails gains optional files?: string[], filled in finalizeOutput from the same array the rendered text is built from, so the two cannot disagree. details is local metadata and is not sent to the model, so the model-visible output is unchanged.
  • An empty result now reports files: []. That distinguishes "the search genuinely matched nothing" from "this backend reports no list" (undefined), which is what lets a consumer trust an empty result instead of re-parsing text.

packages/coding-agent/src/step/tool-profile.ts

  • findFilesFromDetails reads details.files; executeFindFiles uses it first.
  • stripNativeFindNotices becomes the fallback for injected backends that report no list, and now removes the notice by shape — one bracketed block after a blank line, at the end of the output — instead of dropping lines by prefix. Bracket-prefixed paths and paths beginning with WARNING: both survive.

packages/coding-agent/test/step-tool-profile.test.ts

  • A narrowed search keeps [id], [...slug], and [[...slug]] route directories, and asserts matchedFiles / returnedFiles.
  • A path starting with WARNING: is not discarded (skipped on Windows, where : is not a legal filename character).

Why both layers. The structured list removes inference from the main path; shape-based removal keeps the fallback from guessing by prefix. Only the first would still drop paths for injected backends. Only the second would still be reading data out of text.

Behavior and compatibility

  • Rendered output is unchanged for paths that were already parsed unambiguously.
  • details.files is a new optional field. backend, resultLimitReached, and truncation keep their meaning; the Step-facing matchedFiles and returnedFiles are unchanged.
  • One behavior change: a path beginning with WARNING: was previously discarded and is now returned. This cannot occur on Windows (: is not allowed in filenames).
  • details.files is the native match list and can be longer than the Step-facing returnedFiles when max_results truncates. The two are named differently on purpose; neither reinterprets the other.
  • No public interface, configuration, CLI argument, persisted data, or provider response is affected.

Validation

Repro is a routes/ directory holding [id]/page.tsx, [...slug]/page.tsx, and static/page.tsx, queried with find_files { pattern: "*.tsx", path: "routes", max_results: 10 }. All three are genuine matches, so the correct result is three rows and matchedFiles: 3.

Before (main @ be88ee7) — one row survives, and the count is wrong too:

static/page.tsx
details: {"backend":"fd","matchedFiles":1,"returnedFiles":1,...}

After (this branch):

[...slug]/page.tsx
[id]/page.tsx
static/page.tsx
details: {"backend":"fd","files":["[...slug]/page.tsx","[id]/page.tsx","static/page.tsx"],"matchedFiles":3,"returnedFiles":3,...}

The matchedFiles: 1 → 3 part matters as much as the restored rows: the count was telling the model the directory held one file.

The new tests fail on main with all three bracket rows missing, and pass with the change.

  • vitest run test/step-tool-profile.test.ts test/find-fallback.test.ts test/suite/regressions/3302-find-path-glob.test.ts test/suite/regressions/3303-find-nested-gitignore.test.ts test/suite/regressions/6104-find-root-relativization.test.ts — 53/53 passed
  • npm run check — passed (biome, tsgo, repository check scripts)
  • tsgo --noEmit — no errors
  • Verified on Windows 11 x64; the WARNING: path case is skipped there and runs on platforms where the filename is legal.

Limitations

  • The fallback identifies a notice as a bracketed block after a blank line at the end of the output. An injected backend that emitted a blank line inside its file list and ended with a bracketed ordinary path would have that last line removed. Native find never emits a blank line, and the structured list path is not affected.
  • macOS and Linux runs are covered by CI rather than a local run here.

…text

The Step find adapter dropped every native output line starting with "["
or "WARNING:" to remove the notices the find tool appends. A narrowed
search returns paths relative to the search root, so `[id]/page.tsx`
arrives with the bracket in the first position and was discarded as a
notice: `find_files` reported `(no matches)` and `matchedFiles: 0` for a
directory the backend had already resolved.

The find tool now reports its match list in `details.files`, and the adapter
reads that list instead of parsing rendered text. Text parsing stays as a
fallback for injected backends that report no list, and it now removes the
notice block by shape — one bracketed block after a blank line — rather than
by line prefix.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant