tools: drop Tool.origin in favor of registry-side ownership lookup

assistant/src/__tests__/checker.test.ts

@@ -137,7 +137,6 @@ const mockSkillTool: Tool = {
   category: "skill",
   defaultRiskLevel: RiskLevel.Low,
   executionTarget: "sandbox",
-  origin: "skill",
   input_schema: { type: "object" as const, properties: {} },
   execute: async () => ({ content: "ok", isError: false }),
 };
@@ -154,7 +153,6 @@ const mockBundledSkillTool: Tool = {
   category: "skill",
   defaultRiskLevel: RiskLevel.Low,
   executionTarget: "sandbox",
-  origin: "skill",
   input_schema: { type: "object" as const, properties: {} },
   execute: async () => ({ content: "ok", isError: false }),
 };
@@ -390,7 +388,6 @@ describe("Permission Checker", () => {
         category: "skill",
         defaultRiskLevel: RiskLevel.Medium,
         executionTarget: "sandbox",
-        origin: "skill",
         input_schema: { type: "object" as const, properties: {} },
         execute: async () => ({ content: "ok", isError: false }),
       };

assistant/src/__tests__/computer-use-skill-manifest-regression.test.ts

@@ -113,16 +113,16 @@ describe("computer-use skill manifest regression", () => {
     await initializeTools();
 
     // Simulate what projectSkillTools() does when the computer-use skill is
-    // activated: create skill-origin Tool objects matching the manifest names
-    // and register them. This must not throw.
+    // activated: create Tool objects matching the manifest names and register
+    // them through `registerSkillTools(skillId, tools)`, which records
+    // ownership in the registry's `ownersByName` map. This must not throw.
     const skillTools: Tool[] = manifest.tools.map(
       (entry: { name: string; description: string }) => ({
         name: entry.name,
         description: entry.description,
         input_schema: { type: "object" as const, properties: {} },
         category: "computer-use",
         defaultRiskLevel: RiskLevel.Low,
-        origin: "skill" as const,
         execute: async () => ({ content: "stub", isError: false }),
       }),
     );

assistant/src/__tests__/conversation-app-control-instantiation.test.ts

@@ -112,7 +112,6 @@ mock.module("../tools/skills/skill-tool-factory.js", () => ({
       category: entry.category,
       defaultRiskLevel: RiskLevel.Medium,
       executionTarget: "sandbox" as const,
-      origin: "skill" as const,
       input_schema: entry.input_schema as object,
       execute: async () => ({ content: "", isError: false }),
     })),

assistant/src/__tests__/conversation-skill-tools.test.ts

@@ -106,7 +106,6 @@ mock.module("../tools/skills/skill-tool-factory.js", () => ({
       category: entry.category,
       defaultRiskLevel: RiskLevel.Medium,
       executionTarget: "sandbox" as const,
-      origin: "skill" as const,
       input_schema: entry.input_schema as object,
       execute: async () => ({ content: "", isError: false }),
     }));
@@ -1740,14 +1739,11 @@ describe("bundled skill: app-builder", () => {
     expect(tools).toBeDefined();
     expect(tools!.length).toBe(4);
 
-    // All tools should have skill origin metadata. Ownership is asserted by
-    // the fact that `mockRegisteredTools.get("app-builder")` returned the
-    // tools — i.e. the mock `registerSkillTools(skillId, tools)` was called
-    // with skillId "app-builder", which is what the registry would record
-    // in production.
-    for (const tool of tools!) {
-      expect(tool.origin).toBe("skill");
-    }
+    // Ownership is asserted by the fact that
+    // `mockRegisteredTools.get("app-builder")` returned the tools — i.e. the
+    // mock `registerSkillTools(skillId, tools)` was called with skillId
+    // "app-builder", which is what the registry would record in production.
+    // The Tool object itself no longer carries a kind/origin field.
   });
 });
 

assistant/src/__tests__/plugin-tool-contribution.test.ts

@@ -6,9 +6,11 @@
  *
  * - Registering a plugin with `tools: Tool[]`, running `bootstrapPlugins`,
  *   and observing the contributed tool via `getAllTools()` / `getTool()`.
- * - Tool ownership metadata (`origin: "plugin"`, `owner: { kind: "plugin", id: <plugin> }`)
- *   stamped authoritatively by `registerPluginTools` regardless of what the
- *   plugin author set on the incoming object.
+ * - Tool ownership (`owner: { kind: "plugin", id: <plugin> }`) recorded
+ *   authoritatively by `registerPluginTools` into the registry's
+ *   `ownersByName` map (queried via `getToolOwner(name)`), regardless of
+ *   what the plugin author set on the incoming object. The `Tool` itself
+ *   carries no ownership field — the bootstrap is the only writer.
  * - Shutdown hook unregistering the contributed tools so the registry is
  *   clean again after teardown.
  * - Direct `registerPluginTools` / `unregisterPluginTools` semantics,
@@ -169,10 +171,9 @@ describe("plugin tool contributions", () => {
     // registry's `ownersByName` map (keyed by tool name, accessed via
     // `getToolOwner(name)`) — the registry uses it to drive ref-counting
     // and conflict detection when the plugin shuts down or is hot-reloaded.
-    // Plugin tools live in their own `origin: "plugin"` namespace, disjoint
-    // from real skills, so a plugin name that happens to match a skill id
-    // cannot collide. Ownership is not stamped on the `Tool` object itself.
-    expect(retrieved?.origin).toBe("plugin");
+    // Plugin tools live in their own namespace, disjoint from real skills,
+    // so a plugin name that happens to match a skill id cannot collide.
+    // Ownership is not stamped on the `Tool` object itself.
     expect(getToolOwner("plugin-contrib-tool")).toEqual({
       kind: "plugin",
       id: "alpha-contributor",
@@ -254,26 +255,23 @@ describe("registerPluginTools / unregisterPluginTools helpers", () => {
     __resetRegistryForTesting();
   });
 
-  test("registerPluginTools stamps category and origin from the plugin, records ownership in the registry", () => {
-    // Even if the plugin author hands in a tool with no category or
-    // ownership metadata, the helper fills in category/origin on the tool
-    // and records ownership in the registry's `ownersByName` map — the
-    // tool itself never carries an `owner` field, so plugin authors can't
-    // spoof ownership by forging one.
+  test("registerPluginTools stamps category and records ownership in the registry", () => {
+    // Even if the plugin author hands in a tool with no category, the
+    // helper fills it in and records ownership in the registry's
+    // `ownersByName` map — the tool itself never carries an `owner` field,
+    // so plugin authors can't spoof ownership by forging one.
     const accepted = registerPluginTools("my-plugin", [
       makeFakeTool("pt_stamped"),
     ]);
     expect(accepted).toHaveLength(1);
     expect(accepted[0]?.category).toBe("plugin");
-    expect(accepted[0]?.origin).toBe("plugin");
     expect(getToolOwner("pt_stamped")).toEqual({
       kind: "plugin",
       id: "my-plugin",
     });
 
     const retrieved = getTool("pt_stamped");
     expect(retrieved?.category).toBe("plugin");
-    expect(retrieved?.origin).toBe("plugin");
   });
 
   test("registerPluginTools exposes provider-safe aliases for unsafe plugin tool names", async () => {
@@ -323,30 +321,24 @@ describe("registerPluginTools / unregisterPluginTools helpers", () => {
     expect(getTool(paddedAlias!)).toBeDefined();
   });
 
-  test("registerPluginTools overwrites any pre-existing ownership metadata", () => {
+  test("registerPluginTools ignores forged ownership fields on the incoming tool", () => {
     // A plugin author could (maliciously or mistakenly) hand in a tool
-    // pre-tagged with another skill's or plugin's ID. The helper must
-    // overwrite both `origin` and `owner` so the bootstrap is always the
-    // source of truth for ownership and the stamped tool cannot leak across
-    // namespaces or spoof skill-origin auto-allow.
+    // pre-tagged with another skill's or plugin's ID. The `Tool` type now
+    // carries no ownership field at all, so any such forgery is purely
+    // inert extra data — the registry only populates `ownersByName` from
+    // the first argument to `register*Tools`, which is the single source
+    // of truth for ownership and cannot be spoofed by forging fields on
+    // the manifest.
     //
-    // The narrow `ToolDefinition` shape doesn't expose ownership fields, so
-    // the cast through `unknown` simulates a hostile or transpiled artifact
-    // that arrives with spoofed fields baked in — the bootstrap-side
-    // defense is the second layer that must hold.
+    // Cast through `unknown` to simulate a hostile or transpiled artifact
+    // arriving with extra fields baked in.
     const spoofed = {
       ...makeFakeTool("pt_spoof"),
       origin: "skill",
-      // Forging an `owner` field on the tool literal has no effect — the
-      // `Tool` type doesn't carry ownership at all, and the registry only
-      // populates `ownersByName` from the first argument to the
-      // `register*Tools` function. Cast through `unknown` to simulate a
-      // hostile or transpiled artifact arriving with extra fields.
       owner: { kind: "skill", id: "some-other-skill" },
     } as unknown as LoadedTool;
     registerPluginTools("my-plugin", [spoofed]);
-    const retrieved = getTool("pt_spoof");
-    expect(retrieved?.origin).toBe("plugin");
+    expect(getTool("pt_spoof")).toBeDefined();
     expect(getToolOwner("pt_spoof")).toEqual({
       kind: "plugin",
       id: "my-plugin",

assistant/src/__tests__/registry.test.ts

@@ -42,10 +42,10 @@ function makeFakeTool(name: string): Tool {
 }
 
 function makeSkillTool(name: string): Tool {
-  return {
-    ...makeFakeTool(name),
-    origin: "skill" as const,
-  };
+  // Ownership is recorded by the registry (via `registerSkillTools`), not by
+  // any field on the Tool itself. This helper is now a thin alias for
+  // makeFakeTool; the wrapper is kept so call sites stay readable.
+  return makeFakeTool(name);
 }
 
 describe("tool registry host tools", () => {
@@ -217,35 +217,26 @@ describe("baseline characterization: core app tool surface", () => {
   });
 });
 
-describe("tool origin metadata", () => {
+describe("tool ownership metadata", () => {
   beforeEach(() => {
     __resetRegistryForTesting();
   });
 
-  test("registers a skill-origin tool and preserves origin via getTool()", () => {
-    const skillTool: Tool = {
-      ...makeFakeTool("test-skill-origin-tool"),
-      origin: "skill",
-    };
+  test("registerTool does not record ownership (bare-install path)", () => {
+    registerTool(makeFakeTool("test-bare-tool"));
 
-    registerTool(skillTool);
-
-    const retrieved = getTool("test-skill-origin-tool");
-    expect(retrieved).toBeDefined();
-    expect(retrieved?.origin).toBe("skill");
+    expect(getTool("test-bare-tool")).toBeDefined();
     // `registerTool` is the bare-install path used by tests + core
     // bootstraps; it does not record ownership. Tools that need an owner
     // must go through `registerSkillTools(skillId, ...)` or its sibling
     // entry points so the registry populates `ownersByName`.
-    expect(getToolOwner("test-skill-origin-tool")).toBeUndefined();
+    expect(getToolOwner("test-bare-tool")).toBeUndefined();
   });
 
-  test("core tools have no origin metadata and no owner", async () => {
+  test("core tools have no owner", async () => {
     await initializeTools();
 
-    const coreTool = getTool("host_file_read");
-    expect(coreTool).toBeDefined();
-    expect(coreTool?.origin).toBeUndefined();
+    expect(getTool("host_file_read")).toBeDefined();
     expect(getToolOwner("host_file_read")).toBeUndefined();
   });
 });
@@ -262,12 +253,16 @@ describe("dynamic skill tool registry", () => {
     ]);
 
     expect(getTool("sk_tool_a")).toBeDefined();
-    expect(getTool("sk_tool_a")?.origin).toBe("skill");
-    expect(getToolOwner("sk_tool_a")).toEqual({ kind: "skill", id: "my-skill" });
+    expect(getToolOwner("sk_tool_a")).toEqual({
+      kind: "skill",
+      id: "my-skill",
+    });
 
     expect(getTool("sk_tool_b")).toBeDefined();
-    expect(getTool("sk_tool_b")?.origin).toBe("skill");
-    expect(getToolOwner("sk_tool_b")).toEqual({ kind: "skill", id: "my-skill" });
+    expect(getToolOwner("sk_tool_b")).toEqual({
+      kind: "skill",
+      id: "my-skill",
+    });
   });
 
   test("skips skill tool that collides with a core tool without throwing", async () => {
@@ -281,8 +276,7 @@ describe("dynamic skill tool registry", () => {
     // The colliding tool should be silently skipped
     expect(accepted).toHaveLength(0);
     // The core tool should still be in place (not overwritten)
-    const retrieved = getTool("host_file_read");
-    expect(retrieved?.origin).toBeUndefined(); // core tools have no origin
+    expect(getTool("host_file_read")).toBeDefined();
     expect(getToolOwner("host_file_read")).toBeUndefined();
   });
 
@@ -333,7 +327,10 @@ describe("dynamic skill tool registry", () => {
 
     expect(getTool("sk_keep")).toBeDefined();
     expect(getTool("sk_remove")).toBeUndefined();
-    expect(getToolOwner("sk_keep")).toEqual({ kind: "skill", id: "keep-skill" });
+    expect(getToolOwner("sk_keep")).toEqual({
+      kind: "skill",
+      id: "keep-skill",
+    });
     expect(getToolOwner("sk_remove")).toBeUndefined();
   });
 
@@ -369,8 +366,8 @@ describe("dynamic skill tool registry", () => {
       kind: "skill",
       id: "atomic-skill",
     });
-    // The core tool should be untouched
-    expect(getTool("host_file_read")?.origin).toBeUndefined();
+    // The core tool should be untouched (no owner recorded)
+    expect(getTool("host_file_read")).toBeDefined();
     expect(getToolOwner("host_file_read")).toBeUndefined();
   });
 });

assistant/src/__tests__/skill-projection-feature-flag.test.ts

@@ -129,7 +129,6 @@ mock.module("../tools/skills/skill-tool-factory.js", () => ({
       category: entry.category,
       defaultRiskLevel: RiskLevel.Medium,
       executionTarget: "sandbox" as const,
-      origin: "skill" as const,
       input_schema: entry.input_schema as object,
       execute: async () => ({ content: "", isError: false }),
     }));
@@ -229,9 +228,8 @@ mock.module("../util/logger.js", () => ({
 
 const { projectSkillTools, resetSkillToolProjection } =
   await import("../daemon/conversation-skill-tools.js");
-const { setOverridesForTesting } = await import(
-  "./feature-flag-test-helpers.js"
-);
+const { setOverridesForTesting } =
+  await import("./feature-flag-test-helpers.js");
 
 // ---------------------------------------------------------------------------
 // Helpers

assistant/src/__tests__/skill-projection.benchmark.test.ts

@@ -110,7 +110,6 @@ mock.module("../tools/skills/skill-tool-factory.js", () => ({
       description: e.description,
       category: "skill",
       defaultRiskLevel: "low",
-      origin: "skill" as const,
       owner: { kind: "skill" as const, id: skillId },
       input_schema: e.input_schema,
       execute: async () => ({ content: "", isError: false }),