diff --git a/src/mcpd/src/main.ts b/src/mcpd/src/main.ts index 3beb8d0..ac96fe5 100644 --- a/src/mcpd/src/main.ts +++ b/src/mcpd/src/main.ts @@ -587,7 +587,7 @@ async function main(): Promise { // Agentic teams: the Epic is the versioned vision every task walks up to. // Revision-tracked so scope creep is as visible as under-delivery. - const epicService = new EpicService(epicRepo, projectRepo); + const epicService = new EpicService(epicRepo, projectRepo, epicTaskRepo, workTaskRepo); epicService.setRevisionService(resourceRevisionService); const epicTaskService = new EpicTaskService(epicTaskRepo, epicRepo); const workTaskService = new TaskService(workTaskRepo, epicTaskRepo); diff --git a/src/mcpd/src/routes/epics.ts b/src/mcpd/src/routes/epics.ts index 0ecf7b4..68a7da8 100644 --- a/src/mcpd/src/routes/epics.ts +++ b/src/mcpd/src/routes/epics.ts @@ -37,8 +37,24 @@ export function registerEpicRoutes( }, ); + // Reconciliation rides along with the single Epic read, next to the + // completeness it contradicts. Kept off the list route on purpose: it costs a + // query per roadmap node, and a mismatch nobody is looking at is only useful + // where the misleading percentage is already on screen. app.get<{ Params: { id: string } }>('/api/v1/epics/:id', async (request) => { - return withCompleteness(await service.getEpic(request.params.id)); + const epic = await service.getEpic(request.params.id); + return { + ...withCompleteness(epic), + reconciliation: await service.reconciliation(epic.id), + }; + }); + + /** + * Claimed vs delivered, on its own — for callers that want the mismatch + * without dragging along an Epic body that can run to 200KB of content. + */ + app.get<{ Params: { id: string } }>('/api/v1/epics/:id/reconciliation', async (request) => { + return service.reconciliation(request.params.id); }); // ── Create / Update / Delete ── diff --git a/src/mcpd/src/services/epic.service.ts b/src/mcpd/src/services/epic.service.ts index 90b1530..9e5c2ff 100644 --- a/src/mcpd/src/services/epic.service.ts +++ b/src/mcpd/src/services/epic.service.ts @@ -1,7 +1,9 @@ -import type { Epic, EpicStatus, Prisma } from '@prisma/client'; +import type { Epic, EpicStatus, EpicTask, EpicTaskStatus, Prisma, Task } from '@prisma/client'; import type { IEpicRepository } from '../repositories/epic.repository.js'; +import type { IEpicTaskRepository } from '../repositories/epic-task.repository.js'; import type { IProjectRepository } from '../repositories/project.repository.js'; +import type { ITaskRepository } from '../repositories/task.repository.js'; import { CreateEpicSchema, UpdateEpicSchema, @@ -23,12 +25,102 @@ export interface EpicCompleteness { unmet: DoneCriterion[]; } +// ── Reconciliation types ── + +/** How much work sits under something, split by whether it actually landed. */ +export interface TaskRollup { + /** Tasks in `merged` — the only state that means it is in the product. */ + merged: number; + /** + * Neither merged nor cancelled: still owed. `reopened` counts here, because a + * post-merge failure means the delivery did not hold. + */ + outstanding: number; + /** Abandoned. Counted apart: not delivered, but no longer owed either. */ + cancelled: number; +} + +/** A roadmap node that claims a criterion, plus what it has actually landed. */ +export interface OwningEpicTask extends TaskRollup { + id: string; + name: string; + /** The node's own claim about itself. Recorded, never used as evidence. */ + status: EpicTaskStatus; +} + +/** One Epic criterion set against the work delivered under the nodes that own it. */ +export interface CriterionDelivery extends TaskRollup { + id: string; + text: string; + kind: DoneCriterion['kind']; + /** The claim — set by a human or agent. Never inferred from task state. */ + met: boolean; + /** Nodes owning this criterion. Empty means nothing can deliver it. */ + epicTasks: OwningEpicTask[]; +} + +/** + * Claimed completeness measured against delivered work. + * + * Both mismatch directions are reported and neither is resolved: a criterion's + * `met` flag is a judgement, and inferring it from task state would launder the + * exact assumption this comparison exists to challenge. + */ +export interface EpicReconciliation { + epicId: string; + /** + * Unmet criteria whose owning node has merged work. Somebody delivered and + * nobody ticked the box, so the Epic under-reports itself. + */ + deliveredButUnclaimed: CriterionDelivery[]; + /** + * Criteria marked met with nothing merged underneath. The box is ticked and + * nothing shipped — the dangerous direction, and the easy one to forget. + */ + claimedButUndelivered: CriterionDelivery[]; + /** + * Criteria no roadmap node claims. They cannot appear in either list above + * because nothing is positioned to deliver them; reporting them separately + * keeps that gap from hiding inside a clean-looking reconciliation. + */ + unowned: CriterionDelivery[]; + /** Every task under the Epic, so the mismatch has a denominator. */ + tasks: TaskRollup & { total: number }; + /** True only when all three lists are empty. */ + consistent: boolean; +} + +/** + * The fields the reconciliation rule actually reads. Structural rather than the + * full Prisma rows so the pure function stays callable with a small fixture. + */ +export type ReconcileNode = Pick & { doneCriteria: unknown }; +export type ReconcileTask = Pick; + function badRequest(message: string): Error { return Object.assign(new Error(message), { statusCode: 400 }); } -function readCriteria(epic: Epic): DoneCriterion[] { - return (epic.doneCriteria ?? []) as unknown as DoneCriterion[]; +function readCriteria(source: { doneCriteria: unknown }): DoneCriterion[] { + return (source.doneCriteria ?? []) as DoneCriterion[]; +} + +function rollup(tasks: ReconcileTask[]): TaskRollup { + let merged = 0; + let cancelled = 0; + for (const task of tasks) { + if (task.state === 'merged') merged += 1; + else if (task.state === 'cancelled') cancelled += 1; + } + return { merged, outstanding: tasks.length - merged - cancelled, cancelled }; +} + +function sumRollup(parts: TaskRollup[]): TaskRollup { + return { + merged: parts.reduce((n, p) => n + p.merged, 0), + outstanding: parts.reduce((n, p) => n + p.outstanding, 0), + cancelled: parts.reduce((n, p) => n + p.cancelled, 0), + }; } function readTradeoffs(epic: Epic): AcceptedTradeoff[] { @@ -41,6 +133,11 @@ export class EpicService { constructor( private readonly epicRepo: IEpicRepository, private readonly projectRepo: IProjectRepository, + // Reconciliation reads down the whole chain — Epic → EpicTask → Task — so + // the two lower repositories are required, not injected later. An Epic that + // cannot see its own delivered work is the blind spot, not a nice-to-have. + private readonly epicTaskRepo: IEpicTaskRepository, + private readonly taskRepo: ITaskRepository, ) {} setRevisionService(service: ResourceRevisionService): void { @@ -66,6 +163,116 @@ export class EpicService { }; } + // ── Reconciliation: what was claimed vs what was delivered ── + + /** + * Compare the Epic's claimed completeness against the work actually merged + * under its roadmap. + * + * `completeness()` above counts only `met` flags, and nothing flips those + * automatically — so an Epic whose every Task is merged still reads 0%. That + * is precisely the blind spot the Epic exists to remove, hence this second + * reading of the same question from the delivery side. + * + * **How a criterion is attributed to a node.** By criterion id, because that + * is the only link the schema offers: `EpicTask.doneCriteria` is documented + * as "the decomposed subset of the Epic's doneCriteria this node owns" and + * carries the same ids. Nothing enforces it — an id can be missing, shared by + * several nodes, or invented — so this treats the convention as a claim and + * reports where it does not hold (`unowned`) rather than guessing an owner by + * name similarity or ordering. A real foreign key would end the ambiguity. + * + * **Nothing is written.** `met` stays exactly as a human or agent set it; + * deriving it from task state would assert that finishing the tasks means + * delivering the vision, which is the assumption under audit. + */ + static reconcile( + epic: { id: string; doneCriteria: unknown }, + nodes: ReconcileNode[], + tasks: ReconcileTask[], + ): EpicReconciliation { + const tasksByNode = new Map(); + for (const task of tasks) { + tasksByNode.set(task.epicTaskId, [...(tasksByNode.get(task.epicTaskId) ?? []), task]); + } + + const ownersByCriterion = new Map(); + for (const node of nodes) { + const owner: OwningEpicTask = { + id: node.id, + name: node.name, + status: node.status, + ...rollup(tasksByNode.get(node.id) ?? []), + }; + // Deduplicated: nothing stops a node from listing the same id twice, and + // that must not double-count its tasks against the criterion. + for (const ownedId of new Set(readCriteria(node).map((c) => c.id))) { + ownersByCriterion.set(ownedId, [...(ownersByCriterion.get(ownedId) ?? []), owner]); + } + } + + const deliveredButUnclaimed: CriterionDelivery[] = []; + const claimedButUndelivered: CriterionDelivery[] = []; + const unowned: CriterionDelivery[] = []; + + for (const criterion of readCriteria(epic)) { + const epicTasks = ownersByCriterion.get(criterion.id) ?? []; + const entry: CriterionDelivery = { + id: criterion.id, + text: criterion.text, + // Restored backups write doneCriteria as raw JSON without going through + // the zod defaults, so neither field can be trusted to be present. + kind: criterion.kind ?? 'outcome', + met: criterion.met === true, + epicTasks, + ...sumRollup(epicTasks), + }; + + if (epicTasks.length === 0) { + // No node owns it, so no amount of merged work can speak for it. Note + // this catches a *met* orphan too — a box ticked with nothing on the + // roadmap even nominally responsible for it. + unowned.push(entry); + } else if (entry.met && entry.merged === 0) { + claimedButUndelivered.push(entry); + } else if (!entry.met && entry.merged > 0) { + deliveredButUnclaimed.push(entry); + } + } + + return { + epicId: epic.id, + deliveredButUnclaimed, + claimedButUndelivered, + unowned, + tasks: { ...rollup(tasks), total: tasks.length }, + consistent: + deliveredButUnclaimed.length === 0 && + claimedButUndelivered.length === 0 && + unowned.length === 0, + }; + } + + /** + * Load an Epic's roadmap and every task under it, then reconcile. + * + * One query per node rather than a join: a roadmap holds a handful of nodes, + * `ITaskRepository` already filters by `epicTaskId`, and widening that + * interface to save a round-trip on a single-Epic read is not worth the + * churn across its call sites. + */ + async reconciliation(id: string): Promise { + const epic = await this.getEpic(id); + const nodes = await this.epicTaskRepo.findAll(epic.id); + + const tasks: Task[] = []; + for (const node of nodes) { + tasks.push(...(await this.taskRepo.findAll({ epicTaskId: node.id }))); + } + + return EpicService.reconcile(epic, nodes, tasks); + } + // ── CRUD ── async listEpics(projectId?: string): Promise { diff --git a/src/mcpd/tests/epic-routes.test.ts b/src/mcpd/tests/epic-routes.test.ts index 1cc0c27..2244463 100644 --- a/src/mcpd/tests/epic-routes.test.ts +++ b/src/mcpd/tests/epic-routes.test.ts @@ -1,10 +1,12 @@ -import type { Epic, Project } from '@prisma/client'; +import type { Epic, EpicTask, Project, Task } from '@prisma/client'; import Fastify, { type FastifyInstance } from 'fastify'; import { describe, it, expect, vi, afterEach } from 'vitest'; import { errorHandler } from '../src/middleware/error-handler.js'; import type { IEpicRepository } from '../src/repositories/epic.repository.js'; +import type { IEpicTaskRepository } from '../src/repositories/epic-task.repository.js'; import type { IProjectRepository } from '../src/repositories/project.repository.js'; +import type { ITaskRepository, TaskFilters } from '../src/repositories/task.repository.js'; import { registerEpicRoutes } from '../src/routes/epics.js'; import { EpicService } from '../src/services/epic.service.js'; @@ -36,7 +38,19 @@ const makeProject = (o: Partial = {}): Project => const criterion = (id: string, met: boolean) => ({ id, text: `criterion ${id}`, kind: 'outcome' as const, met }); -async function buildApp(epic: Epic | null = makeEpic(), userId?: string): Promise<{ +/** Roadmap node + work item, trimmed to the fields the reconciliation reads. */ +const node = (id: string, criterionIds: string[]): EpicTask => + ({ id, name: id, status: 'todo', doneCriteria: criterionIds.map((c) => criterion(c, false)) } as unknown as EpicTask); + +const workTask = (epicTaskId: string, state: Task['state']): Task => + ({ epicTaskId, state } as Task); + +async function buildApp( + epic: Epic | null = makeEpic(), + userId?: string, + nodes: EpicTask[] = [], + tasks: Task[] = [], +): Promise<{ app: FastifyInstance; epicRepo: IEpicRepository & { update: ReturnType }; }> { @@ -60,13 +74,24 @@ async function buildApp(epic: Epic | null = makeEpic(), userId?: string): Promis findByName: vi.fn(async (n: string) => (n === 'mcpctl' ? makeProject() : null)), } as unknown as IProjectRepository; + const epicTaskRepo = { findAll: vi.fn(async () => nodes) } as unknown as IEpicTaskRepository; + const taskRepo = { + findAll: vi.fn(async (filters: TaskFilters = {}) => + tasks.filter((t) => t.epicTaskId === filters.epicTaskId), + ), + } as unknown as ITaskRepository; + const instance = Fastify(); instance.setErrorHandler(errorHandler); // Stand in for the auth middleware, which normally sets request.userId. if (userId !== undefined) { instance.addHook('onRequest', async (req) => { req.userId = userId; }); } - registerEpicRoutes(instance, new EpicService(epicRepo, projectRepo), projectRepo); + registerEpicRoutes( + instance, + new EpicService(epicRepo, projectRepo, epicTaskRepo, taskRepo), + projectRepo, + ); await instance.ready(); return { app: instance, epicRepo }; } @@ -90,6 +115,54 @@ describe('epic routes', () => { expect(one.json().completeness.unmet).toHaveLength(1); }); + it('puts the delivered-vs-claimed mismatch next to the misleading percentage', async () => { + // 0% claimed, everything merged — reading one without the other is the bug. + const built = await buildApp( + makeEpic({ doneCriteria: [criterion('wheels', false)] as never }), + undefined, + [node('n1', ['wheels'])], + [workTask('n1', 'merged')], + ); + app = built.app; + + const res = await app.inject({ method: 'GET', url: '/api/v1/epics/epic-1' }); + expect(res.statusCode).toBe(200); + expect(res.json().completeness.percent).toBe(0); + expect(res.json().reconciliation.deliveredButUnclaimed.map((c: { id: string }) => c.id)) + .toEqual(['wheels']); + expect(res.json().reconciliation.consistent).toBe(false); + }); + + it('serves the reconciliation on its own, without the Epic body', async () => { + const built = await buildApp( + makeEpic({ doneCriteria: [criterion('engine', true)] as never }), + undefined, + [node('n1', ['engine'])], + [], + ); + app = built.app; + + const res = await app.inject({ method: 'GET', url: '/api/v1/epics/epic-1/reconciliation' }); + expect(res.statusCode).toBe(200); + expect(res.json().content).toBeUndefined(); + expect(res.json().claimedButUndelivered.map((c: { id: string }) => c.id)).toEqual(['engine']); + }); + + it('404s the reconciliation of an unknown Epic', async () => { + const built = await buildApp(null); + app = built.app; + const res = await app.inject({ method: 'GET', url: '/api/v1/epics/ghost/reconciliation' }); + expect(res.statusCode).toBe(404); + }); + + it('keeps the list route free of reconciliation — one query per node is not list work', async () => { + const built = await buildApp(makeEpic({ doneCriteria: [criterion('a', false)] as never })); + app = built.app; + const res = await app.inject({ method: 'GET', url: '/api/v1/epics' }); + expect(res.json()[0].completeness).toBeDefined(); + expect(res.json()[0].reconciliation).toBeUndefined(); + }); + it('creates from a project name and stamps the caller as owner', async () => { const built = await buildApp(null, 'user-42'); app = built.app; diff --git a/src/mcpd/tests/services/epic-service.test.ts b/src/mcpd/tests/services/epic-service.test.ts index f13a7bb..5485e6b 100644 --- a/src/mcpd/tests/services/epic-service.test.ts +++ b/src/mcpd/tests/services/epic-service.test.ts @@ -2,8 +2,11 @@ import type { Epic, Project } from '@prisma/client'; import { describe, it, expect, vi, beforeEach } from 'vitest'; import type { IEpicRepository } from '../../src/repositories/epic.repository.js'; +import type { IEpicTaskRepository } from '../../src/repositories/epic-task.repository.js'; import type { IProjectRepository } from '../../src/repositories/project.repository.js'; +import type { ITaskRepository, TaskFilters } from '../../src/repositories/task.repository.js'; import { EpicService } from '../../src/services/epic.service.js'; +import type { ReconcileNode, ReconcileTask } from '../../src/services/epic.service.js'; function makeEpic(overrides: Partial = {}): Epic { return { @@ -28,7 +31,27 @@ function makeEpic(overrides: Partial = {}): Epic { const criterion = (id: string, met: boolean, kind: 'outcome' | 'quality' = 'outcome') => ({ id, text: `criterion ${id}`, kind, met }); -function mockRepos(epic: Epic | null = makeEpic()) { +/** A roadmap node, trimmed to what the reconciliation rule reads. */ +const node = ( + id: string, + criterionIds: string[], + overrides: Partial = {}, +): ReconcileNode => ({ + id, + name: id, + status: 'todo', + doneCriteria: criterionIds.map((c) => criterion(c, false)), + ...overrides, +}); + +const task = (epicTaskId: string, state: ReconcileTask['state']): ReconcileTask => + ({ epicTaskId, state }); + +function mockRepos( + epic: Epic | null = makeEpic(), + nodes: ReconcileNode[] = [], + tasks: ReconcileTask[] = [], +) { let current = epic; const update = vi.fn(async (_id: string, data: Record) => { current = { ...(current as Epic), ...data } as Epic; @@ -49,7 +72,25 @@ function mockRepos(epic: Epic | null = makeEpic()) { findByName: vi.fn(async () => ({ id: 'proj-1', name: 'mcpctl' } as Project)), } as unknown as IProjectRepository; - return { epicRepo, projectRepo, service: new EpicService(epicRepo, projectRepo) }; + const epicTaskRepo = { findAll: vi.fn(async () => nodes) }; + const taskRepo = { + findAll: vi.fn(async (filters: TaskFilters = {}) => + tasks.filter((t) => t.epicTaskId === filters.epicTaskId), + ), + }; + + return { + epicRepo, + projectRepo, + epicTaskRepo, + taskRepo, + service: new EpicService( + epicRepo, + projectRepo, + epicTaskRepo as unknown as IEpicTaskRepository, + taskRepo as unknown as ITaskRepository, + ), + }; } describe('EpicService.completeness', () => { @@ -78,6 +119,219 @@ describe('EpicService.completeness', () => { }); }); +describe('EpicService.reconcile — claimed vs delivered', () => { + const ids = (list: Array<{ id: string }>) => list.map((c) => c.id); + + it('flags work that was delivered and never claimed', () => { + // The motivating bug: every task merged, the Epic still reads 0%. + const epic = { id: 'epic-1', doneCriteria: [criterion('wheels', false)] }; + const result = EpicService.reconcile( + epic, + [node('n1', ['wheels'])], + [task('n1', 'merged'), task('n1', 'merged')], + ); + + expect(ids(result.deliveredButUnclaimed)).toEqual(['wheels']); + expect(result.deliveredButUnclaimed[0]?.merged).toBe(2); + expect(result.claimedButUndelivered).toEqual([]); + expect(result.consistent).toBe(false); + }); + + it('flags a criterion claimed with nothing merged under it', () => { + const epic = { id: 'epic-1', doneCriteria: [criterion('engine', true)] }; + const result = EpicService.reconcile( + epic, + [node('n1', ['engine'])], + [task('n1', 'awaiting_review')], + ); + + expect(ids(result.claimedButUndelivered)).toEqual(['engine']); + expect(result.claimedButUndelivered[0]).toMatchObject({ merged: 0, outstanding: 1 }); + }); + + it('flags a claim over a node that has no tasks at all', () => { + // Not-yet-decomposed is still nothing shipped — same bucket, and the empty + // rollup is what tells the two apart. + const epic = { id: 'epic-1', doneCriteria: [criterion('engine', true)] }; + const result = EpicService.reconcile(epic, [node('n1', ['engine'])], []); + + expect(ids(result.claimedButUndelivered)).toEqual(['engine']); + expect(result.claimedButUndelivered[0]).toMatchObject({ + merged: 0, + outstanding: 0, + cancelled: 0, + }); + }); + + it('reports both directions from one pass over a mixed Epic', () => { + const epic = { + id: 'epic-1', + doneCriteria: [ + criterion('wheels', false), // delivered, unclaimed + criterion('engine', true), // claimed, nothing merged + criterion('paint', true), // claimed and delivered — agrees + criterion('seats', false), // neither — just not done yet + ], + }; + const result = EpicService.reconcile( + epic, + [node('n1', ['wheels']), node('n2', ['engine']), node('n3', ['paint']), node('n4', ['seats'])], + [task('n1', 'merged'), task('n2', 'in_progress'), task('n3', 'merged'), task('n4', 'todo')], + ); + + expect(ids(result.deliveredButUnclaimed)).toEqual(['wheels']); + expect(ids(result.claimedButUndelivered)).toEqual(['engine']); + expect(result.unowned).toEqual([]); + expect(result.tasks).toEqual({ merged: 2, outstanding: 2, cancelled: 0, total: 4 }); + }); + + it('reports criteria no roadmap node owns, instead of silently dropping them', () => { + // There is no foreign key from criterion to epictask — only the id + // convention. When it does not hold, say so rather than guessing an owner. + const epic = { + id: 'epic-1', + doneCriteria: [criterion('orphan', false), criterion('claimed-orphan', true)], + }; + const result = EpicService.reconcile(epic, [node('n1', ['something-else'])], [task('n1', 'merged')]); + + expect(ids(result.unowned)).toEqual(['orphan', 'claimed-orphan']); + // A met criterion nothing is even nominally responsible for must not be + // laundered into the "agrees" pile by having no owner to contradict it. + expect(result.unowned[1]?.met).toBe(true); + expect(result.deliveredButUnclaimed).toEqual([]); + expect(result.claimedButUndelivered).toEqual([]); + expect(result.consistent).toBe(false); + }); + + it('treats cancelled tasks as neither delivered nor outstanding', () => { + // Counting them as owed would leave an epictask permanently undelivered + // because somebody abandoned a task; counting them as delivered is a lie. + const epic = { id: 'epic-1', doneCriteria: [criterion('engine', true)] }; + const result = EpicService.reconcile( + epic, + [node('n1', ['engine'])], + [task('n1', 'cancelled'), task('n1', 'cancelled')], + ); + + expect(ids(result.claimedButUndelivered)).toEqual(['engine']); + expect(result.tasks).toEqual({ merged: 0, outstanding: 0, cancelled: 2, total: 2 }); + }); + + it('does not count a reopened task as delivered — the merge did not hold', () => { + const epic = { id: 'epic-1', doneCriteria: [criterion('engine', true)] }; + const result = EpicService.reconcile(epic, [node('n1', ['engine'])], [task('n1', 'reopened')]); + + expect(ids(result.claimedButUndelivered)).toEqual(['engine']); + expect(result.tasks.outstanding).toBe(1); + }); + + it('sums across every node that claims the same criterion', () => { + const epic = { id: 'epic-1', doneCriteria: [criterion('wheels', false)] }; + const result = EpicService.reconcile( + epic, + [node('n1', ['wheels']), node('n2', ['wheels'])], + [task('n1', 'todo'), task('n2', 'merged')], + ); + + // One entry, not two, and delivered because one owner landed work. + expect(result.deliveredButUnclaimed).toHaveLength(1); + expect(result.deliveredButUnclaimed[0]?.epicTasks.map((n) => n.id)).toEqual(['n1', 'n2']); + expect(result.deliveredButUnclaimed[0]).toMatchObject({ merged: 1, outstanding: 1 }); + }); + + it('ignores an epictask status claim — only merged tasks count as delivery', () => { + // The node says done. Nothing merged. That is the mismatch, not evidence. + const epic = { id: 'epic-1', doneCriteria: [criterion('engine', true)] }; + const result = EpicService.reconcile( + epic, + [node('n1', ['engine'], { status: 'done' })], + [task('n1', 'awaiting_review')], + ); + + expect(ids(result.claimedButUndelivered)).toEqual(['engine']); + expect(result.claimedButUndelivered[0]?.epicTasks[0]?.status).toBe('done'); + }); + + it('never flips met — it reports the gap and leaves the judgement alone', () => { + const epic = { id: 'epic-1', doneCriteria: [criterion('wheels', false)] }; + const result = EpicService.reconcile(epic, [node('n1', ['wheels'])], [task('n1', 'merged')]); + + expect(result.deliveredButUnclaimed[0]?.met).toBe(false); + expect(epic.doneCriteria[0]?.met).toBe(false); + }); + + it('is consistent only when nothing disagrees', () => { + const epic = { id: 'epic-1', doneCriteria: [criterion('wheels', true)] }; + const result = EpicService.reconcile(epic, [node('n1', ['wheels'])], [task('n1', 'merged')]); + + expect(result.consistent).toBe(true); + expect(result.deliveredButUnclaimed).toEqual([]); + expect(result.claimedButUndelivered).toEqual([]); + expect(result.unowned).toEqual([]); + }); + + it('is consistent on an Epic with no criteria and no work', () => { + expect(EpicService.reconcile({ id: 'epic-1', doneCriteria: [] }, [], []).consistent).toBe(true); + }); + + it('survives criteria restored as raw JSON without kind or met', () => { + // upsertByName writes doneCriteria straight from a backup document, so the + // zod defaults never ran. + const epic = { id: 'epic-1', doneCriteria: [{ id: 'wheels', text: 'has wheels' }] }; + const result = EpicService.reconcile( + epic, + [{ id: 'n1', name: 'n1', status: 'todo', doneCriteria: [{ id: 'wheels' }] }], + [task('n1', 'merged')], + ); + + expect(result.deliveredButUnclaimed[0]).toMatchObject({ kind: 'outcome', met: false }); + }); + + it('tolerates a null doneCriteria column on either side', () => { + const result = EpicService.reconcile( + { id: 'epic-1', doneCriteria: null }, + [{ id: 'n1', name: 'n1', status: 'todo', doneCriteria: null }], + [task('n1', 'merged')], + ); + expect(result.consistent).toBe(true); + expect(result.tasks.merged).toBe(1); + }); +}); + +describe('EpicService.reconciliation — loading the chain', () => { + beforeEach(() => { vi.clearAllMocks(); }); + + it('walks Epic → epictasks → tasks and reconciles what it finds', async () => { + const { service, epicTaskRepo, taskRepo } = mockRepos( + makeEpic({ doneCriteria: [criterion('wheels', false)] as never }), + [node('n1', ['wheels'])], + [task('n1', 'merged')], + ); + + const result = await service.reconciliation('epic-1'); + + expect(epicTaskRepo.findAll).toHaveBeenCalledWith('epic-1'); + expect(taskRepo.findAll).toHaveBeenCalledWith({ epicTaskId: 'n1' }); + expect(result.deliveredButUnclaimed.map((c) => c.id)).toEqual(['wheels']); + }); + + it('writes nothing — reconciliation is a reading, not a repair', async () => { + const { service, epicRepo } = mockRepos( + makeEpic({ doneCriteria: [criterion('wheels', false)] as never }), + [node('n1', ['wheels'])], + [task('n1', 'merged')], + ); + + await service.reconciliation('epic-1'); + expect(epicRepo.update).not.toHaveBeenCalled(); + }); + + it('404s on an unknown Epic', async () => { + const { service } = mockRepos(null); + await expect(service.reconciliation('ghost')).rejects.toThrow(/Epic not found/); + }); +}); + describe('EpicService.setStatus — the honest-done rule', () => { beforeEach(() => { vi.clearAllMocks(); }); diff --git a/src/mcplocal/tests/smoke/agentic-teams.smoke.test.ts b/src/mcplocal/tests/smoke/agentic-teams.smoke.test.ts index b208b4e..74e1129 100644 --- a/src/mcplocal/tests/smoke/agentic-teams.smoke.test.ts +++ b/src/mcplocal/tests/smoke/agentic-teams.smoke.test.ts @@ -169,4 +169,54 @@ describe('agentic teams smoke', () => { expect(withNote.code, withNote.stderr || withNote.stdout).toBe(0); expect(withNote.stdout).toMatch(/rejected → in_progress/); }); + + it('reports work delivered but never claimed, and does not tick the box itself', () => { + if (!up) return; + + // Give the roadmap node the Epic criterion it owns. There is no + // `create epictask --criterion` flag, so apply is the only way in. + const path = join(workDir as string, 'epictask-criteria.yaml'); + writeFileSync( + path, + `kind: epictask\nname: ${EPICTASK}\nepic: ${EPIC}\n` + + 'doneCriteria:\n - id: builds\n text: it builds\n kind: outcome\n met: false\n', + 'utf-8', + ); + const applied = run(`apply -f ${path}`); + expect(applied.code, applied.stderr || applied.stdout).toBe(0); + + // Drive the rejected task all the way to merged. The QA verdict recorded + // earlier still stands, so the risk gate is already satisfied. + for (const state of ['building', 'self_testing', 'done', 'awaiting_review']) { + const r = run(`tasks advance ${TASK} ${state}`); + expect(r.code, `${state}: ${r.stderr || r.stdout}`).toBe(0); + } + const accepted = run(`tasks accept ${TASK} --note "ship it"`); + expect(accepted.code, accepted.stderr || accepted.stdout).toBe(0); + + const json = run(`get epic ${EPIC} -o json`); + expect(json.code, json.stderr).toBe(0); + const [epic] = JSON.parse(json.stdout) as Array<{ + completeness: { percent: number; met: number }; + reconciliation: { + deliveredButUnclaimed: Array<{ id: string; met: boolean; merged: number }>; + unowned: Array<{ id: string }>; + consistent: boolean; + }; + }>; + + // The blind spot, stated out loud: everything merged, still 0% claimed. + expect(epic?.completeness.percent).toBe(0); + expect(epic?.reconciliation.deliveredButUnclaimed.map((c) => c.id)).toEqual(['builds']); + expect(epic?.reconciliation.deliveredButUnclaimed[0]?.merged).toBe(1); + expect(epic?.reconciliation.consistent).toBe(false); + + // The quality criterion no node claimed is reported as unowned rather than + // quietly counted as either delivered or outstanding. + expect(epic?.reconciliation.unowned).toHaveLength(1); + + // And the judgement is still the human's: reporting the gap must not close it. + expect(epic?.completeness.met).toBe(0); + expect(epic?.reconciliation.deliveredButUnclaimed[0]?.met).toBe(false); + }); });