From b47210302cb39403da23285d803d4157c91fd99e Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Thu, 22 Oct 2026 03:36:34 -0700 Subject: [PATCH] feat(mcp): link a PR when updating a task MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Accept an optional normalized GitHub pull request URL, save it before the progress note, and preserve continueAnyway on the note request. Update the capability documentation. Publish assigns package versions. Written by Sprint Impl, a Commonly agent — https://commonly.me/pods/6a692a1be833c668acdb84cf Co-authored-by: sprint-impl Agent-Model: codex/gpt-6.1-sol/medium --- CLAUDE.md | 2 +- commonly-mcp/__tests__/tools.test.mjs | 60 +++++++++++++++++++++++++++ commonly-mcp/src/tools.js | 28 ++++++++++--- 3 files changed, 83 insertions(+), 7 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index ae4a7f0657..2978e242aa 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -535,7 +535,7 @@ Rule (unchanged): any new social-presence primitive (typing-indicator, read-rece | runtime | `commonly_update_task` does | title? | |---|---|---| - | MCP | *"Append an update note to a task **without changing status**"* → `POST /api/v1/tasks/:podId/:taskId/updates`, params `{podId, taskId, text}` | no — and **no MCP tool wraps a PATCH route at all** | + | MCP | *"Append an update note to a task **without changing status**"* → `POST /api/v1/tasks/:podId/:taskId/updates`, params `{podId, taskId, text, priority?, prUrl?, continueAnyway?}`; optional `prUrl` first PATCHes the task URL | no — the PATCH wrapper exposes `prUrl` only, not `title` | | openclaw extension | *"Patch task fields: assignee, status, dep, prUrl, notes, **title**… For progress notes use `commonly_add_task_update` instead"* → `PATCH /api/v1/tasks/:podId/:taskId` | yes | The backend is not the constraint. `PATCH /:podId/:taskId` lists `title` in its `allowed` array and is gated by `auth` + `requirePodMember(podId, userId, { write: true })` — **the same pair guarding the note-append route beside it**, which every seat hits on every lease renewal. So the authority is identical and only the tool surface differs. diff --git a/commonly-mcp/__tests__/tools.test.mjs b/commonly-mcp/__tests__/tools.test.mjs index 03bb94a12b..4caf2cc176 100644 --- a/commonly-mcp/__tests__/tools.test.mjs +++ b/commonly-mcp/__tests__/tools.test.mjs @@ -357,6 +357,66 @@ 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 saves a normalized PR URL before appending the note', async () => { + const fetchSpy = installFetch(async () => okResponse({ ok: true })); + await byName.commonly_update_task.call({ + podId: 'POD', taskId: 'TASK-001', text: 'PR opened', + prUrl: 'https://github.com/Team-Commonly/commonly/pull/2399?tab=files#diff', + }); + expect(fetchSpy).toHaveBeenCalledTimes(2); + const [[patchUrl, patchInit], [noteUrl, noteInit]] = fetchSpy.mock.calls; + expect(patchUrl).toBe('https://x.example/api/v1/tasks/POD/TASK-001'); + expect(patchInit.method).toBe('PATCH'); + expect(JSON.parse(patchInit.body)).toEqual({ + prUrl: 'https://github.com/Team-Commonly/commonly/pull/2399', + }); + expect(noteUrl).toBe('https://x.example/api/v1/tasks/POD/TASK-001/updates'); + expect(noteInit.method).toBe('POST'); + expect(JSON.parse(noteInit.body)).toEqual({ text: 'PR opened' }); + }); + + it.each([ + 'https://example.com/Team-Commonly/commonly/pull/2399', + 'https://github.com/Team-Commonly/commonly/issues/2399', + 'https://github.com/Team-Commonly/commonly/pull/2399/extra', + 'https://github.com/../commonly/pull/2399', + 'https://github.com/Team-Commonly/commonly/pull/0', + 'https://github.com/Team-Commonly/commonly/pull/2399/', + 123, + ])('update_task refuses a non-PR URL before any request: %s', async (prUrl) => { + const fetchSpy = installFetch(async () => okResponse({ ok: true })); + const result = await byName.commonly_update_task.call({ + podId: 'POD', taskId: 'TASK-001', text: 'PR opened', prUrl, + }); + expect(result.isError).toBe(true); + expect(result.content[0].text).toContain('prUrl must be https://github.com/'); + expect(fetchSpy).not.toHaveBeenCalled(); + }); + + it('preserves priority and the freshness override when saving a PR URL and a note', async () => { + const fetchSpy = installFetch(async () => okResponse({ ok: true })); + await byName.commonly_update_task.call({ + podId: 'POD', taskId: 'TASK-001', text: 'PR opened', priority: 'P1', continueAnyway: true, + prUrl: 'https://github.com/Team-Commonly/commonly/pull/2399', + }); + expect(fetchSpy).toHaveBeenCalledTimes(2); + expect(JSON.parse(fetchSpy.mock.calls[1][1].body)).toEqual({ text: 'PR opened', priority: 'P1', continueAnyway: true }); + }); + + it('does not append a note when the PR URL PATCH is refused', async () => { + const fetchSpy = installFetch(async () => ({ + ok: false, status: 409, text: async () => JSON.stringify({ error: 'Task is leased by another agent' }), + })); + const result = await byName.commonly_update_task.call({ + podId: 'POD', taskId: 'TASK-001', text: 'PR opened', + prUrl: 'https://github.com/Team-Commonly/commonly/pull/2399', + }); + expect(result.isError).toBe(true); + expect(result.content[0].text).toContain('409'); + expect(fetchSpy).toHaveBeenCalledTimes(1); + expect(fetchSpy.mock.calls[0][1].method).toBe('PATCH'); + }); }); describe('commonly_create_pod', () => { diff --git a/commonly-mcp/src/tools.js b/commonly-mcp/src/tools.js index bf390f9a44..764c25c713 100644 --- a/commonly-mcp/src/tools.js +++ b/commonly-mcp/src/tools.js @@ -58,6 +58,14 @@ const reqWith = (props, requiredKeys) => ({ ...required(props), required: requir const STRING = { type: 'string' }; const INT = { type: 'integer' }; +const normalizePullRequestUrl = (value) => { + const url = typeof value === 'string' ? value.split(/[?#]/, 1)[0] : ''; + const match = /^https:\/\/github\.com\/[A-Za-z0-9_.-]+\/[A-Za-z0-9_.-]+\/pull\/[1-9]\d*$/.exec(url); + if (!match || match[0] !== url || new URL(url).href !== url) { + throw new Error('prUrl must be https://github.com///pull/'); + } + return url; +}; const CLAIM_OUTCOME = { type: 'string', enum: ['declined', 'completed'] }; const DECISION_CLASS = { type: 'string', enum: ['strategy', 'implementation', 'prioritization'] }; const DECISION_OPTION = { @@ -409,19 +417,27 @@ export const buildTools = (config) => { }, { name: 'commonly_update_task', - description: 'Append an update note and optionally set priority (P0 highest through P3), without changing status — visible in the task drawer history. If a 409 reports unseen messages, read them and retry, or pass continueAnyway:true to update despite them.', + description: 'Append an update note and optionally set priority (P0 highest through P3), without changing status — visible in the task drawer history. Optionally set prUrl to a GitHub pull request URL before appending the note. If a 409 reports unseen messages, read them and retry, or pass continueAnyway:true to update despite them.', inputSchema: reqWith({ podId: STRING, taskId: STRING, text: STRING, priority: { type: 'string', enum: ['P0', 'P1', 'P2', 'P3'] }, + prUrl: STRING, continueAnyway: { type: 'boolean' }, }, ['podId', 'taskId', 'text']), - call: wrap(async ({ podId, taskId, text, priority, continueAnyway }) => request(config, { - method: 'POST', - path: `/api/v1/tasks/${encodeURIComponent(podId)}/${encodeURIComponent(taskId)}/updates`, - body: { text, priority, continueAnyway }, - })), + call: wrap(async ({ podId, taskId, text, priority, prUrl, continueAnyway }) => { + const taskPath = `/api/v1/tasks/${encodeURIComponent(podId)}/${encodeURIComponent(taskId)}`; + if (prUrl !== undefined) { + const normalized = normalizePullRequestUrl(prUrl); + await request(config, { method: 'PATCH', path: taskPath, body: { prUrl: normalized } }); + } + return request(config, { + method: 'POST', + path: `${taskPath}/updates`, + body: { text, priority, continueAnyway }, + }); + }), }, { name: 'commonly_create_pod',