Browse Source

tool/lsp: include request details in permission metadata (#24139)

Simon Klee 1 day ago
parent
commit
9ff999cc2b
2 changed files with 182 additions and 4 deletions
  1. 20 4
      packages/opencode/src/tool/lsp.ts
  2. 162 0
      packages/opencode/test/tool/lsp.test.ts

+ 20 - 4
packages/opencode/src/tool/lsp.ts

@@ -36,7 +36,6 @@ export const LspTool = Tool.define(
   Effect.gen(function* () {
     const lsp = yield* LSP.Service
     const fs = yield* AppFileSystem.Service
-
     return {
       description: DESCRIPTION,
       parameters: Parameters,
@@ -47,12 +46,29 @@ export const LspTool = Tool.define(
         Effect.gen(function* () {
           const file = path.isAbsolute(args.filePath) ? args.filePath : path.join(Instance.directory, args.filePath)
           yield* assertExternalDirectoryEffect(ctx, file)
-          yield* ctx.ask({ permission: "lsp", patterns: ["*"], always: ["*"], metadata: {} })
+          const meta =
+            args.operation === "workspaceSymbol"
+              ? { operation: args.operation }
+              : args.operation === "documentSymbol"
+                ? { operation: args.operation, filePath: file }
+                : { operation: args.operation, filePath: file, line: args.line, character: args.character }
+          yield* ctx.ask({
+            permission: "lsp",
+            patterns: ["*"],
+            always: ["*"],
+            metadata: meta,
+          })
 
           const uri = pathToFileURL(file).href
           const position = { file, line: args.line - 1, character: args.character - 1 }
           const relPath = path.relative(Instance.worktree, file)
-          const title = `${args.operation} ${relPath}:${args.line}:${args.character}`
+          const detail =
+            args.operation === "workspaceSymbol"
+              ? ""
+              : args.operation === "documentSymbol"
+                ? relPath
+                : `${relPath}:${args.line}:${args.character}`
+          const title = detail ? `${args.operation} ${detail}` : args.operation
 
           const exists = yield* fs.existsSafe(file)
           if (!exists) throw new Error(`File not found: ${file}`)
@@ -90,7 +106,7 @@ export const LspTool = Tool.define(
             metadata: { result },
             output: result.length === 0 ? `No results found for ${args.operation}` : JSON.stringify(result, null, 2),
           }
-        }),
+        }).pipe(Effect.orDie),
     }
   }),
 )

+ 162 - 0
packages/opencode/test/tool/lsp.test.ts

@@ -0,0 +1,162 @@
+import { afterEach, describe, expect } from "bun:test"
+import { Effect, Layer } from "effect"
+import path from "path"
+import { Agent } from "../../src/agent/agent"
+import * as CrossSpawnSpawner from "../../src/effect/cross-spawn-spawner"
+import { AppFileSystem } from "@opencode-ai/shared/filesystem"
+import { LSP } from "../../src/lsp"
+import { Permission } from "../../src/permission"
+import { Instance } from "../../src/project/instance"
+import { MessageID, SessionID } from "../../src/session/schema"
+import { Tool, Truncate } from "../../src/tool"
+import { LspTool } from "../../src/tool/lsp"
+import { provideTmpdirInstance } from "../fixture/fixture"
+import { testEffect } from "../lib/effect"
+
+afterEach(async () => {
+  await Instance.disposeAll()
+})
+
+const ctx = {
+  sessionID: SessionID.make("ses_test"),
+  messageID: MessageID.make(""),
+  callID: "",
+  agent: "build",
+  abort: AbortSignal.any([]),
+  messages: [],
+  metadata: () => Effect.void,
+  ask: () => Effect.void,
+}
+
+const lsp = Layer.succeed(
+  LSP.Service,
+  LSP.Service.of({
+    init: () => Effect.void,
+    status: () => Effect.succeed([]),
+    hasClients: () => Effect.succeed(true),
+    touchFile: () => Effect.void,
+    diagnostics: () => Effect.succeed({}),
+    hover: () => Effect.succeed([]),
+    definition: () => Effect.succeed([]),
+    references: () => Effect.succeed([]),
+    implementation: () => Effect.succeed([]),
+    documentSymbol: () => Effect.succeed([]),
+    workspaceSymbol: () => Effect.succeed([]),
+    prepareCallHierarchy: () => Effect.succeed([]),
+    incomingCalls: () => Effect.succeed([]),
+    outgoingCalls: () => Effect.succeed([]),
+  }),
+)
+
+const it = testEffect(
+  Layer.mergeAll(
+    Agent.defaultLayer,
+    AppFileSystem.defaultLayer,
+    CrossSpawnSpawner.defaultLayer,
+    Truncate.defaultLayer,
+    lsp,
+  ),
+)
+
+const init = Effect.fn("LspToolTest.init")(function* () {
+  const info = yield* LspTool
+  return yield* info.init()
+})
+
+const run = Effect.fn("LspToolTest.run")(function* (
+  args: Tool.InferParameters<typeof LspTool>,
+  next: Tool.Context = ctx,
+) {
+  const tool = yield* init()
+  return yield* tool.execute(args, next)
+})
+
+const put = Effect.fn("LspToolTest.put")(function* (file: string) {
+  const fs = yield* AppFileSystem.Service
+  yield* fs.writeWithDirs(file, "export const x = 1\n")
+})
+
+const asks = () => {
+  const items: Array<Omit<Permission.Request, "id" | "sessionID" | "tool">> = []
+  return {
+    items,
+    next: {
+      ...ctx,
+      ask: (req: Omit<Permission.Request, "id" | "sessionID" | "tool">) =>
+        Effect.sync(() => {
+          items.push(req)
+        }),
+    },
+  }
+}
+
+describe("tool.lsp", () => {
+  describe("permission metadata", () => {
+    it.live("keeps cursor details for position-based operations", () =>
+      provideTmpdirInstance(
+        (dir) =>
+          Effect.gen(function* () {
+            const file = path.join(dir, "test.ts")
+            yield* put(file)
+
+            const { items, next } = asks()
+            const result = yield* run({ operation: "goToDefinition", filePath: file, line: 3, character: 7 }, next)
+            const req = items.find((item) => item.permission === "lsp")
+
+            expect(req).toBeDefined()
+            expect(req!.metadata).toEqual({
+              operation: "goToDefinition",
+              filePath: file,
+              line: 3,
+              character: 7,
+            })
+            expect(result.title).toBe("goToDefinition test.ts:3:7")
+          }),
+        { git: true },
+      ),
+    )
+
+    it.live("omits cursor details for documentSymbol", () =>
+      provideTmpdirInstance(
+        (dir) =>
+          Effect.gen(function* () {
+            const file = path.join(dir, "test.ts")
+            yield* put(file)
+
+            const { items, next } = asks()
+            const result = yield* run({ operation: "documentSymbol", filePath: file, line: 3, character: 7 }, next)
+            const req = items.find((item) => item.permission === "lsp")
+
+            expect(req).toBeDefined()
+            expect(req!.metadata).toEqual({
+              operation: "documentSymbol",
+              filePath: file,
+            })
+            expect(result.title).toBe("documentSymbol test.ts")
+          }),
+        { git: true },
+      ),
+    )
+
+    it.live("omits file and cursor details for workspaceSymbol", () =>
+      provideTmpdirInstance(
+        (dir) =>
+          Effect.gen(function* () {
+            const file = path.join(dir, "test.ts")
+            yield* put(file)
+
+            const { items, next } = asks()
+            const result = yield* run({ operation: "workspaceSymbol", filePath: file, line: 3, character: 7 }, next)
+            const req = items.find((item) => item.permission === "lsp")
+
+            expect(req).toBeDefined()
+            expect(req!.metadata).toEqual({
+              operation: "workspaceSymbol",
+            })
+            expect(result.title).toBe("workspaceSymbol")
+          }),
+        { git: true },
+      ),
+    )
+  })
+})