fix(core): align grep behavior and guidance (#38999)

packages/core/src/tool/grep.ts

@@ -7,6 +7,7 @@ import path from "path"
 import { FileSystem } from "../filesystem"
 import { FSUtil } from "@opencode-ai/util/fs-util"
 import { Location } from "../location"
+import { LocationMutation } from "../location-mutation"
 import { PermissionV2 } from "../permission"
 import { Ripgrep } from "../ripgrep"
 import { RelativePath } from "../schema"
@@ -18,27 +19,27 @@ export const Input = Schema.Struct({
   pattern: FileSystem.GrepInput.fields.pattern.check(
     Schema.isMinLength(1, { message: "Pattern must not be empty" }),
   ).annotate({
-    description: "Regex pattern to search for in file contents",
+    description: "Regular expression to search for in file contents (ripgrep syntax)",
   }),
   path: RelativePath.pipe(Schema.optional).annotate({
-    description: "Relative directory to search. Defaults to the active Location.",
+    description: "File or directory to search. Defaults to the current working directory.",
   }),
   include: FileSystem.GrepInput.fields.include.annotate({
-    description: 'File glob to include in the search (for example, "*.js" or "*.{ts,tsx}")',
+    description: 'Glob pattern to filter files (for example, "*.js" or "*.{ts,tsx}")',
   }),
   limit: FileSystem.GrepInput.fields.limit.annotate({
-    description: `Maximum matches to return (default: ${FileSystem.DEFAULT_SEARCH_LIMIT})`,
+    description: `Maximum number of matching lines to return (default: ${FileSystem.DEFAULT_SEARCH_LIMIT})`,
   }),
 })
 
 export const Output = Schema.Array(FileSystem.Match)
-type ModelOutput = typeof Output.Encoded
+type EncodedOutput = typeof Output.Encoded
 
-/** Format raw search matches into the familiar concise model output. */
-export const toModelOutput = (output: ModelOutput, truncated = false) => {
-  const lines = output.length === 0 ? ["No files found"] : [`Found ${output.length} matches`]
+/** Format raw search matches into concise model content. */
+export const toModelContent = (matches: EncodedOutput, truncated = false) => {
+  const lines = matches.length === 0 ? ["No matches found"] : [`Found ${matches.length} matches`]
   let current = ""
-  for (const match of output) {
+  for (const match of matches) {
     if (current !== match.entry.path) {
       if (current) lines.push("")
       current = match.entry.path
@@ -49,7 +50,7 @@ export const toModelOutput = (output: ModelOutput, truncated = false) => {
   if (truncated)
     lines.push(
       "",
-      `(Results are truncated: showing first ${output.length} results. Consider using a more specific path or pattern.)`,
+      `(Results are truncated: showing first ${matches.length} results. Consider using a more specific path or pattern.)`,
     )
   return lines.join("\n")
 }
@@ -61,6 +62,7 @@ export const Plugin = {
     const fs = yield* FSUtil.Service
     const ripgrep = yield* Ripgrep.Service
     const location = yield* Location.Service
+    const mutation = yield* LocationMutation.Service
     const permission = yield* PermissionV2.Service
 
     yield* ctx.tool
@@ -69,11 +71,20 @@ export const Plugin = {
           name,
           Tool.make({
             description:
-              "Search file contents by regular expression within the active Location or an absolute managed tool-output file. Use a path to narrow the search, include to filter files by glob, and limit to bound the match count. Returns concise file resources, line numbers, and bounded line previews.",
+              "Search file contents using regular expressions. Use it to locate specific code, symbols, or text patterns, and narrow searches with `path` or `include`. Returns matching file paths, line numbers, and line previews.",
             input: Input,
             output: Output,
             execute: (input, context) =>
               Effect.gen(function* () {
+                const source = { type: "tool" as const, messageID: context.messageID, callID: context.callID }
+                const target = yield* mutation.resolve({ path: input.path ?? "." })
+                if (target.externalDirectory)
+                  yield* permission.assert({
+                    ...LocationMutation.externalDirectoryPermission(target.externalDirectory),
+                    sessionID: context.sessionID,
+                    agent: context.agent,
+                    source,
+                  })
                 yield* permission.assert({
                   action: name,
                   resources: [input.pattern],
@@ -86,22 +97,23 @@ export const Plugin = {
                   },
                   sessionID: context.sessionID,
                   agent: context.agent,
-                  source: { type: "tool", messageID: context.messageID, callID: context.callID },
+                  source,
                 })
-                const target = path.resolve(location.directory, input.path ?? ".")
+                const root = path.resolve(location.directory, input.path ?? ".")
                 const info = yield* fs
-                  .stat(target)
+                  .stat(root)
                   .pipe(
                     Effect.catchReason("PlatformError", "NotFound", () =>
                       Effect.fail(new ToolFailure({ message: `Search path does not exist: ${input.path ?? "."}` })),
                     ),
                   )
+                const cwd = info?.type === "Directory" ? root : path.dirname(root)
                 const limit = input.limit ?? FileSystem.DEFAULT_SEARCH_LIMIT
                 const matches = yield* ripgrep
                   .grep({
-                    cwd: info?.type === "Directory" ? target : path.dirname(target),
+                    cwd,
                     pattern: input.pattern,
-                    file: info?.type === "File" ? path.basename(target) : undefined,
+                    file: info?.type === "File" ? path.basename(root) : undefined,
                     include: input.include,
                     limit: limit + 1,
                   })
@@ -113,13 +125,7 @@ export const Plugin = {
                           entry: FileSystem.Entry.make({
                             ...match.entry,
                             path: RelativePath.make(
-                              path.relative(
-                                location.directory,
-                                path.resolve(
-                                  info?.type === "Directory" ? target : path.dirname(target),
-                                  match.entry.path,
-                                ),
-                              ),
+                              path.relative(location.directory, path.resolve(cwd, match.entry.path)),
                             ),
                           }),
                         }),
@@ -130,7 +136,7 @@ export const Plugin = {
               }).pipe(
                 Effect.map((result) => ({
                   output: result.matches,
-                  content: toModelOutput(
+                  content: toModelContent(
                     result.matches.map((match) => ({
                       ...match,
                       entry: { ...match.entry, path: path.resolve(location.directory, match.entry.path) },
@@ -142,6 +148,8 @@ export const Plugin = {
                 Effect.mapError((error) =>
                   error instanceof ToolFailure
                     ? error
+                    : error instanceof Ripgrep.InvalidPatternError
+                      ? new ToolFailure({ message: `Invalid regex pattern: ${error.message}` })
                     : new ToolFailure({ message: `Unable to grep for ${input.pattern}`, error }),
                 ),
               ),

packages/core/test/tool-search.test.ts

@@ -37,7 +37,14 @@ const globToolNode = makeLocationNode({
 const grepToolNode = makeLocationNode({
   name: "test/grep-tool-plugin",
   layer: Layer.effectDiscard(registerToolPlugin(GrepTool.Plugin)),
-  deps: [ToolRegistry.toolsNode, FSUtil.node, Ripgrep.node, Location.node, PermissionV2.node],
+  deps: [
+    ToolRegistry.toolsNode,
+    FSUtil.node,
+    Ripgrep.node,
+    Location.node,
+    LocationMutation.node,
+    PermissionV2.node,
+  ],
 })
 const sessionID = SessionV2.ID.make("ses_search_tool_test")
 
@@ -183,6 +190,94 @@ describe("search tools", () => {
     ),
   )
 
+  it.live("reports no grep matches", () =>
+    Effect.acquireUseRelease(
+      Effect.promise(() => tmpdir()),
+      (tmp) =>
+        Effect.promise(() => fs.writeFile(path.join(tmp.path, "file.txt"), "haystack\n")).pipe(
+          Effect.andThen(
+            withTools(tmp.path, (registry) => executeTool(registry, call("grep", { pattern: "needle" }))),
+          ),
+          Effect.tap((result) =>
+            Effect.sync(() => {
+              expect(result).toMatchObject({
+                status: "completed",
+                content: [{ type: "text", text: "No matches found" }],
+                metadata: { matches: 0, truncated: false },
+              })
+            }),
+          ),
+        ),
+      (tmp) => Effect.promise(() => tmp[Symbol.asyncDispose]()),
+    ),
+  )
+
+  it.live("reports invalid grep regex details", () =>
+    Effect.acquireUseRelease(
+      Effect.promise(() => tmpdir()),
+      (tmp) =>
+        withTools(tmp.path, (registry) =>
+          Effect.gen(function* () {
+            const result = yield* executeTool(registry, call("grep", { pattern: "[" }))
+            expect(result).toMatchObject({
+              status: "error",
+              error: { type: "tool.execution" },
+            })
+            if (result.status !== "error") return
+            expect(result.error.message).toStartWith("Invalid regex pattern:")
+            expect(result.error.message).toContain("unclosed character class")
+          }),
+        ),
+      (tmp) => Effect.promise(() => tmp[Symbol.asyncDispose]()),
+    ),
+  )
+
+  it.live("requires external_directory approval for external grep files and directories", () =>
+    Effect.acquireUseRelease(
+      Effect.promise(() => Promise.all([tmpdir(), tmpdir()])),
+      ([active, outside]) => {
+        const assertions: PermissionV2.AssertInput[] = []
+        return Effect.promise(() => fs.writeFile(path.join(outside.path, "outside.txt"), "needle\n")).pipe(
+          Effect.andThen(
+            withTools(
+              active.path,
+              (registry) =>
+                Effect.gen(function* () {
+                  const directory = yield* executeTool(
+                    registry,
+                    call("grep", { path: outside.path, pattern: "needle" }),
+                  )
+                  const file = yield* executeTool(
+                    registry,
+                    call("grep", { path: path.join(outside.path, "outside.txt"), pattern: "needle" }),
+                  )
+                  expect(directory.status).toBe("completed")
+                  expect(file.status).toBe("completed")
+                }),
+              assertions,
+            ),
+          ),
+          Effect.tap(() =>
+            Effect.sync(() => {
+              expect(assertions.map((input) => input.action)).toEqual([
+                "external_directory",
+                "grep",
+                "external_directory",
+                "grep",
+              ])
+              expect(assertions[0]?.resources).toEqual([path.join(outside.path, "*").replaceAll("\\", "/")])
+              expect(assertions[2]?.resources).toEqual([path.join(outside.path, "*").replaceAll("\\", "/")])
+            }),
+          ),
+        )
+      },
+      ([active, outside]) =>
+        Effect.promise(() =>
+          Promise.all([active[Symbol.asyncDispose](), outside[Symbol.asyncDispose]()]).then(() => undefined),
+        ),
+    ),
+  )
+
   for (const name of ["glob", "grep"] as const) {
     it.live(`${name} reports a missing search path`, () =>
       Effect.acquireUseRelease(