Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ mock.module("../tools/skills/skill-tool-factory.js", () => ({
entries: SkillToolManifest["tools"],
skillId: string,
_skillDir: string,
versionHash: string,
_versionHash: string,
_bundled?: boolean,
): Tool[] =>
entries.map((entry) => ({
Expand All @@ -113,7 +113,6 @@ mock.module("../tools/skills/skill-tool-factory.js", () => ({
executionTarget: "sandbox" as const,
origin: "skill" as const,
ownerSkillId: skillId,
ownerSkillVersionHash: versionHash,
input_schema: entry.input_schema as object,
execute: async () => ({ content: "", isError: false }),
})),
Expand Down
117 changes: 1 addition & 116 deletions assistant/src/__tests__/conversation-skill-tools.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ mock.module("../tools/skills/skill-tool-factory.js", () => ({
entries: SkillToolManifest["tools"],
skillId: string,
_skillDir: string,
versionHash: string,
_versionHash: string,
_bundled?: boolean,
): Tool[] => {
return entries.map((entry) => ({
Expand All @@ -107,7 +107,6 @@ mock.module("../tools/skills/skill-tool-factory.js", () => ({
executionTarget: "sandbox" as const,
origin: "skill" as const,
ownerSkillId: skillId,
ownerSkillVersionHash: versionHash,
input_schema: entry.input_schema as object,
execute: async () => ({ content: "", isError: false }),
}));
Expand Down Expand Up @@ -2256,120 +2255,6 @@ describe("hash change re-prompt regressions (PR 35)", () => {
});
});

// ---------------------------------------------------------------------------
// Version hash plumbing regression tests
// Verify that createSkillToolsFromManifest receives the computed hash and
// that projected tools carry ownerSkillVersionHash, which downstream
// components (executor.ts) use to build policy context.
// ---------------------------------------------------------------------------

describe("version hash plumbing to projected tools", () => {
let sessionState: Map<string, string>;

beforeEach(() => {
mockCatalog = [];
mockManifests = {};
mockRegisteredTools = new Map();
mockUnregisteredSkillIds = [];
mockSkillRefCount = new Map();
mockVersionHashes = {};
mockVersionHashErrors = new Set();
sessionState = new Map<string, string>();
});

test("projected tools carry ownerSkillVersionHash matching the computed hash", () => {
mockCatalog = [makeSkill("deploy")];
mockManifests = { deploy: makeManifest(["deploy_run", "deploy_status"]) };
mockVersionHashes = { deploy: "v1:secure-hash-abc" };

const history: Message[] = [
...skillLoadMessages('<loaded_skill id="deploy" />'),
];

projectSkillTools(history, { previouslyActiveSkillIds: sessionState });

const tools = mockRegisteredTools.get("deploy");
expect(tools).toBeDefined();
expect(tools!.length).toBe(2);

// Every tool created for this skill must carry the version hash
for (const tool of tools!) {
expect(tool.ownerSkillVersionHash).toBe("v1:secure-hash-abc");
}
});

test("after hash change re-registration, new tools carry the updated hash", () => {
mockCatalog = [makeSkill("deploy")];
mockManifests = { deploy: makeManifest(["deploy_run"]) };
mockVersionHashes = { deploy: "v1:hash-before" };

const history: Message[] = [
...skillLoadMessages('<loaded_skill id="deploy" />'),
];

// Turn 1: register with original hash
projectSkillTools(history, { previouslyActiveSkillIds: sessionState });
const toolsV1 = mockRegisteredTools.get("deploy");
expect(toolsV1).toBeDefined();
expect(toolsV1![0].ownerSkillVersionHash).toBe("v1:hash-before");

// Simulate file edit — hash changes
mockVersionHashes = { deploy: "v2:hash-after" };

// Turn 2: re-registration with new hash
projectSkillTools(history, { previouslyActiveSkillIds: sessionState });
const toolsV2 = mockRegisteredTools.get("deploy");
expect(toolsV2).toBeDefined();

// The most recently registered tool should carry the new hash
const lastTool = toolsV2![toolsV2!.length - 1];
expect(lastTool.ownerSkillVersionHash).toBe("v2:hash-after");
});

test("tools for multiple co-active skills each carry their own version hash", () => {
mockCatalog = [makeSkill("deploy"), makeSkill("oncall")];
mockManifests = {
deploy: makeManifest(["deploy_run"]),
oncall: makeManifest(["oncall_page"]),
};
mockVersionHashes = {
deploy: "v1:deploy-hash-123",
oncall: "v1:oncall-hash-456",
};

const history: Message[] = [
...skillLoadMessages('<loaded_skill id="deploy" />'),
...skillLoadMessages('<loaded_skill id="oncall" />'),
];

projectSkillTools(history, { previouslyActiveSkillIds: sessionState });

const deployTools = mockRegisteredTools.get("deploy");
expect(deployTools).toBeDefined();
expect(deployTools![0].ownerSkillVersionHash).toBe("v1:deploy-hash-123");

const oncallTools = mockRegisteredTools.get("oncall");
expect(oncallTools).toBeDefined();
expect(oncallTools![0].ownerSkillVersionHash).toBe("v1:oncall-hash-456");
});

test("default hash is used and plumbed when no explicit hash override is set", () => {
mockCatalog = [makeSkill("deploy")];
mockManifests = { deploy: makeManifest(["deploy_run"]) };
// No mockVersionHashes override — mock returns 'v1:default-hash-deploy'

const history: Message[] = [
...skillLoadMessages('<loaded_skill id="deploy" />'),
];

projectSkillTools(history, { previouslyActiveSkillIds: sessionState });

const tools = mockRegisteredTools.get("deploy");
expect(tools).toBeDefined();
expect(tools![0].ownerSkillVersionHash).toBe("v1:default-hash-deploy");
});
});

// ---------------------------------------------------------------------------
// Child skill includes: no auto-activation
// ---------------------------------------------------------------------------
Expand Down
6 changes: 2 additions & 4 deletions assistant/src/__tests__/plugin-tool-contribution.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -318,8 +318,8 @@ describe("registerPluginTools / unregisterPluginTools helpers", () => {
// pre-tagged with another skill's or plugin's ID. The helper must
// overwrite it so the bootstrap is always the source of truth for
// ownership — and it must clear cross-origin fields (ownerSkillId /
// ownerMcpServerId / ownerSkillVersionHash) so the stamped tool cannot
// leak across namespaces or spoof skill-origin auto-allow.
// ownerMcpServerId) so the stamped tool cannot leak across namespaces
// or spoof skill-origin auto-allow.
//
// The narrow `ToolDefinition` shape doesn't expose these ownership
// fields, so the cast through `unknown` simulates a hostile or
Expand All @@ -329,14 +329,12 @@ describe("registerPluginTools / unregisterPluginTools helpers", () => {
...makeFakeTool("pt_spoof"),
origin: "skill",
ownerSkillId: "some-other-skill",
ownerSkillVersionHash: "deadbeef",
} as unknown as LoadedTool;
registerPluginTools("my-plugin", [spoofed]);
const retrieved = getTool("pt_spoof");
expect(retrieved?.origin).toBe("plugin");
expect(retrieved?.ownerPluginId).toBe("my-plugin");
expect(retrieved?.ownerSkillId).toBeUndefined();
expect(retrieved?.ownerSkillVersionHash).toBeUndefined();
});

test("unregisterPluginTools removes the plugin's tools", () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ mock.module("../tools/skills/skill-tool-factory.js", () => ({
entries: SkillToolManifest["tools"],
skillId: string,
_skillDir: string,
versionHash: string,
_versionHash: string,
_bundled?: boolean,
): Tool[] => {
return entries.map((entry) => ({
Expand All @@ -124,7 +124,6 @@ mock.module("../tools/skills/skill-tool-factory.js", () => ({
executionTarget: "sandbox" as const,
origin: "skill" as const,
ownerSkillId: skillId,
ownerSkillVersionHash: versionHash,
input_schema: entry.input_schema as object,
execute: async () => ({ content: "", isError: false }),
}));
Expand Down
3 changes: 1 addition & 2 deletions assistant/src/__tests__/skill-projection.benchmark.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ mock.module("../tools/skills/skill-tool-factory.js", () => ({
entries: Array<{ name: string; description: string; input_schema: object }>,
skillId: string,
_skillDir: string,
versionHash: string,
_versionHash: string,
_bundled?: boolean,
) =>
entries.map((e) => ({
Expand All @@ -112,7 +112,6 @@ mock.module("../tools/skills/skill-tool-factory.js", () => ({
defaultRiskLevel: "low",
origin: "skill" as const,
ownerSkillId: skillId,
ownerSkillVersionHash: versionHash,
input_schema: e.input_schema,
execute: async () => ({ content: "", isError: false }),
})),
Expand Down
32 changes: 0 additions & 32 deletions assistant/src/__tests__/skill-tool-factory.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,18 +97,6 @@ describe("createSkillTool", () => {
expect(tool.ownerSkillId).toBe("weather-skill");
});

test("sets ownerSkillVersionHash from versionHash", () => {
const hash = "v1:abc123def456";
const tool = createSkillTool(
makeEntry(),
"my-skill",
"/skills/my-skill",
hash,
);

expect(tool.ownerSkillVersionHash).toBe(hash);
});

test.each([
["low", RiskLevel.Low],
["medium", RiskLevel.Medium],
Expand Down Expand Up @@ -249,24 +237,6 @@ describe("createSkillToolsFromManifest", () => {
expect(tools).toEqual([]);
});

test("passes versionHash through to all created tools", () => {
const hash = "v1:deadbeef";
const entries: SkillToolEntry[] = [
makeEntry({ name: "alpha" }),
makeEntry({ name: "beta" }),
];

const tools = createSkillToolsFromManifest(
entries,
"versioned-skill",
"/skills/versioned",
hash,
);

for (const tool of tools) {
expect(tool.ownerSkillVersionHash).toBe(hash);
}
});
});

// ---------------------------------------------------------------------------
Expand Down Expand Up @@ -389,7 +359,5 @@ describe("createSkillTool — version hash plumbing to runner", () => {
const parsed = JSON.parse(result.content);
expect(parsed.input).toEqual({ query: "test" });
expect(parsed.workingDir).toBe("/my/project");
// Confirm the tool still has the hash stored
expect(tool.ownerSkillVersionHash).toBe(hash);
});
});
2 changes: 0 additions & 2 deletions assistant/src/__tests__/tool-executor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,6 @@ describe("ToolExecutor policy context plumbing", () => {
defaultRiskLevel: RiskLevel.Low,
origin: "skill" as const,
ownerSkillId: "my-skill-123",
ownerSkillVersionHash: "abc123hash",
executionTarget: "sandbox" as const,
input_schema: { type: "object" as const, properties: {} },
execute: async () => fakeToolResult,
Expand Down Expand Up @@ -455,7 +454,6 @@ describe("ToolExecutor policy context plumbing", () => {
defaultRiskLevel: RiskLevel.Low,
origin: "skill" as const,
ownerSkillId: "host-skill",
ownerSkillVersionHash: "host-hash",
executionTarget: "host" as const,
input_schema: { type: "object" as const, properties: {} },
execute: async () => fakeToolResult,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,6 @@ describe("loadMeetManifestProxies", () => {
expect(t.executionMode).toBe("proxy");
expect(t.origin).toBe("skill");
expect(t.ownerSkillId).toBe("meet-join");
expect(t.ownerSkillVersionHash).toBe(FIXTURE_MANIFEST.sourceHash);
expect(t.input_schema).toEqual(
FIXTURE_MANIFEST.tools[0]!.input_schema,
);
Expand Down
4 changes: 1 addition & 3 deletions assistant/src/daemon/meet-manifest-loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,6 @@ function serializeToolContext(context: ToolContext): Record<string, unknown> {
function buildProxyTool(
entry: ManifestToolEntry,
supervisor: MeetHostSupervisor,
manifestHash: string,
): Tool {
const risk = coerceRiskLevel(entry.risk, entry.name);
return {
Expand All @@ -142,7 +141,6 @@ function buildProxyTool(
executionTarget: "host" as ExecutionTarget,
origin: "skill",
ownerSkillId: MEET_SKILL_ID,
ownerSkillVersionHash: manifestHash,
execute: async (input, context) => {
// `dispatchTool` ensures the meet-host child is up + connected
// before sending the frame, so callers don't need a separate
Expand Down Expand Up @@ -363,7 +361,7 @@ export async function loadMeetManifestProxies(
// contract as the in-process skill's provider closure.
registerTools(() =>
manifest.tools.map((entry) =>
buildProxyTool(entry, supervisor, manifest.sourceHash),
buildProxyTool(entry, supervisor),
),
);

Expand Down
2 changes: 0 additions & 2 deletions assistant/src/ipc/skill-routes/registries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ const ToolManifestSchema = z.object({
// tool registered without an owner has no ref-counted entry to drop and
// would leak into the global registry on socket close.
ownerSkillId: z.string().min(1),
ownerSkillVersionHash: z.string().optional(),
});

export type ToolManifest = z.infer<typeof ToolManifestSchema>;
Expand Down Expand Up @@ -194,7 +193,6 @@ function buildProxyTool(manifest: ToolManifest): Tool {
}),
origin: "skill",
ownerSkillId: manifest.ownerSkillId,
ownerSkillVersionHash: manifest.ownerSkillVersionHash,
execute: async () => {
// Only reached when no supervisor is attached (tests/boot race);
// the supervisor short-circuit above replaces this with the
Expand Down
1 change: 0 additions & 1 deletion assistant/src/tools/registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,6 @@ export function registerPluginTools(
ownerPluginId: pluginName,
ownerSkillId: undefined,
ownerMcpServerId: undefined,
ownerSkillVersionHash: undefined,
};
return withProviderSafeToolName(tool);
});
Expand Down
1 change: 0 additions & 1 deletion assistant/src/tools/skills/skill-tool-factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ export function createSkillTool(
origin: "skill",
ownerSkillId: skillId,
executionTarget: entry.execution_target as ExecutionTarget,
ownerSkillVersionHash: versionHash,

input_schema: entry.input_schema as object,

Expand Down
2 changes: 0 additions & 2 deletions assistant/src/tools/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,4 @@ export interface Tool extends LoadedTool {
ownerMcpServerId?: string;
/** If origin is 'plugin', the name of the owning plugin. */
ownerPluginId?: string;
/** Content-hash of the owning skill's source at registration time. */
ownerSkillVersionHash?: string;
}
1 change: 0 additions & 1 deletion packages/skill-host-contracts/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1083,7 +1083,6 @@ export class SkillHostClient implements SkillHost {
executionTarget: t.executionTarget,
executionMode: t.executionMode ?? "proxy",
ownerSkillId: t.ownerSkillId ?? this.options.skillId,
ownerSkillVersionHash: t.ownerSkillVersionHash,
};
});
// Cache the provider so `skill.dispatch_tool` can resolve a tool
Expand Down
2 changes: 0 additions & 2 deletions packages/skill-host-contracts/src/tool-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -429,8 +429,6 @@ export interface Tool {
ownerMcpServerId?: string;
/** If origin is 'plugin', the name of the owning plugin. */
ownerPluginId?: string;
/** Content-hash of the owning skill's source at registration time. */
ownerSkillVersionHash?: string;
/** Declared execution target from the skill manifest. Used by resolveExecutionTarget
* to accurately label lifecycle events for skill-provided tools. */
executionTarget?: ExecutionTarget;
Expand Down
4 changes: 2 additions & 2 deletions skills/catalog.json
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@
"display-name": "LLM Cost Optimizer"
}
},
"updatedAt": "2026-05-26T13:03:52-05:00"
"updatedAt": "2026-05-26T14:11:54-04:00"
},
{
"id": "macos-automation",
Expand Down Expand Up @@ -426,7 +426,7 @@
"feature-flag": "meet"
}
},
"updatedAt": "2026-05-24T10:49:52-04:00"
"updatedAt": "2026-05-26T18:33:11Z"
},
{
"id": "meme-generator",
Expand Down
2 changes: 0 additions & 2 deletions skills/meet-join/__tests__/entrypoint.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,6 @@ const fakeTool = (name: string) => ({
category: "fake",
defaultRiskLevel: "low",
ownerSkillId: "meet-join",
ownerSkillBundled: true,
ownerSkillVersionHash: "test",
executionTarget: "host" as const,
executionMode: "proxy" as const,
getDefinition: () => ({ name, description: "test fake", input_schema: {} }),
Expand Down
Loading