diff --git a/.claude/wiki.md b/.claude/wiki.md index 7bfe449..b03893e 100644 --- a/.claude/wiki.md +++ b/.claude/wiki.md @@ -1,5 +1,18 @@ # polyskill Project Wiki +## Core JSON schemas are ALSO the server's validator — loosen client-first only + + + +The registry server (private `skill_marketplace` repo) validates publishes with the same +`@polyskill/core` schemas. Changing a schema here therefore needs a lockstep server upgrade +(publish core → bump the server's dependency, per CONTRIBUTING). Safe order: LOOSENING +(accepting more) ships client-first — worst case the old server rejects the new shape with a +clear 400 at publish. TIGHTENING must ship server-first or freshly-valid local skills would +already be live that the server later can't re-validate. 2026-07-03: `parameters` gained +boolean `additionalProperties` (OpenAI strict mode) — a loosening; server catches up on its +next core bump. + ## CLI version comes from package.json at runtime — never hardcode it diff --git a/README.md b/README.md index e2dc00d..1eba6f1 100644 --- a/README.md +++ b/README.md @@ -119,6 +119,8 @@ For skills that define tools, add a `tools.json` and reference it in the manifes } ``` +Tool `parameters` are standard JSON Schema objects. `"additionalProperties": false` is supported — set it (at every object level) if you target OpenAI strict function calling. + ## CLI Commands | Command | Description | diff --git a/packages/cli/src/__tests__/targets.test.ts b/packages/cli/src/__tests__/targets.test.ts index 40b2c2b..f126185 100644 --- a/packages/cli/src/__tests__/targets.test.ts +++ b/packages/cli/src/__tests__/targets.test.ts @@ -17,8 +17,14 @@ vi.mock("../config.js", () => ({ REGISTRY_URL: "http://localhost:3000", })); +// Mock the interactive picker used by resolveTarget on TTYs +vi.mock("@inquirer/prompts", () => ({ + select: vi.fn(), +})); + import { writeFile, mkdir } from "node:fs/promises"; import { existsSync } from "node:fs"; +import { select } from "@inquirer/prompts"; import { generateSkillMd, toSlug } from "../targets/skill-md.js"; import { detectInstalledTargets, resolveTarget, targetRegistry } from "../targets/index.js"; import { codexRootDir } from "../targets/codex.js"; @@ -185,38 +191,70 @@ describe("codexRootDir", () => { // ─── resolveTarget ─────────────────────────────────────────────────────────── describe("resolveTarget", () => { - it("returns specified target when --target flag given", () => { - const target = resolveTarget("claude-code", false); + /** Force isTTY on stdin+stdout for one call; restores afterwards. */ + async function withTTY(value: boolean | undefined, fn: () => Promise): Promise { + const stdinTTY = process.stdin.isTTY; + const stdoutTTY = process.stdout.isTTY; + (process.stdin as any).isTTY = value; + (process.stdout as any).isTTY = value; + try { + return await fn(); + } finally { + (process.stdin as any).isTTY = stdinTTY; + (process.stdout as any).isTTY = stdoutTTY; + } + } + + it("returns specified target when --target flag given", async () => { + const target = await resolveTarget("claude-code", false); expect(target.name).toBe("claude-code"); }); - it("returns local target when --output flag given (no --target)", () => { - const target = resolveTarget(undefined, true); + it("returns local target when --output flag given (no --target)", async () => { + const target = await resolveTarget(undefined, true); expect(target.name).toBe("local"); }); - it("exits with error for unknown target name", () => { - expect(() => resolveTarget("unknown-runtime", false)).toThrow(ExitError); + it("exits with error for unknown target name", async () => { + await expect(resolveTarget("unknown-runtime", false)).rejects.toThrow(ExitError); expect(process.exit).toHaveBeenCalledWith(1); }); - it("auto-detects single runtime and returns it", () => { + it("auto-detects single runtime and returns it", async () => { vi.mocked(existsSync).mockImplementation((p) => String(p) === path.join(os.homedir(), ".claude") ); - const target = resolveTarget(undefined, false); + const target = await resolveTarget(undefined, false); expect(target.name).toBe("claude-code"); }); - it("exits with message when multiple runtimes detected", () => { + it("exits with message when multiple runtimes detected without a TTY", async () => { vi.mocked(existsSync).mockReturnValue(true); - expect(() => resolveTarget(undefined, false)).toThrow(ExitError); + await withTTY(undefined, async () => { + await expect(resolveTarget(undefined, false)).rejects.toThrow(ExitError); + }); expect(process.exit).toHaveBeenCalledWith(1); + expect(select).not.toHaveBeenCalled(); + }); + + it("prompts to pick a runtime when multiple detected on a TTY", async () => { + vi.mocked(existsSync).mockReturnValue(true); + vi.mocked(select).mockResolvedValue("codex"); + + const target = await withTTY(true, () => resolveTarget(undefined, false)); + + expect(target.name).toBe("codex"); + expect(process.exit).not.toHaveBeenCalled(); + const promptArg = vi.mocked(select).mock.calls[0][0] as { + choices: Array<{ value: string }>; + }; + expect(promptArg.choices.map((c) => c.value)).toContain("claude-code"); + expect(promptArg.choices.map((c) => c.value)).toContain("codex"); }); - it("falls back to local when no runtimes detected", () => { + it("falls back to local when no runtimes detected", async () => { vi.mocked(existsSync).mockReturnValue(false); - const target = resolveTarget(undefined, false); + const target = await resolveTarget(undefined, false); expect(target.name).toBe("local"); }); }); diff --git a/packages/cli/src/commands/install.ts b/packages/cli/src/commands/install.ts index 02d5adb..7016969 100644 --- a/packages/cli/src/commands/install.ts +++ b/packages/cli/src/commands/install.ts @@ -118,7 +118,7 @@ export const installCommand = new Command("install") } // Resolve target (--target flag, --output implies local, or auto-detect) - const target = resolveTarget(options.target, options.output !== undefined); + const target = await resolveTarget(options.target, options.output !== undefined); const outputDir = resolve(process.cwd(), options.output ?? "."); if (options.output !== undefined && target.name !== "local") { diff --git a/packages/cli/src/targets/index.ts b/packages/cli/src/targets/index.ts index 3c6ebae..c817130 100644 --- a/packages/cli/src/targets/index.ts +++ b/packages/cli/src/targets/index.ts @@ -2,6 +2,7 @@ import { existsSync } from "node:fs"; import os from "node:os"; import { join } from "node:path"; import chalk from "chalk"; +import { select } from "@inquirer/prompts"; import { claudeCodeTarget } from "./claude-code.js"; import { openclawTarget } from "./openclaw.js"; import { opencodeTarget, openCodeRootDir } from "./opencode.js"; @@ -45,13 +46,14 @@ export function detectInstalledTargets(): string[] { * 2. --output given (implies local, backward compat) * 3. Auto-detect from installed runtimes * - 1 found → use it, print confirmation - * - >1 found → print list, exit with message to re-run with --target + * - >1 found → interactive picker on a TTY; otherwise exit with a + * message to re-run with --target (agents / CI pipes) * - 0 found → fall back to local with a warning */ -export function resolveTarget( +export async function resolveTarget( flag: string | undefined, hasOutputFlag: boolean -): Target { +): Promise { if (flag) { const target = targetRegistry[flag]; if (!target) { @@ -78,6 +80,16 @@ export function resolveTarget( } if (found.length > 1) { + if (process.stdin.isTTY && process.stdout.isTTY) { + const choice = await select({ + message: "Multiple runtimes detected — install to which one?", + choices: found.map((name) => ({ + name: `${name} (${targetRegistry[name].dir})`, + value: name, + })), + }); + return targetRegistry[choice]; + } console.log( chalk.yellow( `\nMultiple runtimes detected: ${found.join(", ")}.\nRe-run with --target to choose one.\n` diff --git a/packages/core/src/__tests__/validator.test.ts b/packages/core/src/__tests__/validator.test.ts index 76c5c88..beed59c 100644 --- a/packages/core/src/__tests__/validator.test.ts +++ b/packages/core/src/__tests__/validator.test.ts @@ -170,6 +170,49 @@ describe("validateTools", () => { expect(result.valid).toBe(true); }); + it("accepts additionalProperties: false in parameters (OpenAI strict mode)", () => { + const result = validateTools({ + tools: [ + { + name: "strict_tool", + description: "Strict-mode tool", + parameters: { + type: "object", + properties: { + location: { type: "string", description: "City name" }, + options: { + type: "object", + properties: { units: { type: "string" } }, + additionalProperties: false, + }, + }, + required: ["location"], + additionalProperties: false, + }, + }, + ], + }); + expect(result.valid).toBe(true); + expect(result.errors).toEqual([]); + }); + + it("rejects non-boolean additionalProperties in parameters", () => { + const result = validateTools({ + tools: [ + { + name: "bad_tool", + description: "Bad additionalProperties", + parameters: { + type: "object", + properties: {}, + additionalProperties: "nope", + }, + }, + ], + }); + expect(result.valid).toBe(false); + }); + it("rejects tools with extra properties on tool object", () => { const result = validateTools({ tools: [ diff --git a/packages/core/src/schema/tool-definition.schema.json b/packages/core/src/schema/tool-definition.schema.json index 9ca74bd..dbcf030 100644 --- a/packages/core/src/schema/tool-definition.schema.json +++ b/packages/core/src/schema/tool-definition.schema.json @@ -33,6 +33,10 @@ "required": { "type": "array", "items": { "type": "string" } + }, + "additionalProperties": { + "type": "boolean", + "description": "Standard JSON Schema key — set false for OpenAI strict function calling" } }, "additionalProperties": false diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts index 464ee59..8707c55 100644 --- a/packages/core/src/types.ts +++ b/packages/core/src/types.ts @@ -41,6 +41,7 @@ export interface JsonSchemaProperty { items?: JsonSchemaProperty; properties?: Record; required?: string[]; + additionalProperties?: boolean; } /** Parameter schema for a tool — standard JSON Schema object */ @@ -48,6 +49,8 @@ export interface ToolParameterSchema { type: "object"; properties: Record; required?: string[]; + /** Set false for OpenAI strict function calling */ + additionalProperties?: boolean; } /** A single canonical tool definition */