diff --git a/packages/opencode/src/tool/lsp.ts b/packages/opencode/src/tool/lsp.ts index 3a555c2ce826..05695fbdf91b 100644 --- a/packages/opencode/src/tool/lsp.ts +++ b/packages/opencode/src/tool/lsp.ts @@ -22,16 +22,10 @@ const operations = [ export const Parameters = Schema.Struct({ operation: Schema.Literals(operations).annotate({ description: "The LSP operation to perform" }), - filePath: Schema.String.annotate({ description: "The absolute or relative path to the file" }), - line: Schema.Int.check(Schema.isGreaterThanOrEqualTo(1)).annotate({ - description: "The line number (1-based, as shown in editors)", - }), - character: Schema.Int.check(Schema.isGreaterThanOrEqualTo(1)).annotate({ - description: "The character offset (1-based, as shown in editors)", - }), - query: Schema.optional(Schema.String).annotate({ - description: "Search query for workspaceSymbol. Empty string requests all symbols.", - }), + filePath: Schema.optional(Schema.String).annotate({ description: "The absolute or relative path to the file. Required for all operations except workspaceSymbol." }), + line: Schema.optional(Schema.Int.check(Schema.isGreaterThanOrEqualTo(1))).annotate({ description: "The line number (1-based, as shown in editors). Required for: goToDefinition, findReferences, hover, goToImplementation, prepareCallHierarchy, incomingCalls, outgoingCalls." }), + character: Schema.optional(Schema.Int.check(Schema.isGreaterThanOrEqualTo(1))).annotate({ description: "The character offset (1-based, as shown in editors). Required for: goToDefinition, findReferences, hover, goToImplementation, prepareCallHierarchy, incomingCalls, outgoingCalls." }), + query: Schema.optional(Schema.String).annotate({ description: "Search query. Required for workspaceSymbol operation." }), }) export const LspTool = Tool.define( @@ -42,16 +36,33 @@ export const LspTool = Tool.define( return { description: DESCRIPTION, parameters: Parameters, - execute: (args: Schema.Schema.Type, ctx: Tool.Context) => + execute: ( + args: Schema.Schema.Type, + ctx: Tool.Context, + ) => Effect.gen(function* () { + if (args.operation === "workspaceSymbol") { + if (!args.query) { + throw new Error(`query is required for operation '${args.operation}'`) + } + const result: unknown[] = yield* lsp.workspaceSymbol(args.query) + return { + title: `workspaceSymbol "${args.query}"`, + metadata: { result }, + output: result.length === 0 ? `No workspace symbols found matching query "${args.query}"` : JSON.stringify(result, null, 2), + } + } + + if (!args.filePath) { + throw new Error(`filePath is required for operation '${args.operation}'`) + } + const file = path.isAbsolute(args.filePath) ? args.filePath : path.join(Instance.directory, args.filePath) yield* assertExternalDirectoryEffect(ctx, file) 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 } + 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: ["*"], @@ -59,17 +70,6 @@ export const LspTool = Tool.define( 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 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}`) @@ -78,6 +78,26 @@ export const LspTool = Tool.define( yield* lsp.touchFile(file, "document") + const uri = pathToFileURL(file).href + const relPath = path.relative(Instance.worktree, file) + + if (args.operation === "documentSymbol") { + const result: unknown[] = yield* lsp.documentSymbol(uri) + return { + title: `documentSymbol ${relPath}`, + metadata: { result }, + output: result.length === 0 ? `No document symbols found` : JSON.stringify(result, null, 2), + } + } + + if (args.line === undefined || args.character === undefined) { + throw new Error(`line and character are required for operation '${args.operation}'`) + } + + const position = { file, line: args.line - 1, character: args.character - 1 } + const detail = `${relPath}:${args.line}:${args.character}` + const title = `${args.operation} ${detail}` + const result: unknown[] = yield* (() => { switch (args.operation) { case "goToDefinition": @@ -86,10 +106,6 @@ export const LspTool = Tool.define( return lsp.references(position) case "hover": return lsp.hover(position) - case "documentSymbol": - return lsp.documentSymbol(uri) - case "workspaceSymbol": - return lsp.workspaceSymbol(args.query ?? "") case "goToImplementation": return lsp.implementation(position) case "prepareCallHierarchy": @@ -98,6 +114,8 @@ export const LspTool = Tool.define( return lsp.incomingCalls(position) case "outgoingCalls": return lsp.outgoingCalls(position) + default: + throw new Error(`Unknown operation: ${args.operation}`) } })() @@ -106,7 +124,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), + }), } }), ) diff --git a/packages/opencode/test/tool/lsp.test.ts b/packages/opencode/test/tool/lsp.test.ts index 8a3189cab9c0..0bc6a3392cc4 100644 --- a/packages/opencode/test/tool/lsp.test.ts +++ b/packages/opencode/test/tool/lsp.test.ts @@ -1,5 +1,5 @@ import { afterEach, describe, expect } from "bun:test" -import { Effect, Layer } from "effect" +import { Cause, Effect, Exit, Layer } from "effect" import path from "path" import { Agent } from "../../src/agent/agent" import { CrossSpawnSpawner } from "@opencode-ai/core/cross-spawn-spawner" @@ -29,8 +29,6 @@ const ctx = { ask: () => Effect.void, } -const workspaceSymbolQueries: string[] = [] - const lsp = Layer.succeed( LSP.Service, LSP.Service.of({ @@ -44,11 +42,7 @@ const lsp = Layer.succeed( references: () => Effect.succeed([]), implementation: () => Effect.succeed([]), documentSymbol: () => Effect.succeed([]), - workspaceSymbol: (query) => - Effect.sync(() => { - workspaceSymbolQueries.push(query) - return [] - }), + workspaceSymbol: () => Effect.succeed([]), prepareCallHierarchy: () => Effect.succeed([]), incomingCalls: () => Effect.succeed([]), outgoingCalls: () => Effect.succeed([]), @@ -78,6 +72,19 @@ const run = Effect.fn("LspToolTest.run")(function* ( return yield* tool.execute(args, next) }) +const fail = Effect.fn("LspToolTest.fail")(function* ( + dir: string, + args: Tool.InferParameters, + next: Tool.Context = ctx, +) { + const exit = yield* run(args, next).pipe(Effect.exit) + if (Exit.isFailure(exit)) { + const err = Cause.squash(exit.cause) + return err instanceof Error ? err : new Error(String(err)) + } + throw new Error("expected lsp to fail") +}) + const put = Effect.fn("LspToolTest.put")(function* (file: string) { const fs = yield* AppFileSystem.Service yield* fs.writeWithDirs(file, "export const x = 1\n") @@ -131,7 +138,7 @@ describe("tool.lsp", () => { yield* put(file) const { items, next } = asks() - const result = yield* run({ operation: "documentSymbol", filePath: file, line: 3, character: 7 }, next) + const result = yield* run({ operation: "documentSymbol", filePath: file }, next) const req = items.find((item) => item.permission === "lsp") expect(req).toBeDefined() @@ -145,40 +152,189 @@ describe("tool.lsp", () => { ), ) - it.live("omits file and cursor details for workspaceSymbol", () => + it.live("workspaceSymbol has no permission request or cursor details", () => + provideTmpdirInstance( + (dir) => + Effect.gen(function* () { + const { items, next } = asks() + const result = yield* run({ operation: "workspaceSymbol", query: "TestSymbol" }, next) + const req = items.find((item) => item.permission === "lsp") + + expect(req).toBeUndefined() + expect(result.title).toBe(`workspaceSymbol "TestSymbol"`) + }), + { git: true }, + ), + ) + }) + + describe("required parameters", () => { + it.live("workspaceSymbol requires query", () => + provideTmpdirInstance( + (dir) => + Effect.gen(function* () { + const err = yield* fail(dir, { operation: "workspaceSymbol" }, ctx) + expect(err.message).toContain("query is required") + }), + { git: true }, + ), + ) + + it.live("workspaceSymbol works with query", () => + provideTmpdirInstance( + (dir) => + Effect.gen(function* () { + const result = yield* run({ operation: "workspaceSymbol", query: "Foo" }, ctx) + expect(result.title).toBe(`workspaceSymbol "Foo"`) + }), + { git: true }, + ), + ) + + const positionOps = [ + "goToDefinition", + "findReferences", + "hover", + "goToImplementation", + "prepareCallHierarchy", + "incomingCalls", + "outgoingCalls", + ] as const + + for (const op of positionOps) { + it.live(`${op} requires filePath, line, character`, () => + provideTmpdirInstance( + (dir) => + Effect.gen(function* () { + const file = path.join(dir, "test.ts") + yield* put(file) + + const err1 = yield* fail(dir, { operation: op, line: 1, character: 0 }, ctx) + expect(err1.message).toContain("filePath is required") + + const err2 = yield* fail(dir, { operation: op, filePath: file, character: 0 }, ctx) + expect(err2.message).toContain("line and character are required") + + const err3 = yield* fail(dir, { operation: op, filePath: file, line: 1 }, ctx) + expect(err3.message).toContain("line and character are required") + + const { items, next } = asks() + const result = yield* run({ operation: op, filePath: file, line: 1, character: 1 }, next) + expect(result.title).toBe(`${op} test.ts:1:1`) + const req = items.find((item) => item.permission === "lsp") + expect(req!.metadata).toEqual({ + operation: op, + filePath: file, + line: 1, + character: 1, + }) + }), + { git: true }, + ), + ) + } + + it.live("documentSymbol requires only filePath", () => provideTmpdirInstance( (dir) => Effect.gen(function* () { - workspaceSymbolQueries.length = 0 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 result = yield* run({ operation: "documentSymbol", filePath: file }, next) + expect(result.title).toBe("documentSymbol test.ts") const req = items.find((item) => item.permission === "lsp") - - expect(req).toBeDefined() expect(req!.metadata).toEqual({ - operation: "workspaceSymbol", + operation: "documentSymbol", + filePath: file, }) - expect(result.title).toBe("workspaceSymbol") }), { git: true }, ), ) - it.live("passes workspaceSymbol query to LSP", () => + it.live("query is ignored by non-workspaceSymbol operations", () => provideTmpdirInstance( (dir) => Effect.gen(function* () { - workspaceSymbolQueries.length = 0 const file = path.join(dir, "test.ts") yield* put(file) - yield* run({ operation: "workspaceSymbol", filePath: file, line: 3, character: 7, query: "TestSymbol" }) - yield* run({ operation: "workspaceSymbol", filePath: file, line: 3, character: 7 }) + const nonPositionOps = [ + "goToDefinition", + "findReferences", + "hover", + "documentSymbol", + "goToImplementation", + "prepareCallHierarchy", + "incomingCalls", + "outgoingCalls", + ] as const + for (const op of nonPositionOps) { + const result = yield* run( + { + operation: op, + filePath: file, + line: 1, + character: 1, + query: "shouldBeIgnored", + } as unknown as Tool.InferParameters, + ctx, + ) + expect(result).toBeDefined() + } + }), + { git: true }, + ), + ) + + it.live("workspaceSymbol ignores line and character", () => + provideTmpdirInstance( + (dir) => + Effect.gen(function* () { + const { items, next } = asks() + const result = yield* run( + { + operation: "workspaceSymbol", + query: "Foo", + filePath: "ignored.ts", + line: 42, + character: 99, + } as unknown as Tool.InferParameters, + next, + ) + expect(result.title).toBe(`workspaceSymbol "Foo"`) + const req = items.find((item) => item.permission === "lsp") + expect(req).toBeUndefined() + }), + { git: true }, + ), + ) + + it.live("documentSymbol ignores line and character", () => + provideTmpdirInstance( + (dir) => + Effect.gen(function* () { + const file = path.join(dir, "test.ts") + yield* put(file) - expect(workspaceSymbolQueries).toEqual(["TestSymbol", ""]) + const { items, next } = asks() + const result = yield* run( + { + operation: "documentSymbol", + filePath: file, + line: 42, + character: 99, + } as unknown as Tool.InferParameters, + next, + ) + expect(result.title).toBe("documentSymbol test.ts") + const req = items.find((item) => item.permission === "lsp") + expect(req!.metadata).toEqual({ + operation: "documentSymbol", + filePath: file, + }) }), { git: true }, ),