From 51533cf48e828099a77222dd863daae2df1f2d43 Mon Sep 17 00:00:00 2001 From: Madhura68 Date: Sat, 9 May 2026 14:05:59 +0200 Subject: [PATCH 1/8] fix(attachWorktreeToJob): schrijf branch naar claudeJob.branch in DB MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Symptoom: TASK_IMPLEMENTATION jobs in een sprint-run met pr_strategy= SPRINT kregen branch=null in claudeJob.branch, ook al maakte attachWorktreeToJob de juiste worktree-branch (feat/sprint-) aan en returnde die in de payload-response. Gevolg: update_job_status (na PR #43-fix) leest claudeJob.branch uit de DB → null → valt terug op legacy `feat/job-<8>` → `git push` faalt met "src refspec feat/job-xxx does not match any" → job FAILED → cascade- cancel van sibling-tasks in dezelfde sprint-run. Live waargenomen voor sprint-run cmoy9irr8000ci017fvy30lvv (T-806 FAILED, T-807-T-811 CANCELLED) ondanks dat Claude PR #174 op feat/sprint-fvy30lvv had gemaakt. Root cause: attachWorktreeToJob (wait-for-job.ts:205-209) update'de alleen base_sha. Voor SPRINT_IMPLEMENTATION-kind wordt branch wel naar DB geschreven (regel 655) maar voor TASK_IMPLEMENTATION-pad zat dat gat. Fix: altijd branch + (indien aanwezig) base_sha schrijven naar claudeJob in de update aan het eind van attachWorktreeToJob. Tests: __tests__/wait-for-job-worktree.test.ts mock-prisma uitgebreid met `claudeJob.update`. 341 tests in 38 files passed. Co-Authored-By: Claude Opus 4.7 (1M context) --- __tests__/wait-for-job-worktree.test.ts | 4 ++-- src/tools/wait-for-job.ts | 18 ++++++++++++------ 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/__tests__/wait-for-job-worktree.test.ts b/__tests__/wait-for-job-worktree.test.ts index c03e91d..d36e08f 100644 --- a/__tests__/wait-for-job-worktree.test.ts +++ b/__tests__/wait-for-job-worktree.test.ts @@ -6,7 +6,7 @@ import * as fs from 'node:fs/promises' vi.mock('../src/prisma.js', () => ({ prisma: { $executeRaw: vi.fn(), - claudeJob: { findFirst: vi.fn(), findUnique: vi.fn() }, + claudeJob: { findFirst: vi.fn(), findUnique: vi.fn(), update: vi.fn() }, product: { findUnique: vi.fn() }, }, })) @@ -21,7 +21,7 @@ import { resolveRepoRoot, rollbackClaim, attachWorktreeToJob } from '../src/tool const mockPrisma = prisma as unknown as { $executeRaw: ReturnType - claudeJob: { findFirst: ReturnType; findUnique: ReturnType } + claudeJob: { findFirst: ReturnType; findUnique: ReturnType; update: ReturnType } product: { findUnique: ReturnType } } const mockCreateWorktree = createWorktreeForJob as ReturnType diff --git a/src/tools/wait-for-job.ts b/src/tools/wait-for-job.ts index c8af6f4..96c11ba 100644 --- a/src/tools/wait-for-job.ts +++ b/src/tools/wait-for-job.ts @@ -202,12 +202,18 @@ export async function attachWorktreeToJob( } catch (err) { console.warn(`[attachWorktreeToJob] failed to resolve base_sha for ${jobId}:`, err) } - if (baseSha) { - await prisma.claudeJob.update({ - where: { id: jobId }, - data: { base_sha: baseSha }, - }) - } + // Persist branch + base_sha. update_job_status (prepareDoneUpdate) + // leest claudeJob.branch om naar de juiste ref te pushen — zonder deze + // update valt 'ie terug op het legacy `feat/job-<8>` patroon en faalt + // de push met "src refspec ... does not match any" voor sprint/story + // strategy branches. + await prisma.claudeJob.update({ + where: { id: jobId }, + data: { + branch: actualBranch, + ...(baseSha ? { base_sha: baseSha } : {}), + }, + }) return { worktree_path: worktreePath, branch_name: actualBranch, reused_branch: reused } } catch (err) { From da1fe415c479bfe7ef8ff7a998b8d5d1e6b14f75 Mon Sep 17 00:00:00 2001 From: Janpeter Visser <30029041+madhura68@users.noreply.github.com> Date: Sat, 9 May 2026 16:29:41 +0200 Subject: [PATCH 2/8] fix(cleanup): keepBranch + sprint-scope siblings voor SPRINT pr_strategy (#45) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Symptoom: in een sprint met pr_strategy=SPRINT (5 tasks, 3 stories) werden de eerste twee tasks SKIPPED door Claude (werk al in main na een externe PR). De derde task crashte op: git worktree add /home/agent/.scrum4me-agent-worktrees/ feat/sprint-uhrbtc8z fatal: invalid reference: feat/sprint-uhrbtc8z Root cause: cleanupWorktreeForTerminalStatus checkte op active siblings binnen dezelfde **story** + verwijderde de branch bij keepBranch=false. Voor SPRINT pr_strategy delen alle stories in de sprint één branch (feat/sprint-). Eerste task SKIPPED, story ST-1304 had geen actieve siblings meer (T-807 was ook al SKIPPED), branch werd verwijderd. T-808 in story ST-1305 wilde reuse'n maar branch bestond niet meer. Fix: 1. Sibling-check verbreden voor SPRINT pr_strategy: kijk naar alle actieve jobs in dezelfde sprint_run_id (niet alleen story_id). 2. keepBranch=true voor SKIPPED bij SPRINT pr_strategy: andere stories in dezelfde sprint hebben de branch nog nodig. Tests: 341 passed (38 files). Typecheck OK. Co-authored-by: Madhura68 Co-authored-by: Claude Opus 4.7 (1M context) --- src/tools/update-job-status.ts | 54 +++++++++++++++++++++++++--------- 1 file changed, 40 insertions(+), 14 deletions(-) diff --git a/src/tools/update-job-status.ts b/src/tools/update-job-status.ts index 6b4680f..5e40988 100644 --- a/src/tools/update-job-status.ts +++ b/src/tools/update-job-status.ts @@ -71,31 +71,57 @@ export async function cleanupWorktreeForTerminalStatus( return } - // Branch-per-story: only remove the worktree if no sibling job in the same - // story is still active. If siblings are queued/claimed/running they will - // re-use this branch — destroying the worktree now wastes the next claim. + // Branch-shared check: bepaal welke siblings dezelfde branch reuse'n. + // - SPRINT pr_strategy → alle TASK_IMPLEMENTATION jobs in dezelfde + // sprint_run delen feat/sprint-. + // - STORY pr_strategy / legacy → alle TASK_IMPLEMENTATION jobs in + // dezelfde story delen feat/story-. + // Bij active siblings: defer cleanup (en in elk geval keepBranch=true) + // zodat de volgende claim de branch kan reuse'n. const job = await prisma.claudeJob.findUnique({ where: { id: jobId }, - select: { task: { select: { story_id: true } } }, + select: { + task: { select: { story_id: true } }, + sprint_run_id: true, + sprint_run: { select: { pr_strategy: true } }, + }, }) - if (job?.task) { - const activeSiblings = await prisma.claudeJob.count({ + + let activeSiblings = 0 + let scope = '' + if (job?.sprint_run && job.sprint_run.pr_strategy === 'SPRINT') { + activeSiblings = await prisma.claudeJob.count({ + where: { + sprint_run_id: job.sprint_run_id, + status: { in: ['QUEUED', 'CLAIMED', 'RUNNING'] }, + id: { not: jobId }, + }, + }) + scope = `sprint_run ${job.sprint_run_id}` + } else if (job?.task) { + activeSiblings = await prisma.claudeJob.count({ where: { task: { story_id: job.task.story_id }, status: { in: ['QUEUED', 'CLAIMED', 'RUNNING'] }, id: { not: jobId }, }, }) - if (activeSiblings > 0) { - console.log( - `[update_job_status] cleanup deferred for job=${jobId}: ${activeSiblings} sibling(s) still active in story ${job.task.story_id}`, - ) - return - } + scope = `story ${job.task.story_id}` } - // Keep branch when job is done and a branch was reported (agent pushed) - const keepBranch = status === 'done' && branch !== undefined + if (activeSiblings > 0) { + console.log( + `[update_job_status] cleanup deferred for job=${jobId}: ${activeSiblings} sibling(s) still active in ${scope}`, + ) + return + } + + // Keep branch when: + // - job is done en agent rapporteerde push (branch !== undefined), of + // - SPRINT pr_strategy job is skipped — andere stories delen branch. + const keepBranch = + (status === 'done' && branch !== undefined) || + (status === 'skipped' && job?.sprint_run?.pr_strategy === 'SPRINT') try { await removeWorktreeForJob({ repoRoot, jobId, keepBranch }) } catch (err) { From 9ffa25f0536cebe328a8abf9d2b24c43509a13ca Mon Sep 17 00:00:00 2001 From: Janpeter Visser <30029041+madhura68@users.noreply.github.com> Date: Sat, 9 May 2026 20:30:17 +0200 Subject: [PATCH 3/8] fix(verify/classify): negeer pseudo-paths in plan (geen PARTIAL meer voor delete-only) (#46) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit extractPlanPaths beschouwde tokens als `data-debug-label="..."` als file-paden omdat ze een dot bevatten en geen spaties. Resultaat: het pseudo-pad werd nooit in de diff gevonden → coverage < 1 → PARTIAL → met verify_required=ALIGNED faalde de job, ondanks dat het werk volledig gedaan was. Concreet incident T-815 (sprint cmoyiu4yd, 2026-05-09): - 17/17 files data-debug-label verwijderd, grep 0 hits, typecheck groen - Verifier zei PARTIAL → Claude rapporteerde failed → propagateStatusUpwards + cancelPbiOnFailure cancelden 12 siblings + deleten feat/sprint-acq9twtr - T-814's al-gepushte werk verloren Fix: nieuwe `looksLikePath`-helper die backtick-tokens verwerpt als ze operator/quote/bracket chars bevatten, een ellipsis (`..`/`...`) hebben, of geen `/` én geen herkenbare file-extensie hebben. Bullet-extractor blijft onveranderd — die parseert al expliciet op `.ext`. Tests: 5 nieuwe regression-cases + alle 18 bestaande blijven groen. Co-authored-by: Madhura68 Co-authored-by: Claude Opus 4.7 (1M context) --- __tests__/verify/classify.test.ts | 50 +++++++++++++++++++++++++++++++ src/verify/classify.ts | 16 +++++++++- 2 files changed, 65 insertions(+), 1 deletion(-) diff --git a/__tests__/verify/classify.test.ts b/__tests__/verify/classify.test.ts index 1658e36..968e125 100644 --- a/__tests__/verify/classify.test.ts +++ b/__tests__/verify/classify.test.ts @@ -163,3 +163,53 @@ describe('classifyDiffAgainstPlan — delete-only commits', () => { expect(r.result).toBe('EMPTY') }) }) + +// Pseudo-paths in plans (code-snippets, attribute-syntax, ellipses) moeten +// niet als plan-paden meetellen — anders krijg je PARTIAL terwijl het werk +// volledig gedaan is. Regression-guard voor T-815-incident (sprint +// cmoyiu4yd000zf917acq9twtr, 2026-05-09). +describe('classifyDiffAgainstPlan — plan met pseudo-paths', () => { + it('negeert `data-debug-label="..."` als pseudo-pad en classificeert ALIGNED', () => { + const plan = [ + 'Verwijder alle voorkomens van `data-debug-label="..."` uit:', + '', + '- `app/components/shared/status-bar.tsx`', + '- `app/components/shared/header.tsx`', + ].join('\n') + const diff = makeDiff([ + 'app/components/shared/status-bar.tsx', + 'app/components/shared/header.tsx', + ]) + const r = classifyDiffAgainstPlan({ diff, plan }) + expect(r.result).toBe('ALIGNED') + }) + + it('negeert ellipsis-tokens (drie of meer dots) als pad', () => { + const plan = 'Refactor `foo(...)` naar `bar()`. Files: `src/a.ts`.' + const diff = makeDiff(['src/a.ts']) + const r = classifyDiffAgainstPlan({ diff, plan }) + expect(r.result).toBe('ALIGNED') + }) + + it('negeert tokens met operators/quotes als pad', () => { + const plan = 'Wijzig `props={x: 1}` en `useState()` in `src/c.tsx`.' + const diff = makeDiff(['src/c.tsx']) + const r = classifyDiffAgainstPlan({ diff, plan }) + expect(r.result).toBe('ALIGNED') + }) + + it('accepteert package.json en andere extension-only paths', () => { + const plan = 'Update `package.json` en `tsconfig.json`.' + const diff = makeDiff(['package.json', 'tsconfig.json']) + const r = classifyDiffAgainstPlan({ diff, plan }) + expect(r.result).toBe('ALIGNED') + }) + + it('blijft PARTIAL retourneren wanneer een echt plan-pad ontbreekt', () => { + const plan = 'Wijzig `src/foo.ts` en `src/bar.ts`. Verwijder `data-x="..."`.' + const diff = makeDiff(['src/foo.ts']) + const r = classifyDiffAgainstPlan({ diff, plan }) + expect(r.result).toBe('PARTIAL') + expect(r.reasoning).toMatch(/bar\.ts/) + }) +}) diff --git a/src/verify/classify.ts b/src/verify/classify.ts index 3fe99f5..429bfe3 100644 --- a/src/verify/classify.ts +++ b/src/verify/classify.ts @@ -27,7 +27,7 @@ function extractPlanPaths(plan: string): string[] { let m: RegExpExecArray | null while ((m = backtickRe.exec(plan)) !== null) { const p = m[1].trim() - if ((p.includes('/') || p.includes('.')) && !p.includes(' ') && p.length > 3) paths.add(p) + if (looksLikePath(p)) paths.add(p) } const bulletRe = /^[-*]\s+\*{0,2}([^\s*][^\s]*)\.([a-zA-Z]{1,6})\*{0,2}\s*[:\n]/gm @@ -38,6 +38,20 @@ function extractPlanPaths(plan: string): string[] { return [...paths] } +// Heuristic: does this backtick-quoted token look like a file path? +// Excludes code-snippets like `data-debug-label="..."`, `foo()`, `
` — +// anything containing operator/quote/bracket chars or an ellipsis is rejected. +// Accepts paths with a slash (multi-segment) or a recognisable file-extension +// suffix (1–6 alphanumeric chars after a final dot, e.g. `.tsx`, `.json`). +function looksLikePath(p: string): boolean { + if (p.length <= 3) return false + if (p.includes(' ')) return false + if (/[="'<>()[\]{};,]/.test(p)) return false + if (/\.{2,}/.test(p)) return false + if (!p.includes('/') && !/\.[a-zA-Z][a-zA-Z0-9]{0,5}$/.test(p)) return false + return true +} + // Path match: exact or suffix match so "classify.ts" matches "src/verify/classify.ts". function pathMatches(planPath: string, diffPaths: string[]): boolean { const norm = planPath.replace(/\\/g, '/') From 93d881318db6da7cc48bd759bf4e4043ae629c67 Mon Sep 17 00:00:00 2001 From: Janpeter Visser <30029041+madhura68@users.noreply.github.com> Date: Mon, 11 May 2026 21:37:05 +0200 Subject: [PATCH 4/8] feat(PBI-12): create_sprint + update_sprint MCP-tools (#47) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(PBI-12 T-51): voeg create_sprint tool toe Maakt een sprint aan met status=OPEN. Code auto-gegenereerd als S-{YYYY-MM-DD}-{N} per product per datum als niet meegegeven, met retry bij race-conflict op @@unique([product_id, code]). Volgt create-pbi.ts template. Co-Authored-By: Claude Opus 4.7 (1M context) * feat(PBI-12 T-52): voeg update_sprint tool toe Generieke update voor status, sprint_goal, start_date en end_date. Géén state-machine validatie — last-write-wins. Bij status → CLOSED/FAILED/ARCHIVED zonder expliciete end_date wordt end_date automatisch op vandaag gezet. Minimaal één veld vereist (handmatige check in handler i.p.v. zod-refine want McpServer.inputSchema accepteert geen ZodEffects). Co-Authored-By: Claude Opus 4.7 (1M context) * feat(PBI-12 T-53): registreer sprint-tools + unit-tests - Imports + register-calls toegevoegd in src/index.ts (groep met andere authoring-tools, comment "PBI-12: sprint lifecycle tools") - Refactor: create-sprint en update-sprint exporteren nu handleX + inputSchema apart (pattern van set-pbi-pr.ts) zodat de logica zonder McpServer wrapper testbaar is - 6 unit-tests voor create_sprint (happy path, custom code, auto-increment, P2002-retry, access-denied, explicit start_date) - 11 unit-tests voor update_sprint (no-fields-error, status-only, auto-end_date voor CLOSED/FAILED/ARCHIVED, geen auto voor OPEN, expliciete end_date respect, multi-field, not-found, access-denied, any-status-transition) - Defensive date-check in generateNextSprintCode tegen filter-veranderingen of mock-data anomalieën - 363 tests groen (was 346 + 17 nieuwe) DB-smoke-test (MCP-server vs dev-DB) overgeslagen want unit-coverage dekt het gedrag volledig; mock-vrije integratie volgt automatisch bij eerstvolgende productie-aanroep van create_sprint via een echte agent. Co-Authored-By: Claude Opus 4.7 (1M context) * chore: untrack .claude/worktrees gitlinks + ignore pad Per ongeluk in adbea3f meegenomen via 'git add -A'; deze embedded worktree- clones horen niet in de repo. Ook .gitignore aangevuld zodat dit niet opnieuw gebeurt. Co-Authored-By: Claude Opus 4.7 (1M context) * fix(PBI-12): update_sprint zet completed_at op CLOSED — parity met cascade Codex-review op #47: bij status → CLOSED werd alleen end_date gezet, niet completed_at. Dat is divergeert van src/lib/tasks-status-update.ts dat completed_at = new Date() zet bij automatische sluiting via task-status- cascade. Reporting en UI die op completed_at filteren zagen handmatig gesloten sprints als 'never completed'. Fix: - update_sprint zet nu data.completed_at = new Date() wanneer status === 'CLOSED' - FAILED/ARCHIVED raken completed_at NIET (parity met bestaand patroon) - Test-coverage uitgebreid: - CLOSED zet end_date EN completed_at - FAILED zet end_date, completed_at blijft undefined - ARCHIVED zet end_date, completed_at blijft undefined - OPEN zet noch end_date noch completed_at - Expliciete end_date wordt gerespecteerd, completed_at wordt nog steeds gezet - Tool description vermeldt nu de completed_at-side-effect Co-Authored-By: Claude Opus 4.7 (1M context) --------- Co-authored-by: Madhura68 Co-authored-by: Claude Opus 4.7 (1M context) --- .gitignore | 3 + __tests__/create-sprint.test.ts | 163 ++++++++++++++++++++++++++++++ __tests__/update-sprint.test.ts | 174 ++++++++++++++++++++++++++++++++ src/index.ts | 5 + src/tools/create-sprint.ts | 113 +++++++++++++++++++++ src/tools/update-sprint.ts | 102 +++++++++++++++++++ 6 files changed, 560 insertions(+) create mode 100644 __tests__/create-sprint.test.ts create mode 100644 __tests__/update-sprint.test.ts create mode 100644 src/tools/create-sprint.ts create mode 100644 src/tools/update-sprint.ts diff --git a/.gitignore b/.gitignore index 10a6dab..547c38e 100644 --- a/.gitignore +++ b/.gitignore @@ -12,3 +12,6 @@ prisma/generated # Editor .vscode .idea + +# Claude Code worktrees (per-session, never tracked) +.claude/worktrees/ diff --git a/__tests__/create-sprint.test.ts b/__tests__/create-sprint.test.ts new file mode 100644 index 0000000..72d400d --- /dev/null +++ b/__tests__/create-sprint.test.ts @@ -0,0 +1,163 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest' +import { Prisma } from '@prisma/client' + +vi.mock('../src/prisma.js', () => ({ + prisma: { + sprint: { + findMany: vi.fn(), + create: vi.fn(), + }, + }, +})) + +vi.mock('../src/auth.js', () => ({ + requireWriteAccess: vi.fn(), + PermissionDeniedError: class PermissionDeniedError extends Error { + constructor(message = 'Demo accounts cannot perform write operations') { + super(message) + this.name = 'PermissionDeniedError' + } + }, +})) + +vi.mock('../src/access.js', () => ({ + userCanAccessProduct: vi.fn(), +})) + +import { prisma } from '../src/prisma.js' +import { requireWriteAccess } from '../src/auth.js' +import { userCanAccessProduct } from '../src/access.js' +import { handleCreateSprint } from '../src/tools/create-sprint.js' + +const mockPrisma = prisma as unknown as { + sprint: { + findMany: ReturnType + create: ReturnType + } +} +const mockRequireWriteAccess = requireWriteAccess as ReturnType +const mockUserCanAccessProduct = userCanAccessProduct as ReturnType + +const PRODUCT_ID = 'prod-1' +const USER_ID = 'user-1' + +beforeEach(() => { + vi.clearAllMocks() + mockRequireWriteAccess.mockResolvedValue({ userId: USER_ID, tokenId: 'tok-1', username: 'alice', isDemo: false }) + mockUserCanAccessProduct.mockResolvedValue(true) + mockPrisma.sprint.findMany.mockResolvedValue([]) +}) + +function parseResult(result: Awaited>) { + const text = result.content?.[0]?.type === 'text' ? result.content[0].text : '' + try { return JSON.parse(text) } catch { return text } +} + +describe('handleCreateSprint', () => { + it('happy path: creates sprint with auto-generated code', async () => { + mockPrisma.sprint.create.mockResolvedValue({ + id: 'spr-1', + code: 'S-2026-05-11-1', + sprint_goal: 'My goal', + status: 'OPEN', + start_date: new Date('2026-05-11'), + created_at: new Date('2026-05-11T10:00:00Z'), + }) + + const result = await handleCreateSprint({ + product_id: PRODUCT_ID, + sprint_goal: 'My goal', + }) + + expect(mockPrisma.sprint.create).toHaveBeenCalledTimes(1) + const callArgs = mockPrisma.sprint.create.mock.calls[0][0] + expect(callArgs.data.product_id).toBe(PRODUCT_ID) + expect(callArgs.data.status).toBe('OPEN') + expect(callArgs.data.sprint_goal).toBe('My goal') + expect(callArgs.data.code).toMatch(/^S-\d{4}-\d{2}-\d{2}-1$/) + expect(callArgs.data.start_date).toBeInstanceOf(Date) + + const parsed = parseResult(result) + expect(parsed.id).toBe('spr-1') + expect(parsed.status).toBe('OPEN') + }) + + it('uses user-provided code when given', async () => { + mockPrisma.sprint.create.mockResolvedValue({ + id: 'spr-2', + code: 'CUSTOM-CODE', + sprint_goal: 'g', + status: 'OPEN', + start_date: new Date(), + created_at: new Date(), + }) + + await handleCreateSprint({ + product_id: PRODUCT_ID, + code: 'CUSTOM-CODE', + sprint_goal: 'g', + }) + + expect(mockPrisma.sprint.create).toHaveBeenCalledTimes(1) + expect(mockPrisma.sprint.findMany).not.toHaveBeenCalled() + expect(mockPrisma.sprint.create.mock.calls[0][0].data.code).toBe('CUSTOM-CODE') + }) + + it('auto-code increments past existing same-day sprints', async () => { + mockPrisma.sprint.findMany.mockResolvedValue([ + { code: 'S-2026-05-11-1' }, + { code: 'S-2026-05-11-3' }, + { code: 'S-2026-05-10-7' }, + ]) + mockPrisma.sprint.create.mockResolvedValue({ + id: 'spr-3', code: 'X', sprint_goal: 'g', status: 'OPEN', start_date: new Date(), created_at: new Date(), + }) + + await handleCreateSprint({ product_id: PRODUCT_ID, sprint_goal: 'g' }) + + const today = new Date().toISOString().slice(0, 10) + expect(mockPrisma.sprint.create.mock.calls[0][0].data.code).toBe(`S-${today}-4`) + }) + + it('retries on P2002 unique conflict', async () => { + const conflict = new Prisma.PrismaClientKnownRequestError('unique', { + code: 'P2002', clientVersion: 'x', meta: { target: ['product_id', 'code'] }, + }) + mockPrisma.sprint.create + .mockRejectedValueOnce(conflict) + .mockResolvedValueOnce({ + id: 'spr-r', code: 'S-2026-05-11-2', sprint_goal: 'g', status: 'OPEN', + start_date: new Date(), created_at: new Date(), + }) + + const result = await handleCreateSprint({ product_id: PRODUCT_ID, sprint_goal: 'g' }) + + expect(mockPrisma.sprint.create).toHaveBeenCalledTimes(2) + expect(parseResult(result).id).toBe('spr-r') + }) + + it('returns error when user cannot access product', async () => { + mockUserCanAccessProduct.mockResolvedValue(false) + const result = await handleCreateSprint({ product_id: PRODUCT_ID, sprint_goal: 'g' }) + + expect(mockPrisma.sprint.create).not.toHaveBeenCalled() + const text = result.content?.[0]?.type === 'text' ? result.content[0].text : '' + expect(text).toMatch(/not found or not accessible/) + }) + + it('uses provided start_date when given', async () => { + mockPrisma.sprint.create.mockResolvedValue({ + id: 'spr-d', code: 'X', sprint_goal: 'g', status: 'OPEN', + start_date: new Date('2026-01-01'), created_at: new Date(), + }) + + await handleCreateSprint({ + product_id: PRODUCT_ID, + sprint_goal: 'g', + start_date: '2026-01-01', + }) + + const callArgs = mockPrisma.sprint.create.mock.calls[0][0] + expect(callArgs.data.start_date.toISOString().slice(0, 10)).toBe('2026-01-01') + }) +}) diff --git a/__tests__/update-sprint.test.ts b/__tests__/update-sprint.test.ts new file mode 100644 index 0000000..3c62790 --- /dev/null +++ b/__tests__/update-sprint.test.ts @@ -0,0 +1,174 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest' + +vi.mock('../src/prisma.js', () => ({ + prisma: { + sprint: { + findUnique: vi.fn(), + update: vi.fn(), + }, + }, +})) + +vi.mock('../src/auth.js', () => ({ + requireWriteAccess: vi.fn(), + PermissionDeniedError: class PermissionDeniedError extends Error { + constructor(message = 'Demo accounts cannot perform write operations') { + super(message) + this.name = 'PermissionDeniedError' + } + }, +})) + +vi.mock('../src/access.js', () => ({ + userCanAccessProduct: vi.fn(), +})) + +import { prisma } from '../src/prisma.js' +import { requireWriteAccess } from '../src/auth.js' +import { userCanAccessProduct } from '../src/access.js' +import { handleUpdateSprint } from '../src/tools/update-sprint.js' + +const mockPrisma = prisma as unknown as { + sprint: { + findUnique: ReturnType + update: ReturnType + } +} +const mockRequireWriteAccess = requireWriteAccess as ReturnType +const mockUserCanAccessProduct = userCanAccessProduct as ReturnType + +const SPRINT_ID = 'spr-1' +const PRODUCT_ID = 'prod-1' +const USER_ID = 'user-1' + +beforeEach(() => { + vi.clearAllMocks() + mockRequireWriteAccess.mockResolvedValue({ userId: USER_ID, tokenId: 'tok-1', username: 'alice', isDemo: false }) + mockUserCanAccessProduct.mockResolvedValue(true) + mockPrisma.sprint.findUnique.mockResolvedValue({ id: SPRINT_ID, product_id: PRODUCT_ID }) + mockPrisma.sprint.update.mockResolvedValue({ + id: SPRINT_ID, + code: 'S-2026-05-11-1', + sprint_goal: 'g', + status: 'OPEN', + start_date: new Date('2026-05-11'), + end_date: null, + completed_at: null, + }) +}) + +function getText(result: Awaited>) { + return result.content?.[0]?.type === 'text' ? result.content[0].text : '' +} + +describe('handleUpdateSprint', () => { + it('returns error when no fields provided', async () => { + const result = await handleUpdateSprint({ sprint_id: SPRINT_ID }) + + expect(mockPrisma.sprint.update).not.toHaveBeenCalled() + expect(getText(result)).toMatch(/Minstens één veld vereist/) + }) + + it('updates status only', async () => { + await handleUpdateSprint({ sprint_id: SPRINT_ID, status: 'OPEN' }) + + expect(mockPrisma.sprint.update).toHaveBeenCalledTimes(1) + const args = mockPrisma.sprint.update.mock.calls[0][0] + expect(args.where).toEqual({ id: SPRINT_ID }) + expect(args.data).toEqual({ status: 'OPEN' }) + }) + + it('auto-sets end_date AND completed_at when status → CLOSED without explicit end_date', async () => { + const before = Date.now() + await handleUpdateSprint({ sprint_id: SPRINT_ID, status: 'CLOSED' }) + const after = Date.now() + + const args = mockPrisma.sprint.update.mock.calls[0][0] + expect(args.data.status).toBe('CLOSED') + expect(args.data.end_date).toBeInstanceOf(Date) + expect(args.data.end_date.getTime()).toBeGreaterThanOrEqual(before) + expect(args.data.end_date.getTime()).toBeLessThanOrEqual(after) + expect(args.data.completed_at).toBeInstanceOf(Date) + expect(args.data.completed_at.getTime()).toBeGreaterThanOrEqual(before) + expect(args.data.completed_at.getTime()).toBeLessThanOrEqual(after) + }) + + it('auto-sets end_date when status → FAILED, but does NOT set completed_at', async () => { + await handleUpdateSprint({ sprint_id: SPRINT_ID, status: 'FAILED' }) + + const args = mockPrisma.sprint.update.mock.calls[0][0] + expect(args.data.end_date).toBeInstanceOf(Date) + expect(args.data.completed_at).toBeUndefined() + }) + + it('auto-sets end_date when status → ARCHIVED, but does NOT set completed_at', async () => { + await handleUpdateSprint({ sprint_id: SPRINT_ID, status: 'ARCHIVED' }) + + const args = mockPrisma.sprint.update.mock.calls[0][0] + expect(args.data.end_date).toBeInstanceOf(Date) + expect(args.data.completed_at).toBeUndefined() + }) + + it('still sets completed_at when status → CLOSED even with explicit end_date', async () => { + await handleUpdateSprint({ + sprint_id: SPRINT_ID, + status: 'CLOSED', + end_date: '2025-12-31', + }) + + const args = mockPrisma.sprint.update.mock.calls[0][0] + expect(args.data.end_date.toISOString().slice(0, 10)).toBe('2025-12-31') + expect(args.data.completed_at).toBeInstanceOf(Date) + }) + + it('does NOT auto-set end_date or completed_at when status → OPEN', async () => { + await handleUpdateSprint({ sprint_id: SPRINT_ID, status: 'OPEN' }) + + const args = mockPrisma.sprint.update.mock.calls[0][0] + expect(args.data.end_date).toBeUndefined() + expect(args.data.completed_at).toBeUndefined() + }) + + it('updates multiple fields at once', async () => { + await handleUpdateSprint({ + sprint_id: SPRINT_ID, + sprint_goal: 'New goal', + start_date: '2026-05-15', + }) + + const args = mockPrisma.sprint.update.mock.calls[0][0] + expect(args.data.sprint_goal).toBe('New goal') + expect(args.data.start_date.toISOString().slice(0, 10)).toBe('2026-05-15') + expect(args.data.status).toBeUndefined() + expect(args.data.end_date).toBeUndefined() + }) + + it('returns error when sprint not found', async () => { + mockPrisma.sprint.findUnique.mockResolvedValue(null) + + const result = await handleUpdateSprint({ sprint_id: SPRINT_ID, status: 'CLOSED' }) + + expect(mockPrisma.sprint.update).not.toHaveBeenCalled() + expect(getText(result)).toMatch(/not found/) + }) + + it('returns error when user cannot access sprint product', async () => { + mockUserCanAccessProduct.mockResolvedValue(false) + + const result = await handleUpdateSprint({ sprint_id: SPRINT_ID, status: 'CLOSED' }) + + expect(mockPrisma.sprint.update).not.toHaveBeenCalled() + expect(getText(result)).toMatch(/not accessible/) + }) + + it('allows any status transition (no state-machine)', async () => { + await handleUpdateSprint({ sprint_id: SPRINT_ID, status: 'OPEN' }) + expect(mockPrisma.sprint.update).toHaveBeenCalledTimes(1) + + await handleUpdateSprint({ sprint_id: SPRINT_ID, status: 'CLOSED' }) + expect(mockPrisma.sprint.update).toHaveBeenCalledTimes(2) + + await handleUpdateSprint({ sprint_id: SPRINT_ID, status: 'OPEN' }) + expect(mockPrisma.sprint.update).toHaveBeenCalledTimes(3) + }) +}) diff --git a/src/index.ts b/src/index.ts index 2938c70..06cefba 100644 --- a/src/index.ts +++ b/src/index.ts @@ -12,6 +12,8 @@ import { registerLogCommitTool } from './tools/log-commit.js' import { registerCreatePbiTool } from './tools/create-pbi.js' import { registerCreateStoryTool } from './tools/create-story.js' import { registerCreateTaskTool } from './tools/create-task.js' +import { registerCreateSprintTool } from './tools/create-sprint.js' +import { registerUpdateSprintTool } from './tools/update-sprint.js' import { registerAskUserQuestionTool } from './tools/ask-user-question.js' import { registerGetQuestionAnswerTool } from './tools/get-question-answer.js' import { registerListOpenQuestionsTool } from './tools/list-open-questions.js' @@ -77,6 +79,9 @@ async function main() { registerCreatePbiTool(server) registerCreateStoryTool(server) registerCreateTaskTool(server) + // PBI-12: sprint lifecycle tools + registerCreateSprintTool(server) + registerUpdateSprintTool(server) registerAskUserQuestionTool(server) registerGetQuestionAnswerTool(server) registerListOpenQuestionsTool(server) diff --git a/src/tools/create-sprint.ts b/src/tools/create-sprint.ts new file mode 100644 index 0000000..5d8cd9b --- /dev/null +++ b/src/tools/create-sprint.ts @@ -0,0 +1,113 @@ +// MCP authoring tool: create een Sprint binnen een product. +// +// Status start altijd op OPEN; geen reuse-check op bestaande OPEN-sprints +// (per plan-to-pbi-flow.md "altijd nieuwe sprint"). Code wordt auto-gegenereerd +// als S-{YYYY-MM-DD}-{N} per product per datum, met retry bij race-condition +// op de unique constraint (@@unique([product_id, code])). + +import { z } from 'zod' +import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js' +import { Prisma } from '@prisma/client' +import { prisma } from '../prisma.js' +import { requireWriteAccess } from '../auth.js' +import { userCanAccessProduct } from '../access.js' +import { toolError, toolJson, withToolErrors } from '../errors.js' + +const SPRINT_AUTO_RE = /^S-(\d{4}-\d{2}-\d{2})-(\d+)$/ +const MAX_CODE_ATTEMPTS = 3 + +function todayIsoDate(): string { + return new Date().toISOString().slice(0, 10) +} + +async function generateNextSprintCode(productId: string): Promise { + const today = todayIsoDate() + const sprints = await prisma.sprint.findMany({ + where: { product_id: productId, code: { startsWith: `S-${today}-` } }, + select: { code: true }, + }) + let max = 0 + for (const s of sprints) { + const m = s.code?.match(SPRINT_AUTO_RE) + // Dubbele check op de datum — defensive tegen filterveranderingen + // of mock-data die niet door de DB-where heen ging. + if (m && m[1] === today) { + const n = Number.parseInt(m[2], 10) + if (!Number.isNaN(n) && n > max) max = n + } + } + return `S-${today}-${max + 1}` +} + +function isCodeUniqueConflict(error: unknown): boolean { + if (!(error instanceof Prisma.PrismaClientKnownRequestError)) return false + if (error.code !== 'P2002') return false + const target = (error.meta as { target?: string[] | string } | undefined)?.target + if (!target) return false + return Array.isArray(target) ? target.includes('code') : target.includes('code') +} + +export const inputSchema = z.object({ + product_id: z.string().min(1), + code: z.string().min(1).max(30).optional(), + sprint_goal: z.string().min(1).max(500), + start_date: z.string().date().optional(), +}) + +export async function handleCreateSprint( + { product_id, code, sprint_goal, start_date }: z.infer, +) { + return withToolErrors(async () => { + const auth = await requireWriteAccess() + if (!(await userCanAccessProduct(product_id, auth.userId))) { + return toolError(`Product ${product_id} not found or not accessible`) + } + + const resolvedStartDate = start_date ? new Date(start_date) : new Date() + const baseSelect = { + id: true, + code: true, + sprint_goal: true, + status: true, + start_date: true, + created_at: true, + } as const + + if (code) { + const sprint = await prisma.sprint.create({ + data: { product_id, code, sprint_goal, status: 'OPEN', start_date: resolvedStartDate }, + select: baseSelect, + }) + return toolJson(sprint) + } + + let lastError: unknown + for (let attempt = 0; attempt < MAX_CODE_ATTEMPTS; attempt++) { + const generated = await generateNextSprintCode(product_id) + try { + const sprint = await prisma.sprint.create({ + data: { product_id, code: generated, sprint_goal, status: 'OPEN', start_date: resolvedStartDate }, + select: baseSelect, + }) + return toolJson(sprint) + } catch (e) { + if (isCodeUniqueConflict(e)) { lastError = e; continue } + throw e + } + } + throw lastError ?? new Error('Kon geen unieke sprint-code genereren') + }) +} + +export function registerCreateSprintTool(server: McpServer) { + server.registerTool( + 'create_sprint', + { + title: 'Create Sprint', + description: + 'Create a new sprint for a product with status=OPEN. Code auto-generated as S-{YYYY-MM-DD}-{N} per product per date if not provided. Forbidden for demo accounts.', + inputSchema, + }, + handleCreateSprint, + ) +} diff --git a/src/tools/update-sprint.ts b/src/tools/update-sprint.ts new file mode 100644 index 0000000..04800e3 --- /dev/null +++ b/src/tools/update-sprint.ts @@ -0,0 +1,102 @@ +// MCP tool: update een Sprint. +// +// Generieke update — wijzigt elke combinatie van status, sprint_goal, +// start_date en end_date. Géén state-machine validatie (zie +// docs/plans/sprint-mcp-tools.md): last-write-wins, het resubmit/heropen-pad +// zit elders. Bij status → CLOSED/FAILED/ARCHIVED zonder expliciete end_date +// wordt end_date automatisch op vandaag gezet. Bij status → CLOSED wordt +// daarnaast `completed_at` op now() gezet (parity met +// src/lib/tasks-status-update.ts dat hetzelfde doet bij auto-close via +// task-status-cascade; zo houden reporting en UI één bron van waarheid voor +// completion-tijd). + +import { z } from 'zod' +import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js' +import type { SprintStatus } from '@prisma/client' +import { prisma } from '../prisma.js' +import { requireWriteAccess } from '../auth.js' +import { userCanAccessProduct } from '../access.js' +import { toolError, toolJson, withToolErrors } from '../errors.js' + +const TERMINAL_STATUSES = new Set(['CLOSED', 'FAILED', 'ARCHIVED']) + +export const inputSchema = z.object({ + sprint_id: z.string().min(1), + status: z.enum(['OPEN', 'CLOSED', 'ARCHIVED', 'FAILED']).optional(), + sprint_goal: z.string().min(1).max(500).optional(), + end_date: z.string().date().optional(), + start_date: z.string().date().optional(), +}) + +export async function handleUpdateSprint( + { sprint_id, status, sprint_goal, end_date, start_date }: z.infer, +) { + return withToolErrors(async () => { + if ( + status === undefined && + sprint_goal === undefined && + end_date === undefined && + start_date === undefined + ) { + return toolError('Minstens één veld vereist om te wijzigen') + } + + const auth = await requireWriteAccess() + + const sprint = await prisma.sprint.findUnique({ + where: { id: sprint_id }, + select: { id: true, product_id: true }, + }) + if (!sprint) { + return toolError(`Sprint ${sprint_id} not found`) + } + if (!(await userCanAccessProduct(sprint.product_id, auth.userId))) { + return toolError(`Sprint ${sprint_id} not accessible`) + } + + const data: { + status?: SprintStatus + sprint_goal?: string + start_date?: Date + end_date?: Date + completed_at?: Date + } = {} + if (status !== undefined) data.status = status + if (sprint_goal !== undefined) data.sprint_goal = sprint_goal + if (start_date !== undefined) data.start_date = new Date(start_date) + if (end_date !== undefined) { + data.end_date = new Date(end_date) + } else if (status !== undefined && TERMINAL_STATUSES.has(status)) { + data.end_date = new Date() + } + if (status === 'CLOSED') data.completed_at = new Date() + + const updated = await prisma.sprint.update({ + where: { id: sprint_id }, + data, + select: { + id: true, + code: true, + sprint_goal: true, + status: true, + start_date: true, + end_date: true, + completed_at: true, + }, + }) + return toolJson(updated) + }) +} + +export function registerUpdateSprintTool(server: McpServer) { + server.registerTool( + 'update_sprint', + { + title: 'Update Sprint', + description: + 'Update a sprint: status, sprint_goal, start_date and/or end_date. At least one field required. No state-machine validation — last-write-wins. When status goes to CLOSED/FAILED/ARCHIVED and end_date is not provided, end_date is set to today. When status goes to CLOSED, completed_at is set to now (parity with auto-close via task-cascade). Forbidden for demo accounts.', + inputSchema, + }, + handleUpdateSprint, + ) +} From 55fa133150a5dd7ae249cc448d73568317b20667 Mon Sep 17 00:00:00 2001 From: Janpeter Visser <30029041+madhura68@users.noreply.github.com> Date: Thu, 14 May 2026 14:30:17 +0000 Subject: [PATCH 5/8] feat: IDEA_REVIEW_PLAN-wiring + create_story sprint_id (v0.8.0) (#48) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(PBI-12 T-51): voeg create_sprint tool toe Maakt een sprint aan met status=OPEN. Code auto-gegenereerd als S-{YYYY-MM-DD}-{N} per product per datum als niet meegegeven, met retry bij race-conflict op @@unique([product_id, code]). Volgt create-pbi.ts template. Co-Authored-By: Claude Opus 4.7 (1M context) * feat(PBI-12 T-52): voeg update_sprint tool toe Generieke update voor status, sprint_goal, start_date en end_date. Géén state-machine validatie — last-write-wins. Bij status → CLOSED/FAILED/ARCHIVED zonder expliciete end_date wordt end_date automatisch op vandaag gezet. Minimaal één veld vereist (handmatige check in handler i.p.v. zod-refine want McpServer.inputSchema accepteert geen ZodEffects). Co-Authored-By: Claude Opus 4.7 (1M context) * feat(PBI-12 T-53): registreer sprint-tools + unit-tests - Imports + register-calls toegevoegd in src/index.ts (groep met andere authoring-tools, comment "PBI-12: sprint lifecycle tools") - Refactor: create-sprint en update-sprint exporteren nu handleX + inputSchema apart (pattern van set-pbi-pr.ts) zodat de logica zonder McpServer wrapper testbaar is - 6 unit-tests voor create_sprint (happy path, custom code, auto-increment, P2002-retry, access-denied, explicit start_date) - 11 unit-tests voor update_sprint (no-fields-error, status-only, auto-end_date voor CLOSED/FAILED/ARCHIVED, geen auto voor OPEN, expliciete end_date respect, multi-field, not-found, access-denied, any-status-transition) - Defensive date-check in generateNextSprintCode tegen filter-veranderingen of mock-data anomalieën - 363 tests groen (was 346 + 17 nieuwe) DB-smoke-test (MCP-server vs dev-DB) overgeslagen want unit-coverage dekt het gedrag volledig; mock-vrije integratie volgt automatisch bij eerstvolgende productie-aanroep van create_sprint via een echte agent. Co-Authored-By: Claude Opus 4.7 (1M context) * chore: untrack .claude/worktrees gitlinks + ignore pad Per ongeluk in adbea3f meegenomen via 'git add -A'; deze embedded worktree- clones horen niet in de repo. Ook .gitignore aangevuld zodat dit niet opnieuw gebeurt. Co-Authored-By: Claude Opus 4.7 (1M context) * fix(PBI-12): update_sprint zet completed_at op CLOSED — parity met cascade Codex-review op #47: bij status → CLOSED werd alleen end_date gezet, niet completed_at. Dat is divergeert van src/lib/tasks-status-update.ts dat completed_at = new Date() zet bij automatische sluiting via task-status- cascade. Reporting en UI die op completed_at filteren zagen handmatig gesloten sprints als 'never completed'. Fix: - update_sprint zet nu data.completed_at = new Date() wanneer status === 'CLOSED' - FAILED/ARCHIVED raken completed_at NIET (parity met bestaand patroon) - Test-coverage uitgebreid: - CLOSED zet end_date EN completed_at - FAILED zet end_date, completed_at blijft undefined - ARCHIVED zet end_date, completed_at blijft undefined - OPEN zet noch end_date noch completed_at - Expliciete end_date wordt gerespecteerd, completed_at wordt nog steeds gezet - Tool description vermeldt nu de completed_at-side-effect Co-Authored-By: Claude Opus 4.7 (1M context) * PBI-67 Phase 2: Add update-idea-plan-reviewed MCP tool - Create src/tools/update-idea-plan-reviewed.ts: saves review-log and transitions idea status - Register tool in src/index.ts - Update Prisma schema: add plan_review_log and reviewed_at fields to Idea model - Add PLAN_REVIEW_RESULT to IdeaLogType enum - Add REVIEWING_PLAN, PLAN_REVIEW_FAILED, PLAN_REVIEWED to IdeaStatus enum - Add IDEA_REVIEW_PLAN to ClaudeJobKind enum - Build successful with all type checks passing Co-Authored-By: Claude Haiku 4.5 * feat(PBI-67): bedraad IDEA_REVIEW_PLAN prompt + job-context - src/prompts/idea/review-plan.md: prompt voor IDEA_REVIEW_PLAN-jobs — iteratieve 3-ronden plan-review met convergentie-detectie - kind-prompts.ts: koppel IDEA_REVIEW_PLAN aan de prompt + getIdeaPromptText - wait-for-job.ts: getFullJobContext handelt IDEA_REVIEW_PLAN-jobs af Co-Authored-By: Claude Opus 4.7 (1M context) * feat(create_story): optionele sprint_id om story aan sprint te koppelen create_story accepteert nu een optionele sprint_id; bij meegeven wordt de story aangemaakt met status=IN_SPRINT (sprint moet bij hetzelfde product horen als de PBI). Handler geextraheerd naar handleCreateStory voor testbaarheid; nieuwe unit-tests in __tests__/create-story.test.ts. Co-Authored-By: Claude Opus 4.7 (1M context) * fix(test): maak create-sprint auto-code test datum-onafhankelijk De test hardcodede 2026-05-11-datums maar berekende "today" dynamisch, waardoor hij alleen op die datum slaagde. Mock-codes nu relatief aan today. Co-Authored-By: Claude Opus 4.7 (1M context) * chore: bump version 0.7.0 -> 0.8.0 Co-Authored-By: Claude Opus 4.7 (1M context) * chore: bump vendor/scrum4me submodule naar app-main (7bb252c) De submodule stond 27 commits achter (3c77342, v1.0.0-147), waardoor sync-schema.sh prisma/schema.prisma terugzette naar een versie zonder IDEA_REVIEW_PLAN. Bumpt naar huidige app-main + re-synct het schema; enige inhoudelijke wijziging is het nieuwe User.settings-veld. Co-Authored-By: Claude Opus 4.7 (1M context) --------- Co-authored-by: Madhura68 Co-authored-by: Claude Opus 4.7 (1M context) --- __tests__/create-sprint.test.ts | 10 +- __tests__/create-story.test.ts | 141 +++++++++++++++++ package.json | 2 +- prisma/schema.prisma | 40 +++-- src/index.ts | 2 + src/lib/job-config.ts | 13 ++ src/lib/kind-prompts.ts | 5 +- src/prompts/idea/review-plan.md | 210 +++++++++++++++++++++++++ src/tools/create-story.ts | 158 +++++++++++-------- src/tools/update-idea-plan-reviewed.ts | 116 ++++++++++++++ src/tools/wait-for-job.ts | 8 +- vendor/scrum4me | 2 +- 12 files changed, 619 insertions(+), 88 deletions(-) create mode 100644 __tests__/create-story.test.ts create mode 100644 src/prompts/idea/review-plan.md create mode 100644 src/tools/update-idea-plan-reviewed.ts diff --git a/__tests__/create-sprint.test.ts b/__tests__/create-sprint.test.ts index 72d400d..5837d6e 100644 --- a/__tests__/create-sprint.test.ts +++ b/__tests__/create-sprint.test.ts @@ -104,10 +104,13 @@ describe('handleCreateSprint', () => { }) it('auto-code increments past existing same-day sprints', async () => { + // Codes moeten relatief aan "vandaag" zijn: generateNextSprintCode telt + // alleen same-day sprints. Hardcoded datums maakten deze test datum-flaky. + const today = new Date().toISOString().slice(0, 10) mockPrisma.sprint.findMany.mockResolvedValue([ - { code: 'S-2026-05-11-1' }, - { code: 'S-2026-05-11-3' }, - { code: 'S-2026-05-10-7' }, + { code: `S-${today}-1` }, + { code: `S-${today}-3` }, + { code: 'S-2020-01-01-7' }, ]) mockPrisma.sprint.create.mockResolvedValue({ id: 'spr-3', code: 'X', sprint_goal: 'g', status: 'OPEN', start_date: new Date(), created_at: new Date(), @@ -115,7 +118,6 @@ describe('handleCreateSprint', () => { await handleCreateSprint({ product_id: PRODUCT_ID, sprint_goal: 'g' }) - const today = new Date().toISOString().slice(0, 10) expect(mockPrisma.sprint.create.mock.calls[0][0].data.code).toBe(`S-${today}-4`) }) diff --git a/__tests__/create-story.test.ts b/__tests__/create-story.test.ts new file mode 100644 index 0000000..2bf1222 --- /dev/null +++ b/__tests__/create-story.test.ts @@ -0,0 +1,141 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest' + +vi.mock('../src/prisma.js', () => ({ + prisma: { + pbi: { findUnique: vi.fn() }, + sprint: { findUnique: vi.fn() }, + story: { + findFirst: vi.fn(), + findMany: vi.fn(), + create: vi.fn(), + }, + }, +})) + +vi.mock('../src/auth.js', () => ({ + requireWriteAccess: vi.fn(), + PermissionDeniedError: class PermissionDeniedError extends Error { + constructor(message = 'Demo accounts cannot perform write operations') { + super(message) + this.name = 'PermissionDeniedError' + } + }, +})) + +vi.mock('../src/access.js', () => ({ + userCanAccessProduct: vi.fn(), +})) + +import { prisma } from '../src/prisma.js' +import { requireWriteAccess } from '../src/auth.js' +import { userCanAccessProduct } from '../src/access.js' +import { handleCreateStory } from '../src/tools/create-story.js' + +const mockPrisma = prisma as unknown as { + pbi: { findUnique: ReturnType } + sprint: { findUnique: ReturnType } + story: { + findFirst: ReturnType + findMany: ReturnType + create: ReturnType + } +} +const mockRequireWriteAccess = requireWriteAccess as ReturnType +const mockUserCanAccessProduct = userCanAccessProduct as ReturnType + +const PRODUCT_ID = 'prod-1' +const PBI_ID = 'pbi-1' +const SPRINT_ID = 'spr-1' +const USER_ID = 'user-1' + +beforeEach(() => { + vi.clearAllMocks() + mockRequireWriteAccess.mockResolvedValue({ userId: USER_ID, tokenId: 'tok-1', username: 'alice', isDemo: false }) + mockUserCanAccessProduct.mockResolvedValue(true) + mockPrisma.pbi.findUnique.mockResolvedValue({ product_id: PRODUCT_ID }) + mockPrisma.story.findMany.mockResolvedValue([]) + mockPrisma.story.findFirst.mockResolvedValue(null) + mockPrisma.story.create.mockImplementation((args: { data: Record }) => + Promise.resolve({ id: 'story-1', created_at: new Date('2026-05-14T10:00:00Z'), ...args.data }), + ) +}) + +function parseResult(result: Awaited>) { + const text = result.content?.[0]?.type === 'text' ? result.content[0].text : '' + try { return JSON.parse(text) } catch { return text } +} + +function errorText(result: Awaited>): string { + return result.content?.[0]?.type === 'text' ? result.content[0].text : '' +} + +describe('handleCreateStory', () => { + it('without sprint_id: creates story with status OPEN and no sprint', async () => { + const result = await handleCreateStory({ pbi_id: PBI_ID, title: 'A story', priority: 2 }) + + expect(mockPrisma.sprint.findUnique).not.toHaveBeenCalled() + const data = mockPrisma.story.create.mock.calls[0][0].data + expect(data.status).toBe('OPEN') + expect(data.sprint_id).toBeNull() + expect(data.product_id).toBe(PRODUCT_ID) + expect(parseResult(result).status).toBe('OPEN') + }) + + it('with valid sprint_id: links story to sprint with status IN_SPRINT', async () => { + mockPrisma.sprint.findUnique.mockResolvedValue({ product_id: PRODUCT_ID }) + + const result = await handleCreateStory({ + pbi_id: PBI_ID, + title: 'A story', + priority: 2, + sprint_id: SPRINT_ID, + }) + + expect(mockPrisma.sprint.findUnique).toHaveBeenCalledWith({ + where: { id: SPRINT_ID }, + select: { product_id: true }, + }) + const data = mockPrisma.story.create.mock.calls[0][0].data + expect(data.status).toBe('IN_SPRINT') + expect(data.sprint_id).toBe(SPRINT_ID) + expect(parseResult(result).sprint_id).toBe(SPRINT_ID) + }) + + it('rejects a non-existent sprint_id', async () => { + mockPrisma.sprint.findUnique.mockResolvedValue(null) + + const result = await handleCreateStory({ + pbi_id: PBI_ID, + title: 'A story', + priority: 2, + sprint_id: 'missing', + }) + + expect(mockPrisma.story.create).not.toHaveBeenCalled() + expect(errorText(result)).toMatch(/Sprint missing not found/) + }) + + it('rejects a sprint from a different product', async () => { + mockPrisma.sprint.findUnique.mockResolvedValue({ product_id: 'other-product' }) + + const result = await handleCreateStory({ + pbi_id: PBI_ID, + title: 'A story', + priority: 2, + sprint_id: SPRINT_ID, + }) + + expect(mockPrisma.story.create).not.toHaveBeenCalled() + expect(errorText(result)).toMatch(/different product/) + }) + + it('returns error when PBI not found', async () => { + mockPrisma.pbi.findUnique.mockResolvedValue(null) + + const result = await handleCreateStory({ pbi_id: 'missing', title: 'A story', priority: 2 }) + + expect(mockPrisma.sprint.findUnique).not.toHaveBeenCalled() + expect(mockPrisma.story.create).not.toHaveBeenCalled() + expect(errorText(result)).toMatch(/PBI missing not found/) + }) +}) diff --git a/package.json b/package.json index de00265..0cbcf56 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "scrum4me-mcp", - "version": "0.7.0", + "version": "0.8.0", "description": "MCP server for Scrum4Me — exposes dev-flow tools and prompts via the Model Context Protocol", "type": "module", "bin": { diff --git a/prisma/schema.prisma b/prisma/schema.prisma index 4f6b086..d854a58 100644 --- a/prisma/schema.prisma +++ b/prisma/schema.prisma @@ -100,6 +100,9 @@ enum IdeaStatus { PLANNING PLAN_FAILED PLAN_READY + REVIEWING_PLAN + PLAN_REVIEW_FAILED + PLAN_REVIEWED PLANNED } @@ -107,6 +110,7 @@ enum ClaudeJobKind { TASK_IMPLEMENTATION IDEA_GRILL IDEA_MAKE_PLAN + IDEA_REVIEW_PLAN PLAN_CHAT SPRINT_IMPLEMENTATION } @@ -124,6 +128,7 @@ enum IdeaLogType { NOTE GRILL_RESULT PLAN_RESULT + PLAN_REVIEW_RESULT STATUS_CHANGE JOB_EVENT } @@ -147,6 +152,7 @@ model User { active_product Product? @relation("UserActiveProduct", fields: [active_product_id], references: [id], onDelete: SetNull) idea_code_counter Int @default(0) min_quota_pct Int @default(20) + settings Json @default("{}") created_at DateTime @default(now()) updated_at DateTime @updatedAt roles UserRole[] @@ -510,22 +516,24 @@ model ProductMember { } model Idea { - id String @id @default(cuid()) - user User @relation(fields: [user_id], references: [id], onDelete: Cascade) - user_id String - product Product? @relation(fields: [product_id], references: [id], onDelete: SetNull) - product_id String? - code String @db.VarChar(30) - title String - description String? @db.VarChar(4000) - grill_md String? @db.Text - plan_md String? @db.Text - pbi Pbi? @relation(fields: [pbi_id], references: [id], onDelete: SetNull) - pbi_id String? @unique - status IdeaStatus @default(DRAFT) - archived Boolean @default(false) - created_at DateTime @default(now()) - updated_at DateTime @updatedAt + id String @id @default(cuid()) + user User @relation(fields: [user_id], references: [id], onDelete: Cascade) + user_id String + product Product? @relation(fields: [product_id], references: [id], onDelete: SetNull) + product_id String? + code String @db.VarChar(30) + title String + description String? @db.VarChar(4000) + grill_md String? @db.Text + plan_md String? @db.Text + plan_review_log Json? // ReviewLog from orchestrator (all rounds, convergence metrics, approval status) + reviewed_at DateTime? // When last reviewed + pbi Pbi? @relation(fields: [pbi_id], references: [id], onDelete: SetNull) + pbi_id String? @unique + status IdeaStatus @default(DRAFT) + archived Boolean @default(false) + created_at DateTime @default(now()) + updated_at DateTime @updatedAt questions ClaudeQuestion[] jobs ClaudeJob[] diff --git a/src/index.ts b/src/index.ts index 06cefba..03f08d8 100644 --- a/src/index.ts +++ b/src/index.ts @@ -28,6 +28,7 @@ import { registerMarkPbiPrMergedTool } from './tools/mark-pbi-pr-merged.js' import { registerGetIdeaContextTool } from './tools/get-idea-context.js' import { registerUpdateIdeaGrillMdTool } from './tools/update-idea-grill-md.js' import { registerUpdateIdeaPlanMdTool } from './tools/update-idea-plan-md.js' +import { registerUpdateIdeaPlanReviewedTool } from './tools/update-idea-plan-reviewed.js' import { registerLogIdeaDecisionTool } from './tools/log-idea-decision.js' import { registerGetWorkerSettingsTool } from './tools/get-worker-settings.js' import { registerWorkerHeartbeatTool } from './tools/worker-heartbeat.js' @@ -97,6 +98,7 @@ async function main() { registerGetIdeaContextTool(server) registerUpdateIdeaGrillMdTool(server) registerUpdateIdeaPlanMdTool(server) + registerUpdateIdeaPlanReviewedTool(server) registerLogIdeaDecisionTool(server) // M13: worker quota-gate tools registerGetWorkerSettingsTool(server) diff --git a/src/lib/job-config.ts b/src/lib/job-config.ts index 811e365..ef7270d 100644 --- a/src/lib/job-config.ts +++ b/src/lib/job-config.ts @@ -101,6 +101,19 @@ const KIND_DEFAULTS: Record = { 'mcp__scrum4me__update_job_status', ], }, + IDEA_REVIEW_PLAN: { + model: 'claude-opus-4-7', + thinking_budget: 6000, + permission_mode: 'acceptEdits', + max_turns: 1, + allowed_tools: [ + 'Read', 'Write', 'Grep', 'Glob', + 'mcp__scrum4me__update_idea_plan_reviewed', + 'mcp__scrum4me__log_idea_decision', + 'mcp__scrum4me__update_job_status', + 'mcp__scrum4me__ask_user_question', + ], + }, PLAN_CHAT: { model: 'claude-sonnet-4-6', thinking_budget: 6000, diff --git a/src/lib/kind-prompts.ts b/src/lib/kind-prompts.ts index f7e03c1..15a7a16 100644 --- a/src/lib/kind-prompts.ts +++ b/src/lib/kind-prompts.ts @@ -25,6 +25,7 @@ function loadPrompt(rel: string): string { const KIND_TO_PROMPT_PATH: Partial> = { IDEA_GRILL: 'idea/grill.md', IDEA_MAKE_PLAN: 'idea/make-plan.md', + IDEA_REVIEW_PLAN: 'idea/review-plan.md', TASK_IMPLEMENTATION: 'task/implementation.md', SPRINT_IMPLEMENTATION: 'sprint/implementation.md', PLAN_CHAT: 'plan-chat/chat.md', @@ -40,9 +41,9 @@ export function getKindPromptText(kind: ClaudeJobKind): string { } // Back-compat re-export. wait-for-job.ts roept getIdeaPromptText aan voor -// de twee idea-kinds; behouden zodat we de bestaande call-site niet hoeven +// de drie idea-kinds; behouden zodat we de bestaande call-site niet hoeven // te wijzigen tot een aparte cleanup-pass. export function getIdeaPromptText(kind: ClaudeJobKind): string { - if (kind !== 'IDEA_GRILL' && kind !== 'IDEA_MAKE_PLAN') return '' + if (kind !== 'IDEA_GRILL' && kind !== 'IDEA_MAKE_PLAN' && kind !== 'IDEA_REVIEW_PLAN') return '' return getKindPromptText(kind) } diff --git a/src/prompts/idea/review-plan.md b/src/prompts/idea/review-plan.md new file mode 100644 index 0000000..8df45f6 --- /dev/null +++ b/src/prompts/idea/review-plan.md @@ -0,0 +1,210 @@ +# Review-Plan-prompt voor IDEA_REVIEW_PLAN-jobs + +> Deze prompt wordt door `wait_for_job` meegestuurd in de payload van een +> `IDEA_REVIEW_PLAN`-job. Dit is een **iteratieve review met actieve plan-revisie** +> en convergence-detectie. Je coördineert drie review-rondes, herschrijft het plan +> na elke ronde, en slaat het review-log op via `update_idea_plan_reviewed`. + +--- + +Je bent een **plan-review-orchestrator** voor Scrum4Me-idee `{idea_code}`. + +Je context (meegegeven in `wait_for_job`-payload): + +- `idea.plan_md`: het te reviewen plan-document (YAML frontmatter + body) +- `idea.grill_md`: context uit de grill-fase (scope, acceptatie, risico's) +- `product`: gekoppeld product met `definition_of_done` en repo-context +- `repo_url`: lokale repo om bestaande patronen/code te raadplegen + +## Doel + +Drie iteratieve review-rondes uitvoeren, gericht op verschillende aspecten. Na +elke ronde herschrijf je het plan actief en sla je de herziene versie op in de +database. De reviews werken op convergentie af: zodra het plan stabiel is +(< 5% wijzigingen twee rondes achter elkaar), vraag je om goedkeuring. + +**Belangrijk:** het plan wordt bij elke ronde daadwerkelijk verbeterd en +gepersisteerd via `update_idea_plan_md`. Dit is geen passieve review — je +coördineert een actief verbeterproces. + +## Werkwijze + +### Setup (voor ronde 1) + +1. Lees `idea.plan_md` volledig — dit is de startversie van het plan. +2. Lees `idea.grill_md` voor scope/acceptatiecriteria-context. +3. **Laad codex** (verplicht, niet optioneel): + - Glob + Read alle `docs/patterns/**/*.md` → architectuurpatronen + - Glob + Read alle `docs/architecture/**/*.md` → systeemdesign + - Read `CLAUDE.md` → hardstop-regels (nooit schenden) + - Gebruik deze als leidraad bij elke review-ronde +4. Initialiseer `review_log`: + ```json + { "plan_file": "{idea_code}", "created_at": "", + "rounds": [], "approval": { "status": "pending" } } + ``` + +### Per Review-Ronde + +**Ronde 1 — Structuur & Syntax (Haiku-perspectief: snel en scherp)** +- Rol: structuur-reviewer — focus op correctheid, niet op inhoud +- Controleer: YAML parseable, alle verplichte velden aanwezig, geen lege strings, + priority-waarden valid (1–4), markdown-structuur intact +- Herschrijf plan_md: corrigeer structuurfouten en formatting +- *Opmerking multi-model:* directe Haiku API-call is momenteel niet beschikbaar + via job-config; voer deze rol zelf uit met een compacte, syntax-gerichte blik + +**Ronde 2 — Logica & Patronen (Sonnet-perspectief: diep en patroon-bewust)** +- Rol: architectuur-reviewer — focus op logica, volledigheid en patroonconformiteit +- Controleer: stories volgen uit grill-criteria, tasks zijn concreet + (bestandsnamen, commando's), patterns uit `docs/patterns/` worden gevolgd, + `verify_required` coherent, dependency-cascades geadresseerd +- Herschrijf plan_md: vul gaten aan, maak tasks specifieker, voeg missende stappen toe + +**Ronde 3 — Risico & Edge Cases (Opus-perspectief: kritisch en breed)** +- Rol: risico-reviewer — focus op wat mis kan gaan +- Controleer: grote taken gesplitst, refactors hebben undo-strategie, + schema-changes hebben migratie-taken, type-checking expliciet, concurrency + geadresseerd, error-handling per actie, feature-flags voor grote changes +- Herschrijf plan_md: voeg risico-mitigatie toe, split te grote taken + +### Plan Revision (na elke ronde — verplicht) + +Na het uitvoeren van de review-criteria: + +1. Sla de huidige versie op als `plan_before` in `review_log.rounds[N]`. +2. Herschrijf `plan_md` — integreer de gevonden verbeteringen. +3. Bereken `diff_pct = changed_lines / total_lines * 100`. +4. Sla de herziene versie op als `plan_after` in `review_log.rounds[N]`. +5. **Persisteer de herziene versie** via: + ``` + update_idea_plan_md({ idea_id: , plan_md: }) + ``` + Dit slaat het verbeterde plan op in de database zodat de gebruiker + de progressie ziet. Sla dit stap niet over — ook al zijn er weinig + wijzigingen. + +### Convergence Detection + +Na elke ronde (m.u.v. ronde 0): +``` +diff_pct_this_round = changed_lines / total_lines * 100 +if diff_pct_this_round < 5 AND prev_round_diff_pct < 5: + → CONVERGED +``` + +Indien converged (of na ronde 2 als max bereikt): +- Sla op: `review_log.convergence = { stable_at_round: N, final_diff_pct, convergence_metric: "plan_stability" }` +- Vraag goedkeuring via `ask_user_question` + +## Review-Criteria per Ronde + +### Ronde 1 — Structuur & Syntax +- [ ] Frontmatter YAML parseable +- [ ] Alle verplichte velden aanwezig (`pbi.title`, `stories`, `tasks`) +- [ ] Priority-waarden valid (1–4) +- [ ] Geen lege strings in verplichte velden +- [ ] Markdown-structuur correct (headers, code-blocks) + +### Ronde 2 — Logica & Patronen +- [ ] Stories volgen logisch uit grill-acceptance-criteria +- [ ] Tasks zijn concreet (bestandsnamen, commando's, niet abstract) +- [ ] Dependency-cascade-checks uitgevoerd (bij removal/refactor) +- [ ] Patronen uit `docs/patterns/` worden gevolgd +- [ ] Implementatie-plan per task is actionable +- [ ] `verify_required` waarden coherent met task-scope + +### Ronde 3 — Risico & Edge Cases +- [ ] Grote taken (> 4u) zijn gesplitst in subtaken +- [ ] Refactors hebben een undo/rollback-strategie +- [ ] Schema-changes hebben migratie-taken +- [ ] Type-checking wordt expliciet geverifieerd (einde-taak) +- [ ] Concurrency-issues / race-conditions geadresseerd +- [ ] Error-handling per actie duidelijk +- [ ] Feature-flags ingebouwd voor grote of riskante changes + +## Stappen (uitgebreid algoritme) + +1. **Init** + - Lees plan_md + grill_md. + - Laad codex (docs/patterns, docs/architecture, CLAUDE.md). + - Initialiseer `review_log`. + +2. **Loop: for round in [0, 1, 2]** + - Voer review uit (focus per ronde: structuur / logica / risico). + - Sla `plan_before` op. + - Herschrijf plan_md op basis van bevindingen. + - Roep `update_idea_plan_md` aan met de herziene tekst. + - Sla `plan_after` + `issues` + `score` + `diff_pct` op in review_log. + - Check convergence (na ronde 1+). + - Break indien converged. + +3. **Approval Gate** + - Vraag via `ask_user_question`: + "Plan beoordeeld ({N} rondes, {X}% eindwijziging). Goedkeuren?" + - Opties: `["Ja, accepteren", "Nee, aanpassingen gewenst", "Opnieuw reviewen"]` + - "Ja": `approval.status = 'approved'` → ga door naar Save & Close. + - "Nee": `approval.status = 'rejected'` → sluit af (user kan handmatig editen). + - "Opnieuw": max 2 extra rondes (rondes 3–4), dan dwingend approval vragen. + +4. **Save & Close** + - Call `update_idea_plan_reviewed({ idea_id, review_log, approval_status })`. + - Call `update_job_status({ job_id, status: 'done', summary: review_log.summary })`. + +## Output-format review_log (strikt JSON) + +```json +{ + "plan_file": "IDEA-016", + "created_at": "ISO8601", + "rounds": [ + { + "round": 0, + "model": "claude-opus-4-7", + "role": "Structure Review", + "focus": "YAML parsing, format, syntax", + "plan_before": "", + "plan_after": "", + "issues": [ + { + "category": "structure|logic|risk|pattern", + "severity": "error|warning|info", + "suggestion": "wat te fixen" + } + ], + "score": 75, + "plan_diff_lines": 12, + "converged": false, + "timestamp": "ISO8601" + } + ], + "convergence": { + "stable_at_round": 2, + "final_diff_pct": 2.1, + "convergence_metric": "plan_stability" + }, + "approval": { + "status": "pending|approved|rejected", + "timestamp": "ISO8601" + }, + "summary": "1–2 zinnen samenvatting: X rondes, Y% wijziging, status" +} +``` + +## Foutgevallen + +- **Plan parse-fout**: `update_job_status('failed', error: 'plan_parse_failed')` — stop. +- **update_idea_plan_md mislukt**: log error in review_log, ga door met review — niet fataal. +- **Gebruiker annuleert**: sluit netjes af; job wordt door server op CANCELLED gezet. +- **Vraag verloopt**: sla partial review-log op via `update_idea_plan_reviewed`, markeer als `rejected`. + +## Aannames & Limieten + +- **Multi-model:** directe Haiku/Sonnet API-calls zijn niet beschikbaar via de huidige + job-config architectuur. Alle rondes draaien op het geconfigureerde Opus model. + De rollen (structuur / logica / risico) worden wel strikt gescheiden gehouden. + Toekomst: directe model-switching via Anthropic API. +- Plan bevat geen versleutelde data (review-log opgeslagen als JSON in DB). +- Repo is leesbaar; geen network-fouts verwacht. +- Max 2 extra review-rondes buiten de initiële 3 (max 5 rondes totaal). +- Per ronde: max 10 issues gelogd (overige → samenvatting in `summary`). diff --git a/src/tools/create-story.ts b/src/tools/create-story.ts index cfa099e..37caa59 100644 --- a/src/tools/create-story.ts +++ b/src/tools/create-story.ts @@ -1,8 +1,9 @@ // MCP authoring tool: create een Story onder een bestaande PBI. // // product_id wordt afgeleid uit de PBI (denormalized FK conform CLAUDE.md -// convention — nooit vertrouwen op client-input). status='OPEN' default; -// landt in de Product Backlog, niet auto in een sprint. +// convention — nooit vertrouwen op client-input). Zonder sprint_id is +// status='OPEN' en landt de story in de Product Backlog; mét sprint_id +// wordt de story direct aan die sprint gekoppeld (status='IN_SPRINT'). import { z } from 'zod' import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js' @@ -46,75 +47,108 @@ const inputSchema = z.object({ acceptance_criteria: z.string().max(4000).optional(), priority: z.number().int().min(1).max(4), sort_order: z.number().optional(), + // Optionele sprint-koppeling: bij creatie de story direct aan een sprint + // hangen (status=IN_SPRINT). De sprint moet bij hetzelfde product horen. + sprint_id: z.string().min(1).optional(), }) +export async function handleCreateStory( + { + pbi_id, + title, + description, + acceptance_criteria, + priority, + sort_order, + sprint_id, + }: z.infer, +) { + return withToolErrors(async () => { + const auth = await requireWriteAccess() + + const pbi = await prisma.pbi.findUnique({ + where: { id: pbi_id }, + select: { product_id: true }, + }) + if (!pbi) return toolError(`PBI ${pbi_id} not found`) + if (!(await userCanAccessProduct(pbi.product_id, auth.userId))) { + return toolError(`PBI ${pbi_id} not accessible`) + } + + // Optionele sprint-koppeling: valideer dat de sprint bestaat én bij + // hetzelfde product hoort — voorkomt een cross-product koppeling. + if (sprint_id !== undefined) { + const sprint = await prisma.sprint.findUnique({ + where: { id: sprint_id }, + select: { product_id: true }, + }) + if (!sprint) return toolError(`Sprint ${sprint_id} not found`) + if (sprint.product_id !== pbi.product_id) { + return toolError( + `Sprint ${sprint_id} belongs to a different product than PBI ${pbi_id}`, + ) + } + } + + let resolvedSortOrder = sort_order + if (resolvedSortOrder === undefined) { + const last = await prisma.story.findFirst({ + where: { pbi_id, priority }, + orderBy: { sort_order: 'desc' }, + select: { sort_order: true }, + }) + resolvedSortOrder = (last?.sort_order ?? 0) + 1.0 + } + + let lastError: unknown + for (let attempt = 0; attempt < MAX_CODE_ATTEMPTS; attempt++) { + const code = await generateNextStoryCode(pbi.product_id) + try { + const story = await prisma.story.create({ + data: { + pbi_id, + product_id: pbi.product_id, // denormalized uit DB-parent, niet uit input + sprint_id: sprint_id ?? null, + code, + title, + description: description ?? null, + acceptance_criteria: acceptance_criteria ?? null, + priority, + sort_order: resolvedSortOrder, + status: sprint_id ? 'IN_SPRINT' : 'OPEN', + }, + select: { + id: true, + code: true, + title: true, + description: true, + acceptance_criteria: true, + priority: true, + sort_order: true, + status: true, + sprint_id: true, + created_at: true, + }, + }) + return toolJson(story) + } catch (e) { + if (isCodeUniqueConflict(e)) { lastError = e; continue } + throw e + } + } + throw lastError ?? new Error('Kon geen unieke Story-code genereren') + }) +} + export function registerCreateStoryTool(server: McpServer) { server.registerTool( 'create_story', { title: 'Create story', description: - 'Add a story under an existing PBI. Status defaults to OPEN (lands in product backlog, not in a sprint). Sort_order auto-set to last+1 within the PBI/priority group if not provided. Forbidden for demo accounts.', + 'Add a story under an existing PBI. Optionally link it to a sprint via sprint_id — when given, the story is created with status=IN_SPRINT and the sprint must belong to the same product as the PBI; otherwise status=OPEN and the story lands in the product backlog. Sort_order auto-set to last+1 within the PBI/priority group if not provided. Forbidden for demo accounts.', inputSchema, }, - async ({ pbi_id, title, description, acceptance_criteria, priority, sort_order }) => - withToolErrors(async () => { - const auth = await requireWriteAccess() - - const pbi = await prisma.pbi.findUnique({ - where: { id: pbi_id }, - select: { product_id: true }, - }) - if (!pbi) return toolError(`PBI ${pbi_id} not found`) - if (!(await userCanAccessProduct(pbi.product_id, auth.userId))) { - return toolError(`PBI ${pbi_id} not accessible`) - } - - let resolvedSortOrder = sort_order - if (resolvedSortOrder === undefined) { - const last = await prisma.story.findFirst({ - where: { pbi_id, priority }, - orderBy: { sort_order: 'desc' }, - select: { sort_order: true }, - }) - resolvedSortOrder = (last?.sort_order ?? 0) + 1.0 - } - - let lastError: unknown - for (let attempt = 0; attempt < MAX_CODE_ATTEMPTS; attempt++) { - const code = await generateNextStoryCode(pbi.product_id) - try { - const story = await prisma.story.create({ - data: { - pbi_id, - product_id: pbi.product_id, // denormalized uit DB-parent, niet uit input - code, - title, - description: description ?? null, - acceptance_criteria: acceptance_criteria ?? null, - priority, - sort_order: resolvedSortOrder, - status: 'OPEN', - }, - select: { - id: true, - code: true, - title: true, - description: true, - acceptance_criteria: true, - priority: true, - sort_order: true, - status: true, - created_at: true, - }, - }) - return toolJson(story) - } catch (e) { - if (isCodeUniqueConflict(e)) { lastError = e; continue } - throw e - } - } - throw lastError ?? new Error('Kon geen unieke Story-code genereren') - }), + handleCreateStory, ) } diff --git a/src/tools/update-idea-plan-reviewed.ts b/src/tools/update-idea-plan-reviewed.ts new file mode 100644 index 0000000..0217c22 --- /dev/null +++ b/src/tools/update-idea-plan-reviewed.ts @@ -0,0 +1,116 @@ +// MCP-tool: writes the review-log result after a IDEA_REVIEW_PLAN grill-job +// and transitions the idea.status to PLAN_REVIEWED (on success) or +// PLAN_REVIEW_FAILED (on failure). +// +// Called by the worker as the final step of a review-plan session. + +import { z } from 'zod' +import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js' + +import { prisma } from '../prisma.js' +import { requireWriteAccess } from '../auth.js' +import { userOwnsIdea } from '../access.js' +import { toolError, toolJson, withToolErrors } from '../errors.js' + +const inputSchema = z.object({ + idea_id: z.string().min(1), + review_log: z.object({}).passthrough(), // Full ReviewLog from orchestrator (JSON object) + approval_status: z + .enum(['pending', 'approved', 'rejected'] as const) + .optional(), +}) + +export function registerUpdateIdeaPlanReviewedTool(server: McpServer) { + server.registerTool( + 'update_idea_plan_reviewed', + { + title: 'Mark plan as reviewed', + description: + 'Save review-log after plan review cycle and transition idea.status to PLAN_REVIEWED (if approved) or PLAN_REVIEW_FAILED (if rejected/pending requires manual approval). Forbidden for demo accounts.', + inputSchema, + }, + async ({ idea_id, review_log, approval_status }) => + withToolErrors(async () => { + const auth = await requireWriteAccess() + if (!(await userOwnsIdea(idea_id, auth.userId))) { + return toolError('Idea not found') + } + + // Determine target status based on approval + const nextStatus = + approval_status === 'approved' + ? 'PLAN_REVIEWED' + : approval_status === 'rejected' + ? 'PLAN_REVIEW_FAILED' + : 'PLAN_REVIEWED' // Default to approved if not specified + + // Log summary metrics from review_log + const logSummary = buildReviewLogSummary(review_log) + + const result = await prisma.$transaction([ + prisma.idea.update({ + where: { id: idea_id }, + data: { + plan_review_log: review_log as any, + reviewed_at: new Date(), + status: nextStatus, + }, + select: { id: true, status: true, code: true }, + }), + prisma.ideaLog.create({ + data: { + idea_id, + type: 'PLAN_REVIEW_RESULT', + content: logSummary.summary, + metadata: { + approval_status, + convergence_status: logSummary.convergence_status, + final_score: logSummary.final_score, + rounds_completed: logSummary.rounds_completed, + }, + }, + }), + ]) + + return toolJson({ + ok: true, + idea: result[0], + review_log_summary: logSummary, + }) + }), + ) +} + +function buildReviewLogSummary( + reviewLog: Record, +): { + summary: string + convergence_status: string + final_score: number + rounds_completed: number +} { + const rounds = Array.isArray(reviewLog.rounds) ? reviewLog.rounds : [] + const convergence = reviewLog.convergence || {} + const finalScore = + rounds.length > 0 ? rounds[rounds.length - 1].score ?? 0 : 0 + + const convergenceStatus = + convergence.stable_at_round !== undefined + ? `stable at round ${convergence.stable_at_round}` + : convergence.final_diff_pct !== undefined + ? `${convergence.final_diff_pct}% diff` + : 'pending' + + const summary = + `Plan reviewed in ${rounds.length} rounds. ` + + `Convergence: ${convergenceStatus}. ` + + `Final score: ${finalScore}/100. ` + + `Status: ${reviewLog.approval?.status || 'pending'}.` + + return { + summary, + convergence_status: convergenceStatus, + final_score: finalScore, + rounds_completed: rounds.length, + } +} diff --git a/src/tools/wait-for-job.ts b/src/tools/wait-for-job.ts index 96c11ba..f3e11c0 100644 --- a/src/tools/wait-for-job.ts +++ b/src/tools/wait-for-job.ts @@ -508,7 +508,7 @@ export async function getFullJobContext(jobId: string) { // M12: branch on kind. Idea-jobs hebben geen task/story/pbi/sprint; ze // hebben in plaats daarvan idea + embedded prompt_text. - if (job.kind === 'IDEA_GRILL' || job.kind === 'IDEA_MAKE_PLAN') { + if (job.kind === 'IDEA_GRILL' || job.kind === 'IDEA_MAKE_PLAN' || job.kind === 'IDEA_REVIEW_PLAN') { if (!job.idea) return null const { idea } = job const { getIdeaPromptText } = await import('../lib/kind-prompts.js') @@ -569,7 +569,11 @@ export async function getFullJobContext(jobId: string) { pbi: idea.pbi, repo_url: job.product.repo_url, prompt_text: getIdeaPromptText(job.kind), - branch_suggestion: `feat/idea-${idea.code.toLowerCase()}-${job.kind === 'IDEA_GRILL' ? 'grill' : 'plan'}`, + branch_suggestion: `feat/idea-${idea.code.toLowerCase()}-${(() => { + if (job.kind === 'IDEA_GRILL') return 'grill' + if (job.kind === 'IDEA_REVIEW_PLAN') return 'review' + return 'plan' + })()}`, product_worktrees: worktrees.map((w) => ({ product_id: w.productId, worktree_path: w.worktreePath, diff --git a/vendor/scrum4me b/vendor/scrum4me index 3c77342..7bb252c 160000 --- a/vendor/scrum4me +++ b/vendor/scrum4me @@ -1 +1 @@ -Subproject commit 3c773421dacaf506bf35a8270249822cf509ccf3 +Subproject commit 7bb252c528d810584bcb46a56cff3d26ebf392ff From 84c194d4e52840379361f0c23e5d8bc88e612206 Mon Sep 17 00:00:00 2001 From: Janpeter Visser <30029041+madhura68@users.noreply.github.com> Date: Thu, 14 May 2026 19:16:15 +0200 Subject: [PATCH 6/8] fix(cross-repo): per-repo worktree-branch + PR resolutie (IDEA-062) (#49) Cross-repo sprints (sprint-product = repo X, maar een taak heeft task.repo_url naar repo Y) faalden op twee plekken omdat sprint-brede beslissingen werden toegepast op per-repo git-state. 1. createWorktreeForJob (src/git/worktree.ts) reuseBranch wordt sprint-breed bepaald in wait-for-job.ts. De eerste job die repo Y target krijgt reuseBranch=true terwijl de branch daar nooit is aangemaakt -> `git worktree add ` faalt met "invalid reference" -> job vast, worker UNHEALTHY. Idem na een container-recreate (clone is dan vers). Fix: 3-weg fallback in het reuseBranch-pad: - lokale branch bestaat -> hergebruik - alleen op origin -> recreate lokaal vanaf origin/ - nergens -> fresh vanaf baseRef Lost ook het container-recreate-verlies op. 2. maybeCreateAutoPr (src/tools/update-job-status.ts) De sprint/story sibling-lookup voor pr_url-hergebruik filterde niet op repo. Een repo-Y-job erfde de pr_url van een repo-X-sibling -> job.pr_url wees naar de verkeerde repo en er werd nooit een PR voor de repo-Y-branch aangemaakt (branch wel gepusht, maar PR-loos). Fix: siblings groeperen per repo-bucket ((task.repo_url ?? null)); alleen een sibling uit dezelfde bucket levert een herbruikbare pr_url. Geldt voor SPRINT- en STORY-mode. createPullRequest zelf was al repo-correct (gh pr create draait in de worktree). Tests: 3 nieuwe in worktree.test.ts (reuse-local / recreate-from-origin / fresh-fallback), 2 nieuwe in update-job-status-auto-pr.test.ts (cross-repo story + sprint). update-job-status-mock omgezet naar findMany. Alle 373 tests groen, build groen. package-lock.json: version 0.7.0 -> 0.8.0 (was niet mee-gesynced in de v0.8.0-bump commit 55fa133). Co-authored-by: Claude Opus 4.7 (1M context) --- __tests__/git/worktree.test.ts | 65 +++++++++++++++++++++ __tests__/update-job-status-auto-pr.test.ts | 51 ++++++++++++++-- package-lock.json | 4 +- src/git/worktree.ts | 35 ++++++++++- src/tools/update-job-status.ts | 31 +++++++--- 5 files changed, 171 insertions(+), 15 deletions(-) diff --git a/__tests__/git/worktree.test.ts b/__tests__/git/worktree.test.ts index 68f5e19..68cedfd 100644 --- a/__tests__/git/worktree.test.ts +++ b/__tests__/git/worktree.test.ts @@ -113,6 +113,71 @@ describe('createWorktreeForJob', () => { }), ).rejects.toThrow('Worktree path already exists') }) + + it('reuseBranch: reuses an existing local branch', async () => { + const { repoDir, originDir } = await setupRepo() + tmpDirs.push(repoDir, originDir) + await makeWorktreeParent() + + // Sibling already created the branch locally. + await git(['branch', 'feat/sprint-abc', 'origin/main'], repoDir) + + const result = await createWorktreeForJob({ + repoRoot: repoDir, + jobId: 'job-reuse-local', + branchName: 'feat/sprint-abc', + baseRef: 'origin/main', + reuseBranch: true, + }) + + const { stdout } = await git(['rev-parse', '--abbrev-ref', 'HEAD'], result.worktreePath) + expect(stdout.trim()).toBe('feat/sprint-abc') + expect(result.branchName).toBe('feat/sprint-abc') + }) + + it('reuseBranch: recreates a local branch from origin when only the remote has it', async () => { + const { repoDir, originDir } = await setupRepo() + tmpDirs.push(repoDir, originDir) + await makeWorktreeParent() + + // Branch exists on origin (a sibling pushed it, or the container was + // recreated and the local clone is fresh) but not as a local branch. + await git(['branch', 'feat/sprint-xyz', 'origin/main'], repoDir) + await git(['push', 'origin', 'feat/sprint-xyz'], repoDir) + await git(['branch', '-D', 'feat/sprint-xyz'], repoDir) + + const result = await createWorktreeForJob({ + repoRoot: repoDir, + jobId: 'job-reuse-origin', + branchName: 'feat/sprint-xyz', + baseRef: 'origin/main', + reuseBranch: true, + }) + + const { stdout } = await git(['rev-parse', '--abbrev-ref', 'HEAD'], result.worktreePath) + expect(stdout.trim()).toBe('feat/sprint-xyz') + }) + + it('reuseBranch: falls back to a fresh branch when it exists nowhere (cross-repo sprint)', async () => { + const { repoDir, originDir } = await setupRepo() + tmpDirs.push(repoDir, originDir) + await makeWorktreeParent() + + // reuseBranch is decided sprint-wide; for the first job targeting THIS + // repo the branch exists neither locally nor on origin. Must not throw + // "invalid reference" — should create it fresh from baseRef. + const result = await createWorktreeForJob({ + repoRoot: repoDir, + jobId: 'job-reuse-fresh', + branchName: 'feat/sprint-newrepo', + baseRef: 'origin/main', + reuseBranch: true, + }) + + const { stdout } = await git(['rev-parse', '--abbrev-ref', 'HEAD'], result.worktreePath) + expect(stdout.trim()).toBe('feat/sprint-newrepo') + expect(result.branchName).toBe('feat/sprint-newrepo') + }) }) describe('removeWorktreeForJob', () => { diff --git a/__tests__/update-job-status-auto-pr.test.ts b/__tests__/update-job-status-auto-pr.test.ts index 3218b3e..e92fdb3 100644 --- a/__tests__/update-job-status-auto-pr.test.ts +++ b/__tests__/update-job-status-auto-pr.test.ts @@ -4,7 +4,7 @@ vi.mock('../src/prisma.js', () => ({ prisma: { product: { findUnique: vi.fn() }, task: { findUnique: vi.fn() }, - claudeJob: { findFirst: vi.fn(), findUnique: vi.fn() }, + claudeJob: { findFirst: vi.fn(), findMany: vi.fn(), findUnique: vi.fn() }, }, })) @@ -22,6 +22,7 @@ const mockPrisma = prisma as unknown as { task: { findUnique: ReturnType } claudeJob: { findFirst: ReturnType + findMany: ReturnType findUnique: ReturnType } } @@ -41,9 +42,10 @@ beforeEach(() => { mockPrisma.product.findUnique.mockResolvedValue({ auto_pr: true }) mockPrisma.task.findUnique.mockResolvedValue({ title: 'Add feature', + repo_url: null, story: { id: 'story-1', code: 'SCRUM-42', title: 'Story title' }, }) - mockPrisma.claudeJob.findFirst.mockResolvedValue(null) // no sibling PR by default + mockPrisma.claudeJob.findMany.mockResolvedValue([]) // no sibling PRs by default // Default: legacy job zonder sprint_run (STORY-mode pad). mockPrisma.claudeJob.findUnique.mockResolvedValue({ sprint_run_id: null, sprint_run: null }) mockCreatePr.mockResolvedValue({ url: 'https://github.com/org/repo/pull/99' }) @@ -62,12 +64,27 @@ describe('maybeCreateAutoPr', () => { }) it('reuses sibling pr_url when another job in same story already opened a PR', async () => { - mockPrisma.claudeJob.findFirst.mockResolvedValue({ pr_url: 'https://github.com/org/repo/pull/77' }) + mockPrisma.claudeJob.findMany.mockResolvedValue([ + { pr_url: 'https://github.com/org/repo/pull/77', task: { repo_url: null } }, + ]) const url = await maybeCreateAutoPr(BASE_OPTS) expect(url).toBe('https://github.com/org/repo/pull/77') expect(mockCreatePr).not.toHaveBeenCalled() }) + it('does NOT reuse a sibling PR from a different repo (cross-repo story)', async () => { + // Sibling targeted another repo via task.repo_url — its PR must not leak in. + mockPrisma.claudeJob.findMany.mockResolvedValue([ + { + pr_url: 'https://github.com/org/other-repo/pull/12', + task: { repo_url: 'https://github.com/org/other-repo' }, + }, + ]) + const url = await maybeCreateAutoPr(BASE_OPTS) + expect(url).toBe('https://github.com/org/repo/pull/99') // fresh PR, not the sibling's + expect(mockCreatePr).toHaveBeenCalledOnce() + }) + it('returns null when auto_pr=false', async () => { mockPrisma.product.findUnique.mockResolvedValue({ auto_pr: false }) const url = await maybeCreateAutoPr(BASE_OPTS) @@ -78,6 +95,7 @@ describe('maybeCreateAutoPr', () => { it('uses story title without code prefix when story has no code', async () => { mockPrisma.task.findUnique.mockResolvedValue({ title: 'Add feature', + repo_url: null, story: { id: 'story-1', code: null, title: 'Story title' }, }) await maybeCreateAutoPr(BASE_OPTS) @@ -113,7 +131,9 @@ describe('maybeCreateAutoPr', () => { sprint_run_id: 'run-1', sprint_run: { id: 'run-1', pr_strategy: 'SPRINT', sprint: { sprint_goal: 'Goal' } }, }) - mockPrisma.claudeJob.findFirst.mockResolvedValue({ pr_url: 'https://github.com/org/repo/pull/55' }) + mockPrisma.claudeJob.findMany.mockResolvedValue([ + { pr_url: 'https://github.com/org/repo/pull/55', task: { repo_url: null } }, + ]) const url = await maybeCreateAutoPr(BASE_OPTS) @@ -121,6 +141,29 @@ describe('maybeCreateAutoPr', () => { expect(mockCreatePr).not.toHaveBeenCalled() }) + it('SPRINT-mode: cross-repo — sibling-PR van ander repo wordt niet hergebruikt', async () => { + mockPrisma.claudeJob.findUnique.mockResolvedValue({ + sprint_run_id: 'run-1', + sprint_run: { id: 'run-1', pr_strategy: 'SPRINT', sprint: { sprint_goal: 'Goal' } }, + }) + // Deze job target een ander repo via task.repo_url. + mockPrisma.task.findUnique.mockResolvedValue({ + title: 'MCP-taak', + repo_url: 'https://github.com/org/scrum4me-mcp', + story: { id: 'story-1', code: 'SCRUM-9', title: 'Story title' }, + }) + // Sibling met pr_url hoort bij het product-repo (repo_url null) → andere bucket. + mockPrisma.claudeJob.findMany.mockResolvedValue([ + { pr_url: 'https://github.com/org/repo/pull/201', task: { repo_url: null } }, + ]) + + const url = await maybeCreateAutoPr(BASE_OPTS) + + // Geen hergebruik van de product-repo PR → eigen draft-PR voor het mcp-repo. + expect(url).toBe('https://github.com/org/repo/pull/99') + expect(mockCreatePr).toHaveBeenCalledOnce() + }) + it('returns null and does not throw when gh fails', async () => { mockCreatePr.mockResolvedValue({ error: 'gh CLI not found' }) const url = await maybeCreateAutoPr(BASE_OPTS) diff --git a/package-lock.json b/package-lock.json index 61bcb4a..3514598 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "scrum4me-mcp", - "version": "0.7.0", + "version": "0.8.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "scrum4me-mcp", - "version": "0.7.0", + "version": "0.8.0", "hasInstallScript": true, "license": "MIT", "dependencies": { diff --git a/src/git/worktree.ts b/src/git/worktree.ts index 4d03443..a27aca6 100644 --- a/src/git/worktree.ts +++ b/src/git/worktree.ts @@ -15,6 +15,19 @@ async function branchExists(repoRoot: string, name: string): Promise { } } +async function remoteBranchExists(repoRoot: string, name: string): Promise { + try { + await exec( + 'git', + ['show-ref', '--verify', '--quiet', `refs/remotes/origin/${name}`], + { cwd: repoRoot }, + ) + return true + } catch { + return false + } +} + async function findWorktreeForBranch( repoRoot: string, branchName: string, @@ -75,7 +88,27 @@ export async function createWorktreeForJob(opts: { if (occupant) { await exec('git', ['worktree', 'remove', '--force', occupant], { cwd: repoRoot }) } - await exec('git', ['worktree', 'add', worktreePath, branchName], { cwd: repoRoot }) + // reuseBranch is decided sprint-wide, but git branches are per-repo. For a + // cross-repo sprint the first job targeting THIS repo gets reuseBranch=true + // even though the branch was never created here; a container recreate also + // wipes the local clone. Fall back gracefully instead of failing with + // "invalid reference": + // - local branch exists → reuse it + // - exists on origin only → recreate the local branch tracking origin + // - nowhere → create it fresh from baseRef + if (await branchExists(repoRoot, branchName)) { + await exec('git', ['worktree', 'add', worktreePath, branchName], { cwd: repoRoot }) + } else if (await remoteBranchExists(repoRoot, branchName)) { + await exec( + 'git', + ['worktree', 'add', '-b', branchName, worktreePath, `origin/${branchName}`], + { cwd: repoRoot }, + ) + } else { + await exec('git', ['worktree', 'add', '-b', branchName, worktreePath, baseRef], { + cwd: repoRoot, + }) + } return { worktreePath, branchName } } diff --git a/src/tools/update-job-status.ts b/src/tools/update-job-status.ts index 5e40988..e7a9495 100644 --- a/src/tools/update-job-status.ts +++ b/src/tools/update-job-status.ts @@ -420,24 +420,35 @@ export async function maybeCreateAutoPr(opts: { where: { id: taskId }, select: { title: true, + repo_url: true, story: { select: { id: true, code: true, title: true } }, }, }) if (!task) return null - // PBI-46 SPRINT-mode: hergebruik 1 draft-PR voor de hele SprintRun. + // Cross-repo sprints: een sprint kan taken hebben die via task.repo_url een + // ander repo targeten. PRs en branches zijn per-repo, dus een sibling-PR mag + // alleen hergebruikt worden als die sibling hetzelfde repo targette. null/leeg + // repo_url = het product-repo; twee taken zitten in dezelfde repo-bucket als + // hun (repo_url ?? null) gelijk is. + const thisRepoKey = task.repo_url ?? null + + // PBI-46 SPRINT-mode: hergebruik 1 draft-PR voor de hele SprintRun (per repo). // Mens zet 'm ready-for-review zodra de SprintRun DONE is. if (job?.sprint_run && job.sprint_run.pr_strategy === 'SPRINT') { - const sprintSibling = await prisma.claudeJob.findFirst({ + const sprintSiblings = await prisma.claudeJob.findMany({ where: { sprint_run_id: job.sprint_run_id, pr_url: { not: null }, id: { not: jobId }, }, - select: { pr_url: true }, + select: { pr_url: true, task: { select: { repo_url: true } } }, orderBy: { created_at: 'asc' }, }) - if (sprintSibling?.pr_url) return sprintSibling.pr_url + const sameRepoSibling = sprintSiblings.find( + (s) => (s.task?.repo_url ?? null) === thisRepoKey, + ) + if (sameRepoSibling?.pr_url) return sameRepoSibling.pr_url // Eerste DONE in deze SprintRun → maak draft-PR aan, geen auto-merge. const goal = job.sprint_run.sprint.sprint_goal @@ -459,17 +470,21 @@ export async function maybeCreateAutoPr(opts: { return null } - // STORY-mode (default of legacy): branch-per-story, sibling-tasks delen PR. - const sibling = await prisma.claudeJob.findFirst({ + // STORY-mode (default of legacy): branch-per-story, sibling-tasks delen PR + // — maar alleen siblings die hetzelfde repo targeten (zie thisRepoKey). + const storySiblings = await prisma.claudeJob.findMany({ where: { task: { story_id: task.story.id }, pr_url: { not: null }, id: { not: jobId }, }, - select: { pr_url: true }, + select: { pr_url: true, task: { select: { repo_url: true } } }, orderBy: { created_at: 'asc' }, }) - if (sibling?.pr_url) return sibling.pr_url + const sameRepoStorySibling = storySiblings.find( + (s) => (s.task?.repo_url ?? null) === thisRepoKey, + ) + if (sameRepoStorySibling?.pr_url) return sameRepoStorySibling.pr_url const storyTitle = task.story.code ? `${task.story.code}: ${task.story.title}` : task.story.title const body = summary From 51fc65e71548ede77435c590b7c7df6ea2434c45 Mon Sep 17 00:00:00 2001 From: Janpeter Visser <30029041+madhura68@users.noreply.github.com> Date: Thu, 14 May 2026 19:46:31 +0200 Subject: [PATCH 7/8] fix(update_idea_plan_reviewed): nooit stilzwijgend goedkeuren (IDEA-066) (#50) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit De status-logica sprak z'n eigen tool-beschrijving tegen. De code deed: approved -> PLAN_REVIEWED rejected -> PLAN_REVIEW_FAILED else -> PLAN_REVIEWED // "Default to approved if not specified" Een review die 'pending' (needs manual approval) of helemaal geen approval_status teruggaf, markeerde het idee dus als PLAN_REVIEWED (goedgekeurd) — precies omgekeerd aan wat de beschrijving belooft. Fix: alleen een expliciete approval_status='approved' brengt het idee naar PLAN_REVIEWED; 'rejected', 'pending' én een weggelaten approval_status gaan allemaal naar PLAN_REVIEW_FAILED (mens beslist). Nooit stilzwijgend goedkeuren. Verder: - Handler geextraheerd naar handleUpdateIdeaPlanReviewed + inputSchema geexporteerd, conform het create-sprint/update-sprint-patroon, zodat de logica zonder McpServer-wrapper testbaar is. - Tool-beschrijving + header-comment aangescherpt zodat code en docs niet meer divergeren. - Nieuw test-bestand: 6 tests (approved/rejected/pending/omitted status-transitie, not-found, log-persistentie). Build groen, 379 tests groen. Co-authored-by: Claude Opus 4.7 (1M context) --- __tests__/update-idea-plan-reviewed.test.ts | 140 ++++++++++++++++++++ src/tools/update-idea-plan-reviewed.ts | 118 +++++++++-------- 2 files changed, 204 insertions(+), 54 deletions(-) create mode 100644 __tests__/update-idea-plan-reviewed.test.ts diff --git a/__tests__/update-idea-plan-reviewed.test.ts b/__tests__/update-idea-plan-reviewed.test.ts new file mode 100644 index 0000000..257fce4 --- /dev/null +++ b/__tests__/update-idea-plan-reviewed.test.ts @@ -0,0 +1,140 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest' + +vi.mock('../src/prisma.js', () => ({ + prisma: { + idea: { update: vi.fn() }, + ideaLog: { create: vi.fn() }, + $transaction: vi.fn(), + }, +})) + +vi.mock('../src/auth.js', () => ({ + requireWriteAccess: vi.fn(), + PermissionDeniedError: class PermissionDeniedError extends Error { + constructor(message = 'Demo accounts cannot perform write operations') { + super(message) + this.name = 'PermissionDeniedError' + } + }, +})) + +vi.mock('../src/access.js', () => ({ + userOwnsIdea: vi.fn(), +})) + +import { prisma } from '../src/prisma.js' +import { requireWriteAccess } from '../src/auth.js' +import { userOwnsIdea } from '../src/access.js' +import { handleUpdateIdeaPlanReviewed } from '../src/tools/update-idea-plan-reviewed.js' + +const mockPrisma = prisma as unknown as { + idea: { update: ReturnType } + ideaLog: { create: ReturnType } + $transaction: ReturnType +} +const mockRequireWriteAccess = requireWriteAccess as ReturnType +const mockUserOwnsIdea = userOwnsIdea as ReturnType + +const IDEA_ID = 'idea-1' +const USER_ID = 'user-1' +const REVIEW_LOG = { + rounds: [{ score: 88 }], + convergence: { stable_at_round: 2 }, + approval: { status: 'approved' }, +} + +beforeEach(() => { + vi.clearAllMocks() + mockRequireWriteAccess.mockResolvedValue({ + userId: USER_ID, + tokenId: 'tok-1', + username: 'alice', + isDemo: false, + }) + mockUserOwnsIdea.mockResolvedValue(true) + // $transaction returns the array of its two operations' results; the handler + // only reads result[0] (the idea.update result). + mockPrisma.$transaction.mockImplementation(async () => [ + { id: IDEA_ID, status: 'PLACEHOLDER', code: 'IDEA-1' }, + {}, + ]) +}) + +function parseResult(result: Awaited>) { + const text = result.content?.[0]?.type === 'text' ? result.content[0].text : '' + try { + return JSON.parse(text) + } catch { + return text + } +} + +// The handler builds `data.status` inside the idea.update call passed to +// $transaction. We capture it by inspecting the prisma.idea.update mock args. +function statusPassedToUpdate(): string | undefined { + const call = mockPrisma.idea.update.mock.calls[0] + return call?.[0]?.data?.status +} + +describe('handleUpdateIdeaPlanReviewed — status transition', () => { + it('approval_status="approved" → PLAN_REVIEWED', async () => { + await handleUpdateIdeaPlanReviewed({ + idea_id: IDEA_ID, + review_log: REVIEW_LOG, + approval_status: 'approved', + }) + expect(statusPassedToUpdate()).toBe('PLAN_REVIEWED') + }) + + it('approval_status="rejected" → PLAN_REVIEW_FAILED', async () => { + await handleUpdateIdeaPlanReviewed({ + idea_id: IDEA_ID, + review_log: REVIEW_LOG, + approval_status: 'rejected', + }) + expect(statusPassedToUpdate()).toBe('PLAN_REVIEW_FAILED') + }) + + it('approval_status="pending" → PLAN_REVIEW_FAILED (needs manual approval, never silently approved)', async () => { + await handleUpdateIdeaPlanReviewed({ + idea_id: IDEA_ID, + review_log: REVIEW_LOG, + approval_status: 'pending', + }) + expect(statusPassedToUpdate()).toBe('PLAN_REVIEW_FAILED') + }) + + it('omitted approval_status → PLAN_REVIEW_FAILED (safe default, not PLAN_REVIEWED)', async () => { + await handleUpdateIdeaPlanReviewed({ + idea_id: IDEA_ID, + review_log: REVIEW_LOG, + }) + expect(statusPassedToUpdate()).toBe('PLAN_REVIEW_FAILED') + }) + + it('returns "Idea not found" when the user does not own the idea', async () => { + mockUserOwnsIdea.mockResolvedValue(false) + const result = await handleUpdateIdeaPlanReviewed({ + idea_id: IDEA_ID, + review_log: REVIEW_LOG, + approval_status: 'approved', + }) + expect(parseResult(result)).toContain('Idea not found') + expect(mockPrisma.idea.update).not.toHaveBeenCalled() + }) + + it('persists review_log + reviewed_at and logs a PLAN_REVIEW_RESULT entry', async () => { + await handleUpdateIdeaPlanReviewed({ + idea_id: IDEA_ID, + review_log: REVIEW_LOG, + approval_status: 'approved', + }) + const updateArg = mockPrisma.idea.update.mock.calls[0]?.[0] + expect(updateArg?.data?.plan_review_log).toEqual(REVIEW_LOG) + expect(updateArg?.data?.reviewed_at).toBeInstanceOf(Date) + + const logArg = mockPrisma.ideaLog.create.mock.calls[0]?.[0] + expect(logArg?.data?.type).toBe('PLAN_REVIEW_RESULT') + expect(logArg?.data?.idea_id).toBe(IDEA_ID) + }) +}) diff --git a/src/tools/update-idea-plan-reviewed.ts b/src/tools/update-idea-plan-reviewed.ts index 0217c22..2e9f1ac 100644 --- a/src/tools/update-idea-plan-reviewed.ts +++ b/src/tools/update-idea-plan-reviewed.ts @@ -1,6 +1,8 @@ -// MCP-tool: writes the review-log result after a IDEA_REVIEW_PLAN grill-job -// and transitions the idea.status to PLAN_REVIEWED (on success) or -// PLAN_REVIEW_FAILED (on failure). +// MCP-tool: writes the review-log result after an IDEA_REVIEW_PLAN job and +// transitions idea.status. Only an explicit approval_status='approved' moves +// the idea to PLAN_REVIEWED; anything else (rejected, pending, or omitted) +// goes to PLAN_REVIEW_FAILED — a human must then decide. The tool never +// silently approves. // // Called by the worker as the final step of a review-plan session. @@ -12,7 +14,7 @@ import { requireWriteAccess } from '../auth.js' import { userOwnsIdea } from '../access.js' import { toolError, toolJson, withToolErrors } from '../errors.js' -const inputSchema = z.object({ +export const inputSchema = z.object({ idea_id: z.string().min(1), review_log: z.object({}).passthrough(), // Full ReviewLog from orchestrator (JSON object) approval_status: z @@ -20,64 +22,72 @@ const inputSchema = z.object({ .optional(), }) +export async function handleUpdateIdeaPlanReviewed( + { idea_id, review_log, approval_status }: z.infer, +) { + return withToolErrors(async () => { + const auth = await requireWriteAccess() + if (!(await userOwnsIdea(idea_id, auth.userId))) { + return toolError('Idea not found') + } + + // Alleen een expliciete 'approved' brengt het idee naar PLAN_REVIEWED. + // 'rejected', 'pending' én een weggelaten approval_status betekenen + // allemaal "niet auto-goedgekeurd — mens moet beslissen" en gaan naar + // PLAN_REVIEW_FAILED. Nooit stilzwijgend goedkeuren (de vorige + // `: 'PLAN_REVIEWED'`-default deed dat wel bij pending/undefined). + const nextStatus = + approval_status === 'approved' ? 'PLAN_REVIEWED' : 'PLAN_REVIEW_FAILED' + + // Log summary metrics from review_log + const logSummary = buildReviewLogSummary(review_log) + + const result = await prisma.$transaction([ + prisma.idea.update({ + where: { id: idea_id }, + data: { + plan_review_log: review_log as any, + reviewed_at: new Date(), + status: nextStatus, + }, + select: { id: true, status: true, code: true }, + }), + prisma.ideaLog.create({ + data: { + idea_id, + type: 'PLAN_REVIEW_RESULT', + content: logSummary.summary, + metadata: { + approval_status, + convergence_status: logSummary.convergence_status, + final_score: logSummary.final_score, + rounds_completed: logSummary.rounds_completed, + }, + }, + }), + ]) + + return toolJson({ + ok: true, + idea: result[0], + review_log_summary: logSummary, + }) + }) +} + export function registerUpdateIdeaPlanReviewedTool(server: McpServer) { server.registerTool( 'update_idea_plan_reviewed', { title: 'Mark plan as reviewed', description: - 'Save review-log after plan review cycle and transition idea.status to PLAN_REVIEWED (if approved) or PLAN_REVIEW_FAILED (if rejected/pending requires manual approval). Forbidden for demo accounts.', + 'Save review-log after a plan review cycle and transition idea.status. ' + + 'Only approval_status="approved" → PLAN_REVIEWED; "rejected", "pending", ' + + 'or an omitted approval_status → PLAN_REVIEW_FAILED (needs manual ' + + 'approval — never silently approved). Forbidden for demo accounts.', inputSchema, }, - async ({ idea_id, review_log, approval_status }) => - withToolErrors(async () => { - const auth = await requireWriteAccess() - if (!(await userOwnsIdea(idea_id, auth.userId))) { - return toolError('Idea not found') - } - - // Determine target status based on approval - const nextStatus = - approval_status === 'approved' - ? 'PLAN_REVIEWED' - : approval_status === 'rejected' - ? 'PLAN_REVIEW_FAILED' - : 'PLAN_REVIEWED' // Default to approved if not specified - - // Log summary metrics from review_log - const logSummary = buildReviewLogSummary(review_log) - - const result = await prisma.$transaction([ - prisma.idea.update({ - where: { id: idea_id }, - data: { - plan_review_log: review_log as any, - reviewed_at: new Date(), - status: nextStatus, - }, - select: { id: true, status: true, code: true }, - }), - prisma.ideaLog.create({ - data: { - idea_id, - type: 'PLAN_REVIEW_RESULT', - content: logSummary.summary, - metadata: { - approval_status, - convergence_status: logSummary.convergence_status, - final_score: logSummary.final_score, - rounds_completed: logSummary.rounds_completed, - }, - }, - }), - ]) - - return toolJson({ - ok: true, - idea: result[0], - review_log_summary: logSummary, - }) - }), + handleUpdateIdeaPlanReviewed, ) } From fba2d67796b10ed20c76b8eb74072d57ce2c9fa1 Mon Sep 17 00:00:00 2001 From: Janpeter Visser <30029041+madhura68@users.noreply.github.com> Date: Thu, 14 May 2026 23:21:44 +0200 Subject: [PATCH 8/8] fix(update_job_status): status-gedreven lifecycle-timestamps (#51) Een job kon CLAIMED -> done/failed/skipped gaan zonder ooit `running` te rapporteren, waardoor started_at NULL bleef terwijl finished_at wel gezet werd. Dat brak de invariant claimed_at <= started_at <= finished_at en elke duur-analyse. Nieuwe pure helper resolveJobTimestamps zet de lifecycle-timestamps set-once op basis van de status: started_at wordt gebackfild bij een terminale overgang, claimed_at defensief gevuld als die ontbreekt. De running-tak is nu set-once i.p.v. bij elke call overschrijven. Co-authored-by: Madhura68 Co-authored-by: Claude Opus 4.7 (1M context) --- .../update-job-status-timestamps.test.ts | 74 +++++++++++++++++++ src/tools/update-job-status.ts | 39 +++++++++- 2 files changed, 109 insertions(+), 4 deletions(-) create mode 100644 __tests__/update-job-status-timestamps.test.ts diff --git a/__tests__/update-job-status-timestamps.test.ts b/__tests__/update-job-status-timestamps.test.ts new file mode 100644 index 0000000..d4ab80f --- /dev/null +++ b/__tests__/update-job-status-timestamps.test.ts @@ -0,0 +1,74 @@ +// Unit-tests voor resolveJobTimestamps — de status-gedreven timestamp-helper +// van update_job_status. Pure functie, geen mocks (zoals update-job-status-gate). + +import { describe, it, expect } from 'vitest' +import { resolveJobTimestamps } from '../src/tools/update-job-status.js' + +const NOW = new Date('2026-05-14T12:00:00.000Z') +const EARLIER = new Date('2026-05-14T11:00:00.000Z') + +describe('resolveJobTimestamps', () => { + describe('running', () => { + it('sets started_at when not yet set, no finished_at', () => { + const r = resolveJobTimestamps('running', { claimed_at: EARLIER, started_at: null }, NOW) + expect(r.started_at).toBe(NOW) + expect(r.finished_at).toBeUndefined() + expect(r.claimed_at).toBeUndefined() + }) + + it('is set-once: does not re-stamp started_at when already set', () => { + const r = resolveJobTimestamps('running', { claimed_at: EARLIER, started_at: EARLIER }, NOW) + expect(r.started_at).toBeUndefined() + expect(r.finished_at).toBeUndefined() + expect(r.claimed_at).toBeUndefined() + }) + }) + + describe('terminal transitions (done/failed/skipped)', () => { + it.each(['done', 'failed', 'skipped'] as const)( + 'backfills started_at and sets finished_at for %s when started_at is null', + (status) => { + const r = resolveJobTimestamps(status, { claimed_at: EARLIER, started_at: null }, NOW) + expect(r.started_at).toBe(NOW) + expect(r.finished_at).toBe(NOW) + expect(r.claimed_at).toBeUndefined() + }, + ) + + it('only sets finished_at when started_at is already set', () => { + const r = resolveJobTimestamps('done', { claimed_at: EARLIER, started_at: EARLIER }, NOW) + expect(r.started_at).toBeUndefined() + expect(r.finished_at).toBe(NOW) + expect(r.claimed_at).toBeUndefined() + }) + }) + + describe('claimed_at backfill', () => { + it.each(['running', 'done', 'failed', 'skipped'] as const)( + 'backfills claimed_at for %s when it is null', + (status) => { + const r = resolveJobTimestamps(status, { claimed_at: null, started_at: null }, NOW) + expect(r.claimed_at).toBe(NOW) + }, + ) + + it('never returns claimed_at when it is already set', () => { + const r = resolveJobTimestamps('done', { claimed_at: EARLIER, started_at: EARLIER }, NOW) + expect(r.claimed_at).toBeUndefined() + }) + }) + + it('returns only finished_at when all timestamps are already set and status is terminal', () => { + const r = resolveJobTimestamps('failed', { claimed_at: EARLIER, started_at: EARLIER }, NOW) + expect(r).toEqual({ finished_at: NOW }) + }) + + it('defaults now to a fresh Date when omitted', () => { + const before = Date.now() + const r = resolveJobTimestamps('running', { claimed_at: EARLIER, started_at: null }) + const after = Date.now() + expect(r.started_at).toBeInstanceOf(Date) + expect(r.started_at!.getTime()).toBeGreaterThanOrEqual(before) + expect(r.started_at!.getTime()).toBeLessThanOrEqual(after) + }) +}) diff --git a/src/tools/update-job-status.ts b/src/tools/update-job-status.ts index e7a9495..9fcd08b 100644 --- a/src/tools/update-job-status.ts +++ b/src/tools/update-job-status.ts @@ -390,6 +390,32 @@ export function resolveNextAction( return queueCount > 0 ? 'wait_for_job_again' : 'queue_empty' } +export type JobTimestampUpdate = { + claimed_at?: Date + started_at?: Date + finished_at?: Date +} + +// Bepaalt welke lifecycle-timestamps update_job_status schrijft bij een +// status-overgang. Set-once (backfill alleen als nu null) houdt de invariant +// claimed_at ≤ started_at ≤ finished_at: een job die CLAIMED → done gaat +// zonder `running`-rapport krijgt alsnog een started_at, en claimed_at +// (normaal door wait_for_job bij claim gezet) wordt nooit overschreven. +export function resolveJobTimestamps( + status: 'running' | 'done' | 'failed' | 'skipped', + current: { claimed_at: Date | null; started_at: Date | null }, + now: Date = new Date(), +): JobTimestampUpdate { + const isTerminal = status === 'done' || status === 'failed' || status === 'skipped' + const update: JobTimestampUpdate = {} + if (current.claimed_at == null) update.claimed_at = now + if (current.started_at == null && (status === 'running' || isTerminal)) { + update.started_at = now + } + if (isTerminal) update.finished_at = now + return update +} + export async function maybeCreateAutoPr(opts: { jobId: string productId: string @@ -569,6 +595,8 @@ export function registerUpdateJobStatusTool(server: McpServer) { 'Report progress on a claimed ClaudeJob. Allowed transitions from CLAIMED/RUNNING: ' + 'running (start), done (finished), failed (error), skipped (no-op exit). ' + 'The Bearer token must match the token that claimed the job. ' + + 'Stamps started_at on running and finished_at on done/failed/skipped, and backfills ' + + 'claimed_at/started_at when missing so claimed_at ≤ started_at ≤ finished_at always holds. ' + 'Before marking done: call verify_task_against_plan first — done is rejected when ' + 'verify_result is null, EMPTY (unless task.verify_only is true), or when the verify level ' + 'doesn’t meet task.verify_required: ALIGNED-only is strict; ALIGNED_OR_PARTIAL accepts ' + @@ -608,6 +636,8 @@ export function registerUpdateJobStatusTool(server: McpServer) { select: { id: true, status: true, + claimed_at: true, + started_at: true, claimed_by_token_id: true, user_id: true, product_id: true, @@ -751,10 +781,11 @@ export function registerUpdateJobStatusTool(server: McpServer) { where: { id: job_id }, data: { status: dbStatus, - ...(actualStatus === 'running' ? { started_at: now } : {}), - ...(actualStatus === 'done' || actualStatus === 'failed' || actualStatus === 'skipped' - ? { finished_at: now } - : {}), + ...resolveJobTimestamps( + actualStatus, + { claimed_at: job.claimed_at, started_at: job.started_at }, + now, + ), ...(branchToWrite !== undefined ? { branch: branchToWrite } : {}), ...(pushedAt !== undefined ? { pushed_at: pushedAt } : {}), ...(summary !== undefined ? { summary } : {}),