From 4b40ad106bdc269da8566cbfab32e7584b432833 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Thu, 22 Oct 2026 23:30:03 -0700 Subject: [PATCH] feat(mcp): commonly_update_task links a PR via optional prUrl (TASK-326) MCP could only append a note to a task, so a seat could not link its PR to the row. Adds an optional prUrl: when present, it is normalised to the canonical https://github.com///pull/ form (trailing slash, /files, query and fragment dropped) and PATCHed onto the row, then the note is appended. Any other URL is refused with the value it was given, before any write, so a refused link leaves the row and its history untouched. Absent prUrl: the wire call is unchanged (one POST to /updates). Tests: absent (one call), set (PATCH canonical then note), refused non-PR path and refused other host (no fetch). Mutation: dropping the PATCH reddens the set test; skipping normalisation reddens the refusal tests. Version 0.3.14 -> 0.3.15 (npm latest was 0.3.14). Co-Authored-By: Claude Haiku 5.5 Co-authored-by: swift Agent-Model: claude/claude-haiku-5-5/default --- commonly-mcp/__tests__/tools.test.mjs | 56 +++++++++++++++++++++++++++ commonly-mcp/package-lock.json | 4 +- commonly-mcp/package.json | 2 +- commonly-mcp/server.json | 4 +- commonly-mcp/src/tools.js | 32 ++++++++++++--- 5 files changed, 87 insertions(+), 11 deletions(-) diff --git a/commonly-mcp/__tests__/tools.test.mjs b/commonly-mcp/__tests__/tools.test.mjs index e749d6ed7d..091319c706 100644 --- a/commonly-mcp/__tests__/tools.test.mjs +++ b/commonly-mcp/__tests__/tools.test.mjs @@ -288,6 +288,62 @@ describe('commonly_get_tasks / create / claim / complete / update', () => { expect(url).toBe('https://x.example/api/v1/tasks/POD/TASK-001/updates'); expect(JSON.parse(init.body)).toEqual({ text: 'still working' }); }); + + it('update_task with no prUrl makes only the note call, as before', async () => { + const fetchSpy = installFetch(async () => okResponse({ ok: true })); + await byName.commonly_update_task.call({ + podId: 'POD', taskId: 'TASK-001', text: 'still working', + }); + expect(fetchSpy).toHaveBeenCalledTimes(1); + const [url, init] = fetchSpy.mock.calls[0]; + expect(init.method).toBe('POST'); + expect(url).toMatch(/\/updates$/); + }); + + it('update_task with prUrl PATCHes the canonical PR URL, then appends the note', async () => { + const fetchSpy = installFetch(async () => okResponse({ ok: true })); + await byName.commonly_update_task.call({ + podId: 'POD', + taskId: 'TASK-001', + text: 'opened the PR', + prUrl: 'https://github.com/Team-Commonly/commonly/pull/2471/files?diff=split#top', + }); + expect(fetchSpy).toHaveBeenCalledTimes(2); + const [patchUrl, patchInit] = fetchSpy.mock.calls[0]; + expect(patchInit.method).toBe('PATCH'); + expect(patchUrl).toBe('https://x.example/api/v1/tasks/POD/TASK-001'); + expect(JSON.parse(patchInit.body)).toEqual({ + prUrl: 'https://github.com/Team-Commonly/commonly/pull/2471', + }); + const [noteUrl, noteInit] = fetchSpy.mock.calls[1]; + expect(noteInit.method).toBe('POST'); + expect(noteUrl).toBe('https://x.example/api/v1/tasks/POD/TASK-001/updates'); + expect(JSON.parse(noteInit.body)).toEqual({ text: 'opened the PR' }); + }); + + it('update_task refuses a non-PR URL before writing anything', async () => { + const fetchSpy = installFetch(async () => okResponse({ ok: true })); + const result = await byName.commonly_update_task.call({ + podId: 'POD', + taskId: 'TASK-001', + text: 'linking', + prUrl: 'https://github.com/Team-Commonly/commonly/issues/2471', + }); + expect(result.isError).toBe(true); + expect(JSON.parse(result.content[0].text).message).toMatch( + /prUrl must be a GitHub pull request URL .*got 'https:\/\/github\.com\/Team-Commonly\/commonly\/issues\/2471'/, + ); + expect(fetchSpy).not.toHaveBeenCalled(); + }); + + it('update_task refuses a PR URL on another host', async () => { + const fetchSpy = installFetch(async () => okResponse({ ok: true })); + const result = await byName.commonly_update_task.call({ + podId: 'POD', taskId: 'TASK-001', text: 'linking', prUrl: 'https://gitlab.com/o/r/pull/3', + }); + expect(result.isError).toBe(true); + expect(fetchSpy).not.toHaveBeenCalled(); + }); }); describe('commonly_create_pod', () => { diff --git a/commonly-mcp/package-lock.json b/commonly-mcp/package-lock.json index fcaf605055..5450019940 100644 --- a/commonly-mcp/package-lock.json +++ b/commonly-mcp/package-lock.json @@ -1,12 +1,12 @@ { "name": "@commonlyai/mcp", - "version": "0.3.14", + "version": "0.3.15", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@commonlyai/mcp", - "version": "0.3.14", + "version": "0.3.15", "license": "Apache-2.0", "dependencies": { "@modelcontextprotocol/sdk": "^1.0.4" diff --git a/commonly-mcp/package.json b/commonly-mcp/package.json index 51da041108..dac64b8697 100644 --- a/commonly-mcp/package.json +++ b/commonly-mcp/package.json @@ -1,7 +1,7 @@ { "name": "@commonlyai/mcp", "mcpName": "io.github.Team-Commonly/commonly", - "version": "0.3.14", + "version": "0.3.15", "license": "Apache-2.0", "repository": { "type": "git", diff --git a/commonly-mcp/server.json b/commonly-mcp/server.json index bd0ab14c9c..1d9b34c9a3 100644 --- a/commonly-mcp/server.json +++ b/commonly-mcp/server.json @@ -9,12 +9,12 @@ "source": "github", "subfolder": "commonly-mcp" }, - "version": "0.3.14", + "version": "0.3.15", "packages": [ { "registryType": "npm", "identifier": "@commonlyai/mcp", - "version": "0.3.14", + "version": "0.3.15", "transport": { "type": "stdio" }, diff --git a/commonly-mcp/src/tools.js b/commonly-mcp/src/tools.js index a364432008..9011f66fbc 100644 --- a/commonly-mcp/src/tools.js +++ b/commonly-mcp/src/tools.js @@ -43,6 +43,20 @@ const ok = (value) => ({ // Convert an HttpError into the MCP `isError: true` shape. The status code // and verbatim backend message are surfaced — Invariant #6. +// A task's prUrl is a GitHub pull request, stored canonically so the board and +// the drawer link to the same place. Trailing slash, /files, /commits, query and +// fragment are accepted and dropped; any other host or path is refused with the +// value it was given. +const GITHUB_PR_URL = /^https:\/\/github\.com\/([A-Za-z0-9_.-]+)\/([A-Za-z0-9_.-]+)\/pull\/(\d+)(?:[/?#].*)?$/; +export const normalizePrUrl = (raw) => { + const value = String(raw ?? '').trim(); + const m = GITHUB_PR_URL.exec(value); + if (!m) { + throw new Error(`prUrl must be a GitHub pull request URL like https://github.com///pull/ (got '${value}')`); + } + return `https://github.com/${m[1]}/${m[2]}/pull/${m[3]}`; +}; + const err = (error) => { const payload = error instanceof HttpError ? { status: error.status, body: error.body, message: error.message } @@ -380,17 +394,23 @@ export const buildTools = (config) => { }, { name: 'commonly_update_task', - description: 'Append an update note to a task without changing status — visible in the task drawer history.', + description: 'Append an update note to a task without changing status — visible in the task drawer history. Pass prUrl to also link the task to its GitHub pull request (https://github.com///pull/); other URLs are refused before anything is written.', inputSchema: reqWith({ podId: STRING, taskId: STRING, text: STRING, + prUrl: STRING, }, ['podId', 'taskId', 'text']), - call: wrap(async ({ podId, taskId, text }) => request(config, { - method: 'POST', - path: `/api/v1/tasks/${encodeURIComponent(podId)}/${encodeURIComponent(taskId)}/updates`, - body: { text }, - })), + call: wrap(async ({ podId, taskId, text, prUrl }) => { + const rowPath = `/api/v1/tasks/${encodeURIComponent(podId)}/${encodeURIComponent(taskId)}`; + // Validate before the PATCH: a refused link must leave the row untouched, + // including its note, rather than half-applying. + const canonicalPrUrl = prUrl != null ? normalizePrUrl(prUrl) : null; + if (canonicalPrUrl) { + await request(config, { method: 'PATCH', path: rowPath, body: { prUrl: canonicalPrUrl } }); + } + return request(config, { method: 'POST', path: `${rowPath}/updates`, body: { text } }); + }), }, { name: 'commonly_create_pod',