diff --git a/mcp/packages/plugin/src/ConnectionMetadata.test.ts b/mcp/packages/plugin/src/ConnectionMetadata.test.ts index 1ae436afd6..f0f26f405d 100644 --- a/mcp/packages/plugin/src/ConnectionMetadata.test.ts +++ b/mcp/packages/plugin/src/ConnectionMetadata.test.ts @@ -4,9 +4,11 @@ import { readFileSync } from "node:fs"; import test from "node:test"; import { runInNewContext } from "node:vm"; import ts from "typescript"; +import * as ErrorUtils from "./ErrorUtils.ts"; import { shouldReconnectAfterClose } from "./ReconnectPolicy.ts"; import { SessionId } from "./SessionId.ts"; import { SessionIdDisplay } from "./SessionIdDisplay.ts"; +import * as VersionCompatibility from "./VersionCompatibility.ts"; /** Compiles the actual plugin entry point for a simulated browser boundary. */ function compile(name: string) { @@ -15,6 +17,22 @@ function compile(name: string) { }).outputText; } +/** Exports of a compiled module that Node's type stripping cannot load directly. */ +function compiledModule(name: string) { + const exports = {}; + runInNewContext(compile(name), { exports }); + return exports; +} + +/** Resolves the imports of `plugin.ts`, stubbing only the code execution handler. */ +function pluginRequire(name: string) { + if (name === "./task-handlers/ExecuteCodeTaskHandler") return { ExecuteCodeTaskHandler: class {} }; + if (name === "./TaskHandler") return compiledModule("TaskHandler.ts"); + if (name === "./ErrorUtils") return ErrorUtils; + if (name === "./VersionCompatibility") return VersionCompatibility; + throw new Error(`Unexpected import: ${name}`); +} + function pluginUi() { const messages: any[] = []; const sockets: FakeSocket[] = []; @@ -184,7 +202,7 @@ test("plugin reads the current file for each connection metadata request", () => on() {}, }, mcp: undefined, - require: () => ({ ExecuteCodeTaskHandler: class {} }), + require: pluginRequire, }); onMessage({ type: "connection-metadata-request", sessionId: "first" }); penpot.currentFile = { id: "file-2", name: "Second design" }; @@ -223,7 +241,7 @@ test("plugin without penpotMgmt derives metadata from the open file", () => { exports: {}, penpot, mcp: undefined, - require: () => ({ ExecuteCodeTaskHandler: class {} }), + require: pluginRequire, }); onMessage({ type: "connection-metadata-request", sessionId: "first" }); penpot.currentFile = null; @@ -313,7 +331,7 @@ test("integrated plugin stays idle until explicitly connected, including while t setMcpStatus: (status: string) => statuses.push(status), on: (event: string, callback: () => void) => events.set(event, callback), }, - require: () => ({ ExecuteCodeTaskHandler: class {} }), + require: pluginRequire, }); assert.ok(!statuses.includes("connecting")); events.get("connect")!(); diff --git a/mcp/packages/server/src/Feedback.test.ts b/mcp/packages/server/src/Feedback.test.ts index b5b6460250..be92aac8ea 100644 --- a/mcp/packages/server/src/Feedback.test.ts +++ b/mcp/packages/server/src/Feedback.test.ts @@ -70,6 +70,21 @@ async function connect(multiUser: boolean, token?: string) { const feedback = { subject: "Export fails", feedback: "Expected a PNG, received an error." }; +/** + * Collects all `pattern` values in a JSON schema. + */ +function schemaPatterns(schema: unknown): string[] { + if (Array.isArray(schema)) { + return schema.flatMap(schemaPatterns); + } + if (schema === null || typeof schema !== "object") { + return []; + } + return Object.entries(schema).flatMap(([key, value]) => + key === "pattern" && typeof value === "string" ? [value] : schemaPatterns(value) + ); +} + for (const multiUser of [false, true]) { test(`feedback is absent without a backend URI (multi-user=${multiUser})`, async () => { const connected = await connect(multiUser, "alice"); @@ -82,6 +97,10 @@ for (const multiUser of [false, true]) { const tool = (await connected.listTools()).tools.find((tool) => tool.name === "submit_feedback"); assert.ok(tool); assert.deepEqual(tool.inputSchema.required, ["subject", "feedback"]); + // advertised patterns must also compile outside ECMAScript (ChatGPT uses Python) + const patterns = schemaPatterns(tool.inputSchema); + assert.ok(patterns.length > 0); + assert.ok(!patterns.some((pattern) => /\\[pP]\{/.test(pattern)), patterns.join(", ")); const result = await connected.callTool({ name: "submit_feedback", arguments: feedback }); assert.deepEqual(result.content, [ { type: "text", text: "Feedback queued. It is emailed in the background, so delivery is not confirmed." }, diff --git a/mcp/packages/server/src/tools/SubmitFeedbackTool.ts b/mcp/packages/server/src/tools/SubmitFeedbackTool.ts index 4c7b92f952..57273991fb 100644 --- a/mcp/packages/server/src/tools/SubmitFeedbackTool.ts +++ b/mcp/packages/server/src/tools/SubmitFeedbackTool.ts @@ -12,7 +12,9 @@ export class SubmitFeedbackArgs implements Feedback { .min(1) .max(120) .regex(/\S/) - .regex(/^\P{Cc}*$/u) + // the Unicode Cc category; `\P{Cc}` is avoided because clients that compile + // advertised patterns with other regex engines (e.g. Python) reject it + .regex(/^[^\x00-\x1F\x7F-\x9F]*$/) .describe("Short, specific summary without line breaks or other control characters (max 120 characters)."), feedback: z .string() diff --git a/plugins/apps/plugin-api-test-suite/README.md b/plugins/apps/plugin-api-test-suite/README.md index a04657cfff..2504756974 100644 --- a/plugins/apps/plugin-api-test-suite/README.md +++ b/plugins/apps/plugin-api-test-suite/README.md @@ -247,13 +247,17 @@ with `name`/`pinned`/`shared` properties and `open`, `moveTo`, `duplicate` and `remove` methods. `management.test.ts` shows the patterns: - Put every team and project a test creates under a name from - `temporaryName()` (`src/framework/temporary.ts`). Tests delete the teams they - create; the live CI driver deletes the teams and projects with that prefix - after the run. Runs from the plugin UI leave the projects behind. The tests - need nothing set up beforehand. -- Assigning `name`, `pinned` or `shared` updates the object at once and saves - in the background. To check the saved result, list again inside - `waitForAsync()` (`src/tests/wait.ts`). + `temporaryName()` (`src/framework/temporary.ts`). The API cannot delete + teams, so the live CI driver deletes the teams and projects with that prefix + after the run. Runs from the plugin UI leave them behind. The tests need + nothing set up beforehand. +- Assigning `name`, `pinned` or `shared` saves to the backend first and updates + the object only when the save succeeds. Wait for the new value with + `waitFor()` (`src/tests/wait.ts`) before reading it; to check what the + backend stored, list again inside `waitForAsync()`. +- `Project.fileCount` is the count when the project was listed; creating or + moving files does not change it. List the project again to see the new + count. - Shape, page, file and library objects taken before `file.open()` (or `openFile`) are stale afterwards; look them up again in the new file. `ctx.board` belongs to the file the test started in, so only use it there. diff --git a/plugins/apps/plugin-api-test-suite/src/tests/management.test.ts b/plugins/apps/plugin-api-test-suite/src/tests/management.test.ts index 1c451f5ac1..67e96d83b6 100644 --- a/plugins/apps/plugin-api-test-suite/src/tests/management.test.ts +++ b/plugins/apps/plugin-api-test-suite/src/tests/management.test.ts @@ -193,6 +193,7 @@ describe('Management', () => { try { team.name = renamed; + await waitFor(() => team.name === renamed, { timeout: 5000 }); expect(team.name).toBe(renamed); await waitForAsync( async () => (await currentTeam(ctx)).name === renamed, @@ -225,7 +226,9 @@ describe('Management', () => { expect(file.projectId).toBe(created.id); expect(file.teamId).toBe(team.id); expect(file.shared).toBe(false); - expect(created.fileCount).toBe(1); + // fileCount is a listing snapshot, so list the project again. + expect(created.fileCount).toBe(0); + expect((await listedProject(ctx, created.id))?.fileCount).toBe(1); expect((await created.listFiles()).map((f) => f.id)).toEqual([file.id]); // Creating does not navigate. expect(ctx.penpotMgmt.workspace).toEqual(home); @@ -240,6 +243,9 @@ describe('Management', () => { created.name = renamed; created.pinned = true; + await waitFor(() => created.name === renamed && created.pinned, { + timeout: 5000, + }); expect(created.name).toBe(renamed); expect(created.pinned).toBe(true); @@ -282,20 +288,18 @@ describe('Management', () => { name: temporaryName('team'), }); - try { - const moved = await team.createProject({ - name: temporaryName('moved'), - }); - await moved.moveTo(target); + // Teams cannot be deleted through the API; the live CI driver deletes + // the temporary team after the run. + const moved = await team.createProject({ + name: temporaryName('moved'), + }); + await moved.moveTo(target); - expect(moved.teamId).toBe(target.id); - expect((await target.listProjects()).map((p) => p.id)).toContain( - moved.id, - ); - expect(await listedProject(ctx, moved.id)).toBeUndefined(); - } finally { - await target.remove(); - } + expect(moved.teamId).toBe(target.id); + expect((await target.listProjects()).map((p) => p.id)).toContain( + moved.id, + ); + expect(await listedProject(ctx, moved.id)).toBeUndefined(); }); }); @@ -305,6 +309,7 @@ describe('Management', () => { const parent = await temporaryProject(ctx); file.name = ' Renamed '; + await waitFor(() => file.name === 'Renamed', { timeout: 5000 }); expect(file.name).toBe('Renamed'); await waitForAsync( async () => (await listedFile(parent, file.id))?.name === 'Renamed', @@ -361,6 +366,9 @@ describe('Management', () => { expect(ctx.penpotMgmt.workspace.fileName).toBe(`${original} (renamed)`); } finally { file.name = original; + await waitFor(() => ctx.penpotMgmt.workspace.fileName === original, { + timeout: 5000, + }); } }); }); @@ -460,6 +468,7 @@ describe('Management', () => { ); file.shared = true; + await waitFor(() => file.shared, { timeout: 5000 }); expect(file.shared).toBe(true); await waitForAsync(isListed); expect(await isListed()).toBe(true);