From dc9f387edceeb92ca94c9bac943871e190130383 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Tue, 3 Nov 2026 06:29:13 -0600 Subject: [PATCH] fix(api): surface existing skill source import in discovery (#18757) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Agents discover supported API operations through the public OpenAPI catalog. > - Importing existing skill content must retain its source provenance. > - A normal source-import query misses the applicable operation on its first page because that operation has little search metadata. > - This pull request describes the existing import operation and its source boundaries. > - The benefit is clearer discovery without changing search scoring or import authority. ## Linked Issues or Issue Description **What happened?** `search_api` does not return `POST /api/companies/{companyId}/skills/import` in the first ten results for `import skill from source SKILL.md`. Source-management operations rank ahead of the import operation. The import entry has only the summary “Import a skill.” **Expected behavior** Ordinary source-import discovery should surface the existing operation. Its documentation should explain that imported content retains provenance and remains subject to company policy and source checks. **Steps to reproduce** Call `searchRunnerApi({ query: "import skill from source SKILL.md", limit: 10 })`. Inspect the operation IDs on the first page. The new regression sends this result through the real bounded MCP serializer. Related: #18717 clarifies the separate `create_skill` tool. This change covers the existing source-import HTTP operation. Neither PR is merged by this work. ## What Changed - Expand only the source-import OpenAPI summary and description. Describe allowed local skill directories, supported remote sources, retained provenance and existing access checks. - Add a first-page discovery regression through the actual authenticated MCP bridge and serializer. Keep non-import lookup, company/mode refusal and denial forwarding controls. - Update the feature and access maps with the test scope and its limits. ## Verification - Parent regression: the controlled loopback MCP response lacked the applicable import operation on the first page. - Focused current checks: 2,060 tests pass in `skill-import-discovery.test.ts` and the existing `runner-api.test.ts` owner. - `node scripts/check-maps.mjs` passes. This checks map structure only. - The first head had one server test job cancelled after a runner shutdown signal. The reason for the shutdown is unknown. The reviewed successor removes only two test comments required by contribution rules; its four focused controls pass. The remaining obsolete CI run was cancelled after that successor was published. No same-head retry was used. - The new test is under `server/src/__tests__`, which the production server typecheck excludes and the server Vitest suite includes. - No local app, database, native agent or provider model was run. Normal PR CI on `a4aaae8e320fb1d56cdbff9f376ec832a634a5da` passed with 48 successful jobs and one skipped job, including the required `ci / verify` and `ci / e2e` checks. No completed agent-import journey or reliability result is claimed. - Greptile is 5/5 on the current head with no unresolved threads. The optional Paperclip Review check remains queued. This is not merge authorization. - Keep this core PR open and unmerged. Do not enable auto-merge. ## Risks The expanded metadata can change where this operation ranks for related queries. Search weights, request schemas, HTTP routes, source parsing, authorization and company skill policy are unchanged. The synthetic 403 control proves denial forwarding; it does not qualify the real policy or source-import route. A live import still needs separate acceptance. ## Model Used OpenAI `gpt-6.1-sol`, through Codex with reasoning, source review and code execution tools. The exact context-window size is not exposed by this environment. A separate agent reviewed the source and contract before publication. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip --- maps/agent-access-map/skills.md | 8 ++ maps/feature-map/skills.md | 6 ++ .../__tests__/skill-import-discovery.test.ts | 75 +++++++++++++++++++ server/src/routes/openapi.ts | 3 +- 4 files changed, 91 insertions(+), 1 deletion(-) create mode 100644 server/src/__tests__/skill-import-discovery.test.ts diff --git a/maps/agent-access-map/skills.md b/maps/agent-access-map/skills.md index f6c06d5d3bf..ab1c27e4835 100644 --- a/maps/agent-access-map/skills.md +++ b/maps/agent-access-map/skills.md @@ -57,6 +57,14 @@ subject to the existing bridge restrictions. Policy: Company skill policy by action/resource plus platform invariants; test execution additionally requires task-assignment authority. +Native `search_api` describes source import separately from authoring. Existing +SKILL.md content may come from an allowed local skill directory or a supported +remote source. Local workspace/managed-root and remote source-access checks still +apply. Discovery metadata grants no import permission and does not assign skills. +The [discovery protocol regression](../../server/src/__tests__/skill-import-discovery.test.ts) +checks the actual first-page result and bounded MCP output. Its synthetic 403 +control proves denial forwarding, not a live skill-policy decision. + Evidence: `source-only` — [implementation](../../server/src/routes/company-skills.ts#L208) at the reviewed commit above. diff --git a/maps/feature-map/skills.md b/maps/feature-map/skills.md index d1addcf3717..38e357c31ef 100644 --- a/maps/feature-map/skills.md +++ b/maps/feature-map/skills.md @@ -26,6 +26,12 @@ Agent access: [companion](../agent-access-map/skills.md). Open `/skills`, Discover, or `/skills/sources`; project import is available through the library’s import flow. In Streamlined UI, Installed, Discover and My Skills are route links styled as tabs beneath the toolbar, with no secondary navigation rail. Sources stays separate under New → Manage sources. Legacy UI retains the Skills sidebar. +Agent API discovery distinguishes importing existing SKILL.md content from +creating newly authored content. The source-import entry documents allowed local +skill directories and supported remote sources without changing source checks or +company skill policy. Its [protocol regression](../../server/src/__tests__/skill-import-discovery.test.ts) +checks discovery and metadata delivery, not a completed agent import journey. + ### `studio` Use New skill or `/skills/studio/:skillId`; writable copies and source-backed originals differ. In Streamlined UI, `/skills/studio` uses the shared Skills toolbar and card collection with My Skills active and Recently updated sorting; opening a card enters Studio. diff --git a/server/src/__tests__/skill-import-discovery.test.ts b/server/src/__tests__/skill-import-discovery.test.ts new file mode 100644 index 00000000000..b32fb8ad2df --- /dev/null +++ b/server/src/__tests__/skill-import-discovery.test.ts @@ -0,0 +1,75 @@ +import { expect, it, vi } from "vitest"; +import { searchRunnerApi } from "../services/native-runtime/runner-api-catalog.js"; +import { executeRunnerApi, validateRunnerApiCall } from "../services/native-runtime/runner-api-client.js"; +import { startOpenCodeMcpBridge } from "../../../packages/paperclip-runner/src/drivers/opencode/mcp-bridge.js"; +import { searchApiAction } from "../../../packages/paperclip-runner/src/protocol-actions/search-api.js"; + +const operationId = "POST /api/companies/{companyId}/skills/import"; +const context = { companyId: "company-a", issueId: "issue-a", runId: "run-a", workMode: "standard" }; + +it("surfaces existing source import on the first discovery page and delivers its contract through MCP", async () => { + const descriptor = searchApiAction.live.descriptor; + let calls = 0; + const bridge = await startOpenCodeMcpBridge({ + tools: [{ name: descriptor.operationId, description: descriptor.description, inputSchema: descriptor.inputSchema }], + handler: async call => { calls++; return { type: "inputText", text: JSON.stringify(searchRunnerApi(call.arguments)) }; }, + }); + const body = JSON.stringify({ jsonrpc: "2.0", id: "source-import-discovery", method: "tools/call", + params: { name: "paperclip_search_api", arguments: { query: "import skill from source SKILL.md", limit: 10 } } }); + try { + const denied = await fetch(bridge.url, { method: "POST", headers: { Authorization: "Bearer wrong-synthetic-bridge-token", "Content-Type": "application/json" }, body }); + expect(denied.status).toBe(401); expect(calls).toBe(0); + await denied.body?.cancel(); + const response = await fetch(bridge.url, { method: "POST", headers: { + Authorization: `Bearer ${bridge.secret}`, "Content-Type": "application/json", Accept: "application/json, text/event-stream", + }, body }); + expect(response.status).toBe(200); + const wire = await response.json() as { result: { isError?: boolean; content: Array<{ type: string; text: string }> } }; + expect(wire.result.isError).not.toBe(true); + const controller = JSON.parse(wire.result.content[0]!.text) as { type: string; text: string }; + expect(controller.type).toBe("inputText"); + expect(Buffer.byteLength(JSON.stringify(controller))).toBeLessThan(500_000); + const discovered = JSON.parse(controller.text) as ReturnType; + const operation = discovered.results.find(row => row.operationId === operationId); + expect(operation).toBeDefined(); + expect(operation!.description).toMatch(/SKILL\.md/i); + expect(operation!.description).toMatch(/local.*director/i); + expect(operation!.description).toMatch(/workspace.*managed/i); + expect(operation!.description).toMatch(/source provenance/i); + expect(operation!.authorization.actor).toBe("board_or_agent"); + expect(operation!.transport).toBe("rest"); + expect(operation!.requestBody?.content["application/json"].schema).toMatchObject({ + type: "object", required: ["source"], properties: { source: { type: "string", minLength: 1 } }, + }); + expect(calls).toBe(1); + } finally { await bridge.close(); } +}); + +it("keeps exact non-import discovery and schema lookup intact", () => { + const id = "DELETE /api/companies/{companyId}/skills/{skillId}"; + const result = searchRunnerApi({ query: id, limit: 10 }); + expect(result.results.map(row => row.operationId)).toEqual([id]); + expect(result.nextCursor).toBeNull(); + expect(result.results[0]!.method).toBe("DELETE"); +}); + +it("keeps foreign-company and read-only-mode import calls denied before dispatch", () => { + const input = { operationId, body: { source: "/owned-company-workspace/example-skill" } }; + expect(() => validateRunnerApiCall({ ...input, pathParams: { companyId: "company-b" } }, context)).toThrow(/another company/); + for (const workMode of ["ask", "planning"]) { + expect(() => validateRunnerApiCall(input, { ...context, workMode })).toThrow(/only reads/); + } +}); + +it("preserves the actual agent credential and source-policy denial returned by the route", async () => { + const request = vi.fn(async () => Response.json({ error: "Skill import policy denied" }, { status: 403 })); + const result = await executeRunnerApi({ operationId, body: { source: "/owned-company-workspace/example-skill" } }, context, + { apiUrl: "https://paperclip.test", token: "synthetic-agent-only", fetch: request, + readFile: async () => { throw new Error("No file read required"); }, + saveResponse: async () => { throw new Error("No capture required"); } }); + expect(result).toMatchObject({ ok: false, status: 403, data: { error: "Skill import policy denied" } }); + expect(request).toHaveBeenCalledOnce(); + const headers = new Headers(request.mock.calls[0]![1]?.headers); + expect(headers.get("Authorization")).toBe("Bearer synthetic-agent-only"); + expect(headers.get("X-Paperclip-Run-Id")).toBe(context.runId); +}); diff --git a/server/src/routes/openapi.ts b/server/src/routes/openapi.ts index f4843102ea3..ccc7bfe0bb5 100644 --- a/server/src/routes/openapi.ts +++ b/server/src/routes/openapi.ts @@ -8799,7 +8799,8 @@ registry.registerPath({ method: "post", path: "/api/companies/{companyId}/skills/import", tags: ["skills"], - summary: "Import a skill", + summary: "Import an existing skill from a source", + description: "Import existing SKILL.md content from an allowed local skill directory or a supported remote source. Retain its source provenance instead of authoring new content with create_skill. Local paths must satisfy the company workspace and managed-root boundary checks. Remote imports keep their source access checks. Importing does not assign the skill to an agent. Company skill policy still applies.", request: { params: z.object({ companyId: z.string() }), body: jsonBody(companySkillImportSchema),