| author | |
| committer | |
| log | 77f67d1766e6b2afa7e3b87a6da52cc652ceb8bf |
| tree | 17f7246709d28f9a233fb39aa19a3ade883d3668 |
| parent | 1c2ed7593712b54bf4aa649739ebd5c64d4a44ad |
- Catch provider approval response failures and emit `provider.approval.respond.failed`
- Auto-resolve approvals when provider reports unknown pending request IDs
- Add tests for Cursor ACP resume via `resumeCursor.acpSessionId` and legacy `sessionId`3 files changed, 253 insertions(+), 7 deletions(-)
apps/server/src/orchestration/Layers/ProviderCommandReactor.test.ts+99-2| ... | @@ -6,6 +6,7 @@ import type { ProviderRuntimeEvent, ProviderSession } from "@t3tools/contracts"; | ... | @@ -6,6 +6,7 @@ import type { ProviderRuntimeEvent, ProviderSession } from "@t3tools/contracts"; |
| 6 | import { | 6 | import { |
| 7 | ApprovalRequestId, | 7 | ApprovalRequestId, |
| 8 | CommandId, | 8 | CommandId, |
| 9 | EventId, | ||
| 9 | MessageId, | 10 | MessageId, |
| 10 | ProjectId, | 11 | ProjectId, |
| 11 | ProviderSessionId, | 12 | ProviderSessionId, |
| ... | @@ -19,6 +20,7 @@ import { afterEach, describe, expect, it, vi } from "vitest"; | ... | @@ -19,6 +20,7 @@ import { afterEach, describe, expect, it, vi } from "vitest"; |
| 19 | 20 | ||
| 20 | import { ServerConfig } from "../../config.ts"; | 21 | import { ServerConfig } from "../../config.ts"; |
| 21 | import { TextGenerationError } from "../../git/Errors.ts"; | 22 | import { TextGenerationError } from "../../git/Errors.ts"; |
| 23 | import { ProviderAdapterRequestError } from "../../provider/Errors.ts"; | ||
| 22 | import { OrchestrationEventStoreLive } from "../../persistence/Layers/OrchestrationEventStore.ts"; | 24 | import { OrchestrationEventStoreLive } from "../../persistence/Layers/OrchestrationEventStore.ts"; |
| 23 | import { OrchestrationCommandReceiptRepositoryLive } from "../../persistence/Layers/OrchestrationCommandReceipts.ts"; | 25 | import { OrchestrationCommandReceiptRepositoryLive } from "../../persistence/Layers/OrchestrationCommandReceipts.ts"; |
| 24 | import { SqlitePersistenceMemory } from "../../persistence/Layers/Sqlite.ts"; | 26 | import { SqlitePersistenceMemory } from "../../persistence/Layers/Sqlite.ts"; |
| ... | @@ -43,10 +45,10 @@ const asApprovalRequestId = (value: string): ApprovalRequestId => | ... | @@ -43,10 +45,10 @@ const asApprovalRequestId = (value: string): ApprovalRequestId => |
| 43 | const asMessageId = (value: string): MessageId => MessageId.makeUnsafe(value); | 45 | const asMessageId = (value: string): MessageId => MessageId.makeUnsafe(value); |
| 44 | const asTurnId = (value: string): TurnId => TurnId.makeUnsafe(value); | 46 | const asTurnId = (value: string): TurnId => TurnId.makeUnsafe(value); |
| 45 | 47 | ||
| 46 | async function waitFor(predicate: () => boolean, timeoutMs = 2000): Promise<void> { | 48 | async function waitFor(predicate: () => boolean | Promise<boolean>, timeoutMs = 2000): Promise<void> { |
| 47 | const deadline = Date.now() + timeoutMs; | 49 | const deadline = Date.now() + timeoutMs; |
| 48 | const poll = async (): Promise<void> => { | 50 | const poll = async (): Promise<void> => { |
| 49 | if (predicate()) { | 51 | if (await predicate()) { |
| 50 | return; | 52 | return; |
| 51 | } | 53 | } |
| 52 | if (Date.now() >= deadline) { | 54 | if (Date.now() >= deadline) { |
| ... | @@ -775,6 +777,101 @@ describe("ProviderCommandReactor", () => { | ... | @@ -775,6 +777,101 @@ describe("ProviderCommandReactor", () => { |
| 775 | }); | 777 | }); |
| 776 | }); | 778 | }); |
| 777 | 779 | ||
| 780 | it("surfaces stale provider approval request failures and resolves stale pending approvals", async () => { | ||
| 781 | const harness = await createHarness(); | ||
| 782 | const now = new Date().toISOString(); | ||
| 783 | harness.respondToRequest.mockImplementation(() => | ||
| 784 | Effect.fail( | ||
| 785 | new ProviderAdapterRequestError({ | ||
| 786 | provider: "cursor", | ||
| 787 | method: "session/request_permission", | ||
| 788 | detail: "Unknown pending permission request: approval-request-1", | ||
| 789 | }), | ||
| 790 | ), | ||
| 791 | ); | ||
| 792 | |||
| 793 | await Effect.runPromise( | ||
| 794 | harness.engine.dispatch({ | ||
| 795 | type: "thread.session.set", | ||
| 796 | commandId: CommandId.makeUnsafe("cmd-session-set-for-approval-error"), | ||
| 797 | threadId: ThreadId.makeUnsafe("thread-1"), | ||
| 798 | session: { | ||
| 799 | threadId: ThreadId.makeUnsafe("thread-1"), | ||
| 800 | status: "running", | ||
| 801 | providerName: "cursor", | ||
| 802 | providerSessionId: asSessionId("sess-1"), | ||
| 803 | providerThreadId: ProviderThreadId.makeUnsafe("provider-thread-1"), | ||
| 804 | approvalPolicy: "on-request", | ||
| 805 | sandboxMode: "workspace-write", | ||
| 806 | activeTurnId: null, | ||
| 807 | lastError: null, | ||
| 808 | updatedAt: now, | ||
| 809 | }, | ||
| 810 | createdAt: now, | ||
| 811 | }), | ||
| 812 | ); | ||
| 813 | |||
| 814 | await Effect.runPromise( | ||
| 815 | harness.engine.dispatch({ | ||
| 816 | type: "thread.activity.append", | ||
| 817 | commandId: CommandId.makeUnsafe("cmd-approval-requested"), | ||
| 818 | threadId: ThreadId.makeUnsafe("thread-1"), | ||
| 819 | activity: { | ||
| 820 | id: EventId.makeUnsafe("activity-approval-requested"), | ||
| 821 | tone: "approval", | ||
| 822 | kind: "approval.requested", | ||
| 823 | summary: "Command approval requested", | ||
| 824 | payload: { | ||
| 825 | requestId: "approval-request-1", | ||
| 826 | requestKind: "command", | ||
| 827 | }, | ||
| 828 | turnId: null, | ||
| 829 | createdAt: now, | ||
| 830 | }, | ||
| 831 | createdAt: now, | ||
| 832 | }), | ||
| 833 | ); | ||
| 834 | |||
| 835 | await Effect.runPromise( | ||
| 836 | harness.engine.dispatch({ | ||
| 837 | type: "thread.approval.respond", | ||
| 838 | commandId: CommandId.makeUnsafe("cmd-approval-respond-stale"), | ||
| 839 | threadId: ThreadId.makeUnsafe("thread-1"), | ||
| 840 | requestId: asApprovalRequestId("approval-request-1"), | ||
| 841 | decision: "acceptForSession", | ||
| 842 | createdAt: now, | ||
| 843 | }), | ||
| 844 | ); | ||
| 845 | |||
| 846 | await waitFor(async () => { | ||
| 847 | const readModel = await Effect.runPromise(harness.engine.getReadModel()); | ||
| 848 | const thread = readModel.threads.find((entry) => entry.id === ThreadId.makeUnsafe("thread-1")); | ||
| 849 | if (!thread) return false; | ||
| 850 | return thread.activities.some((activity) => activity.kind === "provider.approval.respond.failed"); | ||
| 851 | }); | ||
| 852 | |||
| 853 | const readModel = await Effect.runPromise(harness.engine.getReadModel()); | ||
| 854 | const thread = readModel.threads.find((entry) => entry.id === ThreadId.makeUnsafe("thread-1")); | ||
| 855 | expect(thread).toBeDefined(); | ||
| 856 | |||
| 857 | const failureActivity = thread?.activities.find( | ||
| 858 | (activity) => activity.kind === "provider.approval.respond.failed", | ||
| 859 | ); | ||
| 860 | expect(failureActivity).toBeDefined(); | ||
| 861 | expect(failureActivity?.payload).toMatchObject({ | ||
| 862 | requestId: "approval-request-1", | ||
| 863 | }); | ||
| 864 | |||
| 865 | const resolvedActivity = thread?.activities.find( | ||
| 866 | (activity) => | ||
| 867 | activity.kind === "approval.resolved" && | ||
| 868 | typeof activity.payload === "object" && | ||
| 869 | activity.payload !== null && | ||
| 870 | (activity.payload as Record<string, unknown>).requestId === "approval-request-1", | ||
| 871 | ); | ||
| 872 | expect(resolvedActivity).toBeDefined(); | ||
| 873 | }); | ||
| 874 | |||
| 778 | it("reacts to thread.session.stop by stopping provider session and clearing thread session state", async () => { | 875 | it("reacts to thread.session.stop by stopping provider session and clearing thread session state", async () => { |
| 779 | const harness = await createHarness(); | 876 | const harness = await createHarness(); |
| 780 | const now = new Date().toISOString(); | 877 | const now = new Date().toISOString(); |
apps/server/src/orchestration/Layers/ProviderCommandReactor.ts+92-5| ... | @@ -16,6 +16,7 @@ import { Cache, Cause, Duration, Effect, Layer, Option, Queue, Stream } from "ef | ... | @@ -16,6 +16,7 @@ import { Cache, Cause, Duration, Effect, Layer, Option, Queue, Stream } from "ef |
| 16 | 16 | ||
| 17 | import { resolveThreadWorkspaceCwd } from "../../checkpointing/Utils.ts"; | 17 | import { resolveThreadWorkspaceCwd } from "../../checkpointing/Utils.ts"; |
| 18 | import { GitCore } from "../../git/Services/GitCore.ts"; | 18 | import { GitCore } from "../../git/Services/GitCore.ts"; |
| 19 | import { ProviderAdapterRequestError } from "../../provider/Errors.ts"; | ||
| 19 | import { TextGeneration } from "../../git/Services/TextGeneration.ts"; | 20 | import { TextGeneration } from "../../git/Services/TextGeneration.ts"; |
| 20 | import { ProviderService } from "../../provider/Services/ProviderService.ts"; | 21 | import { ProviderService } from "../../provider/Services/ProviderService.ts"; |
| 21 | import { OrchestrationEngineService } from "../Services/OrchestrationEngine.ts"; | 22 | import { OrchestrationEngineService } from "../Services/OrchestrationEngine.ts"; |
| ... | @@ -71,6 +72,32 @@ const DEFAULT_SANDBOX_MODE: ProviderSandboxMode = "workspace-write"; | ... | @@ -71,6 +72,32 @@ const DEFAULT_SANDBOX_MODE: ProviderSandboxMode = "workspace-write"; |
| 71 | const WORKTREE_BRANCH_PREFIX = "t3code"; | 72 | const WORKTREE_BRANCH_PREFIX = "t3code"; |
| 72 | const TEMP_WORKTREE_BRANCH_PATTERN = new RegExp(`^${WORKTREE_BRANCH_PREFIX}\\/[0-9a-f]{8}$`); | 73 | const TEMP_WORKTREE_BRANCH_PATTERN = new RegExp(`^${WORKTREE_BRANCH_PREFIX}\\/[0-9a-f]{8}$`); |
| 73 | 74 | ||
| 75 | function toErrorMessage(error: unknown): string { | ||
| 76 | if (error instanceof Error && error.message.trim().length > 0) { | ||
| 77 | return error.message; | ||
| 78 | } | ||
| 79 | try { | ||
| 80 | return JSON.stringify(error); | ||
| 81 | } catch { | ||
| 82 | return String(error); | ||
| 83 | } | ||
| 84 | } | ||
| 85 | |||
| 86 | function isUnknownPendingApprovalRequestError(error: unknown): boolean { | ||
| 87 | if (error instanceof ProviderAdapterRequestError) { | ||
| 88 | const detail = error.detail.toLowerCase(); | ||
| 89 | return ( | ||
| 90 | detail.includes("unknown pending approval request") || | ||
| 91 | detail.includes("unknown pending permission request") | ||
| 92 | ); | ||
| 93 | } | ||
| 94 | const message = toErrorMessage(error).toLowerCase(); | ||
| 95 | return ( | ||
| 96 | message.includes("unknown pending approval request") || | ||
| 97 | message.includes("unknown pending permission request") | ||
| 98 | ); | ||
| 99 | } | ||
| 100 | |||
| 74 | function isTemporaryWorktreeBranch(branch: string): boolean { | 101 | function isTemporaryWorktreeBranch(branch: string): boolean { |
| 75 | return TEMP_WORKTREE_BRANCH_PATTERN.test(branch.trim().toLowerCase()); | 102 | return TEMP_WORKTREE_BRANCH_PATTERN.test(branch.trim().toLowerCase()); |
| 76 | } | 103 | } |
| ... | @@ -127,6 +154,7 @@ const make = Effect.gen(function* () { | ... | @@ -127,6 +154,7 @@ const make = Effect.gen(function* () { |
| 127 | readonly detail: string; | 154 | readonly detail: string; |
| 128 | readonly turnId: TurnId | null; | 155 | readonly turnId: TurnId | null; |
| 129 | readonly createdAt: string; | 156 | readonly createdAt: string; |
| 157 | readonly requestId?: string; | ||
| 130 | }) => | 158 | }) => |
| 131 | orchestrationEngine.dispatch({ | 159 | orchestrationEngine.dispatch({ |
| 132 | type: "thread.activity.append", | 160 | type: "thread.activity.append", |
| ... | @@ -139,6 +167,7 @@ const make = Effect.gen(function* () { | ... | @@ -139,6 +167,7 @@ const make = Effect.gen(function* () { |
| 139 | summary: input.summary, | 167 | summary: input.summary, |
| 140 | payload: { | 168 | payload: { |
| 141 | detail: input.detail, | 169 | detail: input.detail, |
| 170 | ...(input.requestId ? { requestId: input.requestId } : {}), | ||
| 142 | }, | 171 | }, |
| 143 | turnId: input.turnId, | 172 | turnId: input.turnId, |
| 144 | createdAt: input.createdAt, | 173 | createdAt: input.createdAt, |
| ... | @@ -146,6 +175,33 @@ const make = Effect.gen(function* () { | ... | @@ -146,6 +175,33 @@ const make = Effect.gen(function* () { |
| 146 | createdAt: input.createdAt, | 175 | createdAt: input.createdAt, |
| 147 | }); | 176 | }); |
| 148 | 177 | ||
| 178 | const appendApprovalResolvedActivity = (input: { | ||
| 179 | readonly threadId: ThreadId; | ||
| 180 | readonly requestId: string; | ||
| 181 | readonly decision: "accept" | "acceptForSession" | "decline" | "cancel"; | ||
| 182 | readonly detail: string; | ||
| 183 | readonly createdAt: string; | ||
| 184 | }) => | ||
| 185 | orchestrationEngine.dispatch({ | ||
| 186 | type: "thread.activity.append", | ||
| 187 | commandId: serverCommandId("approval-resolved"), | ||
| 188 | threadId: input.threadId, | ||
| 189 | activity: { | ||
| 190 | id: EventId.makeUnsafe(crypto.randomUUID()), | ||
| 191 | tone: "approval", | ||
| 192 | kind: "approval.resolved", | ||
| 193 | summary: "Approval resolved", | ||
| 194 | payload: { | ||
| 195 | requestId: input.requestId, | ||
| 196 | decision: input.decision, | ||
| 197 | detail: input.detail, | ||
| 198 | }, | ||
| 199 | turnId: null, | ||
| 200 | createdAt: input.createdAt, | ||
| 201 | }, | ||
| 202 | createdAt: input.createdAt, | ||
| 203 | }); | ||
| 204 | |||
| 149 | const setThreadSession = (input: { | 205 | const setThreadSession = (input: { |
| 150 | readonly threadId: ThreadId; | 206 | readonly threadId: ThreadId; |
| 151 | readonly session: OrchestrationSession; | 207 | readonly session: OrchestrationSession; |
| ... | @@ -488,14 +544,45 @@ const make = Effect.gen(function* () { | ... | @@ -488,14 +544,45 @@ const make = Effect.gen(function* () { |
| 488 | detail: "No active provider session is bound to this thread.", | 544 | detail: "No active provider session is bound to this thread.", |
| 489 | turnId: null, | 545 | turnId: null, |
| 490 | createdAt: event.payload.createdAt, | 546 | createdAt: event.payload.createdAt, |
| 547 | requestId: event.payload.requestId, | ||
| 491 | }); | 548 | }); |
| 492 | } | 549 | } |
| 493 | 550 | ||
| 494 | yield* providerService.respondToRequest({ | 551 | yield* providerService |
| 495 | sessionId, | 552 | .respondToRequest({ |
| 496 | requestId: event.payload.requestId, | 553 | sessionId, |
| 497 | decision: event.payload.decision, | 554 | requestId: event.payload.requestId, |
| 498 | }); | 555 | decision: event.payload.decision, |
| 556 | }) | ||
| 557 | .pipe( | ||
| 558 | Effect.catchCause((cause) => | ||
| 559 | Effect.gen(function* () { | ||
| 560 | const error = Cause.squash(cause); | ||
| 561 | const detail = toErrorMessage(error); | ||
| 562 | yield* appendProviderFailureActivity({ | ||
| 563 | threadId: event.payload.threadId, | ||
| 564 | kind: "provider.approval.respond.failed", | ||
| 565 | summary: "Provider approval response failed", | ||
| 566 | detail, | ||
| 567 | turnId: null, | ||
| 568 | createdAt: event.payload.createdAt, | ||
| 569 | requestId: event.payload.requestId, | ||
| 570 | }); | ||
| 571 | |||
| 572 | if (!isUnknownPendingApprovalRequestError(error)) { | ||
| 573 | return; | ||
| 574 | } | ||
| 575 | |||
| 576 | yield* appendApprovalResolvedActivity({ | ||
| 577 | threadId: event.payload.threadId, | ||
| 578 | requestId: event.payload.requestId, | ||
| 579 | decision: event.payload.decision, | ||
| 580 | detail: "Approval request is no longer pending in the provider session.", | ||
| 581 | createdAt: event.payload.createdAt, | ||
| 582 | }); | ||
| 583 | }), | ||
| 584 | ), | ||
| 585 | ); | ||
| 499 | }); | 586 | }); |
| 500 | 587 | ||
| 501 | const processSessionStopRequested = Effect.fnUntraced(function* ( | 588 | const processSessionStopRequested = Effect.fnUntraced(function* ( |
apps/server/src/provider/Layers/CursorAdapter.test.ts+62| ... | @@ -359,6 +359,68 @@ describe("CursorAdapterLive", () => { | ... | @@ -359,6 +359,68 @@ describe("CursorAdapterLive", () => { |
| 359 | }).pipe(Effect.provide(layer)); | 359 | }).pipe(Effect.provide(layer)); |
| 360 | }); | 360 | }); |
| 361 | 361 | ||
| 362 | it.effect("resumes ACP session using resumeCursor.acpSessionId", () => { | ||
| 363 | const fake = new FakeCursorAcpProcess(); | ||
| 364 | const layer = makeCursorAdapterLive({ | ||
| 365 | createProcess: () => fake as never, | ||
| 366 | }); | ||
| 367 | |||
| 368 | return Effect.gen(function* () { | ||
| 369 | const adapter = yield* CursorAdapter; | ||
| 370 | const session = yield* adapter.startSession({ | ||
| 371 | provider: "cursor", | ||
| 372 | cwd: "/tmp/project", | ||
| 373 | resumeCursor: { | ||
| 374 | acpSessionId: "acp-session-resume", | ||
| 375 | }, | ||
| 376 | }); | ||
| 377 | |||
| 378 | const methods = new Set(fake.requests.map((request) => request.method)); | ||
| 379 | assert.equal(methods.has("session/load"), true); | ||
| 380 | assert.equal(methods.has("session/new"), false); | ||
| 381 | |||
| 382 | const loadRequest = fake.requests.find((request) => request.method === "session/load"); | ||
| 383 | assert.deepEqual(loadRequest?.params, { | ||
| 384 | sessionId: "acp-session-resume", | ||
| 385 | cwd: "/tmp/project", | ||
| 386 | mcpServers: [], | ||
| 387 | }); | ||
| 388 | assert.equal(session.threadId, "acp-session-resume"); | ||
| 389 | assert.deepEqual(session.resumeCursor, { | ||
| 390 | acpSessionId: "acp-session-resume", | ||
| 391 | }); | ||
| 392 | }).pipe(Effect.provide(layer)); | ||
| 393 | }); | ||
| 394 | |||
| 395 | it.effect("accepts legacy resumeCursor.sessionId for ACP session resume", () => { | ||
| 396 | const fake = new FakeCursorAcpProcess(); | ||
| 397 | const layer = makeCursorAdapterLive({ | ||
| 398 | createProcess: () => fake as never, | ||
| 399 | }); | ||
| 400 | |||
| 401 | return Effect.gen(function* () { | ||
| 402 | const adapter = yield* CursorAdapter; | ||
| 403 | const session = yield* adapter.startSession({ | ||
| 404 | provider: "cursor", | ||
| 405 | cwd: "/tmp/project", | ||
| 406 | resumeCursor: { | ||
| 407 | sessionId: "acp-session-legacy", | ||
| 408 | }, | ||
| 409 | }); | ||
| 410 | |||
| 411 | const loadRequest = fake.requests.find((request) => request.method === "session/load"); | ||
| 412 | assert.deepEqual(loadRequest?.params, { | ||
| 413 | sessionId: "acp-session-legacy", | ||
| 414 | cwd: "/tmp/project", | ||
| 415 | mcpServers: [], | ||
| 416 | }); | ||
| 417 | assert.equal(session.threadId, "acp-session-legacy"); | ||
| 418 | assert.deepEqual(session.resumeCursor, { | ||
| 419 | acpSessionId: "acp-session-legacy", | ||
| 420 | }); | ||
| 421 | }).pipe(Effect.provide(layer)); | ||
| 422 | }); | ||
| 423 | |||
| 362 | it.effect("bridges permission requests to request.opened/request.resolved", () => { | 424 | it.effect("bridges permission requests to request.opened/request.resolved", () => { |
| 363 | const fake = new FakeCursorAcpProcess(); | 425 | const fake = new FakeCursorAcpProcess(); |
| 364 | const layer = makeCursorAdapterLive({ | 426 | const layer = makeCursorAdapterLive({ |