mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-09 16:02:36 +01:00
fix(plugins): align tool run context and interaction input types (#19493)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Plugins add tools and issue-thread interactions through `@paperclipai/plugin-sdk` > - The SDK types are the public contract between the host and plugin authors > - Two parts of that contract do not match what the host does at runtime > - First, `ToolRunContext.projectId` says it is always a project UUID. The tool gateway sends an empty string when a run has no project. The direct tool route rejects such runs with 400 > - Second, three interaction helpers ask for the parsed output type, so plugin authors must pass fields that the host fills in with defaults > - This pull request makes the types and both tool paths agree with the runtime > - The benefit is that agents with no project can run plugin tools, and plugin authors can omit the fields that have defaults ## Linked Issues or Issue Description Fixes #3273 Related: #3274 removes the `projectId` requirement from the route. That change alone is not enough today: the route now also looks up the project to check its company, and that lookup needs a value. This pull request also skips that lookup when the run has no project. It gives plugins one value for "no project" on both tool paths. The new test typecheck in #18739 and #18839 also listed both problems as possible product problems. ## What Changed - `server/src/routes/plugins.ts`: `POST /api/plugins/tools/execute` accepts a `runContext.projectId` that is missing, `null`, or an empty string. It passes an empty string to the plugin, the same value that the tool gateway sends. It checks the project's company only when a project ID is present. A `projectId` that is not a string returns 400. - `server/src/routes/openapi.ts`: the request schema marks `runContext.projectId` as optional and nullable. - `packages/plugins/sdk/src/types.ts`: - The `ToolRunContext.projectId` comment says the value is an empty string when the run has no project. - `suggestTasks`, `requestConfirmation` and `requestCheckboxConfirmation` take `CreateIssueThreadInteractionInput`, as `askUserQuestions` already does. The host parses this input with the schema, so the defaults apply. - `packages/plugins/examples/plugin-kitchen-sink-example/src/worker.ts`: the example passes `undefined` instead of an empty project ID to `issues.create`. - `doc/plugins/PLUGIN_SPEC.md` (section 13.10): documents the empty project ID. - Tests: - `server/src/__tests__/plugin-routes-authz.test.ts`: tests a run with an empty, missing, or `null` project ID, which skips the project lookup. Also tests that a project ID that is not a string returns 400. - `packages/plugins/sdk/tests/testing-actions.test.ts`: removes the cast that the output type required. ## Verification - `pnpm exec vitest run server/src/__tests__/plugin-routes-authz.test.ts server/src/__tests__/openapi-routes.test.ts packages/plugins/sdk/tests/testing-actions.test.ts`: 3 files, 81 tests pass. - `cd server && pnpm typecheck`: passes. This includes the source layers and the three test-tree shards. - `tsc --noEmit` passes for `packages/plugins/sdk` (`tsconfig.json` and `tsconfig.test.json`) and for the kitchen-sink example. - Not run locally: the full `pnpm test:run` and the root `pnpm build`. Required CI covers them. ## Risks - Low risk. The direct tool route now accepts calls that it rejected before. It still checks that the agent and the run belong to the company. With no project ID, there is no project to check. - A plugin that reads `runCtx.projectId` can get an empty string on both tool paths. The tool gateway already sent this value before this change. Only the documentation is new. - The SDK parameter types are wider. Existing plugin code still compiles. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`) in Claude Code, with tool use (shell, file edits) and extended thinking. ## 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 - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [ ] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
7840d894fa
commit
e1332b997f
7 files changed
+126
-23
No files matched your search
@@ -682,6 +682,7 @@ The host provides:
|
||||
- tool name (without plugin namespace prefix)
|
||||
- parsed parameters matching the tool's declared schema
|
||||
- run context: agent ID, run ID, company ID, project ID
|
||||
(the project ID is an empty string when the run has no project)
|
||||
|
||||
The worker executes the tool and returns a typed result (string content, structured data, or error).
|
||||
|
||||
|
||||
@@ -897,7 +897,8 @@ async function registerToolHandlers(ctx: PluginContext): Promise<void> {
|
||||
}
|
||||
const issue = await ctx.issues.create({
|
||||
companyId: runCtx.companyId,
|
||||
projectId: runCtx.projectId,
|
||||
// The run context has an empty project ID when the run has no project.
|
||||
projectId: runCtx.projectId || undefined,
|
||||
title: payload.title,
|
||||
description: payload.description,
|
||||
});
|
||||
|
||||
@@ -36,7 +36,6 @@ import type {
|
||||
AskUserQuestionsInteraction,
|
||||
RequestConfirmationInteraction,
|
||||
RequestCheckboxConfirmationInteraction,
|
||||
CreateIssueThreadInteraction,
|
||||
CreateIssueThreadInteractionInput,
|
||||
PluginIssueOriginKind,
|
||||
IssueSurfaceVisibility,
|
||||
@@ -270,7 +269,10 @@ export interface ToolRunContext {
|
||||
runId: string;
|
||||
/** UUID of the company the run belongs to. */
|
||||
companyId: string;
|
||||
/** UUID of the project the run belongs to. */
|
||||
/**
|
||||
* UUID of the project the run belongs to, or an empty string when the run
|
||||
* has no project. Check the value before you use it as a project ID.
|
||||
*/
|
||||
projectId: string;
|
||||
}
|
||||
|
||||
@@ -1600,7 +1602,7 @@ export interface PluginIssuesClient {
|
||||
): Promise<IssueThreadInteraction>;
|
||||
suggestTasks(
|
||||
issueId: string,
|
||||
interaction: Omit<Extract<CreateIssueThreadInteraction, { kind: "suggest_tasks" }>, "kind">,
|
||||
interaction: Omit<Extract<CreateIssueThreadInteractionInput, { kind: "suggest_tasks" }>, "kind">,
|
||||
companyId: string,
|
||||
options?: { authorAgentId?: string },
|
||||
): Promise<SuggestTasksInteraction>;
|
||||
@@ -1612,13 +1614,13 @@ export interface PluginIssuesClient {
|
||||
): Promise<AskUserQuestionsInteraction>;
|
||||
requestConfirmation(
|
||||
issueId: string,
|
||||
interaction: Omit<Extract<CreateIssueThreadInteraction, { kind: "request_confirmation" }>, "kind">,
|
||||
interaction: Omit<Extract<CreateIssueThreadInteractionInput, { kind: "request_confirmation" }>, "kind">,
|
||||
companyId: string,
|
||||
options?: { authorAgentId?: string },
|
||||
): Promise<RequestConfirmationInteraction>;
|
||||
requestCheckboxConfirmation(
|
||||
issueId: string,
|
||||
interaction: Omit<Extract<CreateIssueThreadInteraction, { kind: "request_checkbox_confirmation" }>, "kind">,
|
||||
interaction: Omit<Extract<CreateIssueThreadInteractionInput, { kind: "request_checkbox_confirmation" }>, "kind">,
|
||||
companyId: string,
|
||||
options?: { authorAgentId?: string },
|
||||
): Promise<RequestCheckboxConfirmationInteraction>;
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
|
||||
import { createTestHarness } from "../src/testing.js";
|
||||
import type { PaperclipPluginManifestV1, PluginIssuesClient } from "../src/types.js";
|
||||
import type { PaperclipPluginManifestV1 } from "../src/types.js";
|
||||
|
||||
const manifest = {
|
||||
id: "paperclip.test-actions",
|
||||
@@ -136,8 +136,8 @@ describe("createTestHarness issue interactions", () => {
|
||||
minSelected: 1,
|
||||
maxSelected: 2,
|
||||
},
|
||||
// Omits the fields the schema defaults, so the assertions below cover those defaults.
|
||||
} as Parameters<PluginIssuesClient["requestCheckboxConfirmation"]>[1],
|
||||
// Omits the fields the schema defaults. The input type allows that.
|
||||
},
|
||||
"company-1",
|
||||
{ authorAgentId: "agent-1" },
|
||||
);
|
||||
|
||||
@@ -673,6 +673,94 @@ describe("plugin tool and bridge authz", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("allows tool execution for a run with no project and skips the project lookup", async () => {
|
||||
const executeTool = vi.fn().mockResolvedValue({ content: "ok" });
|
||||
const { app } = await createApp(boardActor(), {}, {
|
||||
// Only the agent and run lookups. A project lookup would get no row and fail.
|
||||
db: createSelectQueueDb([
|
||||
[{ companyId: companyA }],
|
||||
[{ companyId: companyA, agentId: agentA }],
|
||||
]),
|
||||
toolDeps: {
|
||||
toolDispatcher: {
|
||||
listToolsForAgent: vi.fn(),
|
||||
getTool: vi.fn(() => ({ name: "paperclip.example:search" })),
|
||||
executeTool,
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const res = await request(app)
|
||||
.post("/api/plugins/tools/execute")
|
||||
.send({
|
||||
tool: "paperclip.example:search",
|
||||
parameters: {},
|
||||
runContext: { agentId: agentA, runId: runA, companyId: companyA, projectId: "" },
|
||||
});
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(executeTool).toHaveBeenCalledWith(
|
||||
"paperclip.example:search",
|
||||
{},
|
||||
{ agentId: agentA, runId: runA, companyId: companyA, projectId: "" },
|
||||
);
|
||||
});
|
||||
|
||||
it.each([
|
||||
["missing", {}],
|
||||
["null", { projectId: null }],
|
||||
])("passes an empty projectId to the plugin when runContext.projectId is %s", async (_label, project) => {
|
||||
const executeTool = vi.fn().mockResolvedValue({ content: "ok" });
|
||||
const { app } = await createApp(boardActor(), {}, {
|
||||
db: createSelectQueueDb([
|
||||
[{ companyId: companyA }],
|
||||
[{ companyId: companyA, agentId: agentA }],
|
||||
]),
|
||||
toolDeps: {
|
||||
toolDispatcher: {
|
||||
listToolsForAgent: vi.fn(),
|
||||
getTool: vi.fn(() => ({ name: "paperclip.example:search" })),
|
||||
executeTool,
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const res = await request(app)
|
||||
.post("/api/plugins/tools/execute")
|
||||
.send({
|
||||
tool: "paperclip.example:search",
|
||||
parameters: {},
|
||||
runContext: { agentId: agentA, runId: runA, companyId: companyA, ...project },
|
||||
});
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(executeTool).toHaveBeenCalledWith(
|
||||
"paperclip.example:search",
|
||||
{},
|
||||
{ agentId: agentA, runId: runA, companyId: companyA, projectId: "" },
|
||||
);
|
||||
});
|
||||
|
||||
it("rejects tool execution when runContext.projectId is not a string", async () => {
|
||||
const executeTool = vi.fn();
|
||||
const { app } = await createApp(boardActor(), {}, {
|
||||
toolDeps: {
|
||||
toolDispatcher: { listToolsForAgent: vi.fn(), getTool: vi.fn(), executeTool },
|
||||
},
|
||||
});
|
||||
|
||||
const res = await request(app)
|
||||
.post("/api/plugins/tools/execute")
|
||||
.send({
|
||||
tool: "paperclip.example:search",
|
||||
parameters: {},
|
||||
runContext: { agentId: agentA, runId: runA, companyId: companyA, projectId: 42 },
|
||||
});
|
||||
|
||||
expect(res.status).toBe(400);
|
||||
expect(executeTool).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it.each([
|
||||
["legacy data", "post", `/api/plugins/${pluginId}/bridge/data`, { key: "health" }],
|
||||
["legacy action", "post", `/api/plugins/${pluginId}/bridge/action`, { key: "sync" }],
|
||||
|
||||
@@ -9586,7 +9586,7 @@ registry.registerPath({
|
||||
agentId: z.string(),
|
||||
runId: z.string(),
|
||||
companyId: z.string(),
|
||||
projectId: z.string(),
|
||||
projectId: z.string().nullable().optional(),
|
||||
}),
|
||||
}),
|
||||
),
|
||||
|
||||
@@ -482,8 +482,8 @@ interface PluginToolExecuteRequest {
|
||||
tool: string;
|
||||
/** Parameters matching the tool's declared JSON Schema. */
|
||||
parameters?: unknown;
|
||||
/** Agent run context. */
|
||||
runContext: ToolRunContext;
|
||||
/** Agent run context. projectId may be omitted or null when the run has no project. */
|
||||
runContext: Omit<ToolRunContext, "projectId"> & { projectId?: string | null };
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -862,13 +862,15 @@ export function pluginRoutes(
|
||||
return '"runContext.runId" does not belong to "runContext.agentId"';
|
||||
}
|
||||
|
||||
const [project] = await db
|
||||
.select({ companyId: projects.companyId })
|
||||
.from(projects)
|
||||
.where(eq(projects.id, runContext.projectId))
|
||||
.limit(1);
|
||||
if (!project || project.companyId !== runContext.companyId) {
|
||||
return '"runContext.projectId" does not belong to "runContext.companyId"';
|
||||
if (runContext.projectId) {
|
||||
const [project] = await db
|
||||
.select({ companyId: projects.companyId })
|
||||
.from(projects)
|
||||
.where(eq(projects.id, runContext.projectId))
|
||||
.limit(1);
|
||||
if (!project || project.companyId !== runContext.companyId) {
|
||||
return '"runContext.projectId" does not belong to "runContext.companyId"';
|
||||
}
|
||||
}
|
||||
|
||||
return null;
|
||||
@@ -1060,7 +1062,7 @@ export function pluginRoutes(
|
||||
return;
|
||||
}
|
||||
|
||||
const { tool, parameters, runContext } = body;
|
||||
const { tool, parameters, runContext: requestedRunContext } = body;
|
||||
|
||||
// Validate required fields
|
||||
if (!tool || typeof tool !== "string") {
|
||||
@@ -1068,18 +1070,27 @@ export function pluginRoutes(
|
||||
return;
|
||||
}
|
||||
|
||||
if (!runContext || typeof runContext !== "object") {
|
||||
if (!requestedRunContext || typeof requestedRunContext !== "object") {
|
||||
res.status(400).json({ error: '"runContext" is required and must be an object' });
|
||||
return;
|
||||
}
|
||||
|
||||
if (!runContext.agentId || !runContext.runId || !runContext.companyId || !runContext.projectId) {
|
||||
if (!requestedRunContext.agentId || !requestedRunContext.runId || !requestedRunContext.companyId) {
|
||||
res.status(400).json({
|
||||
error: '"runContext" must include agentId, runId, companyId, and projectId',
|
||||
error: '"runContext" must include agentId, runId, and companyId',
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
const requestedProjectId: unknown = requestedRunContext.projectId;
|
||||
if (requestedProjectId != null && typeof requestedProjectId !== "string") {
|
||||
res.status(400).json({ error: '"runContext.projectId" must be a string when present' });
|
||||
return;
|
||||
}
|
||||
// A run can have no project. Plugins then get an empty projectId, the same
|
||||
// value the tool gateway sends, so both paths give plugins one contract.
|
||||
const runContext: ToolRunContext = { ...requestedRunContext, projectId: requestedProjectId ?? "" };
|
||||
|
||||
assertCompanyAccess(req, runContext.companyId);
|
||||
const scopeError = await validateToolRunContextScope(runContext);
|
||||
if (scopeError) {
|
||||
|
||||
Reference in new issue
Block a user