Prechádzať zdrojové kódy

fix(core): keep execute tool cache stable (#38783)

Aiden Cline 3 týždňov pred
rodič
commit
33390cc457

+ 0 - 1
packages/core/src/codemode.ts

@@ -63,7 +63,6 @@ const layer = Layer.effect(
           if (rule?.resource === "*" && rule.effect === "deny") continue
           registrations.set(name, registration)
         }
-        if (registrations.size === 0) return {}
         const executeRule = rules.findLast((rule) => Wildcard.match("execute", rule.action))
         if (executeRule?.resource === "*" && executeRule.effect === "deny") return {}
         return {

+ 14 - 12
packages/core/src/codemode/instructions.ts

@@ -17,7 +17,8 @@ Use \`search\` to discover exact paths and signatures for additional tools:
 ## Available tools`
 
 export function render(catalog: CodeModeCatalog.Summary) {
-  if (catalog.total === 0) return "No tools are currently available."
+  if (catalog.total === 0)
+    return "No Code Mode tools are currently available. Do not call `execute` until a later system update announces available tools."
 
   const tools = catalog.namespaces.flatMap((namespace) => {
     const count = namespace.count === 1 ? "1 tool" : `${namespace.count} tools`
@@ -36,12 +37,13 @@ ${tools.join("\n")}`
 }
 
 export function update(previous: CodeModeCatalog.Summary, current: CodeModeCatalog.Summary) {
-  const full = `The Code Mode tool catalog has changed. This catalog supersedes the previous Code Mode tool catalog.
+  const replacement = `The Code Mode tool catalog has changed. This catalog supersedes the previous Code Mode tool catalog.
 
 ${render(current)}`
+  if (current.total === 0) return replacement
   const previousComplete = previous.shown === previous.total
   const currentComplete = current.shown === current.total
-  if (previousComplete !== currentComplete) return full
+  if (previousComplete !== currentComplete) return replacement
 
   const diff = Instructions.diffByKey(
     previous.namespaces.flatMap((namespace) => namespace.entries),
@@ -52,7 +54,7 @@ ${render(current)}`
   const entriesChanged = diff.added.length > 0 || diff.removed.length > 0 || diff.changed.length > 0
 
   if (!currentComplete) {
-    if (entriesChanged) return full
+    if (entriesChanged) return replacement
     const namespaces = Instructions.diffByKey(
       previous.namespaces,
       current.namespaces,
@@ -60,7 +62,7 @@ ${render(current)}`
       (before, after) => before.count !== after.count,
     )
     const changed = namespaces.added.length > 0 || namespaces.removed.length > 0 || namespaces.changed.length > 0
-    if (!changed) return full
+    if (!changed) return replacement
 
     const parts = ["The Code Mode tool catalog has changed."]
     if (namespaces.added.length > 0) {
@@ -85,11 +87,11 @@ ${render(current)}`
       )
     }
     const delta = parts.join("\n\n")
-    if (delta.length < full.length) return delta
-    return full
+    if (delta.length < replacement.length) return delta
+    return replacement
   }
 
-  if (!entriesChanged) return full
+  if (!entriesChanged) return replacement
   const parts = ["The Code Mode tool catalog has changed."]
   if (diff.added.length > 0) {
     parts.push(
@@ -115,19 +117,19 @@ ${render(current)}`
     )
   }
   const delta = parts.join("\n\n")
-  if (delta.length < full.length) return delta
-  return full
+  if (delta.length < replacement.length) return delta
+  return replacement
 }
 
 const key = Instructions.Key.make("core/codemode")
 const codec = Schema.toCodecJson(CodeModeCatalog.Summary)
 
 export const make = (entries?: ReadonlyArray<CodeModeCatalog.Entry>): Instructions.Instructions => {
-  const catalog = CodeModeCatalog.summarize(entries ?? [])
+  const catalog = entries === undefined ? Instructions.removed : CodeModeCatalog.summarize(entries)
   return Instructions.make({
     key,
     codec,
-    read: Effect.succeed(catalog.total === 0 ? Instructions.removed : catalog),
+    read: Effect.succeed(catalog),
     render: {
       initial: render,
       changed: update,

+ 3 - 1
packages/core/test/codemode/catalog.test.ts

@@ -113,7 +113,9 @@ describe("CodeModeInstructions.render", () => {
   })
 
   test("renders only the no-tools notice for an empty catalog", () => {
-    expect(render([])).toBe("No tools are currently available.")
+    expect(render([])).toBe(
+      "No Code Mode tools are currently available. Do not call `execute` until a later system update announces available tools.",
+    )
   })
 })
 

+ 19 - 0
packages/core/test/codemode/instructions.test.ts

@@ -21,6 +21,25 @@ const lookup: CodeModeCatalog.Entry = {
 }
 
 describe("CodeModeInstructions", () => {
+  it.effect("instructs the model not to call execute while the catalog is empty", () =>
+    Effect.gen(function* () {
+      const initialized = yield* readInitial(CodeModeInstructions.make([]))
+      expect(initialized.text).toBe(
+        "No Code Mode tools are currently available. Do not call `execute` until a later system update announces available tools.",
+      )
+
+      const added = yield* readUpdate(CodeModeInstructions.make([echo]), initialized)
+      expect(added.text).toContain("New tools are available in addition to those previously listed:")
+      expect(added.text).toContain(echo.signature)
+
+      expect(yield* readUpdate(CodeModeInstructions.make([]), { values: added.values })).toMatchObject({
+        text:
+          "The Code Mode tool catalog has changed. This catalog supersedes the previous Code Mode tool catalog.\n\n" +
+          "No Code Mode tools are currently available. Do not call `execute` until a later system update announces available tools.",
+      })
+    }),
+  )
+
   it.effect("renders the initial catalog, semantic deltas, and removal", () =>
     Effect.gen(function* () {
       const initialized = yield* readInitial(CodeModeInstructions.make([echo]))

+ 16 - 0
packages/core/test/lib/tool.ts

@@ -32,6 +32,22 @@ export function waitForTool(
   })
 }
 
+export function waitForCodeModeTool(
+  registry: ToolRegistry.Interface,
+  path: string,
+  remaining = 1000,
+): Effect.Effect<ToolRegistry.ToolSet, Error> {
+  return Effect.gen(function* () {
+    const toolSet = yield* registry.snapshot()
+    if (toolSet.codeModeCatalog?.some((tool) => tool.path === path)) return toolSet
+    if (remaining === 0) {
+      return yield* Effect.fail(new Error(`Timed out waiting for Code Mode tool: ${path}`))
+    }
+    yield* Effect.promise(() => Bun.sleep(1))
+    return yield* waitForCodeModeTool(registry, path, remaining - 1)
+  })
+}
+
 /**
  * Registers a core tool plugin's tools against the real registry without booting the
  * full plugin host. Only the tool domain is live; focused tool tests exercise

+ 15 - 9
packages/core/test/mcp.test.ts

@@ -33,7 +33,7 @@ import { Image } from "@opencode-ai/core/image"
 import { testEffect } from "./lib/effect"
 import { imagePassthrough } from "./lib/image"
 import { location } from "./fixture/location"
-import { executeTool, toolDefinitions, toolIdentity, waitForTool } from "./lib/tool"
+import { executeTool, toolDefinitions, toolIdentity, waitForCodeModeTool, waitForTool } from "./lib/tool"
 
 let assertion: Deferred.Deferred<PermissionV2.AssertInput> | undefined
 let decision: Effect.Effect<void, PermissionV2.Error> = Effect.void
@@ -802,10 +802,16 @@ test("serializes concurrent MCP lifecycle operations", async () => {
 it.effect("advertises MCP output schemas to Code Mode", () =>
   Effect.gen(function* () {
     const registry = yield* ToolRegistry.Service
-    yield* waitForTool(registry, "execute")
-    const definitions = yield* toolDefinitions(registry)
-    const execute = definitions.find((tool) => tool.name === "execute")
-
+    const toolSet = yield* waitForCodeModeTool(registry, "demo.search")
+    const execute = toolSet.definitions.find((tool) => tool.name === "execute")
+
+    expect(toolSet.definitions.map((tool) => tool.name)).toEqual([
+      "direct_fail",
+      "direct_lookup",
+      "direct_media",
+      "execute",
+    ])
+    expect(toolSet.codeModeCatalog?.find((tool) => tool.path === "demo.search")?.signature).toContain("ok: boolean")
     expect(execute?.description).not.toContain("tools.demo.search")
   }),
 )
@@ -873,9 +879,9 @@ it.effect("waits for permission before calling an MCP tool", () =>
     const permission = yield* Deferred.make<void>()
     decision = Deferred.await(permission)
     const registry = yield* ToolRegistry.Service
-    yield* waitForTool(registry, "execute")
+    const toolSet = yield* waitForCodeModeTool(registry, "demo.search")
 
-    const fiber = yield* executeTool(registry, {
+    const fiber = yield* toolSet.execute({
       sessionID: SessionV2.ID.make("ses_mcp_permission"),
       ...toolIdentity,
       call: {
@@ -912,9 +918,9 @@ it.effect("does not call MCP when permission is blocked", () =>
     assertion = yield* Deferred.make<PermissionV2.AssertInput>()
     decision = Effect.fail(new PermissionV2.BlockedError({ rules: [], permission: "demo_search", resources: ["*"] }))
     const registry = yield* ToolRegistry.Service
-    yield* waitForTool(registry, "execute")
+    const toolSet = yield* waitForCodeModeTool(registry, "demo.search")
 
-    const execution = yield* executeTool(registry, {
+    const execution = yield* toolSet.execute({
       sessionID: SessionV2.ID.make("ses_mcp_blocked"),
       ...toolIdentity,
       call: {

+ 1 - 0
packages/core/test/session-runner-recorded.test.ts

@@ -186,6 +186,7 @@ describe("SessionRunnerLLM recorded", () => {
       yield* agents.transform((draft) =>
         draft.update(AgentV2.ID.make("build"), (agent) => {
           agent.mode = "primary"
+          agent.permissions.push({ action: "execute", resource: "*", effect: "deny" })
         }),
       )
       const pluginHost = host({

+ 33 - 10
packages/core/test/session-runner-tool-registry.test.ts

@@ -88,7 +88,7 @@ describe("ToolRegistry", () => {
 
       expect(error).toBeInstanceOf(Tool.RegistrationError)
       expect(error.message).toBe('Invalid tool namespace: "slack..admin"')
-      expect((yield* service.snapshot()).definitions).toEqual([])
+      expect((yield* service.snapshot()).definitions.map((tool) => tool.name)).toEqual(["execute"])
     }),
   )
 
@@ -102,7 +102,7 @@ describe("ToolRegistry", () => {
         .register({ "echo.tool": make(), echo_tool: make() }, { codemode: false })
         .pipe(Effect.flip)
       expect(collision.message).toBe("Duplicate normalized tool name: echo_tool")
-      expect((yield* service.snapshot()).definitions).toEqual([])
+      expect((yield* service.snapshot()).definitions.map((tool) => tool.name)).toEqual(["execute"])
     }),
   )
 
@@ -117,7 +117,7 @@ describe("ToolRegistry", () => {
         .pipe(Effect.flip)
 
       expect(error).toBeInstanceOf(Tool.RegistrationError)
-      expect((yield* service.snapshot()).definitions).toEqual([])
+      expect((yield* service.snapshot()).definitions.map((tool) => tool.name)).toEqual(["execute"])
     }),
   )
 
@@ -148,6 +148,20 @@ describe("ToolRegistry", () => {
     }),
   )
 
+  it.effect("keeps execute available without Code Mode tools unless explicitly denied", () =>
+    Effect.gen(function* () {
+      const service = yield* ToolRegistry.Service
+
+      const available = yield* service.snapshot()
+      expect(available.definitions.map((tool) => tool.name)).toEqual(["execute"])
+      expect(available.codeModeCatalog).toEqual([])
+
+      const denied = yield* service.snapshot([{ action: "execute", resource: "*", effect: "deny" }])
+      expect(denied.definitions).toEqual([])
+      expect(denied.codeModeCatalog).toBeUndefined()
+    }),
+  )
+
   it.effect("filters disabled tools with edit aliases and ordered wildcard precedence", () =>
     Effect.gen(function* () {
       const service = yield* ToolRegistry.Service
@@ -156,7 +170,12 @@ describe("ToolRegistry", () => {
       const names = (permissions: PermissionV2.Ruleset) =>
         toolDefinitions(service, permissions).pipe(Effect.map((definitions) => definitions.map((tool) => tool.name)))
 
-      expect(yield* names([{ action: "question", resource: "*", effect: "deny" }])).toEqual(["bash", "edit", "write"])
+      expect(yield* names([{ action: "question", resource: "*", effect: "deny" }])).toEqual([
+        "bash",
+        "edit",
+        "write",
+        "execute",
+      ])
       expect(
         yield* names([
           { action: "*", resource: "*", effect: "deny" },
@@ -169,7 +188,11 @@ describe("ToolRegistry", () => {
           { action: "*", resource: "*", effect: "deny" },
         ]),
       ).toEqual([])
-      expect(yield* names([{ action: "edit", resource: "*", effect: "deny" }])).toEqual(["bash", "question"])
+      expect(yield* names([{ action: "edit", resource: "*", effect: "deny" }])).toEqual([
+        "bash",
+        "question",
+        "execute",
+      ])
     }),
   )
 
@@ -182,7 +205,7 @@ describe("ToolRegistry", () => {
 
       expect(
         (yield* toolDefinitions(service, [{ action: "edit", resource: "*", effect: "deny" }])).map((tool) => tool.name),
-      ).toEqual(["first"])
+      ).toEqual(["first", "execute"])
     }),
   )
 
@@ -191,9 +214,9 @@ describe("ToolRegistry", () => {
       const service = yield* ToolRegistry.Service
       const scope = yield* Scope.make()
       yield* service.register({ echo: make() }, { codemode: false }).pipe(Scope.provide(scope))
-      expect((yield* toolDefinitions(service)).map((tool) => tool.name)).toEqual(["echo"])
+      expect((yield* toolDefinitions(service)).map((tool) => tool.name)).toEqual(["echo", "execute"])
       yield* Scope.close(scope, Exit.void)
-      expect(yield* toolDefinitions(service)).toEqual([])
+      expect((yield* toolDefinitions(service)).map((tool) => tool.name)).toEqual(["execute"])
     }),
   )
 
@@ -213,9 +236,9 @@ describe("ToolRegistry", () => {
       yield* Deferred.await(registered)
       yield* Fiber.interrupt(fiber)
 
-      expect((yield* toolDefinitions(service)).map((tool) => tool.name)).toEqual(["echo"])
+      expect((yield* toolDefinitions(service)).map((tool) => tool.name)).toEqual(["echo", "execute"])
       yield* Scope.close(scope, Exit.void)
-      expect(yield* toolDefinitions(service)).toEqual([])
+      expect((yield* toolDefinitions(service)).map((tool) => tool.name)).toEqual(["execute"])
     }),
   )
 

+ 40 - 0
packages/core/test/session-runner.test.ts

@@ -880,6 +880,46 @@ describe("SessionRunnerLLM", () => {
     }),
   )
 
+  it.effect("advertises execute and durable guidance for an empty Code Mode catalog", () =>
+    Effect.gen(function* () {
+      const session = yield* setup
+      const empty = {
+        catalog: [],
+        tool: Tool.make({
+          description: "Execute Code Mode",
+          input: Schema.Struct({ code: Schema.String }),
+          output: Schema.String,
+          execute: () => Effect.succeed({ output: "unused" }),
+        }),
+      }
+      codeModeMaterializations = [empty, empty, {}]
+      yield* admit(session, "Continue without Code Mode tools")
+      response = reply.stop()
+
+      yield* session.resume(sessionID)
+      yield* admit(session, "Still no Code Mode tools")
+      yield* session.resume(sessionID)
+      yield* admit(session, "Code Mode denied")
+      yield* session.resume(sessionID)
+
+      expect(requests).toHaveLength(3)
+      expect(requests[0]?.tools.map((tool) => tool.name)).toEqual(["defect", "echo", "storefail", "execute"])
+      expect(requests[0]?.system.some((part) => part.text.includes("Do not call `execute`"))).toBe(true)
+      expect(requests[1]?.tools.map((tool) => tool.name)).toEqual(["defect", "echo", "storefail", "execute"])
+      expect(requests[1]?.messages.filter((message) => message.role === "system")).toEqual([])
+      expect(requests[2]?.tools.map((tool) => tool.name)).toEqual(["defect", "echo", "storefail"])
+      expect(
+        requests[2]?.messages.some(
+          (message) =>
+            message.role === "system" &&
+            message.content.some(
+              (part) => part.type === "text" && part.text.includes("Code Mode tools are no longer available"),
+            ),
+        ),
+      ).toBe(true)
+    }),
+  )
+
   it.effect("applies session context hooks without exposing unavailable tools", () =>
     Effect.gen(function* () {
       const session = yield* setup

+ 6 - 4
packages/core/test/tool-edit.test.ts

@@ -137,10 +137,12 @@ describe("EditTool", () => {
           Effect.andThen(
             withTool(tmp.path, (registry) =>
               Effect.gen(function* () {
-                expect((yield* toolDefinitions(registry)).map((tool) => tool.name)).toEqual(["edit"])
-                expect(yield* toolDefinitions(registry, [{ action: "edit", resource: "*", effect: "deny" }])).toEqual(
-                  [],
-                )
+                expect((yield* toolDefinitions(registry)).map((tool) => tool.name)).toEqual(["edit", "execute"])
+                expect(
+                  (yield* toolDefinitions(registry, [{ action: "edit", resource: "*", effect: "deny" }])).map(
+                    (tool) => tool.name,
+                  ),
+                ).toEqual(["execute"])
                 const settled = yield* executeTool(
                   registry,
                   call({ path: "hello.txt", oldString: "before", newString: "after" }),

+ 1 - 1
packages/core/test/tool-patch.test.ts

@@ -181,7 +181,7 @@ describe("PatchTool", () => {
           Effect.andThen(
             withTool(tmp.path, (registry) =>
               Effect.gen(function* () {
-                expect((yield* toolDefinitions(registry)).map((tool) => tool.name)).toEqual(["patch"])
+                expect((yield* toolDefinitions(registry)).map((tool) => tool.name)).toEqual(["patch", "execute"])
                 const settled = yield* executeTool(
                   registry,
                   call(

+ 6 - 2
packages/core/test/tool-question.test.ts

@@ -97,7 +97,11 @@ describe("QuestionTool", () => {
       deny = true
       const registry = yield* ToolRegistry.Service
 
-      expect(yield* toolDefinitions(registry, [{ action: "question", resource: "*", effect: "deny" }])).toEqual([])
+      expect(
+        (yield* toolDefinitions(registry, [{ action: "question", resource: "*", effect: "deny" }])).map(
+          (tool) => tool.name,
+        ),
+      ).toEqual(["execute"])
       expect(
         yield* executeTool(registry, {
           sessionID,
@@ -142,7 +146,7 @@ describe("QuestionTool", () => {
         },
       ]
 
-      expect((yield* toolDefinitions(registry)).map((definition) => definition.name)).toEqual(["question"])
+      expect((yield* toolDefinitions(registry)).map((definition) => definition.name)).toEqual(["question", "execute"])
       expect(
         yield* executeTool(registry, {
           sessionID,

+ 6 - 2
packages/core/test/tool-read.test.ts

@@ -197,8 +197,12 @@ describe("ReadTool", () => {
     Effect.gen(function* () {
       const registry = yield* ToolRegistry.Service
 
-      expect(yield* toolDefinitions(registry)).toMatchObject([{ name: "read" }])
-      expect(yield* toolDefinitions(registry, [{ action: "read", resource: "*", effect: "deny" }])).toEqual([])
+      expect((yield* toolDefinitions(registry)).map((tool) => tool.name)).toEqual(["read", "execute"])
+      expect(
+        (yield* toolDefinitions(registry, [{ action: "read", resource: "*", effect: "deny" }])).map(
+          (tool) => tool.name,
+        ),
+      ).toEqual(["execute"])
       const execution = yield* executeTool(registry, {
         sessionID,
         ...toolIdentity,

+ 1 - 1
packages/core/test/tool-webfetch.test.ts

@@ -92,7 +92,7 @@ describe("WebFetchTool registration", () => {
       const registry = yield* ToolRegistry.Service
       const url = "http://example.com/public"
 
-      expect((yield* toolDefinitions(registry)).map((tool) => tool.name)).toEqual(["webfetch"])
+      expect((yield* toolDefinitions(registry)).map((tool) => tool.name)).toEqual(["webfetch", "execute"])
       expect(yield* executeTool(registry, call({ url, format: "text", timeout: 4 }))).toEqual({
         status: "completed",
         output: { url, contentType: "text/plain", format: "text", output: "hello" },

+ 1 - 1
packages/core/test/tool-websearch.test.ts

@@ -154,7 +154,7 @@ describe("WebSearchTool registration", () => {
       config = { provider: "exa", enableExa: false, enableParallel: false }
       const registry = yield* ToolRegistry.Service
 
-      expect((yield* toolDefinitions(registry)).map((tool) => tool.name)).toEqual(["websearch"])
+      expect((yield* toolDefinitions(registry)).map((tool) => tool.name)).toEqual(["websearch", "execute"])
       expect(
         yield* executeTool(registry, {
           sessionID,

+ 1 - 1
packages/core/test/tool-write.test.ts

@@ -118,7 +118,7 @@ describe("WriteTool", () => {
         reset()
         return withTool(tmp.path, (registry) =>
           Effect.gen(function* () {
-            expect((yield* toolDefinitions(registry)).map((tool) => tool.name)).toEqual(["write"])
+            expect((yield* toolDefinitions(registry)).map((tool) => tool.name)).toEqual(["write", "execute"])
             const settled = yield* executeTool(registry, call({ path: "src/new.txt", content: "created" }))
             expect(settled).toEqual({
               status: "completed",