From 4a68db029d9589e881292dd2b98e47062eaaf214 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Mon, 25 May 2026 17:06:22 +0900 Subject: [PATCH] fix(tool-execute-after): gate metadata recovery warnings Root cause: tool.execute.after attempted metadata recovery for every tool, including built-in tools that never publish recoverable OMO metadata. On Windows this produced repeated recovery warnings across read/bash/glob/grep/todowrite/lsp/apply_patch and amplified stalled-session diagnostics, while metadata-linked tools still needed diagnostic visibility when their linkage was stale. Limit the warning to tools expected to publish recoverable metadata and keep all other built-in tools fail-open after missing metadata. Fixes #4449 Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- ...ol-execute-after-metadata-recovery.test.ts | 83 +++++++++++++++++++ src/plugin/tool-execute-after.ts | 15 +++- 2 files changed, 97 insertions(+), 1 deletion(-) create mode 100644 src/plugin/tool-execute-after-metadata-recovery.test.ts diff --git a/src/plugin/tool-execute-after-metadata-recovery.test.ts b/src/plugin/tool-execute-after-metadata-recovery.test.ts new file mode 100644 index 000000000..8e9fdb618 --- /dev/null +++ b/src/plugin/tool-execute-after-metadata-recovery.test.ts @@ -0,0 +1,83 @@ +/// + +import { beforeEach, describe, expect, it } from "bun:test" +import { existsSync, mkdtempSync, readFileSync, rmSync } from "node:fs" +import { tmpdir } from "node:os" +import { join } from "node:path" + +import { clearPendingStore } from "../features/tool-metadata-store" +import { _flushForTesting, _resetLoggerForTesting, _setLoggerForTesting } from "../shared/logger" +import { createToolExecuteAfterHandler } from "./tool-execute-after" + +function readLogIfPresent(filePath: string): string { + return existsSync(filePath) ? readFileSync(filePath, "utf8") : "" +} + +describe("createToolExecuteAfterHandler metadata recovery", () => { + beforeEach(() => { + clearPendingStore() + _resetLoggerForTesting() + }) + + it("#given builtin tool has no recoverable metadata #when tool.execute.after runs #then it fails open without warning spam", async () => { + // given + const logDir = mkdtempSync(join(tmpdir(), "omo-tool-after-")) + const logPath = join(logDir, "omo.log") + _setLoggerForTesting({ filePath: logPath }) + const handler = createToolExecuteAfterHandler({ + ctx: { directory: "/repo" } as never, + hooks: {} as never, + }) + const output = { title: "result", output: "read output", metadata: {} } + + try { + // when + await handler( + { tool: "read", sessionID: "ses_parent", callID: "call_read" }, + output, + ) + _flushForTesting() + + // then + expect(output).toEqual({ title: "result", output: "read output", metadata: {} }) + expect(readLogIfPresent(logPath)).not.toContain("Unable to recover stored metadata") + } finally { + _resetLoggerForTesting() + rmSync(logDir, { force: true, recursive: true }) + } + }) + + it("#given metadata-linked tool has stale metadata #when tool.execute.after runs #then it warns and still completes hooks", async () => { + // given + const logDir = mkdtempSync(join(tmpdir(), "omo-tool-after-")) + const logPath = join(logDir, "omo.log") + _setLoggerForTesting({ filePath: logPath }) + let hookRan = false + const handler = createToolExecuteAfterHandler({ + ctx: { directory: "/repo" } as never, + hooks: { + categorySkillReminder: { + "tool.execute.after": async () => { + hookRan = true + }, + }, + } as never, + }) + + try { + // when + await handler( + { tool: "task", sessionID: "ses_parent", callID: "call_missing" }, + { title: "result", output: "task output", metadata: {} }, + ) + _flushForTesting() + + // then + expect(hookRan).toBe(true) + expect(readLogIfPresent(logPath)).toContain("Unable to recover stored metadata") + } finally { + _resetLoggerForTesting() + rmSync(logDir, { force: true, recursive: true }) + } + }) +}) diff --git a/src/plugin/tool-execute-after.ts b/src/plugin/tool-execute-after.ts index 859bbb065..2b033d1f8 100644 --- a/src/plugin/tool-execute-after.ts +++ b/src/plugin/tool-execute-after.ts @@ -6,6 +6,15 @@ import type { PluginContext } from "./types" const VERIFICATION_ATTEMPT_PATTERN = /(.*?)<\/ulw_verification_attempt_id>/i +const METADATA_LINKED_TOOLS = new Set([ + "background_output", + "background_task", + "call_omo_agent", + "edit", + "hashline_edit", + "task", +]) + type ToolExecuteAfterInput = { readonly tool: string readonly sessionID: string @@ -40,6 +49,10 @@ function getPluginDirectory(ctx: PluginContext): string | null { return null } +function expectsRecoverableMetadata(tool: string): boolean { + return METADATA_LINKED_TOOLS.has(tool) +} + export function createToolExecuteAfterHandler(args: { ctx: PluginContext hooks: CreatedHooks @@ -84,7 +97,7 @@ export function createToolExecuteAfterHandler(args: { output.metadata = { ...output.metadata, ...stored.metadata } } } - } else if (!nativeSessionId) { + } else if (!nativeSessionId && expectsRecoverableMetadata(input.tool)) { log("[tool-execute-after] Unable to recover stored metadata and no native session linkage was present", { tool: input.tool, sessionID: input.sessionID,