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 <clio-agent@sisyphuslabs.ai>
This commit is contained in:
@@ -0,0 +1,83 @@
|
||||
/// <reference types="bun-types" />
|
||||
|
||||
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 })
|
||||
}
|
||||
})
|
||||
})
|
||||
@@ -6,6 +6,15 @@ import type { PluginContext } from "./types"
|
||||
|
||||
const VERIFICATION_ATTEMPT_PATTERN = /<ulw_verification_attempt_id>(.*?)<\/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,
|
||||
|
||||
Reference in New Issue
Block a user