mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-04 11:01:16 +01:00
fix(api): surface existing skill source import in discovery (#18757)
## 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 <noreply@paperclip.ing>
This commit is contained in:
1 parent
ef5b98594f
commit
dc9f387edc
4 files changed
+91
-1
No files matched your search
@@ -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.
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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<typeof searchRunnerApi>;
|
||||
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<typeof fetch>(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);
|
||||
});
|
||||
@@ -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),
|
||||
|
||||
Reference in new issue
Block a user