Compare commits
1 commit
main
...
fix/cross-
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
70cb4ad2b0 |
4 changed files with 58 additions and 313 deletions
|
|
@ -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<typeof vi.fn> }
|
|
||||||
ideaLog: { create: ReturnType<typeof vi.fn> }
|
|
||||||
$transaction: ReturnType<typeof vi.fn>
|
|
||||||
}
|
|
||||||
const mockRequireWriteAccess = requireWriteAccess as ReturnType<typeof vi.fn>
|
|
||||||
const mockUserOwnsIdea = userOwnsIdea as ReturnType<typeof vi.fn>
|
|
||||||
|
|
||||||
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<ReturnType<typeof handleUpdateIdeaPlanReviewed>>) {
|
|
||||||
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)
|
|
||||||
})
|
|
||||||
})
|
|
||||||
|
|
@ -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)
|
|
||||||
})
|
|
||||||
})
|
|
||||||
|
|
@ -1,8 +1,6 @@
|
||||||
// MCP-tool: writes the review-log result after an IDEA_REVIEW_PLAN job and
|
// MCP-tool: writes the review-log result after a IDEA_REVIEW_PLAN grill-job
|
||||||
// transitions idea.status. Only an explicit approval_status='approved' moves
|
// and transitions the idea.status to PLAN_REVIEWED (on success) or
|
||||||
// the idea to PLAN_REVIEWED; anything else (rejected, pending, or omitted)
|
// PLAN_REVIEW_FAILED (on failure).
|
||||||
// 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.
|
// 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 { userOwnsIdea } from '../access.js'
|
||||||
import { toolError, toolJson, withToolErrors } from '../errors.js'
|
import { toolError, toolJson, withToolErrors } from '../errors.js'
|
||||||
|
|
||||||
export const inputSchema = z.object({
|
const inputSchema = z.object({
|
||||||
idea_id: z.string().min(1),
|
idea_id: z.string().min(1),
|
||||||
review_log: z.object({}).passthrough(), // Full ReviewLog from orchestrator (JSON object)
|
review_log: z.object({}).passthrough(), // Full ReviewLog from orchestrator (JSON object)
|
||||||
approval_status: z
|
approval_status: z
|
||||||
|
|
@ -22,22 +20,29 @@ export const inputSchema = z.object({
|
||||||
.optional(),
|
.optional(),
|
||||||
})
|
})
|
||||||
|
|
||||||
export async function handleUpdateIdeaPlanReviewed(
|
export function registerUpdateIdeaPlanReviewedTool(server: McpServer) {
|
||||||
{ idea_id, review_log, approval_status }: z.infer<typeof inputSchema>,
|
server.registerTool(
|
||||||
) {
|
'update_idea_plan_reviewed',
|
||||||
return withToolErrors(async () => {
|
{
|
||||||
|
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()
|
const auth = await requireWriteAccess()
|
||||||
if (!(await userOwnsIdea(idea_id, auth.userId))) {
|
if (!(await userOwnsIdea(idea_id, auth.userId))) {
|
||||||
return toolError('Idea not found')
|
return toolError('Idea not found')
|
||||||
}
|
}
|
||||||
|
|
||||||
// Alleen een expliciete 'approved' brengt het idee naar PLAN_REVIEWED.
|
// Determine target status based on approval
|
||||||
// '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 =
|
const nextStatus =
|
||||||
approval_status === 'approved' ? 'PLAN_REVIEWED' : 'PLAN_REVIEW_FAILED'
|
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
|
// Log summary metrics from review_log
|
||||||
const logSummary = buildReviewLogSummary(review_log)
|
const logSummary = buildReviewLogSummary(review_log)
|
||||||
|
|
@ -72,22 +77,7 @@ export async function handleUpdateIdeaPlanReviewed(
|
||||||
idea: result[0],
|
idea: result[0],
|
||||||
review_log_summary: logSummary,
|
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.',
|
|
||||||
inputSchema,
|
|
||||||
},
|
|
||||||
handleUpdateIdeaPlanReviewed,
|
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -390,32 +390,6 @@ export function resolveNextAction(
|
||||||
return queueCount > 0 ? 'wait_for_job_again' : 'queue_empty'
|
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: {
|
export async function maybeCreateAutoPr(opts: {
|
||||||
jobId: string
|
jobId: string
|
||||||
productId: string
|
productId: string
|
||||||
|
|
@ -595,8 +569,6 @@ export function registerUpdateJobStatusTool(server: McpServer) {
|
||||||
'Report progress on a claimed ClaudeJob. Allowed transitions from CLAIMED/RUNNING: ' +
|
'Report progress on a claimed ClaudeJob. Allowed transitions from CLAIMED/RUNNING: ' +
|
||||||
'running (start), done (finished), failed (error), skipped (no-op exit). ' +
|
'running (start), done (finished), failed (error), skipped (no-op exit). ' +
|
||||||
'The Bearer token must match the token that claimed the job. ' +
|
'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 ' +
|
'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 ' +
|
'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 ' +
|
'doesn’t meet task.verify_required: ALIGNED-only is strict; ALIGNED_OR_PARTIAL accepts ' +
|
||||||
|
|
@ -636,8 +608,6 @@ export function registerUpdateJobStatusTool(server: McpServer) {
|
||||||
select: {
|
select: {
|
||||||
id: true,
|
id: true,
|
||||||
status: true,
|
status: true,
|
||||||
claimed_at: true,
|
|
||||||
started_at: true,
|
|
||||||
claimed_by_token_id: true,
|
claimed_by_token_id: true,
|
||||||
user_id: true,
|
user_id: true,
|
||||||
product_id: true,
|
product_id: true,
|
||||||
|
|
@ -781,11 +751,10 @@ export function registerUpdateJobStatusTool(server: McpServer) {
|
||||||
where: { id: job_id },
|
where: { id: job_id },
|
||||||
data: {
|
data: {
|
||||||
status: dbStatus,
|
status: dbStatus,
|
||||||
...resolveJobTimestamps(
|
...(actualStatus === 'running' ? { started_at: now } : {}),
|
||||||
actualStatus,
|
...(actualStatus === 'done' || actualStatus === 'failed' || actualStatus === 'skipped'
|
||||||
{ claimed_at: job.claimed_at, started_at: job.started_at },
|
? { finished_at: now }
|
||||||
now,
|
: {}),
|
||||||
),
|
|
||||||
...(branchToWrite !== undefined ? { branch: branchToWrite } : {}),
|
...(branchToWrite !== undefined ? { branch: branchToWrite } : {}),
|
||||||
...(pushedAt !== undefined ? { pushed_at: pushedAt } : {}),
|
...(pushedAt !== undefined ? { pushed_at: pushedAt } : {}),
|
||||||
...(summary !== undefined ? { summary } : {}),
|
...(summary !== undefined ? { summary } : {}),
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue