diff --git a/__tests__/git/worktree.test.ts b/__tests__/git/worktree.test.ts index 68cedfd..68f5e19 100644 --- a/__tests__/git/worktree.test.ts +++ b/__tests__/git/worktree.test.ts @@ -113,71 +113,6 @@ 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-idea-plan-reviewed.test.ts b/__tests__/update-idea-plan-reviewed.test.ts deleted file mode 100644 index 257fce4..0000000 --- a/__tests__/update-idea-plan-reviewed.test.ts +++ /dev/null @@ -1,140 +0,0 @@ -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/__tests__/update-job-status-auto-pr.test.ts b/__tests__/update-job-status-auto-pr.test.ts index e92fdb3..3218b3e 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(), findMany: vi.fn(), findUnique: vi.fn() }, + claudeJob: { findFirst: vi.fn(), findUnique: vi.fn() }, }, })) @@ -22,7 +22,6 @@ const mockPrisma = prisma as unknown as { task: { findUnique: ReturnType } claudeJob: { findFirst: ReturnType - findMany: ReturnType findUnique: ReturnType } } @@ -42,10 +41,9 @@ 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.findMany.mockResolvedValue([]) // no sibling PRs by default + mockPrisma.claudeJob.findFirst.mockResolvedValue(null) // no sibling PR 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' }) @@ -64,27 +62,12 @@ describe('maybeCreateAutoPr', () => { }) it('reuses sibling pr_url when another job in same story already opened a PR', async () => { - mockPrisma.claudeJob.findMany.mockResolvedValue([ - { pr_url: 'https://github.com/org/repo/pull/77', task: { repo_url: null } }, - ]) + mockPrisma.claudeJob.findFirst.mockResolvedValue({ pr_url: 'https://github.com/org/repo/pull/77' }) 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) @@ -95,7 +78,6 @@ 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) @@ -131,9 +113,7 @@ describe('maybeCreateAutoPr', () => { sprint_run_id: 'run-1', sprint_run: { id: 'run-1', pr_strategy: 'SPRINT', sprint: { sprint_goal: 'Goal' } }, }) - mockPrisma.claudeJob.findMany.mockResolvedValue([ - { pr_url: 'https://github.com/org/repo/pull/55', task: { repo_url: null } }, - ]) + mockPrisma.claudeJob.findFirst.mockResolvedValue({ pr_url: 'https://github.com/org/repo/pull/55' }) const url = await maybeCreateAutoPr(BASE_OPTS) @@ -141,29 +121,6 @@ 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/__tests__/update-job-status-timestamps.test.ts b/__tests__/update-job-status-timestamps.test.ts deleted file mode 100644 index d4ab80f..0000000 --- a/__tests__/update-job-status-timestamps.test.ts +++ /dev/null @@ -1,74 +0,0 @@ -// 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/package-lock.json b/package-lock.json index 3514598..61bcb4a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "scrum4me-mcp", - "version": "0.8.0", + "version": "0.7.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "scrum4me-mcp", - "version": "0.8.0", + "version": "0.7.0", "hasInstallScript": true, "license": "MIT", "dependencies": { diff --git a/src/git/worktree.ts b/src/git/worktree.ts index a27aca6..4d03443 100644 --- a/src/git/worktree.ts +++ b/src/git/worktree.ts @@ -15,19 +15,6 @@ 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, @@ -88,27 +75,7 @@ export async function createWorktreeForJob(opts: { if (occupant) { await exec('git', ['worktree', 'remove', '--force', occupant], { 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, - }) - } + await exec('git', ['worktree', 'add', worktreePath, branchName], { cwd: repoRoot }) return { worktreePath, branchName } } diff --git a/src/lib/code.ts b/src/lib/code.ts new file mode 100644 index 0000000..6b7376d --- /dev/null +++ b/src/lib/code.ts @@ -0,0 +1,10 @@ +// Sync met Scrum4Me/lib/code.ts — bewust duplicate (geen gedeeld package) +// om de MCP-server eigenstandig te houden. +// +// Extraheert het achterste getal uit een code-string (bijv. "ST-001" → 1, +// "T-42" → 42). Gebruikt als sort_order bij create_story / create_task. +export function parseCodeNumber(code: string | null | undefined): number { + if (!code) return 0 + const m = code.match(/(\d+)$/) + return m ? Number.parseInt(m[1], 10) : 0 +} diff --git a/src/tools/create-story.ts b/src/tools/create-story.ts index 37caa59..2e419d4 100644 --- a/src/tools/create-story.ts +++ b/src/tools/create-story.ts @@ -12,6 +12,7 @@ import { prisma } from '../prisma.js' import { requireWriteAccess } from '../auth.js' import { userCanAccessProduct } from '../access.js' import { toolError, toolJson, withToolErrors } from '../errors.js' +import { parseCodeNumber } from '../lib/code.js' const STORY_AUTO_RE = /^ST-(\d+)$/ const MAX_CODE_ATTEMPTS = 3 @@ -46,7 +47,6 @@ const inputSchema = z.object({ description: z.string().max(4000).optional(), 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(), @@ -59,7 +59,6 @@ export async function handleCreateStory( description, acceptance_criteria, priority, - sort_order, sprint_id, }: z.infer, ) { @@ -90,19 +89,10 @@ export async function handleCreateStory( } } - 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) + const resolvedSortOrder = parseCodeNumber(code) try { const story = await prisma.story.create({ data: { @@ -146,7 +136,7 @@ export function registerCreateStoryTool(server: McpServer) { { title: 'Create story', description: - '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.', + '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 is derived from the auto-generated code number. Forbidden for demo accounts.', inputSchema, }, handleCreateStory, diff --git a/src/tools/create-task.ts b/src/tools/create-task.ts index 8308146..3882220 100644 --- a/src/tools/create-task.ts +++ b/src/tools/create-task.ts @@ -10,6 +10,7 @@ import { prisma } from '../prisma.js' import { requireWriteAccess } from '../auth.js' import { userCanAccessProduct } from '../access.js' import { toolError, toolJson, withToolErrors } from '../errors.js' +import { parseCodeNumber } from '../lib/code.js' const TASK_AUTO_RE = /^T-(\d+)$/ const MAX_CODE_ATTEMPTS = 3 @@ -44,7 +45,6 @@ const inputSchema = z.object({ description: z.string().max(4000).optional(), implementation_plan: z.string().max(8000).optional(), priority: z.number().int().min(1).max(4), - sort_order: z.number().optional(), // Cross-repo override: zet expliciet de repo waarop de worker deze task // moet uitvoeren (overrides product.repo_url). Gebruik dit voor PBI's die // werk in meerdere repos coördineren — bv. PBI op Scrum4Me-product met @@ -60,10 +60,10 @@ export function registerCreateTaskTool(server: McpServer) { { title: 'Create task', description: - 'Add a task under an existing story. Inherits sprint_id from the story (denormalized). Status defaults to TO_DO. Sort_order auto-set to last+1 within the story/priority group if not provided. Optional repo_url overrides the product.repo_url for cross-repo work (e.g. tasks targeting scrum4me-mcp under a Scrum4Me PBI). Forbidden for demo accounts.', + 'Add a task under an existing story. Inherits sprint_id from the story (denormalized). Status defaults to TO_DO. Sort_order is derived from the auto-generated code number. Optional repo_url overrides the product.repo_url for cross-repo work (e.g. tasks targeting scrum4me-mcp under a Scrum4Me PBI). Forbidden for demo accounts.', inputSchema, }, - async ({ story_id, title, description, implementation_plan, priority, sort_order, repo_url }) => + async ({ story_id, title, description, implementation_plan, priority, repo_url }) => withToolErrors(async () => { const auth = await requireWriteAccess() @@ -76,19 +76,10 @@ export function registerCreateTaskTool(server: McpServer) { return toolError(`Story ${story_id} not accessible`) } - let resolvedSortOrder = sort_order - if (resolvedSortOrder === undefined) { - const last = await prisma.task.findFirst({ - where: { story_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 generateNextTaskCode(story.product_id) + const resolvedSortOrder = parseCodeNumber(code) try { const task = await prisma.task.create({ data: { diff --git a/src/tools/get-claude-context.ts b/src/tools/get-claude-context.ts index 80f7a4c..2a94892 100644 --- a/src/tools/get-claude-context.ts +++ b/src/tools/get-claude-context.ts @@ -63,7 +63,7 @@ export function registerGetClaudeContextTool(server: McpServer) { { tasks: { some: { status: { not: 'DONE' } } } }, ], }, - orderBy: [{ priority: 'asc' }, { sort_order: 'asc' }], + orderBy: [{ sort_order: 'asc' }], select: { id: true, code: true, @@ -73,7 +73,7 @@ export function registerGetClaudeContextTool(server: McpServer) { priority: true, status: true, tasks: { - orderBy: [{ priority: 'asc' }, { sort_order: 'asc' }], + orderBy: [{ sort_order: 'asc' }], select: { id: true, title: true, diff --git a/src/tools/update-idea-plan-reviewed.ts b/src/tools/update-idea-plan-reviewed.ts index 2e9f1ac..0217c22 100644 --- a/src/tools/update-idea-plan-reviewed.ts +++ b/src/tools/update-idea-plan-reviewed.ts @@ -1,8 +1,6 @@ -// 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. +// 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. @@ -14,7 +12,7 @@ import { requireWriteAccess } from '../auth.js' import { userOwnsIdea } from '../access.js' import { toolError, toolJson, withToolErrors } from '../errors.js' -export const inputSchema = z.object({ +const inputSchema = z.object({ idea_id: z.string().min(1), review_log: z.object({}).passthrough(), // Full ReviewLog from orchestrator (JSON object) approval_status: z @@ -22,72 +20,64 @@ export 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 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.', + '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, }, - handleUpdateIdeaPlanReviewed, + 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, + }) + }), ) } diff --git a/src/tools/update-job-status.ts b/src/tools/update-job-status.ts index 9fcd08b..5e40988 100644 --- a/src/tools/update-job-status.ts +++ b/src/tools/update-job-status.ts @@ -390,32 +390,6 @@ 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 @@ -446,35 +420,24 @@ 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 - // 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). + // PBI-46 SPRINT-mode: hergebruik 1 draft-PR voor de hele SprintRun. // Mens zet 'm ready-for-review zodra de SprintRun DONE is. if (job?.sprint_run && job.sprint_run.pr_strategy === 'SPRINT') { - const sprintSiblings = await prisma.claudeJob.findMany({ + const sprintSibling = await prisma.claudeJob.findFirst({ where: { sprint_run_id: job.sprint_run_id, pr_url: { not: null }, id: { not: jobId }, }, - select: { pr_url: true, task: { select: { repo_url: true } } }, + select: { pr_url: true }, orderBy: { created_at: 'asc' }, }) - const sameRepoSibling = sprintSiblings.find( - (s) => (s.task?.repo_url ?? null) === thisRepoKey, - ) - if (sameRepoSibling?.pr_url) return sameRepoSibling.pr_url + if (sprintSibling?.pr_url) return sprintSibling.pr_url // Eerste DONE in deze SprintRun → maak draft-PR aan, geen auto-merge. const goal = job.sprint_run.sprint.sprint_goal @@ -496,21 +459,17 @@ export async function maybeCreateAutoPr(opts: { return null } - // 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({ + // STORY-mode (default of legacy): branch-per-story, sibling-tasks delen PR. + const sibling = await prisma.claudeJob.findFirst({ where: { task: { story_id: task.story.id }, pr_url: { not: null }, id: { not: jobId }, }, - select: { pr_url: true, task: { select: { repo_url: true } } }, + select: { pr_url: true }, orderBy: { created_at: 'asc' }, }) - const sameRepoStorySibling = storySiblings.find( - (s) => (s.task?.repo_url ?? null) === thisRepoKey, - ) - if (sameRepoStorySibling?.pr_url) return sameRepoStorySibling.pr_url + if (sibling?.pr_url) return sibling.pr_url const storyTitle = task.story.code ? `${task.story.code}: ${task.story.title}` : task.story.title const body = summary @@ -595,8 +554,6 @@ 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 ' + @@ -636,8 +593,6 @@ 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, @@ -781,11 +736,10 @@ export function registerUpdateJobStatusTool(server: McpServer) { where: { id: job_id }, data: { status: dbStatus, - ...resolveJobTimestamps( - actualStatus, - { claimed_at: job.claimed_at, started_at: job.started_at }, - now, - ), + ...(actualStatus === 'running' ? { started_at: now } : {}), + ...(actualStatus === 'done' || actualStatus === 'failed' || actualStatus === 'skipped' + ? { finished_at: now } + : {}), ...(branchToWrite !== undefined ? { branch: branchToWrite } : {}), ...(pushedAt !== undefined ? { pushed_at: pushedAt } : {}), ...(summary !== undefined ? { summary } : {}), diff --git a/src/tools/wait-for-job.ts b/src/tools/wait-for-job.ts index f3e11c0..dd322c9 100644 --- a/src/tools/wait-for-job.ts +++ b/src/tools/wait-for-job.ts @@ -606,10 +606,10 @@ export async function getFullJobContext(jobId: string) { }, tasks: { where: { status: 'TO_DO' }, - orderBy: [{ priority: 'asc' }, { sort_order: 'asc' }], + orderBy: [{ sort_order: 'asc' }], }, }, - orderBy: [{ priority: 'asc' }, { sort_order: 'asc' }], + orderBy: [{ sort_order: 'asc' }], }, }, },