fix(tools): resolved search and find behavior for missing path inputs

- Updated search, find, ast-edit, and ast-grep to skip missing paths and run on existing ones.
- Added multi-path error handling to throw ToolError only when all provided paths are missing.
- Updated search and find outputs to surface skipped `missingPaths` in text and renderer warnings.
- Updated CHANGELOG with multi-path tool behavior updates and `search_repos` global-search notes.
- Added `PartitionedPaths` and `partitionExistingPaths` to classify existing versus missing path inputs.
- Added test helpers and fixtures for multi-path missing-path cases in search/find behavior tests.
This commit is contained in:
can1357
2026-05-03 05:43:18 +02:00
parent 58b895db23
commit 7f2b25ea95
7 changed files with 280 additions and 20 deletions
+2
View File
@@ -1,12 +1,14 @@
# Changelog
## [Unreleased]
### Added
- Added `search_code`, `search_commits`, and `search_repos` ops to the `github` tool so the search surface mirrors `gh search`'s subcommands
### Changed
- Updated multi-path `search`, `find`, `ast-edit`, and `ast-grep` calls to skip missing base paths, returning matches from remaining paths and reporting skipped paths in output
- Changed `search_repos` to run as a global repository search using query qualifiers without applying the `repo` filter
### Fixed
+12 -3
View File
@@ -20,6 +20,7 @@ import {
hasGlobPathChars,
normalizePathLikeInput,
parseSearchPath,
partitionExistingPaths,
resolveExplicitSearchPaths,
resolveToCwd,
} from "./path-utils";
@@ -226,13 +227,21 @@ export class AstEditTool implements AgentTool<typeof astEditSchema, AstEditToolD
}
resolvedPathInputs.push(resource.sourcePath);
}
if (resolvedPathInputs.length === 1) {
const parsedPath = parseSearchPath(resolvedPathInputs[0] ?? ".");
let effectivePathInputs = resolvedPathInputs;
if (resolvedPathInputs.length > 1) {
const partition = await partitionExistingPaths(resolvedPathInputs, this.session.cwd, parseSearchPath);
if (partition.valid.length === 0) {
throw new ToolError(`Path not found: ${partition.missing.join(", ")}`);
}
effectivePathInputs = partition.valid;
}
if (effectivePathInputs.length === 1) {
const parsedPath = parseSearchPath(effectivePathInputs[0] ?? ".");
searchPath = resolveToCwd(parsedPath.basePath, this.session.cwd);
globFilter = parsedPath.glob;
scopePath = formatScopePath(searchPath);
} else {
const multiSearchPath = await resolveExplicitSearchPaths(resolvedPathInputs, this.session.cwd, globFilter);
const multiSearchPath = await resolveExplicitSearchPaths(effectivePathInputs, this.session.cwd, globFilter);
if (!multiSearchPath) {
throw new ToolError("`paths` must contain at least one path or glob");
}
+12 -3
View File
@@ -20,6 +20,7 @@ import {
hasGlobPathChars,
normalizePathLikeInput,
parseSearchPath,
partitionExistingPaths,
resolveExplicitSearchPaths,
resolveToCwd,
} from "./path-utils";
@@ -171,13 +172,21 @@ export class AstGrepTool implements AgentTool<typeof astGrepSchema, AstGrepToolD
}
resolvedPathInputs.push(resource.sourcePath);
}
if (resolvedPathInputs.length === 1) {
const parsedPath = parseSearchPath(resolvedPathInputs[0] ?? ".");
let effectivePathInputs = resolvedPathInputs;
if (resolvedPathInputs.length > 1) {
const partition = await partitionExistingPaths(resolvedPathInputs, this.session.cwd, parseSearchPath);
if (partition.valid.length === 0) {
throw new ToolError(`Path not found: ${partition.missing.join(", ")}`);
}
effectivePathInputs = partition.valid;
}
if (effectivePathInputs.length === 1) {
const parsedPath = parseSearchPath(effectivePathInputs[0] ?? ".");
searchPath = resolveToCwd(parsedPath.basePath, this.session.cwd);
globFilter = parsedPath.glob;
scopePath = formatScopePath(searchPath);
} else {
const multiSearchPath = await resolveExplicitSearchPaths(resolvedPathInputs, this.session.cwd, globFilter);
const multiSearchPath = await resolveExplicitSearchPaths(effectivePathInputs, this.session.cwd, globFilter);
if (!multiSearchPath) {
throw new ToolError("`paths` must contain at least one path or glob");
}
+47 -7
View File
@@ -27,6 +27,7 @@ import {
formatPathRelativeToCwd,
normalizePathLikeInput,
parseFindPattern,
partitionExistingPaths,
resolveExplicitFindPatterns,
resolveToCwd,
} from "./path-utils";
@@ -59,6 +60,10 @@ export interface FindToolDetails {
files?: string[];
truncated?: boolean;
error?: string;
/** User-supplied paths whose base directory was missing on disk. The tool
* skipped these and continued with the surviving entries; surfaced as a
* non-fatal warning in the renderer and in the model-facing text. */
missingPaths?: string[];
}
/**
@@ -114,8 +119,23 @@ export class FindTool implements AgentTool<typeof findSchema, FindToolDetails> {
throw new ToolError("`paths` must contain non-empty globs or paths");
}
const multiPattern = await resolveExplicitFindPatterns(normalizedPatterns, this.session.cwd);
const parsedPattern = multiPattern ? null : parseFindPattern(normalizedPatterns[0] ?? ".");
// Tolerate missing entries in a multi-path call: skip ones whose base
// directory is gone, and only error if every entry is missing. Single
// missing path keeps the original ENOENT semantics — the user explicitly
// asked about that one path, so silent empty results would be misleading.
let missingPaths: string[] = [];
let effectivePatterns = normalizedPatterns;
if (normalizedPatterns.length > 1 && !this.#customOps) {
const partition = await partitionExistingPaths(normalizedPatterns, this.session.cwd, parseFindPattern);
if (partition.valid.length === 0) {
throw new ToolError(`Path not found: ${partition.missing.join(", ")}`);
}
effectivePatterns = partition.valid;
missingPaths = partition.missing;
}
const multiPattern = await resolveExplicitFindPatterns(effectivePatterns, this.session.cwd);
const parsedPattern = multiPattern ? null : parseFindPattern(effectivePatterns[0] ?? ".");
const hasGlob = multiPattern ? true : (parsedPattern?.hasGlob ?? false);
const globPattern = multiPattern?.globPattern ?? parsedPattern?.globPattern ?? "**/*";
const searchPath = resolveToCwd(multiPattern?.basePath ?? parsedPattern?.basePath ?? ".", this.session.cwd);
@@ -124,7 +144,6 @@ export class FindTool implements AgentTool<typeof findSchema, FindToolDetails> {
if (searchPath === "/") {
throw new ToolError("Searching from root directory '/' is not allowed");
}
const rawLimit = limit ?? DEFAULT_LIMIT;
const effectiveLimit = Number.isFinite(rawLimit) ? Math.floor(rawLimit) : Number.NaN;
if (!Number.isFinite(effectiveLimit) || effectiveLimit <= 0) {
@@ -141,16 +160,29 @@ export class FindTool implements AgentTool<typeof findSchema, FindToolDetails> {
});
};
const missingPathsNote =
missingPaths.length > 0 ? `Skipped missing paths: ${missingPaths.join(", ")}` : undefined;
const buildResult = (files: string[]): AgentToolResult<FindToolDetails> => {
if (files.length === 0) {
const details: FindToolDetails = { scopePath, fileCount: 0, files: [], truncated: false };
return toolResult(details).text("No files found matching pattern").done();
const details: FindToolDetails = {
scopePath,
fileCount: 0,
files: [],
truncated: false,
missingPaths: missingPaths.length > 0 ? missingPaths : undefined,
};
const text = missingPathsNote
? `No files found matching pattern\n${missingPathsNote}`
: "No files found matching pattern";
return toolResult(details).text(text).done();
}
const listLimit = applyListLimit(files, { limit: effectiveLimit });
const limited = listLimit.items;
const limitMeta = listLimit.meta;
const rawOutput = limited.join("\n");
const baseOutput = limited.join("\n");
const rawOutput = missingPathsNote ? `${baseOutput}\n\n${missingPathsNote}` : baseOutput;
const truncation = truncateHead(rawOutput, { maxLines: Number.MAX_SAFE_INTEGER });
const details: FindToolDetails = {
@@ -160,6 +192,7 @@ export class FindTool implements AgentTool<typeof findSchema, FindToolDetails> {
truncated: Boolean(limitMeta.resultLimit || truncation.truncated),
resultLimitReached: limitMeta.resultLimit?.reached,
truncation: truncation.truncated ? truncation : undefined,
missingPaths: missingPaths.length > 0 ? missingPaths : undefined,
};
const resultBuilder = toolResult(details)
@@ -380,12 +413,18 @@ export const findToolRenderer = {
const truncated = Boolean(details?.truncated || truncation || details?.resultLimitReached || limits?.resultLimit);
const files = details?.files ?? [];
const missingPaths = details?.missingPaths ?? [];
const missingNote =
missingPaths.length > 0 ? uiTheme.fg("warning", `skipped missing: ${missingPaths.join(", ")}`) : undefined;
if (fileCount === 0) {
const header = renderStatusLine(
{ icon: "warning", title: "Find", description: args?.paths?.join(", "), meta: ["0 files"] },
uiTheme,
);
return new Text([header, formatEmptyMessage("No files found", uiTheme)].join("\n"), 0, 0);
const lines = [header, formatEmptyMessage("No files found", uiTheme)];
if (missingNote) lines.push(missingNote);
return new Text(lines.join("\n"), 0, 0);
}
const meta: string[] = [formatCount("file", fileCount)];
if (details?.scopePath) meta.push(`in ${details.scopePath}`);
@@ -406,6 +445,7 @@ export const findToolRenderer = {
if (truncationReasons.length > 0) {
extraLines.push(uiTheme.fg("warning", `truncated: ${truncationReasons.join(", ")}`));
}
if (missingNote) extraLines.push(missingNote);
let cached: RenderCache | undefined;
return {
@@ -2,6 +2,7 @@ import * as fs from "node:fs";
import * as os from "node:os";
import * as path from "node:path";
import * as url from "node:url";
import { isEnoent } from "@oh-my-pi/pi-utils";
const UNICODE_SPACES = /[\u00A0\u2000-\u200A\u202F\u205F\u3000]/g;
const NARROW_NO_BREAK_SPACE = "\u202F";
@@ -442,6 +443,60 @@ export async function resolveExplicitFindPatterns(
return resolveFindPatternItems([...new Set(patternItems)], cwd);
}
/**
* Result of partitioning a list of user-supplied paths/globs into entries whose
* base directory currently exists on disk versus those that do not.
*
* Used by multi-path tools (search, find, ast_grep, ast_edit) to tolerate one
* or more missing entries in a multi-path call: the surviving entries should
* still be searched, with the missing entries surfaced as a non-fatal warning.
*/
export interface PartitionedPaths {
/** Raw input strings whose resolved base path exists. */
valid: string[];
/** Raw input strings whose resolved base path is missing (ENOENT). */
missing: string[];
}
/**
* Stat each input's base path concurrently; return entries split by existence.
*
* `splitter` is expected to be {@link parseFindPattern} or
* {@link parseSearchPath}: both return a `basePath` field that this helper
* resolves against `cwd` and stats. ENOENT is the only swallowed error — every
* other stat failure (permission, IO, etc.) propagates so callers do not silently
* skip paths that exist but are unreadable.
*
* Order of `valid` and `missing` follows the input order, so callers can rely
* on `valid[0]` matching the first surviving user-supplied entry.
*/
export async function partitionExistingPaths(
items: string[],
cwd: string,
splitter: (item: string) => { basePath: string },
): Promise<PartitionedPaths> {
const settled = await Promise.all(
items.map(async item => {
const { basePath } = splitter(item);
const absoluteBasePath = resolveToCwd(basePath, cwd);
try {
await fs.promises.stat(absoluteBasePath);
return { item, exists: true } as const;
} catch (err) {
if (isEnoent(err)) return { item, exists: false } as const;
throw err;
}
}),
);
const valid: string[] = [];
const missing: string[] = [];
for (const entry of settled) {
if (entry.exists) valid.push(entry.item);
else missing.push(entry.item);
}
return { valid, missing };
}
export function resolveReadPath(filePath: string, cwd: string): string {
const resolved = resolveToCwd(filePath, cwd);
const shellEscapedVariant = tryShellEscapedPath(resolved);
+44 -7
View File
@@ -22,6 +22,7 @@ import {
hasGlobPathChars,
normalizePathLikeInput,
parseSearchPath,
partitionExistingPaths,
resolveExplicitSearchPaths,
resolveToCwd,
} from "./path-utils";
@@ -68,6 +69,10 @@ export interface SearchToolDetails {
* `result.text` lines but uses a `│` gutter and `*` to mark match lines (vs space for
* context). The TUI uses this directly so it never parses model-facing hashline anchors. */
displayContent?: string;
/** User-supplied paths whose base directory was missing on disk. The tool
* skipped these and continued with the surviving entries; surfaced as a
* non-fatal warning in the renderer and in the model-facing text. */
missingPaths?: string[];
}
type SearchParams = Static<typeof searchSchema>;
@@ -140,13 +145,26 @@ export class SearchTool implements AgentTool<typeof searchSchema, SearchToolDeta
}
resolvedPathInputs.push(resource.sourcePath);
}
if (resolvedPathInputs.length === 1) {
const parsedPath = parseSearchPath(resolvedPathInputs[0] ?? ".");
// Tolerate missing entries in a multi-path call: skip ones whose base
// directory is gone, and only error if every entry is missing. Single
// missing path keeps the original ENOENT semantics.
let missingPaths: string[] = [];
let effectivePaths = resolvedPathInputs;
if (resolvedPathInputs.length > 1) {
const partition = await partitionExistingPaths(resolvedPathInputs, this.session.cwd, parseSearchPath);
if (partition.valid.length === 0) {
throw new ToolError(`Path not found: ${partition.missing.join(", ")}`);
}
effectivePaths = partition.valid;
missingPaths = partition.missing;
}
if (effectivePaths.length === 1) {
const parsedPath = parseSearchPath(effectivePaths[0] ?? ".");
searchPath = resolveToCwd(parsedPath.basePath, this.session.cwd);
globFilter = parsedPath.glob;
scopePath = formatScopePath(searchPath);
} else {
const multiSearchPath = await resolveExplicitSearchPaths(resolvedPathInputs, this.session.cwd, globFilter);
const multiSearchPath = await resolveExplicitSearchPaths(effectivePaths, this.session.cwd, globFilter);
if (!multiSearchPath) {
throw new ToolError("`paths` must contain at least one path or glob");
}
@@ -285,6 +303,8 @@ export class SearchTool implements AgentTool<typeof searchSchema, SearchToolDeta
const limitMessage = `Result limit reached; narrow paths or use skip=${nextSkip}.`;
const { record: recordFile, list: fileList } = createFileRecorder();
const fileMatchCounts = new Map<string, number>();
const missingPathsNote =
missingPaths.length > 0 ? `Skipped missing paths: ${missingPaths.join(", ")}` : undefined;
if (selectedMatches.length === 0) {
const details: SearchToolDetails = {
scopePath,
@@ -292,8 +312,10 @@ export class SearchTool implements AgentTool<typeof searchSchema, SearchToolDeta
fileCount: 0,
files: [],
truncated: false,
missingPaths: missingPaths.length > 0 ? missingPaths : undefined,
};
return toolResult(details).text("No matches found").done();
const text = missingPathsNote ? `No matches found\n${missingPathsNote}` : "No matches found";
return toolResult(details).text(text).done();
}
const outputLines: string[] = [];
let linesTruncated = false;
@@ -365,6 +387,9 @@ export class SearchTool implements AgentTool<typeof searchSchema, SearchToolDeta
if (matchLimitReached || result.limitReached) {
outputLines.push("", limitMessage);
}
if (missingPathsNote) {
outputLines.push("", missingPathsNote);
}
const rawOutput = outputLines.join("\n");
const truncation = truncateHead(rawOutput, { maxLines: Number.MAX_SAFE_INTEGER });
const output = truncation.content;
@@ -382,6 +407,7 @@ export class SearchTool implements AgentTool<typeof searchSchema, SearchToolDeta
matchLimitReached: matchLimitReached ? effectiveLimit : undefined,
resultLimitReached: result.limitReached ? internalLimit : undefined,
displayContent: displayLines.join("\n"),
missingPaths: missingPaths.length > 0 ? missingPaths : undefined,
};
if (truncation.truncated) details.truncation = truncation;
if (linesTruncated) details.linesTruncated = true;
@@ -487,12 +513,20 @@ export const searchToolRenderer = {
details?.truncated || truncation || limits?.matchLimit || limits?.resultLimit || limits?.columnTruncated,
);
const missingPathsList = details?.missingPaths ?? [];
const missingNote =
missingPathsList.length > 0
? uiTheme.fg("warning", `skipped missing: ${missingPathsList.join(", ")}`)
: undefined;
if (matchCount === 0) {
const header = renderStatusLine(
{ icon: "warning", title: "Search", description: args?.pattern, meta: ["0 matches"] },
uiTheme,
);
return new Text([header, formatEmptyMessage("No matches found", uiTheme)].join("\n"), 0, 0);
const lines = [header, formatEmptyMessage("No matches found", uiTheme)];
if (missingNote) lines.push(missingNote);
return new Text(lines.join("\n"), 0, 0);
}
const summaryParts = [formatCount("match", matchCount), formatCount("file", fileCount)];
@@ -538,8 +572,11 @@ export const searchToolRenderer = {
if (limits?.columnTruncated) truncationReasons.push(`line length ${limits.columnTruncated.maxColumn}`);
if (truncation?.artifactId) truncationReasons.push(formatFullOutputReference(truncation.artifactId));
const extraLines =
truncationReasons.length > 0 ? [uiTheme.fg("warning", `truncated: ${truncationReasons.join(", ")}`)] : [];
const extraLines: string[] = [];
if (truncationReasons.length > 0) {
extraLines.push(uiTheme.fg("warning", `truncated: ${truncationReasons.join(", ")}`));
}
if (missingNote) extraLines.push(missingNote);
let cached: RenderCache | undefined;
return {
@@ -0,0 +1,108 @@
import { afterEach, beforeEach, describe, expect, it } from "bun:test";
import * as fs from "node:fs/promises";
import * as os from "node:os";
import * as path from "node:path";
import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings";
import { createTools, type ToolSession } from "@oh-my-pi/pi-coding-agent/tools";
// Regression for grievances #208 (find) and #209 (search): a multi-path call
// that includes an entry which does not exist on disk must not abort the whole
// lookup. The tool should skip the missing entry and return matches from the
// surviving entries, with a non-fatal "skipped missing paths" notice.
function createTestSession(cwd: string, overrides: Partial<ToolSession> = {}): ToolSession {
return {
cwd,
hasUI: false,
getSessionFile: () => null,
getSessionSpawns: () => "*",
settings: Settings.isolated(),
...overrides,
};
}
function getText(result: { content: Array<{ type: string; text?: string }> }): string {
return result.content
.filter(entry => entry.type === "text")
.map(entry => entry.text ?? "")
.join("\n");
}
describe("multi-path tools tolerate missing entries", () => {
let tempDir: string;
beforeEach(async () => {
tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "pi-multi-path-missing-"));
await fs.mkdir(path.join(tempDir, "src"), { recursive: true });
await Bun.write(path.join(tempDir, "src", "alpha.ts"), "shared-needle alpha\n");
await Bun.write(path.join(tempDir, "src", "beta.ts"), "shared-needle beta\n");
});
afterEach(async () => {
await fs.rm(tempDir, { recursive: true, force: true });
});
it("search returns matches from existing paths and reports the missing one", async () => {
const tools = await createTools(createTestSession(tempDir));
const tool = tools.find(entry => entry.name === "search");
if (!tool) throw new Error("Missing search tool");
const result = await tool.execute("search-multi-missing", {
pattern: "shared-needle",
paths: ["src/", "tests/"],
});
const text = getText(result);
const details = result.details as { fileCount?: number; missingPaths?: string[] } | undefined;
expect(text).toContain("shared-needle alpha");
expect(text).toContain("shared-needle beta");
expect(text).toContain("Skipped missing paths: tests/");
expect(details?.fileCount).toBe(2);
expect(details?.missingPaths).toEqual(["tests/"]);
});
it("search errors only when every path is missing", async () => {
const tools = await createTools(createTestSession(tempDir));
const tool = tools.find(entry => entry.name === "search");
if (!tool) throw new Error("Missing search tool");
const promise = tool.execute("search-all-missing", {
pattern: "shared-needle",
paths: ["does-not-exist/", "also-missing/"],
});
await expect(promise).rejects.toThrow(/Path not found.*does-not-exist.*also-missing/s);
});
it("find returns matches from existing globs and reports the missing one", async () => {
const tools = await createTools(createTestSession(tempDir));
const tool = tools.find(entry => entry.name === "find");
if (!tool) throw new Error("Missing find tool");
const result = await tool.execute("find-multi-missing", {
paths: ["src/**/*.ts", "tests/**/*.ts"],
});
const text = getText(result);
const details = result.details as { fileCount?: number; missingPaths?: string[] } | undefined;
expect(text).toContain("src/alpha.ts");
expect(text).toContain("src/beta.ts");
expect(text).toContain("Skipped missing paths: tests/**/*.ts");
expect(details?.fileCount).toBe(2);
expect(details?.missingPaths).toEqual(["tests/**/*.ts"]);
});
it("find errors only when every glob's base directory is missing", async () => {
const tools = await createTools(createTestSession(tempDir));
const tool = tools.find(entry => entry.name === "find");
if (!tool) throw new Error("Missing find tool");
const promise = tool.execute("find-all-missing", {
paths: ["nope/**/*.ts", "also-nope/**/*.ts"],
});
await expect(promise).rejects.toThrow(/Path not found.*nope.*also-nope/s);
});
});