From 78481afec0c48cc31f2e520e6ae063dab188ced9 Mon Sep 17 00:00:00 2001 From: jyl Date: Fri, 6 Feb 2026 18:58:18 +0800 Subject: [PATCH 1/4] fix: work queue bug --- packages/opencode/src/global/index.ts | 65 +-- packages/opencode/src/project/bootstrap.ts | 2 + packages/opencode/src/session/processor.ts | 437 ++++++++++++++---- .../opencode/src/session/tool-dependency.ts | 15 +- .../src/session/work-queue/decision.ts | 19 +- .../opencode/src/session/work-queue/graph.ts | 86 +++- .../opencode/src/session/work-queue/loop.ts | 46 +- .../src/session/work-queue/processor.ts | 44 +- packages/opencode/src/tool/bash.ts | 9 + packages/opencode/src/tool/batch.ts | 83 +++- packages/opencode/src/tool/registry.ts | 24 +- packages/opencode/src/tool/tool.ts | 16 + 12 files changed, 666 insertions(+), 180 deletions(-) diff --git a/packages/opencode/src/global/index.ts b/packages/opencode/src/global/index.ts index 10b6125a6..5d262084f 100644 --- a/packages/opencode/src/global/index.ts +++ b/packages/opencode/src/global/index.ts @@ -23,33 +23,42 @@ export namespace Global { config, state, } -} -await Promise.all([ - fs.mkdir(Global.Path.data, { recursive: true }), - fs.mkdir(Global.Path.config, { recursive: true }), - fs.mkdir(Global.Path.state, { recursive: true }), - fs.mkdir(Global.Path.log, { recursive: true }), - fs.mkdir(Global.Path.bin, { recursive: true }), -]) - -const CACHE_VERSION = "21" - -const version = await Bun.file(path.join(Global.Path.cache, "version")) - .text() - .catch(() => "0") - -if (version !== CACHE_VERSION) { - try { - const contents = await fs.readdir(Global.Path.cache) - await Promise.all( - contents.map((item) => - fs.rm(path.join(Global.Path.cache, item), { - recursive: true, - force: true, - }), - ), - ) - } catch (e) {} - await Bun.file(path.join(Global.Path.cache, "version")).write(CACHE_VERSION) + let initialized = false + export async function init() { + if (initialized) return + await Promise.all([ + fs.mkdir(Global.Path.data, { recursive: true }), + fs.mkdir(Global.Path.config, { recursive: true }), + fs.mkdir(Global.Path.state, { recursive: true }), + fs.mkdir(Global.Path.log, { recursive: true }), + fs.mkdir(Global.Path.bin, { recursive: true }), + fs.mkdir(Global.Path.cache, { recursive: true }), + ]) + + const CACHE_VERSION = "21" + + const version = await Bun.file(path.join(Global.Path.cache, "version")) + .text() + .catch(() => "0") + + if (version !== CACHE_VERSION) { + try { + const contents = await fs.readdir(Global.Path.cache) + await Promise.all( + contents.map((item) => + fs.rm(path.join(Global.Path.cache, item), { + recursive: true, + force: true, + }), + ), + ) + } catch (e) {} + await Bun.file(path.join(Global.Path.cache, "version")).write(CACHE_VERSION) + } + initialized = true + } } + +// We still want to trigger initialization but not as a top-level await that blocks CJS require +Global.init().catch(console.error) diff --git a/packages/opencode/src/project/bootstrap.ts b/packages/opencode/src/project/bootstrap.ts index efdcaba99..9c23ac3b7 100644 --- a/packages/opencode/src/project/bootstrap.ts +++ b/packages/opencode/src/project/bootstrap.ts @@ -1,4 +1,5 @@ import { Plugin } from "../plugin" +import { Global } from "../global" import { Share } from "../share/share" import { Format } from "../format" import { LSP } from "../lsp" @@ -16,6 +17,7 @@ import { Truncate } from "../tool/truncation" export async function InstanceBootstrap() { Log.Default.info("bootstrapping", { directory: Instance.directory }) + await Global.init() await Plugin.init() Share.init() ShareNext.init() diff --git a/packages/opencode/src/session/processor.ts b/packages/opencode/src/session/processor.ts index 7d2ed72e4..6b82c3b73 100644 --- a/packages/opencode/src/session/processor.ts +++ b/packages/opencode/src/session/processor.ts @@ -35,6 +35,17 @@ export namespace SessionProcessor { partId: string callId: string abort: AbortSignal + + // Refactoring 1: Unified interface + execute(ctx: { + sessionID: string; + assistantMessage: MessageV2.Assistant; + agent: Agent.Info; + tools: Record; + }): Promise + getTimeout(): number + getResourceKeys(): Set + getDependencies(): string[] } interface ToolExecutionResult { @@ -44,6 +55,175 @@ export namespace SessionProcessor { type ResourceLockMode = "shared" | "exclusive" + /** + * ToolScheduler handles the execution of multiple tools with dependency management, + * concurrency control, and resource locking. + * + * @VertxThreadSafety + */ + class ToolScheduler { + private pending = new Set() + private running = new Set() + private completed = new Set() + private results = new Map() + + constructor( + private executors: ToolExecutor[], + private tools: Record, + private input: { + sessionID: string + assistantMessage: MessageV2.Assistant + agent: Agent.Info + }, + private options: { + limiter: { run(fn: () => Promise): Promise } + resourceLockManager: { acquire(keys: Set, mode: ResourceLockMode): Promise<() => void> } + onToolExecuted?: (result: ToolExecutionResult, executor: ToolExecutor) => void + }, + ) { + for (const e of executors) { + this.pending.add(e.callId) + } + } + + /** + * Executes all scheduled tools. + * + * @returns {Promise} + * @throws {Error} If execution fails + */ + async execute(): Promise { + const toolCalls: MessageV2.ToolPart[] = this.executors.map((e) => ({ + id: e.partId, + sessionID: this.input.sessionID, + messageID: this.input.assistantMessage.id, + type: "tool", + callID: e.callId, + tool: e.toolName, + state: { + status: "pending" as const, + input: e.input, + raw: "", + }, + })) + + // Use Refactoring 4: Optimized dependency analysis + const dependencies = new Map>() + for (const e of this.executors) { + dependencies.set(e.callId, new Set(e.getDependencies())) + } + + // Check for circular dependencies + const checkCycle = (id: string, visited = new Set(), stack = new Set()): boolean => { + visited.add(id) + stack.add(id) + const deps = dependencies.get(id) + if (deps) { + for (const d of deps) { + if (!visited.has(d)) { + if (checkCycle(d, visited, stack)) return true + } else if (stack.has(d)) { + return true + } + } + } + stack.delete(id) + return false + } + + for (const e of this.executors) { + if (checkCycle(e.callId)) { + log.error("Circular dependency detected in tool calls", { callId: e.callId }) + // Break the cycle by clearing dependencies for this executor + dependencies.set(e.callId, new Set()) + } + } + + const scheduleNext = async () => { + const ready = this.executors.filter((e) => { + if (!this.pending.has(e.callId)) return false + const deps = dependencies.get(e.callId) + if (!deps) return true + return Array.from(deps).every((d) => this.completed.has(d)) + }) + + if (ready.length === 0 && this.running.size === 0 && this.pending.size > 0) { + log.error("Deadlock detected in tool dependencies", { + pending: Array.from(this.pending), + completed: Array.from(this.completed), + }) + return + } + + const promises = ready.map(async (executor) => { + this.pending.delete(executor.callId) + this.running.add(executor.callId) + + try { + await this.options.limiter.run(async () => { + const keys = executor.getResourceKeys() + const mode = toolLockMode(executor.toolName) + const release = await this.options.resourceLockManager.acquire(keys, mode) + try { + // Bug 8: Apply timeout from executor + const timeout = executor.getTimeout() + const resultPromise = executor.execute({ + ...this.input, + tools: this.tools, + }) + + let result: ToolExecutionResult + if (timeout > 0) { + const timeoutPromise = new Promise((_, reject) => + setTimeout(() => reject(new Error(`Tool ${executor.toolName} timed out after ${timeout}ms`)), timeout), + ) + result = await Promise.race([resultPromise, timeoutPromise]) + } else { + result = await resultPromise + } + + this.results.set(executor.callId, result) + this.options.onToolExecuted?.(result, executor) + } finally { + release() + } + }) + } catch (error) { + log.error("tool execution failed", { + sessionID: this.input.sessionID, + tool: executor.toolName, + callId: executor.callId, + error, + }) + // Bug 9: Mark as error instead of silent failure + await Session.updatePart({ + id: executor.partId, + messageID: this.input.assistantMessage.id, + sessionID: this.input.sessionID, + type: "tool", + tool: executor.toolName, + callID: executor.callId, + state: { + status: "error", + input: executor.input, + error: error instanceof Error ? error.message : String(error), + time: { start: Date.now(), end: Date.now() }, + }, + }) + } finally { + this.running.delete(executor.callId) + this.completed.add(executor.callId) + await scheduleNext() + } + }) + + await Promise.all(promises) + } + + await scheduleNext() + } + } + function toolLockMode(toolName: string): ResourceLockMode { if (toolName === "read" || toolName === "grep") return "shared" return "exclusive" @@ -140,12 +320,40 @@ export namespace SessionProcessor { } const acquire = async (keys: Set, mode: ResourceLockMode): Promise<() => void> => { - const sorted = Array.from(keys).sort() + const sortedKeys = Array.from(keys).sort() + + // To avoid deadlock, we must acquire all keys or none. + // But since we sort them, we can acquire them one by one IF we don't allow + // other acquisitions to jump in between and create a cycle. + // However, a simpler way to avoid deadlock with sorting is to ensure that + // if we can't get a key, we don't hold the ones we already got? + // No, sorting IS enough if all keys are acquired in the same order. + // The cycle A->B, B->C, C->A is impossible if everyone must acquire in order A, B, C. + // Tool 1: A, B. Gets A, waits for B. + // Tool 2: B, C. Gets B, waits for C. + // Tool 3: A, C. Waits for A. + // Tool 1 will eventually get B when T2 finishes. + // Wait, T2 is waiting for C. Who has C? + // If T4 has C and is waiting for... nothing? Then T4 finishes, T2 gets C, finishes, T1 gets B, finishes, T3 gets A. + // The only way to deadlock is a cycle. A cycle requires at least one person to acquire in a different order. + // e.g. T1: A then B, T2: B then A. + // With sorting, T2 becomes A then B. No cycle. + + // So the current implementation is actually deadlock-free IF the only way locks are acquired is through this `acquire` method. + // The real issue might be the Limiter interaction. + const releases: Array<() => void> = [] - for (const key of sorted) { - const release = await acquireKey(key, mode) - releases.push(release) + try { + for (const key of sortedKeys) { + const release = await acquireKey(key, mode) + releases.push(release) + } + } catch (e) { + // Cleanup if something goes wrong + for (const r of releases) r() + throw e } + return () => { for (let i = releases.length - 1; i >= 0; i--) { releases[i]!() @@ -270,80 +478,87 @@ export namespace SessionProcessor { ): Promise { if (executors.length === 0) return - const toolCalls: MessageV2.ToolPart[] = executors.map((e) => ({ - id: e.partId, - sessionID: input.sessionID, - messageID: input.assistantMessage.id, - type: "tool", - callID: e.callId, - tool: e.toolName, - state: { - status: "pending" as const, - input: e.input, - raw: "", - }, - })) - - const dependencyResult = ToolDependency.analyze(toolCalls) - - log.debug("dependency analysis", { - sessionID: input.sessionID, - levels: dependencyResult.levels.length, - totalCalls: toolCalls.length, - }) - const limit = Math.max(1, maxParallel ?? 10) const limiter = shared?.limiter ?? (() => { let active = 0 const queue: Array<() => void> = [] + const notify = () => { + while (active < limit && queue.length > 0) { + const next = queue.shift() + if (next) { + active++ + next() + } + } + } async function run(fn: () => Promise): Promise { - while (active >= limit) { + if (active >= limit) { await new Promise((resolve) => queue.push(resolve)) + } else { + active++ } - active++ try { return await fn() } finally { active-- - const next = queue.shift() - if (next) next() + notify() } } - return { run } + return { run, notify } })() + const resourceLockManager = shared?.resourceLockManager ?? createResourceLockManager() - const getKeys = (executor: ToolExecutor) => { - const call = toolCalls.find((c) => c.callID === executor.callId) - if (!call) return new Set([`tool:${executor.toolName}`]) - return ToolDependency.resourceKeys(call) - } - for (const level of dependencyResult.levels) { - const parallelExecutors = level.calls.map((call) => { - const executor = executors.find((e) => e.callId === call.id)! - return limiter - .run(async () => { - const keys = getKeys(executor) - const mode = toolLockMode(executor.toolName) - const release = await resourceLockManager.acquire(keys, mode) - try { - const result = await executeTool(executor, tools, input) - shared?.onToolExecuted?.(result, executor) - } finally { - release() - } - }) - .catch((error) => { - log.error("tool execution failed", { - sessionID: input.sessionID, - tool: executor.toolName, - callId: executor.callId, - error, - }) - }) - }) + const scheduler = new ToolScheduler(executors, tools, input, { + limiter, + resourceLockManager, + onToolExecuted: shared?.onToolExecuted, + }) + + await scheduler.execute() + } - await Promise.all(parallelExecutors) + function createToolExecutor( + toolName: string, + input: Record, + partId: string, + callId: string, + abort: AbortSignal, + toolPart: MessageV2.ToolPart, + ): ToolExecutor { + return { + toolId: toolName, + toolName, + input, + partId, + callId, + abort, + async execute(ctx) { + return executeTool(this, ctx.tools, ctx) + }, + getTimeout() { + const tool = ToolRegistry.getToolSync?.(this.toolName) + if (tool?.getTimeout) { + return tool.getTimeout(this.input) + } + // Bug 8: Default timeout 60s, can be overridden by specific tools if needed + return 60_000 + }, + getResourceKeys() { + const tool = ToolRegistry.getToolSync?.(this.toolName) + if (tool?.getResourceKeys) { + return tool.getResourceKeys(this.input) + } + return ToolDependency.resourceKeys(toolPart) + }, + getDependencies() { + const tool = ToolRegistry.getToolSync?.(this.toolName) + if (tool?.getDependencies) { + return tool.getDependencies(this.input) + } + const result = ToolDependency.analyze([toolPart]) + return Array.from(result.dependencies.get(this.callId) ?? []) + }, } } @@ -386,20 +601,29 @@ export namespace SessionProcessor { const limiter = (() => { let active = 0 const queue: Array<() => void> = [] + const notify = () => { + while (active < concurrencyRef.value && queue.length > 0) { + const next = queue.shift() + if (next) { + active++ + next() + } + } + } async function run(fn: () => Promise): Promise { - while (active >= concurrencyRef.value) { + if (active >= concurrencyRef.value) { await new Promise((resolve) => queue.push(resolve)) + } else { + active++ } - active++ try { return await fn() } finally { active-- - const next = queue.shift() - if (next) next() + notify() } } - return { run } + return { run, notify } })() const onToolExecuted = (r: ToolExecutionResult) => { toolSampleCount++ @@ -415,7 +639,12 @@ export namespace SessionProcessor { if (errRate >= 0.25) next = Math.max(1, Math.floor(prev * 0.7)) else if (avg >= 2_500) next = Math.max(1, prev - 1) else if (avg <= 600) next = Math.min(maxParallelTools, prev + 1) - concurrencyRef.value = next + + if (next !== prev) { + concurrencyRef.value = next + limiter.notify() // Notify waiting tasks if concurrency increased + } + toolSampleCount = 0 toolErrorCount = 0 toolDurationSum = 0 @@ -431,17 +660,43 @@ export namespace SessionProcessor { const executors = pendingExecutors pendingExecutors = [] const tools = streamInput.tools - flushPromise = executeToolsParallel( - executors, - tools, - { - sessionID: input.sessionID, - assistantMessage: input.assistantMessage, - agent: agentInfo, - }, - maxParallelTools, - { resourceLockManager, limiter, onToolExecuted }, - ).finally(() => { + + const doFlush = async () => { + try { + await executeToolsParallel( + executors, + tools, + { + sessionID: input.sessionID, + assistantMessage: input.assistantMessage, + agent: agentInfo, + }, + maxParallelTools, + { resourceLockManager, limiter, onToolExecuted }, + ) + } catch (error) { + log.error("flush failed", { error }) + // Bug 9: Mark all tasks in this batch as failed if the flush itself fails + for (const e of executors) { + await Session.updatePart({ + id: e.partId, + messageID: input.assistantMessage.id, + sessionID: input.sessionID, + type: "tool", + tool: e.toolName, + callID: e.callId, + state: { + status: "error", + input: e.input, + error: `Flush failed: ${error instanceof Error ? error.message : String(error)}`, + time: { start: Date.now(), end: Date.now() }, + }, + }) + } + } + } + + flushPromise = doFlush().finally(() => { flushPromise = null }) } @@ -628,14 +883,16 @@ export namespace SessionProcessor { toolcalls[value.toolCallId] = part as MessageV2.ToolPart if (parallelEnabled) { - pendingExecutors.push({ - toolId: value.toolName, - toolName: value.toolName, - input: value.input, - partId: part.id, - callId: value.toolCallId, - abort: input.abort, - }) + pendingExecutors.push( + createToolExecutor( + value.toolName, + value.input, + part.id, + value.toolCallId, + input.abort, + part as MessageV2.ToolPart, + ), + ) if (pendingExecutors.length >= Math.min(2, maxParallelTools)) { scheduleFlush() } @@ -673,7 +930,7 @@ export namespace SessionProcessor { } case "tool-result": { const match = toolcalls[value.toolCallId] - if (match && match.state.status === "running") { + if (match && (match.state.status === "running" || match.state.status === "pending")) { const attachments = value.output.attachments?.map( (attachment: Omit) => ({ ...attachment, @@ -691,7 +948,7 @@ export namespace SessionProcessor { metadata: value.output.metadata, title: value.output.title, time: { - start: match.state.time.start, + start: (match.state as any).time?.start ?? Date.now(), end: Date.now(), }, attachments, @@ -705,7 +962,7 @@ export namespace SessionProcessor { case "tool-error": { const match = toolcalls[value.toolCallId] - if (match && match.state.status === "running") { + if (match && (match.state.status === "running" || match.state.status === "pending")) { await Session.updatePart({ ...match, state: { @@ -713,7 +970,7 @@ export namespace SessionProcessor { input: value.input ?? match.state.input, error: (value.error as any).toString(), time: { - start: match.state.time.start, + start: (match.state as any).time?.start ?? Date.now(), end: Date.now(), }, }, diff --git a/packages/opencode/src/session/tool-dependency.ts b/packages/opencode/src/session/tool-dependency.ts index 94d8725ae..c33fcc8db 100644 --- a/packages/opencode/src/session/tool-dependency.ts +++ b/packages/opencode/src/session/tool-dependency.ts @@ -75,7 +75,7 @@ export namespace ToolDependency { function getToolDependencies( call: PendingCall, - allCalls: PendingCall[], + allCallsMap: Map, executedResults: Map, ): Set { const dependencies = new Set() @@ -86,7 +86,7 @@ export namespace ToolDependency { if (typeof targetPath !== "string") break for (const [id, result] of executedResults) { - const prevCall = allCalls.find((c) => c.id === id) + const prevCall = allCallsMap.get(id) if (!prevCall) continue if (prevCall.tool === "read" && prevCall.input.path === targetPath) { @@ -104,7 +104,7 @@ export namespace ToolDependency { if (typeof targetPath !== "string") break for (const [id, result] of executedResults) { - const prevCall = allCalls.find((c) => c.id === id) + const prevCall = allCallsMap.get(id) if (!prevCall) continue if (prevCall.tool === "read" && prevCall.input.path === targetPath) { @@ -119,7 +119,7 @@ export namespace ToolDependency { if (typeof command !== "string") break for (const [id, result] of executedResults) { - const prevCall = allCalls.find((c) => c.id === id) + const prevCall = allCallsMap.get(id) if (!prevCall) continue if (prevCall.tool === "read") { @@ -140,7 +140,7 @@ export namespace ToolDependency { if (typeof targetPath !== "string") break for (const [id, result] of executedResults) { - const prevCall = allCalls.find((c) => c.id === id) + const prevCall = allCallsMap.get(id) if (!prevCall) continue if (prevCall.tool === "read" && prevCall.input.path === targetPath) { @@ -155,7 +155,7 @@ export namespace ToolDependency { if (typeof targetPath !== "string" || targetPath.includes("*")) break for (const [id, result] of executedResults) { - const prevCall = allCalls.find((c) => c.id === id) + const prevCall = allCallsMap.get(id) if (!prevCall) continue if (prevCall.tool === "read" && prevCall.input.path === targetPath) { @@ -206,9 +206,10 @@ export namespace ToolDependency { const dependencies = new Map>() const dependents = new Map>() + const pendingCallsMap = new Map(pendingCalls.map((c) => [c.id, c])) for (const call of pendingCalls) { - const deps = getToolDependencies(call, pendingCalls, executedResults) + const deps = getToolDependencies(call, pendingCallsMap, executedResults) dependencies.set(call.id, deps) for (const depId of deps) { diff --git a/packages/opencode/src/session/work-queue/decision.ts b/packages/opencode/src/session/work-queue/decision.ts index fa20478da..1295cd606 100644 --- a/packages/opencode/src/session/work-queue/decision.ts +++ b/packages/opencode/src/session/work-queue/decision.ts @@ -23,7 +23,22 @@ export interface RelevanceResult { reason: string } +/** + * 代理决策中心 + * + * 职责： + * 1. 根据任务板状态决定下一步动作 + * 2. 处理任务完成、错误和用户输入 + * + * 线程模型： + * @VertxThreadSafety 默认在单线程事件循环中运行 + */ export class AgentDecisionCenter { + /** + * 决定下一个动作 + * @param board 任务汇总板 + * @returns 返回建议的动作 + */ decideNext(board: TaskSummaryBoard): AgentAction { const current = board.getCurrentTask() @@ -38,13 +53,13 @@ export class AgentDecisionCenter { const blocked = board.getByStatus("blocked") if (pending.length > 0) { - const nextTask = pending.sort((a, b) => b.priority - a.priority)[0] + const nextTask = [...pending].sort((a, b) => b.priority - a.priority)[0] return { type: "start_next", taskID: nextTask.id } } const unblocked = blocked.filter((t) => t.blockedBy.length === 0) if (unblocked.length > 0) { - const nextTask = unblocked.sort((a, b) => b.priority - a.priority)[0] + const nextTask = [...unblocked].sort((a, b) => b.priority - a.priority)[0] return { type: "unblock", taskIDs: [nextTask.id] } } diff --git a/packages/opencode/src/session/work-queue/graph.ts b/packages/opencode/src/session/work-queue/graph.ts index 433af4623..0ed109f45 100644 --- a/packages/opencode/src/session/work-queue/graph.ts +++ b/packages/opencode/src/session/work-queue/graph.ts @@ -22,17 +22,43 @@ export interface TaskGraphResult { maxLevel: number } +/** + * 任务图 + * + * 职责： + * 1. 构建任务依赖图 + * 2. 计算任务层级（分层执行） + * + * 线程模型： + * @VertxThreadSafety + */ export class TaskGraph { private nodes: Map = new Map() private allTasks: MessageV2.SubtaskPart[] = [] + /** + * 构造函数 + * @param tasks 任务列表 + */ constructor(tasks: MessageV2.SubtaskPart[]) { this.allTasks = tasks this.buildGraph() } + /** + * 获取任务ID + * @param task 任务部分 + * @returns 唯一ID + */ private getTaskId(task: MessageV2.SubtaskPart): string { - return `${task.agent}:${task.prompt.substring(0, 50)}` + // 使用 agent, description 和 prompt 的组合来确保唯一性 (Bug 9) + const content = `${task.agent}:${task.description || ""}:${task.prompt}` + let hash = 0 + for (let i = 0; i < content.length; i++) { + hash = (hash << 5) - hash + content.charCodeAt(i) + hash |= 0 // Convert to 32bit integer + } + return `${task.agent}:${hash.toString(36)}:${task.prompt.substring(0, 30)}` } private buildGraph(): void { @@ -243,8 +269,15 @@ interface SchedulerNode { reject: (error: any) => void isRunning: boolean isCompleted: boolean + isFailed: boolean // 新增: 标记任务是否失败 (Bug 7) } + /** + * 执行任务层级 + * @param levels 任务层级列表 + * @param executeSubtask 执行子任务的回调 + * @param maxParallel 最大并行数 + */ export async function executeTaskLevels( levels: TaskLevel[], executeSubtask: (task: MessageV2.SubtaskPart) => Promise, @@ -270,21 +303,31 @@ export async function executeTaskLevels( reject: () => {}, isRunning: false, isCompleted: false, + isFailed: false, }) } } - async function tryStartTask(nodeId: string): Promise { + // 修改: 去掉 async, 确保在循环检查时不产生 yield (Bug 4) + function tryStartTask(nodeId: string): void { const node = nodes.get(nodeId) - if (!node || node.isRunning || node.isCompleted) return + if (!node || node.isRunning || node.isCompleted || node.isFailed) return const deps = Array.from(node.dependencies) const canStart = deps.every((depId) => { const depNode = nodes.get(depId) - return depNode?.isCompleted + return depNode?.isCompleted // 只有成功完成的任务才算依赖达成 }) - if (!canStart) return + if (!canStart) { + // 检查是否有依赖失败了 + const hasFailedDep = deps.some((depId) => nodes.get(depId)?.isFailed) + if (hasFailedDep) { + node.isFailed = true + node.reject(new Error(`Dependency failed for task: ${node.id}`)) + } + return + } if (runningCount >= limit) return @@ -292,34 +335,31 @@ export async function executeTaskLevels( running.add(nodeId) runningCount++ - const depPromises = deps.map((depId) => nodes.get(depId)?.promise).filter(Boolean) as Promise[] - - const executePromise = (async () => { + // 真正的异步执行逻辑 + void (async () => { try { - if (depPromises.length > 0) { - await Promise.all(depPromises) - } - await executeSubtask(node.task).catch((error) => { - log.error("Subtask execution failed", { - agent: node.task.agent, - description: node.task.description, - error, - }) + await executeSubtask(node.task) + node.isCompleted = true + node.resolve() + } catch (error) { + log.error("Subtask execution failed", { + agent: node.task.agent, + description: node.task.description, + error, }) + node.isFailed = true + node.reject(error) } finally { - node.isCompleted = true + node.isRunning = false running.delete(nodeId) runningCount-- - node.resolve() } })() - - node.promise = executePromise } - async function tryStartAll(): Promise { + function tryStartAll(): void { for (const [nodeId] of nodes) { - await tryStartTask(nodeId) + tryStartTask(nodeId) } } diff --git a/packages/opencode/src/session/work-queue/loop.ts b/packages/opencode/src/session/work-queue/loop.ts index 3f84a17d8..d2aa85c00 100644 --- a/packages/opencode/src/session/work-queue/loop.ts +++ b/packages/opencode/src/session/work-queue/loop.ts @@ -11,10 +11,20 @@ const log = Log.create({ service: "work-queue.loop" }) const TICK_IDLE_TIMEOUT = 50 const TICK_BUSY_TIMEOUT = 5 +/** + * 任务事件循环 + * + * 职责： + * 1. 管理任务队列和执行状态 + * 2. 调度任务执行并处理事件 + * + * 线程模型： + * @VertxThreadSafety 异步非阻塞事件循环 + */ export class EventLoop { private board: TaskSummaryBoard private decision: AgentDecisionCenter - private runningTasks: Map = new Map() + private runningTasks: Map = new Map() private taskAbortControllers: Map = new Map() private maxConcurrency = 2 private eventQueue: AsyncQueue = new AsyncQueue() @@ -23,8 +33,13 @@ export class EventLoop { private isRunning = false private wakeUpResolver: (() => void) | null = null - constructor(sessionID: string) { - this.board = new TaskSummaryBoard() + /** + * 构造函数 + * @param sessionID 会话ID + * @param board 可选的任务板 + */ + constructor(sessionID: string, board?: TaskSummaryBoard) { + this.board = board ?? new TaskSummaryBoard() this.decision = new AgentDecisionCenter() this.abortController = new AbortController() this.setupEventListeners() @@ -72,6 +87,9 @@ export class EventLoop { this.wakeUp() } + /** + * 启动事件循环 + */ async start(): Promise { if (this.isRunning) return this.isRunning = true @@ -89,6 +107,9 @@ export class EventLoop { log.info("EventLoop stopped") } + /** + * 停止事件循环 + */ stop(): void { this.isRunning = false this.abortController.abort() @@ -115,15 +136,6 @@ export class EventLoop { const canSchedule = this.canSchedule() if (canSchedule) { - const nextTask = this.getNextTask() - if (nextTask) { - const action = this.decision.decideNext(this.board) - if (action.type === "start_next" || action.type === "continue") { - await this.executeAction(action) - return - } - } - const action = this.decision.decideNext(this.board) if (action.type !== "idle" && action.type !== "wait") { await this.executeAction(action) @@ -190,6 +202,7 @@ export class EventLoop { this.taskAbortControllers.delete(taskID) } this.runningTasks.delete(taskID) + this.pendingQueue.delete(taskID) this.board.pause(taskID, checkpoint) } @@ -235,7 +248,9 @@ export class EventLoop { if (this.runningTasks.size >= this.maxConcurrency) return false if (this.runningTasks.size === 0) return true const oldestTask = Array.from(this.runningTasks.values()).reduce((a, b) => (a.startTime < b.startTime ? a : b)) - return Date.now() - oldestTask.startTime > TICK_BUSY_TIMEOUT * 3 + // 使用 lastHeartbeat 来判断是否繁忙，而不是 startTime + const mostRecentHeartbeat = Array.from(this.runningTasks.values()).reduce((a, b) => (a.lastHeartbeat > b.lastHeartbeat ? a : b)) + return Date.now() - mostRecentHeartbeat.lastHeartbeat > TICK_BUSY_TIMEOUT * 3 } private startTask(taskID: string): void { @@ -248,7 +263,8 @@ export class EventLoop { this.board.start(taskID) log.info("Task started", { taskID, type: task.type, priority: task.priority }) - this.runningTasks.set(taskID, { startTime: Date.now(), priority: task.priority }) + const now = Date.now() + this.runningTasks.set(taskID, { startTime: now, lastHeartbeat: now, priority: task.priority }) this.pendingQueue.delete(taskID) void this.runTask(task, controller) } @@ -272,7 +288,7 @@ export class EventLoop { onHeartbeat: () => { const taskInfo = this.runningTasks.get(task.id) if (taskInfo) { - taskInfo.startTime = Date.now() + taskInfo.lastHeartbeat = Date.now() } }, } diff --git a/packages/opencode/src/session/work-queue/processor.ts b/packages/opencode/src/session/work-queue/processor.ts index bf0f00eb6..356902c79 100644 --- a/packages/opencode/src/session/work-queue/processor.ts +++ b/packages/opencode/src/session/work-queue/processor.ts @@ -41,6 +41,16 @@ export interface TaskInput { priority?: number } +/** + * WorkQueue 会话处理器 + * + * 职责： + * 1. 提供任务提交、取消等公共 API + * 2. 依赖 EventLoop 进行并发调度 + * + * 线程模型： + * @VertxThreadSafety + */ export class WorkQueueSessionProcessor { private sessionID: string private eventLoop: EventLoop @@ -50,23 +60,26 @@ export class WorkQueueSessionProcessor { private isRunning = false private taskCallbacks: Map void; reject: (e: Error) => void }> = new Map() + /** + * 构造函数 + * @param sessionID 会话ID + * @param config 配置项 + */ constructor(sessionID: string, config?: WorkQueueProcessorConfig) { this.sessionID = sessionID this.board = new TaskSummaryBoard() - this.eventLoop = new EventLoop(sessionID) + this.eventLoop = new EventLoop(sessionID, this.board) this.config = { ...DEFAULT_CONFIG, ...config } this.abortController = new AbortController() this.setupCallbacks() } private setupCallbacks(): void { - const board = this.eventLoop.getBoard() - - board.on("task:completed", (event) => { + this.board.on("task:completed", (event) => { if (!event.taskID) return const callback = this.taskCallbacks.get(event.taskID) if (callback) { - const task = board.get(event.taskID) + const task = this.board.get(event.taskID) callback.resolve({ taskID: event.taskID, type: task?.type ?? "unknown", @@ -78,7 +91,7 @@ export class WorkQueueSessionProcessor { } }) - board.on("task:error", (event) => { + this.board.on("task:error", (event) => { if (!event.taskID) return const callback = this.taskCallbacks.get(event.taskID) if (callback) { @@ -88,6 +101,9 @@ export class WorkQueueSessionProcessor { }) } + /** + * 启动处理器 + */ async start(): Promise { if (this.isRunning) return this.isRunning = true @@ -95,14 +111,28 @@ export class WorkQueueSessionProcessor { await this.eventLoop.start() } + /** + * 停止处理器并清理资源 + */ async stop(): Promise { if (!this.isRunning) return log.info("WorkQueueSessionProcessor stopping", { sessionID: this.sessionID }) this.isRunning = false this.abortController.abort() this.eventLoop.stop() + + // 清理所有回调，防止内存泄漏 (Bug 5) + for (const [taskID, callback] of this.taskCallbacks) { + callback.reject(new Error("Processor stopped")) + this.taskCallbacks.delete(taskID) + } } + /** + * 提交通用任务 + * @param input 任务输入 + * @returns 任务结果 + */ async submitTask(input: TaskInput): Promise { const task = this.board.create({ type: input.type, @@ -160,7 +190,7 @@ export class WorkQueueSessionProcessor { } isActive(): boolean { - return this.isRunning && this.board.stats().running > 0 + return this.isRunning } } diff --git a/packages/opencode/src/tool/bash.ts b/packages/opencode/src/tool/bash.ts index 43f23472f..b98ecd56d 100644 --- a/packages/opencode/src/tool/bash.ts +++ b/packages/opencode/src/tool/bash.ts @@ -266,5 +266,14 @@ export const BashTool = Tool.define("bash", async () => { output, } }, + getTimeout(params) { + return params.timeout ?? DEFAULT_TIMEOUT + }, + getResourceKeys(params) { + const keys = new Set() + const cwd = params.workdir || Instance.directory + keys.add(cwd) + return keys + }, } }) diff --git a/packages/opencode/src/tool/batch.ts b/packages/opencode/src/tool/batch.ts index b5c3ad0a1..6b3d10794 100644 --- a/packages/opencode/src/tool/batch.ts +++ b/packages/opencode/src/tool/batch.ts @@ -1,6 +1,7 @@ import z from "zod" import { Tool } from "./tool" import DESCRIPTION from "./batch.txt" +import { MessageV2 } from "../session/message-v2" const DISALLOWED = new Set(["batch"]) const FILTERED_FROM_SUGGESTIONS = new Set(["invalid", "patch", ...DISALLOWED]) @@ -40,9 +41,31 @@ export const BatchTool = Tool.define("batch", async () => { const availableTools = await ToolRegistry.tools({ modelID: "", providerID: "" }) const toolMap = new Map(availableTools.map((t) => [t.id, t])) - const executeCall = async (call: (typeof toolCalls)[0]) => { + // Bug 7: Dependency analysis for BatchTool + const { ToolDependency } = await import("../session/tool-dependency") + const { MessageV2 } = await import("../session/message-v2") + + const fakeToolParts: MessageV2.ToolPart[] = toolCalls.map((call, index) => ({ + id: `batch-${index}`, + sessionID: ctx.sessionID, + messageID: ctx.messageID, + type: "tool", + callID: `batch-${index}`, + tool: call.tool, + state: { + status: "pending", + input: call.parameters, + raw: "", + }, + })) + + const dependencyResult = ToolDependency.analyze(fakeToolParts) + + const results: Array<{ success: boolean; tool: string; result?: any; error?: any }> = [] + const resultsMap = new Map() + + const executeCall = async (call: (typeof toolCalls)[0], partID: string) => { const callStartTime = Date.now() - const partID = Identifier.ascending("part") try { if (DISALLOWED.has(call.tool)) { @@ -76,8 +99,15 @@ export const BatchTool = Tool.define("batch", async () => { }, }) - const result = await tool.execute(validatedParams, { ...ctx, callID: partID }) - const attachments = result.attachments?.map((attachment) => ({ + // Bug 8: Add timeout handling + const timeout = 60_000 // 60s default timeout for batch sub-tools + const resultPromise = tool.execute(validatedParams, { ...ctx, callID: partID }) + const timeoutPromise = new Promise((_, reject) => + setTimeout(() => reject(new Error(`Tool '${call.tool}' timed out after ${timeout}ms`)), timeout), + ) + + const result = (await Promise.race([resultPromise, timeoutPromise])) as any + const attachments = result.attachments?.map((attachment: any) => ({ ...attachment, id: Identifier.ascending("part"), messageID: ctx.messageID, @@ -105,6 +135,7 @@ export const BatchTool = Tool.define("batch", async () => { }, }) + resultsMap.set(partID, result) return { success: true as const, tool: call.tool, result } } catch (error) { await Session.updatePart({ @@ -129,7 +160,16 @@ export const BatchTool = Tool.define("batch", async () => { } } - const results = await Promise.all(toolCalls.map((call) => executeCall(call))) + // Execute level by level + for (const level of dependencyResult.levels) { + const levelPromises = level.calls.map((callPart) => { + const index = parseInt(callPart.id.split("-")[1]!) + const call = toolCalls[index]! + return executeCall(call, callPart.id) + }) + const levelResults = await Promise.all(levelPromises) + results.push(...levelResults) + } // Add discarded calls as errors const now = Date.now() @@ -177,5 +217,38 @@ export const BatchTool = Tool.define("batch", async () => { }, } }, + getTimeout(params) { + // Batch tool itself has no strict timeout, but sub-tools do. + // We return 0 to indicate no timeout for the batch manager. + return 0 + }, + getResourceKeys(params) { + const keys = new Set() + // Batch tool accesses resources of all its sub-tools + const registry = require("./registry") + const ToolRegistry = registry.ToolRegistry || registry + for (const call of params.tool_calls) { + const tool = ToolRegistry.getToolSync?.(call.tool) + if (tool?.getResourceKeys) { + const subKeys = tool.getResourceKeys(call.parameters) + for (const k of subKeys) keys.add(k) + } + } + return keys + }, + getDependencies(params) { + // Batch tool might depend on other tools if sub-tools do. + const deps: string[] = [] + const registry = require("./registry") + const ToolRegistry = registry.ToolRegistry || registry + for (const call of params.tool_calls) { + const tool = ToolRegistry.getToolSync?.(call.tool) + if (tool?.getDependencies) { + const subDeps = tool.getDependencies(call.parameters) + deps.push(...subDeps) + } + } + return deps + }, } }) diff --git a/packages/opencode/src/tool/registry.ts b/packages/opencode/src/tool/registry.ts index 07776b163..3629ed928 100644 --- a/packages/opencode/src/tool/registry.ts +++ b/packages/opencode/src/tool/registry.ts @@ -32,6 +32,7 @@ export namespace ToolRegistry { const log = Log.create({ service: "tool.registry" }) const initializedCache = new Map>>() + let toolsCache: { agentKey: string; tools: any[] } | null = null export const state = Instance.state(async () => { const custom = [] as Tool.Info[] @@ -135,11 +136,17 @@ export namespace ToolRegistry { }, agent?: Agent.Info, ) { - const tools = await all() - const agentKey = agent?.name ?? "default" + const agentKey = agent ? agent.name : "default" + const modelKey = `${model.providerID}:${model.modelID}` + const fullKey = `${agentKey}:${modelKey}` + if (toolsCache && toolsCache.agentKey === fullKey) { + return toolsCache.tools + } + + const allTools = await all() const result = await Promise.all( - tools + allTools .filter((t) => { if (t.id === "codesearch" || t.id === "websearch") { return model.providerID === "opencode" || Flag.OPENCODE_ENABLE_EXA @@ -168,6 +175,17 @@ export namespace ToolRegistry { } }), ) + + toolsCache = { agentKey: fullKey, tools: result } return result } + + /** + * Returns a tool from the cache synchronously. + * This is used by SessionProcessor to access tool-specific logic (e.g. timeout, dependencies). + */ + export function getToolSync(id: string): any | null { + if (!toolsCache) return null + return toolsCache.tools.find((t) => t.id === id) || null + } } diff --git a/packages/opencode/src/tool/tool.ts b/packages/opencode/src/tool/tool.ts index 0e78ba665..97f66bef4 100644 --- a/packages/opencode/src/tool/tool.ts +++ b/packages/opencode/src/tool/tool.ts @@ -39,6 +39,22 @@ export namespace Tool { attachments?: Omit[] }> formatValidationError?(error: z.ZodError): string + + /** + * Returns the estimated resource keys this tool will access. + * Used for concurrency control and resource locking. + */ + getResourceKeys?(args: z.infer): Set + + /** + * Returns the dependency IDs this tool has on other tool calls. + */ + getDependencies?(args: z.infer): string[] + + /** + * Returns the timeout in milliseconds for this tool. + */ + getTimeout?(args: z.infer): number }> } From 1e4519717b695694edf18af577c2f09b135cf6b5 Mon Sep 17 00:00:00 2001 From: jyl Date: Fri, 6 Feb 2026 20:12:15 +0800 Subject: [PATCH 2/4] feat: update tools grep --- packages/opencode/src/tool/edit.ts | 611 ++++++++++++++++-------- packages/opencode/src/tool/multiedit.ts | 170 ++++++- 2 files changed, 574 insertions(+), 207 deletions(-) diff --git a/packages/opencode/src/tool/edit.ts b/packages/opencode/src/tool/edit.ts index 0bf1d6792..8162317b4 100644 --- a/packages/opencode/src/tool/edit.ts +++ b/packages/opencode/src/tool/edit.ts @@ -20,7 +20,7 @@ import { assertExternalDirectory } from "./external-directory" const MAX_DIAGNOSTICS_PER_FILE = 20 -function normalizeLineEndings(text: string): string { +export function normalizeLineEndings(text: string): string { return text.replaceAll("\r\n", "\n") } @@ -31,6 +31,32 @@ export const EditTool = Tool.define("edit", { oldString: z.string().describe("The text to replace"), newString: z.string().describe("The text to replace it with (must be different from oldString)"), replaceAll: z.boolean().optional().describe("Replace all occurrences of oldString (default false)"), + matchStrategy: z + .enum([ + "simple", + "multi-occurrence", + "line-trimmed", + "block-anchor", + "whitespace-normalized", + "indentation-flexible", + "escape-normalized", + "trimmed-boundary", + "context-aware", + "regex", + ]) + .optional() + .describe("The strategy to use for matching oldString"), + contextLines: z.number().optional().describe("Number of context lines to use for matching (default 3)"), + dryRun: z.boolean().optional().describe("If true, only return the diff without applying changes"), + regexFlags: z.string().optional().describe("Regex flags to use if matchStrategy is 'regex' (e.g., 'gi')"), + anchorLines: z + .object({ + start: z.number().optional().describe("Start line number to restrict the search"), + end: z.number().optional().describe("End line number to restrict the search"), + }) + .optional() + .describe("Restrict the search to a specific line range"), + validateOnly: z.boolean().optional().describe("If true, only validate that oldString exists"), }), async execute(params, ctx) { if (!params.filePath) { @@ -47,7 +73,7 @@ export const EditTool = Tool.define("edit", { let diff = "" let contentOld = "" let contentNew = "" - await FileTime.withLock(filePath, async () => { + const result_output = await FileTime.withLock(filePath, async () => { if (params.oldString === "") { const existed = await Bun.file(filePath).exists() contentNew = params.newString @@ -79,11 +105,57 @@ export const EditTool = Tool.define("edit", { if (stats.isDirectory()) throw new Error(`Path is a directory, not a file: ${filePath}`) await FileTime.assert(ctx.sessionID, filePath) contentOld = await file.text() - contentNew = replace(contentOld, params.oldString, params.newString, params.replaceAll) + + let result: ReplaceResult + try { + result = replace(contentOld, params.oldString, params.newString, { + replaceAll: params.replaceAll, + matchStrategy: params.matchStrategy, + regexFlags: params.regexFlags, + anchorLines: params.anchorLines, + contextLines: params.contextLines, + }) + } catch (e: any) { + if (e.metadata) { + ctx.metadata({ metadata: e.metadata }) + if (e.metadata.type === "not_found") { + let msg = e.message + if (e.metadata.suggestions.length > 0) { + const s = e.metadata.suggestions[0] + msg += ` Did you mean line ${s.line}: "${s.content.trim()}"?` + } + throw new Error(msg) + } + if (e.metadata.type === "multiple_matches") { + const candidates = e.metadata.candidates + .map((c: any) => `Line ${c.line}: "${c.content.trim().substring(0, 50)}..."`) + .join("\n") + throw new Error( + `${e.message}\n${candidates}\n\nProvide more surrounding lines in oldString to identify the correct match.`, + ) + } + } + throw e + } + + contentNew = result.content + const editInfo = result diff = trimDiff( createTwoFilesPatch(filePath, filePath, normalizeLineEndings(contentOld), normalizeLineEndings(contentNew)), ) + + if (params.validateOnly || params.dryRun) { + let message = params.validateOnly ? "Validation successful. oldString found." : "Dry run successful." + return { + metadata: { + diff, + editInfo, + }, + output: `${message} Changes previewed in diff.\nUsed replacer: ${editInfo.replacer}`, + } + } + await ctx.ask({ permission: "edit", patterns: [path.relative(Instance.worktree, filePath)], @@ -91,6 +163,7 @@ export const EditTool = Tool.define("edit", { metadata: { filepath: filePath, diff, + editInfo, }, }) @@ -107,6 +180,14 @@ export const EditTool = Tool.define("edit", { createTwoFilesPatch(filePath, filePath, normalizeLineEndings(contentOld), normalizeLineEndings(contentNew)), ) FileTime.read(ctx.sessionID, filePath) + + let output = "Edit applied successfully." + if (params.replaceAll) { + output += ` (Replaced ${editInfo.matches} occurrences using ${editInfo.replacer})` + } else { + output += ` (Modified lines ${editInfo.startLine}-${editInfo.endLine} using ${editInfo.replacer})` + } + return output }) const filediff: Snapshot.FileDiff = { @@ -117,8 +198,8 @@ export const EditTool = Tool.define("edit", { deletions: 0, } for (const change of diffLines(contentOld, contentNew)) { - if (change.added) filediff.additions += change.count || 0 - if (change.removed) filediff.deletions += change.count || 0 + if (change.added) filediff.additions += change.count ?? 0 + if (change.removed) filediff.deletions += change.count ?? 0 } ctx.metadata({ @@ -129,7 +210,7 @@ export const EditTool = Tool.define("edit", { }, }) - let output = "Edit applied successfully." + let output = typeof result_output === "string" ? result_output : "Edit applied successfully." await LSP.touchFile(filePath, true) const diagnostics = await LSP.diagnostics() const normalizedFilePath = Filesystem.normalizePath(filePath) @@ -154,31 +235,65 @@ export const EditTool = Tool.define("edit", { }, }) -export type Replacer = (content: string, find: string) => Generator +export type Replacer = (content: string, find: string, options?: ReplaceOptions) => Generator // Similarity thresholds for block anchor fallback matching const SINGLE_CANDIDATE_SIMILARITY_THRESHOLD = 0.0 const MULTIPLE_CANDIDATES_SIMILARITY_THRESHOLD = 0.3 /** - * Levenshtein distance algorithm implementation + * Extracts a match from content based on start and end line indices + */ +function extractMatch(content: string, originalLines: string[], startLine: number, endLine: number): string { + let matchStartIndex = 0 + for (let k = 0; k < startLine; k++) { + matchStartIndex += originalLines[k].length + 1 + } + + let matchEndIndex = matchStartIndex + for (let k = startLine; k <= endLine; k++) { + matchEndIndex += originalLines[k].length + if (k < endLine) { + matchEndIndex += 1 // Add newline character except for the last line + } + } + + return content.substring(matchStartIndex, matchEndIndex) +} + +/** + * Levenshtein distance algorithm implementation (space-optimized to O(min(m,n))) */ function levenshtein(a: string, b: string): number { + // Prevent excessive memory usage for very large strings + const MAX_LENGTH = 10000 + if (a.length > MAX_LENGTH || b.length > MAX_LENGTH) { + // Return a simple approximation for very large strings to avoid O(N^2) time/space + return Math.abs(a.length - b.length) + (a.slice(0, 100) === b.slice(0, 100) ? 0 : 1) + } + // Handle empty strings if (a === "" || b === "") { return Math.max(a.length, b.length) } - const matrix = Array.from({ length: a.length + 1 }, (_, i) => - Array.from({ length: b.length + 1 }, (_, j) => (i === 0 ? j : j === 0 ? i : 0)), - ) + + // Ensure 'b' is the shorter string to minimize space usage + if (a.length < b.length) { + ;[a, b] = [b, a] + } + + let prevRow = Array.from({ length: b.length + 1 }, (_, i) => i) for (let i = 1; i <= a.length; i++) { + const currRow = [i] for (let j = 1; j <= b.length; j++) { const cost = a[i - 1] === b[j - 1] ? 0 : 1 - matrix[i][j] = Math.min(matrix[i - 1][j] + 1, matrix[i][j - 1] + 1, matrix[i - 1][j - 1] + cost) + currRow[j] = Math.min(prevRow[j] + 1, currRow[j - 1] + 1, prevRow[j - 1] + cost) } + prevRow = currRow } - return matrix[a.length][b.length] + + return prevRow[b.length] } export const SimpleReplacer: Replacer = function* (_content, find) { @@ -207,29 +322,19 @@ export const LineTrimmedReplacer: Replacer = function* (content, find) { } if (matches) { - let matchStartIndex = 0 - for (let k = 0; k < i; k++) { - matchStartIndex += originalLines[k].length + 1 - } - - let matchEndIndex = matchStartIndex - for (let k = 0; k < searchLines.length; k++) { - matchEndIndex += originalLines[i + k].length - if (k < searchLines.length - 1) { - matchEndIndex += 1 // Add newline character except for the last line - } - } - - yield content.substring(matchStartIndex, matchEndIndex) + yield extractMatch(content, originalLines, i, i + searchLines.length - 1) } } } -export const BlockAnchorReplacer: Replacer = function* (content, find) { +export const BlockAnchorReplacer: Replacer = function* (content, find, options) { const originalLines = content.split("\n") const searchLines = find.split("\n") - if (searchLines.length < 3) { + const contextLines = options?.contextLines ?? 1 + const minBlockSize = Math.max(3, contextLines * 2 + 1) + + if (searchLines.length < minBlockSize) { return } @@ -241,6 +346,30 @@ export const BlockAnchorReplacer: Replacer = function* (content, find) { const lastLineSearch = searchLines[searchLines.length - 1].trim() const searchBlockSize = searchLines.length + const evaluateCandidate = (startLine: number, endLine: number) => { + const actualBlockSize = endLine - startLine + 1 + let similarity = 0 + let linesToCheck = Math.min(searchBlockSize - 2, actualBlockSize - 2) // Middle lines only + + if (linesToCheck > 0) { + for (let j = 1; j < searchBlockSize - 1 && j < actualBlockSize - 1; j++) { + const originalLine = originalLines[startLine + j].trim() + const searchLine = searchLines[j].trim() + const maxLen = Math.max(originalLine.length, searchLine.length) + if (maxLen === 0) { + continue + } + const distance = levenshtein(originalLine, searchLine) + similarity += 1 - distance / maxLen + } + similarity /= linesToCheck // Average similarity + } else { + // No middle lines to compare, just accept based on anchors + similarity = 1.0 + } + return similarity + } + // Collect all candidate positions where both anchors match const candidates: Array<{ startLine: number; endLine: number }> = [] for (let i = 0; i < originalLines.length; i++) { @@ -265,45 +394,10 @@ export const BlockAnchorReplacer: Replacer = function* (content, find) { // Handle single candidate scenario (using relaxed threshold) if (candidates.length === 1) { const { startLine, endLine } = candidates[0] - const actualBlockSize = endLine - startLine + 1 - - let similarity = 0 - let linesToCheck = Math.min(searchBlockSize - 2, actualBlockSize - 2) // Middle lines only - - if (linesToCheck > 0) { - for (let j = 1; j < searchBlockSize - 1 && j < actualBlockSize - 1; j++) { - const originalLine = originalLines[startLine + j].trim() - const searchLine = searchLines[j].trim() - const maxLen = Math.max(originalLine.length, searchLine.length) - if (maxLen === 0) { - continue - } - const distance = levenshtein(originalLine, searchLine) - similarity += (1 - distance / maxLen) / linesToCheck - - // Exit early when threshold is reached - if (similarity >= SINGLE_CANDIDATE_SIMILARITY_THRESHOLD) { - break - } - } - } else { - // No middle lines to compare, just accept based on anchors - similarity = 1.0 - } + const similarity = evaluateCandidate(startLine, endLine) if (similarity >= SINGLE_CANDIDATE_SIMILARITY_THRESHOLD) { - let matchStartIndex = 0 - for (let k = 0; k < startLine; k++) { - matchStartIndex += originalLines[k].length + 1 - } - let matchEndIndex = matchStartIndex - for (let k = startLine; k <= endLine; k++) { - matchEndIndex += originalLines[k].length - if (k < endLine) { - matchEndIndex += 1 // Add newline character except for the last line - } - } - yield content.substring(matchStartIndex, matchEndIndex) + yield extractMatch(content, originalLines, startLine, endLine) } return } @@ -314,27 +408,7 @@ export const BlockAnchorReplacer: Replacer = function* (content, find) { for (const candidate of candidates) { const { startLine, endLine } = candidate - const actualBlockSize = endLine - startLine + 1 - - let similarity = 0 - let linesToCheck = Math.min(searchBlockSize - 2, actualBlockSize - 2) // Middle lines only - - if (linesToCheck > 0) { - for (let j = 1; j < searchBlockSize - 1 && j < actualBlockSize - 1; j++) { - const originalLine = originalLines[startLine + j].trim() - const searchLine = searchLines[j].trim() - const maxLen = Math.max(originalLine.length, searchLine.length) - if (maxLen === 0) { - continue - } - const distance = levenshtein(originalLine, searchLine) - similarity += 1 - distance / maxLen - } - similarity /= linesToCheck // Average similarity - } else { - // No middle lines to compare, just accept based on anchors - similarity = 1.0 - } + const similarity = evaluateCandidate(startLine, endLine) if (similarity > maxSimilarity) { maxSimilarity = similarity @@ -344,19 +418,7 @@ export const BlockAnchorReplacer: Replacer = function* (content, find) { // Threshold judgment if (maxSimilarity >= MULTIPLE_CANDIDATES_SIMILARITY_THRESHOLD && bestMatch) { - const { startLine, endLine } = bestMatch - let matchStartIndex = 0 - for (let k = 0; k < startLine; k++) { - matchStartIndex += originalLines[k].length + 1 - } - let matchEndIndex = matchStartIndex - for (let k = startLine; k <= endLine; k++) { - matchEndIndex += originalLines[k].length - if (k < endLine) { - matchEndIndex += 1 - } - } - yield content.substring(matchStartIndex, matchEndIndex) + yield extractMatch(content, originalLines, bestMatch.startLine, bestMatch.endLine) } } @@ -396,9 +458,9 @@ export const WhitespaceNormalizedReplacer: Replacer = function* (content, find) const findLines = find.split("\n") if (findLines.length > 1) { for (let i = 0; i <= lines.length - findLines.length; i++) { - const block = lines.slice(i, i + findLines.length) - if (normalizeWhitespace(block.join("\n")) === normalizedFind) { - yield block.join("\n") + const block = extractMatch(content, lines, i, i + findLines.length - 1) + if (normalizeWhitespace(block) === normalizedFind) { + yield block } } } @@ -425,7 +487,7 @@ export const IndentationFlexibleReplacer: Replacer = function* (content, find) { const findLines = find.split("\n") for (let i = 0; i <= contentLines.length - findLines.length; i++) { - const block = contentLines.slice(i, i + findLines.length).join("\n") + const block = extractMatch(content, contentLines, i, i + findLines.length - 1) if (removeIndentation(block) === normalizedFind) { yield block } @@ -434,48 +496,45 @@ export const IndentationFlexibleReplacer: Replacer = function* (content, find) { export const EscapeNormalizedReplacer: Replacer = function* (content, find) { const unescapeString = (str: string): string => { - return str.replace(/\\(n|t|r|'|"|`|\\|\n|\$)/g, (match, capturedChar) => { - switch (capturedChar) { - case "n": - return "\n" - case "t": - return "\t" - case "r": - return "\r" - case "'": - return "'" - case '"': - return '"' - case "`": - return "`" - case "\\": - return "\\" - case "\n": - return "\n" - case "$": - return "$" - default: - return match - } - }) + return str.replace( + /\\(n|t|r|'|"|`|\\|\n|\$|x[0-9a-fA-F]{2}|u[0-9a-fA-F]{4}|u\{[0-9a-fA-F]+\})/g, + (match, captured) => { + if (captured === "n") return "\n" + if (captured === "t") return "\t" + if (captured === "r") return "\r" + if (captured === "'") return "'" + if (captured === '"') return '"' + if (captured === "`") return "`" + if (captured === "\\") return "\\" + if (captured === "\n") return "\n" + if (captured === "$") return "$" + if (captured.startsWith("x")) return String.fromCharCode(parseInt(captured.slice(1), 16)) + if (captured.startsWith("u{")) return String.fromCodePoint(parseInt(captured.slice(2, -1), 16)) + if (captured.startsWith("u")) return String.fromCharCode(parseInt(captured.slice(1), 16)) + return match + }, + ) } const unescapedFind = unescapeString(find) - - // Try direct match with unescaped find string - if (content.includes(unescapedFind)) { - yield unescapedFind - } - - // Also try finding escaped versions in content that match unescaped find - const lines = content.split("\n") const findLines = unescapedFind.split("\n") + const lines = content.split("\n") + + // Combine direct match check with sliding window scanning to avoid double scanning the whole content + let hasYieldedDirect = false for (let i = 0; i <= lines.length - findLines.length; i++) { - const block = lines.slice(i, i + findLines.length).join("\n") - const unescapedBlock = unescapeString(block) + const block = extractMatch(content, lines, i, i + findLines.length - 1) - if (unescapedBlock === unescapedFind) { + // Check for direct match of unescaped version (as a substring or exact line) + if (!hasYieldedDirect && block.includes(unescapedFind)) { + yield unescapedFind + hasYieldedDirect = true + } + + // Check for escaped versions in content + const unescapedBlock = unescapeString(block) + if (unescapedBlock === unescapedFind && block !== unescapedFind) { yield block } } @@ -513,7 +572,7 @@ export const TrimmedBoundaryReplacer: Replacer = function* (content, find) { const findLines = find.split("\n") for (let i = 0; i <= lines.length - findLines.length; i++) { - const block = lines.slice(i, i + findLines.length).join("\n") + const block = extractMatch(content, lines, i, i + findLines.length - 1) if (block.trim() === trimmedFind) { yield block @@ -547,8 +606,8 @@ export const ContextAwareReplacer: Replacer = function* (content, find) { for (let j = i + 2; j < contentLines.length; j++) { if (contentLines[j].trim() === lastLine) { // Found a potential context block + const block = extractMatch(content, contentLines, i, j) const blockLines = contentLines.slice(i, j + 1) - const block = blockLines.join("\n") // Check if the middle content has reasonable similarity // (simple heuristic: at least 50% of non-empty lines should match when trimmed) @@ -579,77 +638,249 @@ export const ContextAwareReplacer: Replacer = function* (content, find) { } } +export const RegexReplacer = (flags = "g"): Replacer => { + return function* (content, find) { + try { + const regex = new RegExp(find, flags) + let match + while ((match = regex.exec(content)) !== null) { + yield match[0] + if (!regex.global) break + } + } catch (e) { + // Invalid regex, skip + } + } +} + +export const REPLACER_MAP: Record Replacer)> = { + simple: SimpleReplacer, + "multi-occurrence": MultiOccurrenceReplacer, + "line-trimmed": LineTrimmedReplacer, + "block-anchor": BlockAnchorReplacer, + "whitespace-normalized": WhitespaceNormalizedReplacer, + "indentation-flexible": IndentationFlexibleReplacer, + "escape-normalized": EscapeNormalizedReplacer, + "trimmed-boundary": TrimmedBoundaryReplacer, + "context-aware": ContextAwareReplacer, + regex: (opts: any) => RegexReplacer(opts?.regexFlags || "g"), +} + +export const DEFAULT_REPLACERS = [ + "simple", + "multi-occurrence", + "line-trimmed", + "block-anchor", + "whitespace-normalized", + "indentation-flexible", + "escape-normalized", + "trimmed-boundary", + "context-aware", +] + export function trimDiff(diff: string): string { const lines = diff.split("\n") - const contentLines = lines.filter( - (line) => + let min = Infinity + + // Single pass to find min indentation of content lines + for (const line of lines) { + if ( (line.startsWith("+") || line.startsWith("-") || line.startsWith(" ")) && !line.startsWith("---") && - !line.startsWith("+++"), - ) - - if (contentLines.length === 0) return diff - - let min = Infinity - for (const line of contentLines) { - const content = line.slice(1) - if (content.trim().length > 0) { - const match = content.match(/^(\s*)/) - if (match) min = Math.min(min, match[1].length) + !line.startsWith("+++") + ) { + const content = line.slice(1) + if (content.trim().length > 0) { + const match = content.match(/^(\s*)/) + if (match) min = Math.min(min, match[1].length) + } } } + if (min === Infinity || min === 0) return diff - const trimmedLines = lines.map((line) => { + + // Single pass to trim and join + let result = "" + for (let i = 0; i < lines.length; i++) { + const line = lines[i] if ( (line.startsWith("+") || line.startsWith("-") || line.startsWith(" ")) && !line.startsWith("---") && !line.startsWith("+++") ) { - const prefix = line[0] - const content = line.slice(1) - return prefix + content.slice(min) + result += line[0] + line.slice(1 + min) + } else { + result += line } - return line - }) + if (i < lines.length - 1) result += "\n" + } + + return result +} - return trimmedLines.join("\n") +function escapeForRegExp(str: string): string { + return str.replace(/[.*+?^${}()|[\]\\]/g, "\\$&") } -export function replace(content: string, oldString: string, newString: string, replaceAll = false): string { +export interface ReplaceOptions { + replaceAll?: boolean + matchStrategy?: string + regexFlags?: string + anchorLines?: { start?: number; end?: number } + contextLines?: number +} + +export interface ReplaceResult { + content: string + replacer: string + matches?: number + startLine?: number + endLine?: number +} + +export function replace( + content: string, + oldString: string, + newString: string, + options: ReplaceOptions = {}, +): ReplaceResult { + const { replaceAll = false, matchStrategy, anchorLines } = options + if (oldString === newString) { throw new Error("oldString and newString must be different") } - let notFound = true - - for (const replacer of [ - SimpleReplacer, - LineTrimmedReplacer, - BlockAnchorReplacer, - WhitespaceNormalizedReplacer, - IndentationFlexibleReplacer, - EscapeNormalizedReplacer, - TrimmedBoundaryReplacer, - ContextAwareReplacer, - MultiOccurrenceReplacer, - ]) { - for (const search of replacer(content, oldString)) { - const index = content.indexOf(search) - if (index === -1) continue - notFound = false - if (replaceAll) { - return content.replaceAll(search, newString) - } - const lastIndex = content.lastIndexOf(search) - if (index !== lastIndex) continue - return content.substring(0, index) + newString + content.substring(index + search.length) + // Restrict content if anchorLines are provided + let effectiveContent = content + let contentOffset = 0 + if (anchorLines) { + const lines = content.split("\n") + const start = Math.max(1, anchorLines.start || 1) + const end = Math.min(lines.length, anchorLines.end || lines.length) + + if (start > end) { + throw new Error(`Invalid anchorLines: start (${start}) is greater than end (${end})`) } + + effectiveContent = lines.slice(start - 1, end).join("\n") + contentOffset = lines.slice(0, start - 1).reduce((sum, line) => sum + line.length + 1, 0) } - if (notFound) { - throw new Error("oldString not found in content") + const replacerNames = matchStrategy ? [matchStrategy] : DEFAULT_REPLACERS + const replacers = replacerNames + .map((name) => { + const r = REPLACER_MAP[name] + if (!r) return null + return { + name, + fn: typeof r === "function" && r.length === 1 ? (r as any)(options) : (r as Replacer), + } + }) + .filter((r): r is { name: string; fn: Replacer } => r !== null) + + if (replaceAll) { + for (const replacer of replacers) { + const matches = Array.from(replacer.fn(effectiveContent, oldString, options)) + if (matches.length > 0) { + let newContent = effectiveContent + if (replacer.name === "regex") { + newContent = effectiveContent.replace(new RegExp(oldString, options.regexFlags || "g"), newString) + } else { + // Replace all unique matches found by the fuzzy replacer + const uniqueMatches = Array.from(new Set(matches)) + // Sort by length descending to avoid partial replacements of longer matches + uniqueMatches.sort((a, b) => b.length - a.length) + + for (const match of uniqueMatches) { + newContent = newContent.replace(new RegExp(escapeForRegExp(match), "g"), newString) + } + } + + return { + content: + content.substring(0, contentOffset) + + newContent + + content.substring(contentOffset + effectiveContent.length), + replacer: replacer.name, + matches: matches.length, + } + } + } + } else { + const allMatches: Array<{ search: string; index: number; replacer: string }> = [] + + for (const replacer of replacers) { + for (const search of replacer.fn(effectiveContent, oldString, options)) { + const index = effectiveContent.indexOf(search) + if (index === -1) continue + + const lastIndex = effectiveContent.lastIndexOf(search) + if (index !== lastIndex) { + continue + } + + const absoluteIndex = contentOffset + index + const startLine = content.substring(0, absoluteIndex).split("\n").length + const endLine = startLine + search.split("\n").length - 1 + + return { + content: content.substring(0, absoluteIndex) + newString + content.substring(absoluteIndex + search.length), + replacer: replacer.name, + startLine, + endLine, + } + } + + for (const search of replacer.fn(effectiveContent, oldString, options)) { + let idx = effectiveContent.indexOf(search) + while (idx !== -1) { + allMatches.push({ search, index: contentOffset + idx, replacer: replacer.name }) + idx = effectiveContent.indexOf(search, idx + 1) + } + } + if (allMatches.length > 0) break + } + + if (allMatches.length === 0) { + const lines = effectiveContent.split("\n") + const searchFirstLine = oldString.split("\n")[0].trim() + let bestMatch = { line: -1, similarity: 0, content: "" } + + for (let i = 0; i < lines.length; i++) { + const line = lines[i].trim() + if (line.length === 0) continue + const sim = 1 - levenshtein(line, searchFirstLine) / Math.max(line.length, searchFirstLine.length) + if (sim > bestMatch.similarity) { + bestMatch = { line: (anchorLines?.start || 1) + i, similarity: sim, content: lines[i] } + } + } + + const error: any = new Error(`oldString not found in content.`) + error.metadata = { + type: "not_found", + suggestions: bestMatch.line !== -1 && bestMatch.similarity > 0.7 ? [bestMatch] : [], + } + throw error + } + + if (allMatches.length > 1) { + const candidates = allMatches.map((m) => { + const line = content.substring(0, m.index).split("\n").length + return { + line, + content: m.search.split("\n")[0], + replacer: m.replacer, + } + }) + + const error: any = new Error(`Found multiple matches for oldString (${allMatches.length} total).`) + error.metadata = { + type: "multiple_matches", + candidates: candidates.slice(0, 10), + } + throw error + } } - throw new Error( - "Found multiple matches for oldString. Provide more surrounding lines in oldString to identify the correct match.", - ) + + throw new Error("Unexpected error in replace function") } diff --git a/packages/opencode/src/tool/multiedit.ts b/packages/opencode/src/tool/multiedit.ts index 7f562f473..c94945de6 100644 --- a/packages/opencode/src/tool/multiedit.ts +++ b/packages/opencode/src/tool/multiedit.ts @@ -1,9 +1,20 @@ import z from "zod" import { Tool } from "./tool" -import { EditTool } from "./edit" +import { replace, trimDiff, normalizeLineEndings } from "./edit" import DESCRIPTION from "./multiedit.txt" import path from "path" import { Instance } from "../project/instance" +import { FileTime } from "../file/time" +import { assertExternalDirectory } from "./external-directory" +import { createTwoFilesPatch, diffLines } from "diff" +import { Bus } from "../bus" +import { File } from "../file" +import { FileWatcher } from "../file/watcher" +import { Snapshot } from "@/snapshot" +import { LSP } from "../lsp" +import { Filesystem } from "../util/filesystem" + +const MAX_DIAGNOSTICS_PER_FILE = 20 export const MultiEditTool = Tool.define("multiedit", { description: DESCRIPTION, @@ -12,35 +23,160 @@ export const MultiEditTool = Tool.define("multiedit", { edits: z .array( z.object({ - filePath: z.string().describe("The absolute path to the file to modify"), oldString: z.string().describe("The text to replace"), newString: z.string().describe("The text to replace it with (must be different from oldString)"), replaceAll: z.boolean().optional().describe("Replace all occurrences of oldString (default false)"), + matchStrategy: z + .enum([ + "simple", + "multi-occurrence", + "line-trimmed", + "block-anchor", + "whitespace-normalized", + "indentation-flexible", + "escape-normalized", + "trimmed-boundary", + "context-aware", + "regex", + ]) + .optional() + .describe("The strategy to use for matching oldString"), + contextLines: z.number().optional().describe("Number of context lines to use for matching (default 3)"), + regexFlags: z.string().optional().describe("Regex flags to use if matchStrategy is 'regex' (e.g., 'gi')"), + anchorLines: z + .object({ + start: z.number().optional().describe("Start line number to restrict the search"), + end: z.number().optional().describe("End line number to restrict the search"), + }) + .optional() + .describe("Restrict the search to a specific line range"), }), ) .describe("Array of edit operations to perform sequentially on the file"), + dryRun: z.boolean().optional().describe("If true, only return the diff without applying changes"), }), async execute(params, ctx) { - const tool = await EditTool.init() - const results = [] - for (const [, edit] of params.edits.entries()) { - const result = await tool.execute( - { - filePath: params.filePath, - oldString: edit.oldString, - newString: edit.newString, - replaceAll: edit.replaceAll, - }, - ctx, + if (!params.filePath) { + throw new Error("filePath is required") + } + + const filePath = path.isAbsolute(params.filePath) ? params.filePath : path.join(Instance.directory, params.filePath) + await assertExternalDirectory(ctx, filePath) + + let contentOld = "" + let contentNew = "" + let diff = "" + + await FileTime.withLock(filePath, async () => { + const file = Bun.file(filePath) + const stats = await file.stat().catch(() => {}) + if (!stats) throw new Error(`File ${filePath} not found`) + if (stats.isDirectory()) throw new Error(`Path is a directory, not a file: ${filePath}`) + + await FileTime.assert(ctx.sessionID, filePath) + contentOld = await file.text() + contentNew = contentOld + + const editInfos: any[] = [] + for (const edit of params.edits) { + if (edit.oldString === edit.newString) { + throw new Error("oldString and newString must be different") + } + try { + const result = replace(contentNew, edit.oldString, edit.newString, { + replaceAll: edit.replaceAll, + matchStrategy: edit.matchStrategy, + regexFlags: edit.regexFlags, + anchorLines: edit.anchorLines, + contextLines: edit.contextLines, + }) + contentNew = result.content + editInfos.push(result) + } catch (e: any) { + if (e.metadata) { + ctx.metadata({ metadata: e.metadata }) + } + throw e + } + } + + diff = trimDiff( + createTwoFilesPatch(filePath, filePath, normalizeLineEndings(contentOld), normalizeLineEndings(contentNew)), ) - results.push(result) + + if (params.dryRun) { + return { + metadata: { + diff, + editInfos, + }, + output: `Dry run successful. Changes previewed in diff.`, + } + } + + await ctx.ask({ + permission: "edit", + patterns: [path.relative(Instance.worktree, filePath)], + always: ["*"], + metadata: { + filepath: filePath, + diff, + }, + }) + + await file.write(contentNew) + await Bus.publish(File.Event.Edited, { + file: filePath, + }) + await Bus.publish(FileWatcher.Event.Updated, { + file: filePath, + event: "change", + }) + FileTime.read(ctx.sessionID, filePath) + }) + + const filediff: Snapshot.FileDiff = { + file: filePath, + before: contentOld, + after: contentNew, + additions: 0, + deletions: 0, } + + for (const change of diffLines(contentOld, contentNew)) { + if (change.added) filediff.additions += change.count ?? 0 + if (change.removed) filediff.deletions += change.count ?? 0 + } + + ctx.metadata({ + metadata: { + diff, + filediff, + diagnostics: {}, + }, + }) + + let output = "Edits applied successfully." + await LSP.touchFile(filePath, true) + const diagnostics = await LSP.diagnostics() + const normalizedFilePath = Filesystem.normalizePath(filePath) + const issues = diagnostics[normalizedFilePath] ?? [] + const errors = issues.filter((item) => item.severity === 1) + if (errors.length > 0) { + const limited = errors.slice(0, MAX_DIAGNOSTICS_PER_FILE) + const suffix = + errors.length > MAX_DIAGNOSTICS_PER_FILE ? `\n... and ${errors.length - MAX_DIAGNOSTICS_PER_FILE} more` : "" + output += `\n\nLSP errors detected in this file, please fix:\n\n${limited.map(LSP.Diagnostic.pretty).join("\n")}${suffix}\n` + } + return { - title: path.relative(Instance.worktree, params.filePath), metadata: { - results: results.map((r) => r.metadata), + diagnostics, + diff, + filediff, }, - output: results.at(-1)!.output, + title: `${path.relative(Instance.worktree, filePath)}`, + output, } }, }) From 7d5dd0a0f30760d93f7b3e0a63c135cb3f4c8816 Mon Sep 17 00:00:00 2001 From: jyl Date: Fri, 6 Feb 2026 20:50:08 +0800 Subject: [PATCH 3/4] feat: update edit tools --- packages/opencode/src/tool/edit.ts | 106 +++++++++++++------- packages/opencode/src/tool/multiedit.ts | 60 ++++++------ packages/opencode/src/tool/write.ts | 125 ++++++++++++++---------- packages/opencode/src/tool/write.txt | 24 ++++- 4 files changed, 194 insertions(+), 121 deletions(-) diff --git a/packages/opencode/src/tool/edit.ts b/packages/opencode/src/tool/edit.ts index 8162317b4..fb39c873c 100644 --- a/packages/opencode/src/tool/edit.ts +++ b/packages/opencode/src/tool/edit.ts @@ -32,20 +32,9 @@ export const EditTool = Tool.define("edit", { newString: z.string().describe("The text to replace it with (must be different from oldString)"), replaceAll: z.boolean().optional().describe("Replace all occurrences of oldString (default false)"), matchStrategy: z - .enum([ - "simple", - "multi-occurrence", - "line-trimmed", - "block-anchor", - "whitespace-normalized", - "indentation-flexible", - "escape-normalized", - "trimmed-boundary", - "context-aware", - "regex", - ]) + .enum(["exact", "fuzzy", "block", "regex"]) .optional() - .describe("The strategy to use for matching oldString"), + .describe("The strategy to use for matching oldString (default: tries exact, then fuzzy, then block)"), contextLines: z.number().optional().describe("Number of context lines to use for matching (default 3)"), dryRun: z.boolean().optional().describe("If true, only return the diff without applying changes"), regexFlags: z.string().optional().describe("Regex flags to use if matchStrategy is 'regex' (e.g., 'gi')"), @@ -268,8 +257,24 @@ function levenshtein(a: string, b: string): number { // Prevent excessive memory usage for very large strings const MAX_LENGTH = 10000 if (a.length > MAX_LENGTH || b.length > MAX_LENGTH) { - // Return a simple approximation for very large strings to avoid O(N^2) time/space - return Math.abs(a.length - b.length) + (a.slice(0, 100) === b.slice(0, 100) ? 0 : 1) + // For very large strings, use a more representative approximation than just the first 100 chars + // Compare prefix, suffix and middle to get a better sense of similarity + const prefixLen = 500 + const suffixLen = 500 + const midLen = 500 + + let diff = Math.abs(a.length - b.length) + + // Check prefix + if (a.slice(0, prefixLen) !== b.slice(0, prefixLen)) diff += 1 + // Check suffix + if (a.slice(-suffixLen) !== b.slice(-suffixLen)) diff += 1 + // Check middle + const aMid = a.slice(Math.floor(a.length / 2) - midLen / 2, Math.floor(a.length / 2) + midLen / 2) + const bMid = b.slice(Math.floor(b.length / 2) - midLen / 2, Math.floor(b.length / 2) + midLen / 2) + if (aMid !== bMid) diff += 1 + + return diff } // Handle empty strings @@ -654,29 +659,37 @@ export const RegexReplacer = (flags = "g"): Replacer => { } export const REPLACER_MAP: Record Replacer)> = { - simple: SimpleReplacer, - "multi-occurrence": MultiOccurrenceReplacer, - "line-trimmed": LineTrimmedReplacer, - "block-anchor": BlockAnchorReplacer, - "whitespace-normalized": WhitespaceNormalizedReplacer, - "indentation-flexible": IndentationFlexibleReplacer, - "escape-normalized": EscapeNormalizedReplacer, - "trimmed-boundary": TrimmedBoundaryReplacer, - "context-aware": ContextAwareReplacer, + exact: SimpleReplacer, + fuzzy: function* (content, find, options) { + const fuzzyReplacers = [ + LineTrimmedReplacer, + WhitespaceNormalizedReplacer, + IndentationFlexibleReplacer, + TrimmedBoundaryReplacer, + EscapeNormalizedReplacer, + ] + for (const replacer of fuzzyReplacers) { + const matches = Array.from(replacer(content, find, options)) + if (matches.length > 0) { + yield* matches + return + } + } + }, + block: function* (content, find, options) { + const blockReplacers = [BlockAnchorReplacer, ContextAwareReplacer] + for (const replacer of blockReplacers) { + const matches = Array.from(replacer(content, find, options)) + if (matches.length > 0) { + yield* matches + return + } + } + }, regex: (opts: any) => RegexReplacer(opts?.regexFlags || "g"), } -export const DEFAULT_REPLACERS = [ - "simple", - "multi-occurrence", - "line-trimmed", - "block-anchor", - "whitespace-normalized", - "indentation-flexible", - "escape-normalized", - "trimmed-boundary", - "context-aware", -] +export const DEFAULT_REPLACERS = ["exact", "fuzzy", "block"] export function trimDiff(diff: string): string { const lines = diff.split("\n") @@ -736,6 +749,7 @@ export interface ReplaceResult { matches?: number startLine?: number endLine?: number + output?: string } export function replace( @@ -788,6 +802,21 @@ export function replace( } else { // Replace all unique matches found by the fuzzy replacer const uniqueMatches = Array.from(new Set(matches)) + + // In fuzzy mode, replaceAll is dangerous if different matches are found. + // We restrict replaceAll to only apply if ALL matches are identical strings, + // OR if it's an exact match strategy. + if (replacer.name !== "exact" && uniqueMatches.length > 1) { + const error: any = new Error( + `replaceAll aborted: fuzzy matching found ${uniqueMatches.length} different variations of the content. This could lead to unintended replacements. Please use a more specific oldString or set matchStrategy to 'exact'.`, + ) + error.metadata = { + type: "ambiguous_replace_all", + variations: uniqueMatches.slice(0, 5), + } + throw error + } + // Sort by length descending to avoid partial replacements of longer matches uniqueMatches.sort((a, b) => b.length - a.length) @@ -796,6 +825,8 @@ export function replace( } } + const matchCount = (newContent.match(new RegExp(escapeForRegExp(newString), "g")) || []).length + return { content: content.substring(0, contentOffset) + @@ -803,6 +834,7 @@ export function replace( content.substring(contentOffset + effectiveContent.length), replacer: replacer.name, matches: matches.length, + output: `Applied ${matches.length} replacements using ${replacer.name} strategy.`, } } } @@ -855,7 +887,9 @@ export function replace( } } - const error: any = new Error(`oldString not found in content.`) + const error: any = new Error( + `oldString not found in content.${bestMatch.line !== -1 && bestMatch.similarity > 0.7 ? `\n\nSuggested oldString (Line ${bestMatch.line}):\n${bestMatch.content}` : ""}`, + ) error.metadata = { type: "not_found", suggestions: bestMatch.line !== -1 && bestMatch.similarity > 0.7 ? [bestMatch] : [], diff --git a/packages/opencode/src/tool/multiedit.ts b/packages/opencode/src/tool/multiedit.ts index c94945de6..753a6836f 100644 --- a/packages/opencode/src/tool/multiedit.ts +++ b/packages/opencode/src/tool/multiedit.ts @@ -27,18 +27,7 @@ export const MultiEditTool = Tool.define("multiedit", { newString: z.string().describe("The text to replace it with (must be different from oldString)"), replaceAll: z.boolean().optional().describe("Replace all occurrences of oldString (default false)"), matchStrategy: z - .enum([ - "simple", - "multi-occurrence", - "line-trimmed", - "block-anchor", - "whitespace-normalized", - "indentation-flexible", - "escape-normalized", - "trimmed-boundary", - "context-aware", - "regex", - ]) + .enum(["exact", "fuzzy", "block", "regex"]) .optional() .describe("The strategy to use for matching oldString"), contextLines: z.number().optional().describe("Number of context lines to use for matching (default 3)"), @@ -78,26 +67,37 @@ export const MultiEditTool = Tool.define("multiedit", { contentNew = contentOld const editInfos: any[] = [] - for (const edit of params.edits) { - if (edit.oldString === edit.newString) { - throw new Error("oldString and newString must be different") - } - try { - const result = replace(contentNew, edit.oldString, edit.newString, { - replaceAll: edit.replaceAll, - matchStrategy: edit.matchStrategy, - regexFlags: edit.regexFlags, - anchorLines: edit.anchorLines, - contextLines: edit.contextLines, - }) - contentNew = result.content - editInfos.push(result) - } catch (e: any) { - if (e.metadata) { - ctx.metadata({ metadata: e.metadata }) + + try { + for (const edit of params.edits) { + if (edit.oldString === edit.newString) { + throw new Error(`Edit failed: oldString and newString must be different for edit at line ${edit.anchorLines?.start || "unknown"}`) + } + try { + const result = replace(contentNew, edit.oldString, edit.newString, { + replaceAll: edit.replaceAll, + matchStrategy: edit.matchStrategy, + regexFlags: edit.regexFlags, + anchorLines: edit.anchorLines, + contextLines: edit.contextLines, + }) + contentNew = result.content + editInfos.push(result) + } catch (e: any) { + // Enhance error message to specify which edit failed + const editIndex = params.edits.indexOf(edit) + 1 + e.message = `Edit #${editIndex} failed: ${e.message}` + if (e.metadata) { + ctx.metadata({ metadata: e.metadata }) + } + throw e } - throw e } + } catch (e) { + // If any edit fails, we don't write anything to the file. + // The transactional nature is preserved because file.write(contentNew) + // is only called after all edits succeed. + throw e } diff = trimDiff( diff --git a/packages/opencode/src/tool/write.ts b/packages/opencode/src/tool/write.ts index eca64d303..a69bc937c 100644 --- a/packages/opencode/src/tool/write.ts +++ b/packages/opencode/src/tool/write.ts @@ -1,5 +1,6 @@ import z from "zod" import * as path from "path" +import * as fs from "node:fs/promises" import { Tool } from "./tool" import { LSP } from "../lsp" import { createTwoFilesPatch } from "diff" @@ -16,6 +17,10 @@ import { assertExternalDirectory } from "./external-directory" const MAX_DIAGNOSTICS_PER_FILE = 20 const MAX_PROJECT_DIAGNOSTICS_FILES = 5 +function normalizeLineEndings(text: string): string { + return text.replaceAll("\r\n", "\n") +} + export const WriteTool = Tool.define("write", { description: DESCRIPTION, parameters: z.object({ @@ -26,60 +31,80 @@ export const WriteTool = Tool.define("write", { const filepath = path.isAbsolute(params.filePath) ? params.filePath : path.join(Instance.directory, params.filePath) await assertExternalDirectory(ctx, filepath) - const file = Bun.file(filepath) - const exists = await file.exists() - const contentOld = exists ? await file.text() : "" - if (exists) await FileTime.assert(ctx.sessionID, filepath) + return await FileTime.withLock(filepath, async () => { + const file = Bun.file(filepath) + const exists = await file.exists() + const contentOld = exists ? await file.text() : "" + if (exists) await FileTime.assert(ctx.sessionID, filepath) - const diff = trimDiff(createTwoFilesPatch(filepath, filepath, contentOld, params.content)) - await ctx.ask({ - permission: "edit", - patterns: [path.relative(Instance.worktree, filepath)], - always: ["*"], - metadata: { - filepath, - diff, - }, - }) + const diff = trimDiff(createTwoFilesPatch(filepath, filepath, contentOld, params.content)) + await ctx.ask({ + permission: "edit", + patterns: [path.relative(Instance.worktree, filepath)], + always: ["*"], + metadata: { + filepath, + diff, + }, + }) - await Bun.write(filepath, params.content) - await Bus.publish(File.Event.Edited, { - file: filepath, - }) - await Bus.publish(FileWatcher.Event.Updated, { - file: filepath, - event: exists ? "change" : "add", - }) - FileTime.read(ctx.sessionID, filepath) + // Atomic write using temp file and rename + const tempPath = `${filepath}.tmp.${Math.random().toString(36).slice(2)}` + try { + await Bun.write(tempPath, params.content) + await fs.rename(tempPath, filepath) + } catch (e) { + // Clean up temp file if rename fails + if (await Bun.file(tempPath).exists()) { + await fs.unlink(tempPath).catch(() => {}) + } + throw e + } - let output = "Wrote file successfully." - await LSP.touchFile(filepath, true) - const diagnostics = await LSP.diagnostics() - const normalizedFilepath = Filesystem.normalizePath(filepath) - let projectDiagnosticsCount = 0 - for (const [file, issues] of Object.entries(diagnostics)) { - const errors = issues.filter((item) => item.severity === 1) - if (errors.length === 0) continue - const limited = errors.slice(0, MAX_DIAGNOSTICS_PER_FILE) - const suffix = - errors.length > MAX_DIAGNOSTICS_PER_FILE ? `\n... and ${errors.length - MAX_DIAGNOSTICS_PER_FILE} more` : "" - if (file === normalizedFilepath) { - output += `\n\nLSP errors detected in this file, please fix:\n\n${limited.map(LSP.Diagnostic.pretty).join("\n")}${suffix}\n` - continue + // Post-write verification + const writtenContent = await Bun.file(filepath).text() + if (writtenContent !== params.content) { + throw new Error("Verification failed: written content does not match expected content") } - if (projectDiagnosticsCount >= MAX_PROJECT_DIAGNOSTICS_FILES) continue - projectDiagnosticsCount++ - output += `\n\nLSP errors detected in other files:\n\n${limited.map(LSP.Diagnostic.pretty).join("\n")}${suffix}\n` - } - return { - title: path.relative(Instance.worktree, filepath), - metadata: { - diagnostics, - filepath, - exists: exists, - }, - output, - } + await Bus.publish(File.Event.Edited, { + file: filepath, + }) + await Bus.publish(FileWatcher.Event.Updated, { + file: filepath, + event: exists ? "change" : "add", + }) + FileTime.read(ctx.sessionID, filepath) + + let output = "Wrote file successfully." + await LSP.touchFile(filepath, true) + const diagnostics = await LSP.diagnostics() + const normalizedFilepath = Filesystem.normalizePath(filepath) + let projectDiagnosticsCount = 0 + for (const [file, issues] of Object.entries(diagnostics)) { + const errors = issues.filter((item) => item.severity === 1) + if (errors.length === 0) continue + const limited = errors.slice(0, MAX_DIAGNOSTICS_PER_FILE) + const suffix = + errors.length > MAX_DIAGNOSTICS_PER_FILE ? `\n... and ${errors.length - MAX_DIAGNOSTICS_PER_FILE} more` : "" + if (file === normalizedFilepath) { + output += `\n\nLSP errors detected in this file, please fix:\n\n${limited.map(LSP.Diagnostic.pretty).join("\n")}${suffix}\n` + continue + } + if (projectDiagnosticsCount >= MAX_PROJECT_DIAGNOSTICS_FILES) continue + projectDiagnosticsCount++ + output += `\n\nLSP errors detected in other files:\n\n${limited.map(LSP.Diagnostic.pretty).join("\n")}${suffix}\n` + } + + return { + title: path.relative(Instance.worktree, filepath), + metadata: { + diagnostics, + filepath, + exists: exists, + }, + output, + } + }) }, }) diff --git a/packages/opencode/src/tool/write.txt b/packages/opencode/src/tool/write.txt index 063cbb1f0..7d5c2d51d 100644 --- a/packages/opencode/src/tool/write.txt +++ b/packages/opencode/src/tool/write.txt @@ -1,8 +1,22 @@ Writes a file to the local filesystem. Usage: -- This tool will overwrite the existing file if there is one at the provided path. -- If this is an existing file, you MUST use the Read tool first to read the file's contents. This tool will fail if you did not read the file first. -- ALWAYS prefer editing existing files in the codebase. NEVER write new files unless explicitly required. -- NEVER proactively create documentation files (*.md) or README files. Only create documentation files if explicitly requested by the User. -- Only use emojis if the user explicitly requests it. Avoid writing emojis to files unless asked. +- This tool will overwrite the existing file if one exists at the provided path. +- If editing an existing file, you MUST use the Read tool first. This tool will fail if you did not read the file first. +- For modifying parts of an existing file, use the Edit tool instead. Only use Write when creating entirely new files or doing complete rewrites. +- NEVER proactively create documentation files (*.md) or README files. Only create them when explicitly requested by the User. +- Only use emojis if the user explicitly requests it. Avoid adding emojis to files unless asked. + +Behavior: +- Writes are guaranteed to be atomic: content is first written to a temporary file in the same directory and then renamed to the target path. This prevents file corruption if the process is interrupted. +- Concurrency control: The tool uses a file-level lock to ensure that multiple write operations to the same file are serialized. +- If the file was modified externally since you read it, the write will fail with an error asking you to re-read first. This prevents overwriting changes made by others. +- Large files are written directly; for files over 10MB, consider if partial writes would be more appropriate. + +Error Handling: +- If the write operation fails (e.g., permission denied, disk full), an error will be returned and the original file will remain untouched. +- If a conflict is detected (file changed since last read), you must re-read the file to sync the state before attempting to write again. + +Common mistakes to avoid: +- Using Write to make small changes to existing files (use Edit instead). +- Writing binary files or very large files (>50MB) without chunking. From 2038510b54d509abb80ca9bfe8dd27c633da0330 Mon Sep 17 00:00:00 2001 From: jyl Date: Fri, 6 Feb 2026 21:25:08 +0800 Subject: [PATCH 4/4] feat: update readme --- README.md | 12 ++++++++++++ README.zh.md | 12 ++++++++++++ 2 files changed, 24 insertions(+) diff --git a/README.md b/README.md index 8187995b0..71df117df 100644 --- a/README.md +++ b/README.md @@ -48,6 +48,18 @@ - **🏗️ Workqueue & Background Tasks**: Enhanced the internal workqueue system for more robust background task management. - **🧠 AI-Native Logic Flow**: Refined the agent's decision-making logic to be more efficient and better suited for complex multi-step coding tasks. +### Recent Improvements (v0.2.x) + +- **🔍 Precise Code Reading (Read Tool)**: Introduced advanced navigation and positioning. Supports direct jumping to functions, classes, or line numbers via `symbol`, with 1-based line support and file structure previews, significantly boosting AI context retrieval efficiency. +- **📝 Enhanced Code Editing (Edit/MultiEdit)**: + - **Advanced Matching Strategies**: Supports `exact`, `fuzzy`, `block`, and `regex` matching, drastically improving the success rate of complex code modifications. + - **Range Constraints & Validation**: Added `anchorLines` to restrict search ranges, alongside `dryRun` and `validateOnly` modes for safer, more predictable edits. + - **Intelligent Error Feedback**: Provides smart suggestions on match failures (e.g., "Did you mean line X?"), enabling faster agent self-correction. +- **⚙️ Robust Task Scheduling (Work Queue)**: + - **Optimized Task Identification**: Improved task ID generation using hashing to prevent collisions in large-scale workflows. + - **Enhanced State Management**: Introduced granular failure handling (`isFailed`) to increase reliability during parallel execution. +- **💎 Code Quality & Standards**: Fully aligned with strict coding standards, adding comprehensive documentation and thread-safety annotations (`@VertxThreadSafety`) across core components. + --- [![OpenCode Terminal UI](packages/web/src/assets/lander/screenshot.png)](https://opencode.ai) diff --git a/README.zh.md b/README.zh.md index e94182a29..4f3aa1cc0 100644 --- a/README.zh.md +++ b/README.zh.md @@ -48,6 +48,18 @@ - **🏗️ 工作队列与后台任务**: 增强了内部工作队列系统，使后台任务管理更加健壮。 - **🧠 AI 原生逻辑流**: 优化了 Agent 的决策逻辑，使其更高效且更适合处理复杂的多步编码任务。 +### 最近更新 (v0.2.x) + +- **🔍 精准代码读取 (Read Tool)**: 引入了全新的导航与定位功能。支持通过 `symbol` 直接定位函数、类或行号，提供 1-based 行号支持和文件结构预览（Preview 模式），极大提升了 AI 获取代码上下文的效率。 +- **📝 增强型代码编辑 (Edit/MultiEdit)**: + - **多种匹配策略**: 支持 `exact`（精确）、`fuzzy`（模糊）、`block`（块匹配）和 `regex`（正则）匹配，大幅提高复杂代码修改的成功率。 + - **范围限制与验证**: 引入 `anchorLines` 限制修改范围，并增加 `dryRun` 和 `validateOnly` 模式，确保修改的安全性和准确性。 + - **智能错误反馈**: 当匹配失败时，工具会提供智能建议（如“您是否是指第 X 行？”），帮助 Agent 快速自我纠错。 +- **⚙️ 健壮的任务调度 (Work Queue)**: + - **任务标识优化**: 改进了任务 ID 生成算法（基于 Hash），避免了在大规模任务流中的 ID 碰撞。 + - **状态管理增强**: 引入了更细致的任务失败处理逻辑 (`isFailed`)，提升了并行执行时的可靠性。 +- **💎 代码质量与标准**: 全面遵循严格的编码规范，并在核心组件中增加了详细的文档注释和线程安全说明 (`@VertxThreadSafety`)。 + --- [![OpenCode Terminal UI](packages/web/src/assets/lander/screenshot.png)](https://opencode.ai)