From 610b10b70645102a76e39cfdb42cd688dfbcdaa4 Mon Sep 17 00:00:00 2001 From: Bailey Dixon Date: Sat, 25 Jul 2026 10:20:34 -0400 Subject: [PATCH] fix: reconcile MCP delivery silence --- DEVLOG.md | 21 ++++ docs/engineering/work-management.md | 15 +++ .../__tests__/completion-candidate.test.ts | 74 +++++++++++++++ .../services/__tests__/work-session.test.ts | 95 ++++++++++++++++++- src/server/services/completion-candidate.ts | 71 +++++++++----- src/server/services/work-session.ts | 72 ++++++++++---- 6 files changed, 305 insertions(+), 43 deletions(-) diff --git a/DEVLOG.md b/DEVLOG.md index 67a1e372..53da8a07 100644 --- a/DEVLOG.md +++ b/DEVLOG.md @@ -2,6 +2,27 @@ > Append-only session log. Read at session start. Update at session end. +## 2026-07-25 — MCP Delivery silence reconciliation + +- Audited the production Attention Queue and traced five MCP-unconfirmed asks + to older session-specific Codex Desktop connections whose runs had completed + while their Delivery sessions remained non-terminal. +- Kept endpoint liveness honest: a newer Desktop task or MCP reconnect does not + confirm the older client process merely because it uses the same Agent + Profile or API key. +- Changed quiet-session recovery copy to distinguish an actually active run + from a completed run whose Delivery session simply needs an explicit + disposition. +- Excluded MCP-quiet recovery notices from generic completion-decision counts. + Safe Ready to Close evidence now dismisses the redundant recovery ask, while + genuine operator decisions continue to block completion. +- Reconciled open MCP-quiet asks for issues already in Done/Canceled without + fabricating a Delivered, Released, or Verified state for the historical + session. +- Added integration coverage for terminal-issue cleanup, request + deduplication, active-run wording, completed-run disposition wording, and the + boundary between quiet recovery and real operator decisions. + ## 2026-07-22 — v0.29.6 release preparation Prepared the serialized patch release for AXI-143 after implementation PR #85 diff --git a/docs/engineering/work-management.md b/docs/engineering/work-management.md index 7562adeb..40b4e887 100644 --- a/docs/engineering/work-management.md +++ b/docs/engineering/work-management.md @@ -56,6 +56,21 @@ run has completed because the client is still connected. Ready to Close and stale-run recovery use `AgentRun` lifecycle records, never connection presence or an `IN_REVIEW` Delivery session, to decide whether work is still active. +An expired MCP observation lease remains scoped to the exact negotiated client +session. Activity from a newer Desktop task, CLI process, or reconnect does not +prove that the older endpoint is alive, even when both use the same Agent +Profile or API key. When that older Delivery session has no active run, Forge +asks for a Delivery disposition instead of implying that work is still +executing. Resume it, attach or advance its delivery evidence, hand it off, or +abandon it explicitly. + +MCP-quiet recovery is coordination evidence, not an independent product +decision. It does not by itself block a completion recommendation, and a safe +Ready to Close assessment supersedes the redundant recovery ask. Terminal +issues resolve any remaining MCP-quiet asks during reconciliation without +rewriting the historical Delivery state or falsely marking an unverified +session as deployed. + ## Project branch contract Branch topology is project configuration, not an Axiom-wide constant. Every diff --git a/src/server/services/__tests__/completion-candidate.test.ts b/src/server/services/__tests__/completion-candidate.test.ts index f87e215e..fb896cef 100644 --- a/src/server/services/__tests__/completion-candidate.test.ts +++ b/src/server/services/__tests__/completion-candidate.test.ts @@ -156,6 +156,80 @@ describe("completion candidate policy", () => { ).toBe(2); }); + it("supersedes MCP-quiet recovery with ready evidence without ignoring real decisions", async () => { + const { fixture, prisma } = await setup(); + const issue = await createIssue(fixture, { statusCategory: "IN_PROGRESS" }); + const quiet = await prisma.actionRequest.create({ + data: { + workspaceId: fixture.workspace.id, + issueId: issue.id, + title: "MCP status is unconfirmed", + status: ActionRequestStatus.OPEN, + kind: "FREE_FORM", + sourceType: "work-session", + sourceId: "session-1", + dedupeKey: "work-session-mcp-quiet:session-1", + }, + }); + + const ready = await evaluateIssueCompletionCandidate(prisma, { + workspaceId: fixture.workspace.id, + issueId: issue.id, + actorId: fixture.user.id, + sourceType: "agent-run", + sourceId: "run-1", + sourceLabel: "@codex", + }); + + expect(ready.outcome).toBe("RECOMMENDED"); + const readyRequest = await prisma.actionRequest.findUniqueOrThrow({ + where: { id: "requestId" in ready ? ready.requestId : "" }, + }); + expect(readyRequest.payload).toMatchObject({ + assessment: { + state: "READY", + facts: expect.arrayContaining([ + expect.objectContaining({ key: "decisions", status: "PASS" }), + ]), + }, + }); + await expect( + prisma.actionRequest.findUniqueOrThrow({ where: { id: quiet.id } }), + ).resolves.toMatchObject({ + status: "DISMISSED", + resolution: "Completion evidence confirms there is no active work to recover.", + }); + + await prisma.actionRequest.create({ + data: { + workspaceId: fixture.workspace.id, + issueId: issue.id, + title: "Operator decision required", + status: ActionRequestStatus.OPEN, + kind: "FREE_FORM", + }, + }); + await evaluateIssueCompletionCandidate(prisma, { + workspaceId: fixture.workspace.id, + issueId: issue.id, + actorId: fixture.user.id, + sourceType: "agent-run", + sourceId: "run-2", + sourceLabel: "@codex", + }); + const blocked = await prisma.actionRequest.findUniqueOrThrow({ + where: { id: readyRequest.id }, + }); + expect(blocked.payload).toMatchObject({ + assessment: { + state: "BLOCKED", + facts: expect.arrayContaining([ + expect.objectContaining({ key: "decisions", status: "FAIL" }), + ]), + }, + }); + }); + it("automatically completes only when the shared safety gate is clear", async () => { const { fixture, prisma, done } = await setup(CompletionAutomation.AUTO_WHEN_SAFE); const issue = await createIssue(fixture, { statusCategory: "IN_PROGRESS" }); diff --git a/src/server/services/__tests__/work-session.test.ts b/src/server/services/__tests__/work-session.test.ts index d4456778..9889bbe0 100644 --- a/src/server/services/__tests__/work-session.test.ts +++ b/src/server/services/__tests__/work-session.test.ts @@ -772,7 +772,7 @@ describe("work session coordination", () => { ); }); - it("keeps a quiet MCP-owned lease intact and requests explicit recovery", async () => { + it("keeps a quiet MCP-owned lease intact and distinguishes disposition from active work", async () => { const { fixture, prisma, issue } = await setup(); await prisma.workspace.update({ where: { id: fixture.workspace.id }, @@ -828,18 +828,39 @@ describe("work session coordination", () => { }, }), ).toBe(1); + await expect( + prisma.actionRequest.findFirstOrThrow({ + where: { dedupeKey: `work-session-mcp-quiet:${session.id}` }, + }), + ).resolves.toMatchObject({ + title: expect.stringContaining("needs a disposition"), + body: expect.stringContaining("has no active run"), + }); // The run watchdog may mark the shared connection quiet before the // delivery sweep runs. Recovery must still be materialized in that order. await prisma.actionRequest.deleteMany({ where: { dedupeKey: `work-session-mcp-quiet:${session.id}` }, }); + await prisma.agentRun.create({ + data: { + workspaceId: fixture.workspace.id, + issueId: issue.id, + agentId: agent.id, + connectionId: connection.id, + status: "ACTIVE", + engagementMode: "EXECUTE", + }, + }); expect(await sweepStaleWorkSessions(prisma)).toBe(0); - expect( - await prisma.actionRequest.count({ + await expect( + prisma.actionRequest.findFirstOrThrow({ where: { dedupeKey: `work-session-mcp-quiet:${session.id}` }, }), - ).toBe(1); + ).resolves.toMatchObject({ + title: expect.stringContaining("MCP status"), + body: expect.stringContaining("Delivery remains owned"), + }); expect( await resolveMcpQuietRequestsForConnection(prisma, fixture.workspace.id, connection.id), @@ -866,6 +887,72 @@ describe("work session coordination", () => { ).resolves.toMatchObject({ status: "RESOLVED", resolution: "Work session resumed." }); }); + it("resolves MCP recovery requests when the issue is already terminal", async () => { + const { fixture, prisma, issue } = await setup(); + await prisma.workspace.update({ + where: { id: fixture.workspace.id }, + data: { workSessionStaleMinutes: 1 }, + }); + const agent = await prisma.agent.create({ + data: { + workspaceId: fixture.workspace.id, + profileKey: `terminal-mcp-${Date.now()}`, + name: "Terminal MCP agent", + }, + }); + const connection = await prisma.agentConnection.create({ + data: { + workspaceId: fixture.workspace.id, + agentId: agent.id, + kind: "MCP_CLIENT", + livenessModel: "LEASE", + status: "ACTIVE", + confidence: "CONFIRMED", + instanceKey: `terminal-${Date.now()}`, + }, + }); + const session = await claimWorkSession(prisma, { + workspaceId: fixture.workspace.id, + issueId: issue.id, + repoFullName: "acme/forge", + branch: "codex/terminal-mcp", + source: WorkSessionSource.MCP, + actor: { + userId: fixture.user.id, + agentId: agent.id, + connectionId: connection.id, + }, + }); + await prisma.workSession.update({ + where: { id: session.id }, + data: { lastHeartbeatAt: new Date(Date.now() - 5 * 60_000) }, + }); + + await sweepStaleWorkSessions(prisma); + const request = await prisma.actionRequest.findFirstOrThrow({ + where: { dedupeKey: `work-session-mcp-quiet:${session.id}` }, + }); + const done = await prisma.status.findFirstOrThrow({ + where: { workspaceId: fixture.workspace.id, category: "DONE" }, + }); + await prisma.issue.update({ + where: { id: issue.id }, + data: { statusId: done.id, completedAt: new Date() }, + }); + + await sweepStaleWorkSessions(prisma); + + await expect( + prisma.actionRequest.findUniqueOrThrow({ where: { id: request.id } }), + ).resolves.toMatchObject({ + status: "RESOLVED", + resolution: "Issue is already in a terminal state.", + }); + await expect( + prisma.workSession.findUniqueOrThrow({ where: { id: session.id } }), + ).resolves.toMatchObject({ status: "CLAIMED", endedAt: null }); + }); + it("resolves legacy delivery conflicts after their candidate is terminal or session ends", async () => { const { fixture, prisma, issue } = await setup(); const agent = await prisma.agent.create({ diff --git a/src/server/services/completion-candidate.ts b/src/server/services/completion-candidate.ts index 0c2a127a..eb807ac0 100644 --- a/src/server/services/completion-candidate.ts +++ b/src/server/services/completion-candidate.ts @@ -108,7 +108,9 @@ async function dismissOpenRequests( params: { workspaceId: string; actorId: string | null; + issueId?: string; dedupeKey?: string; + dedupeKeyPrefix?: string; sourceType?: string; sourceId?: string; resolution: string; @@ -118,7 +120,9 @@ async function dismissOpenRequests( where: { workspaceId: params.workspaceId, status: ActionRequestStatus.OPEN, + ...(params.issueId ? { issueId: params.issueId } : {}), ...(params.dedupeKey ? { dedupeKey: params.dedupeKey } : {}), + ...(params.dedupeKeyPrefix ? { dedupeKey: { startsWith: params.dedupeKeyPrefix } } : {}), ...(params.sourceType ? { sourceType: params.sourceType } : {}), ...(params.sourceId ? { sourceId: params.sourceId } : {}), }, @@ -201,27 +205,40 @@ async function completionContext(db: PrismaClient, workspaceId: string, issueId: ...issue.executionPlans.map(({ id: targetId }) => ({ targetType: "execution-plan", targetId })), ...issue.executionSteps.map(({ id: targetId }) => ({ targetType: "execution-step", targetId })), ]; - const [pendingGates, otherRequests, blockingRelations] = await Promise.all([ - db.reviewGate.count({ - where: { - workspaceId, - status: "PENDING", - OR: gateTargets, - }, - }), - db.actionRequest.count({ - where: { - workspaceId, - issueId, - status: ActionRequestStatus.OPEN, - OR: [{ sourceType: null }, { sourceType: { notIn: [COMPLETION_SOURCE, RECOVERY_SOURCE] } }], - }, - }), - db.issueRelation.findMany({ - where: { workspaceId, fromIssueId: issueId, kind: "BLOCKED_BY" }, - select: { toIssue: { select: { status: { select: { category: true } } } } }, - }), - ]); + const [pendingGates, openDecisionRequests, mcpQuietRequests, blockingRelations] = + await Promise.all([ + db.reviewGate.count({ + where: { + workspaceId, + status: "PENDING", + OR: gateTargets, + }, + }), + db.actionRequest.count({ + where: { + workspaceId, + issueId, + status: ActionRequestStatus.OPEN, + OR: [ + { sourceType: null }, + { sourceType: { notIn: [COMPLETION_SOURCE, RECOVERY_SOURCE] } }, + ], + }, + }), + db.actionRequest.count({ + where: { + workspaceId, + issueId, + status: ActionRequestStatus.OPEN, + sourceType: "work-session", + dedupeKey: { startsWith: "work-session-mcp-quiet:" }, + }, + }), + db.issueRelation.findMany({ + where: { workspaceId, fromIssueId: issueId, kind: "BLOCKED_BY" }, + select: { toIssue: { select: { status: { select: { category: true } } } } }, + }), + ]); const completionStatus = issue.workspace.completionStatusId ? await db.status.findFirst({ @@ -246,7 +263,7 @@ async function completionContext(db: PrismaClient, workspaceId: string, issueId: pullRequests, liveRuns: liveRunIds.length, pendingGates, - otherRequests, + otherRequests: Math.max(0, openDecisionRequests - mcpQuietRequests), unresolvedBlockers, }; } @@ -556,6 +573,16 @@ export async function evaluateIssueCompletionCandidate( })), ].slice(0, 12); const held = autoHeldReasons(context, assessment); + if (assessment.state === "READY") { + await dismissOpenRequests(db, { + workspaceId: params.workspaceId, + actorId: params.actorId, + issueId: params.issueId, + sourceType: "work-session", + dedupeKeyPrefix: "work-session-mcp-quiet:", + resolution: "Completion evidence confirms there is no active work to recover.", + }); + } if ( context.automation === CompletionAutomation.AUTO_WHEN_SAFE && diff --git a/src/server/services/work-session.ts b/src/server/services/work-session.ts index 5974f73e..fd5e6284 100644 --- a/src/server/services/work-session.ts +++ b/src/server/services/work-session.ts @@ -978,21 +978,32 @@ export async function sweepStaleWorkSessions(db: PrismaClient): Promise ), ]; if (recoverySessionIds.length > 0) { - const deliveredSessions = await db.workSession.findMany({ + const reconciledSessions = await db.workSession.findMany({ where: { id: { in: recoverySessionIds }, - status: { in: ["MERGED", "RELEASED", "DEPLOYED", "VERIFIED", "ABANDONED"] }, + OR: [ + { status: { in: ["MERGED", "RELEASED", "DEPLOYED", "VERIFIED", "ABANDONED"] } }, + { issue: { status: { category: { in: ["DONE", "CANCELED"] } } } }, + ], + }, + select: { + id: true, + workspaceId: true, + status: true, + issue: { select: { status: { select: { category: true } } } }, }, - select: { id: true, workspaceId: true, status: true }, }); - for (const session of deliveredSessions) { + for (const session of reconciledSessions) { + const issueIsTerminal = ["DONE", "CANCELED"].includes(session.issue.status.category); await resolveRecoveryRequests( db, session.workspaceId, session.id, - session.status === WorkSessionStatus.ABANDONED - ? "Work session abandoned." - : "Implementation pull request merged.", + issueIsTerminal + ? "Issue is already in a terminal state." + : session.status === WorkSessionStatus.ABANDONED + ? "Work session abandoned." + : "Implementation pull request merged.", ); } } @@ -1010,10 +1021,34 @@ export async function sweepStaleWorkSessions(db: PrismaClient): Promise status: { in: [...STALEABLE_WORK_SESSION_STATUSES] }, lastHeartbeatAt: { lt: cutoff }, }, - include: { ownerConnection: { select: { id: true, kind: true, status: true } } }, + include: { + ownerConnection: { select: { id: true, kind: true, status: true } }, + issue: { + select: { + authorId: true, + number: true, + status: { select: { category: true } }, + workspace: { select: { key: true } }, + agentRuns: { + where: { status: { in: [AgentRunStatus.ACTIVE, AgentRunStatus.WAITING] } }, + select: { id: true }, + take: 1, + }, + }, + }, + }, }); for (const session of sessions) { if (session.ownerConnection?.kind === "MCP_CLIENT") { + if (["DONE", "CANCELED"].includes(session.issue.status.category)) { + await resolveRecoveryRequests( + db, + workspace.id, + session.id, + "Issue is already in a terminal state.", + ); + continue; + } // MCP silence is an expired observation lease, not proof that the // client process failed. Keep delivery ownership intact so the // dispatcher cannot start a competing branch while status is unknown. @@ -1023,20 +1058,23 @@ export async function sweepStaleWorkSessions(db: PrismaClient): Promise where: { id: session.ownerConnection.id, status: "ACTIVE" }, data: { status: "QUIET", confidence: "UNCONFIRMED" }, }); - const issue = await db.issue.findUnique({ - where: { id: session.issueId }, - select: { authorId: true, workspace: { select: { key: true } }, number: true }, - }); + const issue = session.issue; + const issueKey = `${issue.workspace.key}-${issue.number}`; + const hasLiveRun = issue.agentRuns.length > 0; await createActionRequest(db, { workspaceId: workspace.id, actorId: null, - title: `MCP status for ${issue ? `${issue.workspace.key}-${issue.number}` : "an issue"} is unconfirmed`, - body: - `${session.repoFullName}:${session.branch} has not sent a lifecycle signal for ` + - `${workspace.workSessionStaleMinutes} minutes. Delivery remains owned; request status, hand off, or abandon it explicitly.`, + title: hasLiveRun + ? `MCP status for ${issueKey} is unconfirmed` + : `Delivery for ${issueKey} needs a disposition`, + body: hasLiveRun + ? `${session.repoFullName}:${session.branch} has not sent a lifecycle signal for ` + + `${workspace.workSessionStaleMinutes} minutes. Delivery remains owned; request status, hand off, or abandon it explicitly.` + : `${session.repoFullName}:${session.branch} has no active run and its owning MCP connection has not sent a lifecycle signal for ` + + `${workspace.workSessionStaleMinutes} minutes. Silence does not mean work is still running. Resume the session, attach or advance delivery evidence, hand it off, or abandon it explicitly.`, severity: NotificationSeverity.WARNING, kind: ActionRequestKind.FREE_FORM, - assignedUserId: session.ownerUserId ?? issue?.authorId ?? null, + assignedUserId: session.ownerUserId ?? issue.authorId ?? null, assignedAgentId: session.ownerAgentId, sourceType: "work-session", sourceId: session.id,