From a0250284e368e9778ef82d2e3d98cfd224440cb6 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Wed, 4 Nov 2026 22:04:12 -0600 Subject: [PATCH] fix: preserve proposal revision turns and prove chat completion (#19105) ## Thinking Path > - Paperclip manages work done by AI agents. > - A completed delegated task must report its result to the originating chat. > - That report uses a real provider turn and must stay in cost accounting. > - Ordinary first-task proposals must also support the promised chat acceptance path. > - A revision response can reject an older proposal before it creates the replacement. > - The old rejection transition clears that active response's ownership and blocks its next tool. > - This change proves required completion work, guides ordinary proposal policy and preserves the revision turn under the existing locks. ## Linked Issues or Issue Description No separate issue exists. The original failures remain in [the complete Product run](https://github.com/paperclipai/paperclip/actions/runs/41915965118). The independent continuation repair is #19076. Related #12209 concerns prior-run confirmation binding. This change fixes the ownership reset during rejection, rather than changing binding authorization. **What happened?** The complete 20-case run finished with 13 passes and 7 failures. Three delegated-chat cases counted one required `chat_task_completed` provider turn as unexpected work. Two first-task agents chose optional human-only cards, then correctly received 403 when they tried conversational acceptance. A [reviewed two-case candidate run](https://github.com/paperclipai/paperclip/actions/runs/41931445311) then used the changed first-task skill. Both models inherited the ordinary resolver policy. Clarify/propose/accept passed all 12 predicates. Revise/accept failed after the new response rejected its predecessor's proposal. The saved rejection was followed by `paperclip_runner_tool_binding_not_authorized` on its next tools. The final `native_session_lease_lost` label followed those binding failures; this PR does not assume lease expiry caused them. Both cells cleaned up and published their original grades. **Expected behavior** Keep the existing ordinary-turn bounds and account for one independently proved completion notification. Inherit the normal resolver policy for ordinary proposals that accept chat replies. When an active response rejects an older proposal, save the rejection and retain that response's current ownership so it can create the revised proposal. Keep human, current-source and native completion-review rejection behavior and all real approval restrictions. **Steps to reproduce** The original full run used `4740d251`. The two-case candidate used trusted workflow `7d1e9b34` and target `74da56c4`. Run the focused provider-free controls below. The dual-run API regression first fails because rejection changes the issue to `todo` and clears the newer run's execution and checkout fields. ## What Changed - Prove one completion notification only for native Codex/local `multi-repository`, `clarify-reuse` and `plan-handoff`. Keep the ordinary bounds of 2, 3 and 4. Preserve actual totals of 3, 4 and 5, all run IDs and all usage. - Read those declared totals from the current catalog, which now includes the completion reply. Check the total once and prove that exactly one turn is the required notification. Do not add another turn while waiting. The required-server controls use the actual catalog tasks. - Bind the notification to the same company, agent, conversation generation, completed child, single delivery, accounted native run, saved reply and rendered result link. Reject extra work, duplicate deliveries, unknown events and mutations, including native work without a Paperclip tool call. - Collect workspace operations through the real run API for these three proofs. Keep other collector calls unchanged. Keep the proof controls in the existing required server test lane. - Stop idle polling on a permanent completion-proof error. Continue to retry transient observations within the original 240-second window. - Guide ordinary chat-accepting first-task proposals to omit optional `resolverPolicy` and inherit server/company defaults. Preserve explicit and governed human-only restrictions. Do not lower or recreate saved restricted cards. - Under the existing locked issue and interaction rows, preserve the active responding run when it rejects a different source run's proposal. Change only the ordinary requeue predicate. Keep the saved rejection, notifications, activity and existing Telemetry calls. Keep global status, lease and tool-authority guards unchanged. - Extend the existing native API controls with the actual dual-run rejection/revision sequence and policy, provenance, human, current-source and native completion-review countercontrols. No new workflow or test framework is needed. ## Verification - Current combined head is `a8567ded67e5ab811675390e9a5c5ed2194aee2f`, integrated cleanly on `fb96537e28c4c7bfced4939909cb334392320868`. The upstream completion-reply waiter is preserved. [Current required CI](https://github.com/paperclipai/paperclip/actions/runs/41940780852) passes. Actual test checkout `ff93c034d7bb40f3df7bc75cf51828fd07bb0d29` merges this head into `9c6993f0aab7ebb22a26d78f5d227accb35a6ad1`. - Fresh Greptile is 5/5 on exact head `a8567ded`, with no blocking finding. All addressed review threads are resolved. The separate automated reviewer emitted a paused-work response; that response is not a review approval. - Current required server CI actually passed all 11 native confirmation API cases, 46 completion-proof controls and 13 onboarding asset checks. Canonical typecheck, Build, the native package/server suites, all browser and serialized shards, `ci / e2e` and `ci / verify` pass. No current-head CI retry was needed. - Local dual-run RED: the unmodified transition returned HTTP 200 and saved rejection, then changed the issue to `todo` and cleared the newer run's execution/checkout fields. Whole setup and check took 28.95 seconds; the case took 596 ms. - Local treatment: all 11 real native/API cases pass. The same-session response retains execution/checkout ownership, reads task context and persists the recorded-shape revised confirmation. Its stored lease receipt stays intact and one rejection activity is saved. Human-only rejection, current-source rejection, human rejection with/without a newer owner, Plan/Ask, foreign provenance, authentication and formal approval controls pass. - All 59 existing confirmation resolver controls pass in 16.62 seconds. These cover cancellation, lost ownership, company/user scope, policy changes, transactional rollback, concurrent retries and stale revisions. - All 46 completion controls pass in 4.07 seconds. The two added clock controls preserve the original idle deadline and prove immediate permanent-proof failure plus transient observation retry. The existing live collector-to-proof and native mutation negatives remain in this required server file. - The first current-catalog integration check failed 5 controls in 2.70 seconds. The old consumer added the already-declared reply again and could accept an extra ordinary turn. The corrected consumer passes all 46 controls using the actual catalog. The existing full 20-case catalog/reporting compatibility check also passes in 4.93 seconds. Case identities, configured models and reporting match the declared 20. - The API controls used one owned loopback PostgreSQL instance through existing supported test hooks. It was stopped and removed. Earlier missing-library/setup failures and one corrected current-source test expectation remain retained; they are not counted as product failures or passes. - The unchanged first-task skill has SHA-256 `b789090996e7925676590c006418eb8b57f4514e646f6bca18ffd7b2b8e864de`. Both candidate runtime copies matched it. The inherited policy was observed in both models; one complete acceptance journey passed. This does not qualify the new rejection repair. - The three retained real completion streams pass the stronger local proof, including native items, workspace observations and recorded DOM links. Their original failed grades stay unchanged. - Earlier related checks passed 153 hiring/non-execution controls and 13 asset checks. Their source is unchanged. Required discovery includes the completion, API and asset files exactly once. - Prior candidate `74da56c4` passed all required CI, including canonical typecheck and build, in [CI run 41929148958](https://github.com/paperclipai/paperclip/actions/runs/41929148958). Its fresh Greptile was 5/5. This is prior-head evidence. - The intermediate `053150d9` head also received Greptile 5/5. Its [CI run](https://github.com/paperclipai/paperclip/actions/runs/41940014165) and review stay retained as superseded evidence. They do not qualify the final catalog correction. - A clean full local typecheck/build is not claimed because the reused dependency cache has stale adapter-utils/Runner declarations. No source or lock workaround was made. Fresh canonical CI must pass before merge. - The original full-plan result remains 13/20. The separate two-case probe remains 1/2. A fresh complete saved-plan run against delivered source is required for acceptance. No case, assertion, model, budget, timeout or failed grade is changed. Focused commands: ```sh # From server/; database URLs point only to an owned disposable local instance. node ../node_modules/vitest/vitest.mjs run src/__tests__/chat-completion-proof.test.ts PAPERCLIP_TEST_DATABASE_URL="$OWNED_TEST_DATABASE_URL" node ../node_modules/vitest/vitest.mjs run src/services/native-runtime/confirmation-comment-api.integration.test.ts PAPERCLIP_TEST_POSTGRES_ADMIN_URL="$OWNED_TEST_DATABASE_URL" node ../node_modules/vitest/vitest.mjs run src/__tests__/confirmation-comment-resolution.test.ts ``` ## Risks - The ownership exception applies only to an agent acting as the current execution run against a different non-null source run. Human, unowned and current-source requeue behavior must stay intact. Native completion-review handling remains separate. - The completion proof fails closed on new native tools or event shapes. A new shape needs review. - A stored unchanged lease receipt is supporting local evidence. The final real journey must prove full revision, execution and delivery. - The new head still needs normal protected merge approval. The optional queued Paperclip review is not a passing review. No causal repair of unrelated CI intermittency is claimed. ## Model Used OpenAI `gpt-6.1-sol` in Codex, with `max` reasoning effort. The runtime reports a 475,000-token context window. Used reasoning, code editing, shell tools and provider-free controls. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip --- .../__tests__/chat-completion-proof.test.ts | 213 ++++++++++++++++++ .../first-task/skills/first-task/SKILL.md | 2 + .../src/services/issue-thread-interactions.ts | 10 + ...nfirmation-comment-api.integration.test.ts | 155 ++++++++++++- tests/runner-e2e/chat-completion-proof.ts | 152 +++++++++++++ tests/runner-e2e/chat-flow.ts | 64 +++++- 6 files changed, 581 insertions(+), 15 deletions(-) create mode 100644 server/src/__tests__/chat-completion-proof.test.ts create mode 100644 tests/runner-e2e/chat-completion-proof.ts diff --git a/server/src/__tests__/chat-completion-proof.test.ts b/server/src/__tests__/chat-completion-proof.test.ts new file mode 100644 index 00000000000..f91dd273317 --- /dev/null +++ b/server/src/__tests__/chat-completion-proof.test.ts @@ -0,0 +1,213 @@ +import { describe, expect, it, vi } from "vitest"; +import { assertChatFlowRunCount, collectChatRunEvidence, retryChatCompletionObservation } from "../../../tests/runner-e2e/chat-flow.js"; +import { needsChatCompletionProof, verifyChatCompletion, type ChatCompletionEvidence } from "../../../tests/runner-e2e/chat-completion-proof.js"; +import { pollUntil } from "../../../tests/runner-e2e/api.js"; +import { chatTasks } from "../../../tests/runner-e2e/chat-cases.js"; + +// Shape and ordering from run 41915965118's three retained failing journeys. +// The ordinary turns keep their existing bound; the one real completion turn +// must prove its own child, delivery, native tools, reply and rendered link. +function completionRecording(id = "multi-repository", ordinaryRunCount = 2) { + const task = chatTasks.find(task => task.id === id); + if (!task) throw new Error("Completion recording must bind an actual catalog task"); + const companyId = "company", agentId = "agent", sourceId = "chat", childId = "child"; + const completedAt = "2026-11-04T21:35:52.000Z"; + const issue = { id: sourceId, companyId, conversationAgentId: agentId, conversationSessionGeneration: 0 }; + const child = { id: childId, companyId, assigneeAgentId: agentId, status: "done", completedAt, identifier: "RUN-2" }; + const ordinary = Array.from({ length: ordinaryRunCount }, (_, index) => ({ + id: `ordinary-${index}`, companyId, agentId, status: "succeeded", runtimeMode: "native", + issueId: index === ordinaryRunCount - 1 ? childId : sourceId, nativeIssueId: index === ordinaryRunCount - 1 ? childId : sourceId, + contextSnapshot: { issueId: index === ordinaryRunCount - 1 ? childId : sourceId }, + startedAt: "2026-11-04T21:35:00.000Z", processStartedAt: "2026-11-04T21:35:01.000Z", + finishedAt: "2026-11-04T21:35:51.000Z", + nativeSessionId: `native-${index}`, usageJson: { accountingReceiptReady: true, outputTokens: 10 }, + })); + const notification = { + id: "notification", companyId, agentId, issueId: sourceId, nativeIssueId: sourceId, + status: "succeeded", runtimeMode: "native", invocationSource: "automation", triggerDetail: "system", + startedAt: "2026-11-04T21:35:53.000Z", processStartedAt: "2026-11-04T21:35:54.000Z", + finishedAt: "2026-11-04T21:36:15.000Z", nativeSessionId: "native-chat-session", + contextSnapshot: { issueId: sourceId, wakeReason: "chat_task_completed", wakeSource: "automation", + conversationSessionGeneration: 0, chatCompletionDeliveryIds: ["delivery"], + chatCompletionUpdates: [{ id: childId, status: "done", completedAt, identifier: "RUN-2", url: "/issues/RUN-2" }] }, + usageJson: { inputTokens: 40_178, outputTokens: 2071, cachedInputTokens: 222_147, + costUsdExact: "0.290990800", costStatus: "estimated", accountingReceiptReady: true }, + }; + const comments = [{ id: "reply", companyId, issueId: sourceId, authorAgentId: agentId, authorType: "agent", + createdByRunId: notification.id, conversationSessionGeneration: 0, + createdAt: "2026-11-04T21:36:15.100Z", deletedAt: null, + body: "[RUN-2](/issues/RUN-2) is complete. Its requested document is saved on the task." }]; + const events: ChatCompletionEvidence["runEvidence"][number]["events"] = ["tool.execution.started", "tool.execution.completed", "run.result.accepted"].map((eventType, index) => ({ + companyId, runId: notification.id, agentId, eventType, seq: index + 1, + payload: { prpEvent: { schema: "paperclip.prp.event.v1", runId: notification.id, eventType, + payload: eventType.startsWith("tool.") ? { schema: "paperclip.tool.execution.v1", name: "paperclip_finish", + executionId: "finish", status: index === 0 ? "running" : "completed" } : {} } }, + })); + const runs = [...ordinary, notification]; + return { suiteId: "agent-chat", profileId: "runner-codex", environmentId: "local", notification, + task: { ...task }, runs, + completionEvidence: { companyId, agentId, companyPrefix: "RUN", baseURL: "http://fixture.invalid", issue, child, + tasks: [child], comments, runEvidence: [{ runId: notification.id, events, + workspaceOperations: [] as Array> }], + renderedLinks: [{ commentId: "reply", href: "/RUN/issues/RUN-2" }] } }; +} + +describe("delegated chat completion provider accounting", () => { + it("reports a permanent completion proof failure without retrying to the idle deadline", async () => { + vi.useFakeTimers(); + try { + const input = completionRecording(); + input.completionEvidence.tasks.push({ ...input.completionEvidence.child, id: "extra-child" }); + const load = vi.fn(async () => verifyChatCompletion(input.runs, input.completionEvidence)); + const outcome = pollUntil({ label: "chat turn settles to waiting", deadlineAt: Date.now() + 240_000, + intervalMs: 1000, load, accept: Boolean, retryOnError: retryChatCompletionObservation }) + .then(value => ({ value, error: undefined }), error => ({ value: undefined, error })); + await vi.advanceTimersByTimeAsync(240_000); + expect((await outcome).error?.message).toBe("Chat completion proof: extra_work"); + expect(load).toHaveBeenCalledTimes(1); + expect(vi.getTimerCount()).toBe(0); + } finally { vi.useRealTimers(); } + }); + it("still retries a transient observation error within the original idle deadline", async () => { + vi.useFakeTimers(); + try { + const input = completionRecording(); + const load = vi.fn(async () => verifyChatCompletion(input.runs, input.completionEvidence)); + load.mockRejectedValueOnce(new Error("temporary API observation failure")); + const outcome = pollUntil({ label: "chat turn settles to waiting", deadlineAt: Date.now() + 240_000, + intervalMs: 1000, load, accept: Boolean, retryOnError: retryChatCompletionObservation }); + await vi.advanceTimersByTimeAsync(1000); + expect(await outcome).toMatchObject({ ordinaryProviderRuns: 2, totalProviderRuns: 3 }); + expect(load).toHaveBeenCalledTimes(2); + expect(vi.getTimerCount()).toBe(0); + } finally { vi.useRealTimers(); } + }); + it("collects the live run API evidence before proving a completion", async () => { + const input = completionRecording(); + const recorded = input.completionEvidence.runEvidence[0]!; + const native = (eventType: string, payload: Record) => ({ companyId: "company", agentId: "agent", + runId: input.notification.id, eventType, + payload: { prpEvent: { schema: "paperclip.prp.event.v1", runId: input.notification.id, eventType, payload } } }); + // The native item/tool shapes come from the retained completion stream. + // Use the actual collector, including its complete paged-event read. + recorded.events = [ + native("item.completed", { kind: "model", model: "gpt-5.6-sol" }), + native("item.completed", { kind: "reasoning", item: { type: "reasoning" } }), + native("item.started", { kind: "dynamicToolCall", item: { type: "tool_use", id: "read", name: "get_task" } }), + native("tool.execution.started", { schema: "paperclip.tool.execution.v1", name: "get_task", executionId: "read", status: "running" }), + native("item.completed", { kind: "dynamicToolCall", item: { type: "tool_result", id: "read", tool_use_id: "read" } }), + native("tool.execution.completed", { schema: "paperclip.tool.execution.v1", name: "get_task", executionId: "read", status: "completed" }), + native("item.completed", { kind: "agentMessage", item: { type: "agentMessage" } }), + ...recorded.events, + native("item.completed", { kind: "usage", usage: input.notification.usageJson }), + ].map((event, index) => ({ ...event, seq: index + 1 })); + const workspaceOperations = [{ companyId: "company", heartbeatRunId: input.notification.id, + phase: "workspace_finalize", status: "succeeded", command: null }]; + const requested: string[] = []; + const api = { async get(path: string): Promise { + requested.push(path); + const url = new URL(path, "http://fixture.invalid"); + if (url.pathname.endsWith("/workspace-operations")) return workspaceOperations as T; + if (url.pathname.endsWith("/log")) return { content: "recorded completion notification" } as T; + if (url.pathname.endsWith("/events")) return recorded.events.filter(e => e.seq > Number(url.searchParams.get("afterSeq"))) as T; + throw new Error("Unexpected evidence endpoint"); + } }; + const collected = await collectChatRunEvidence(api, input.notification, false, true); + expect(verifyChatCompletion(input.runs, { ...input.completionEvidence, runEvidence: [collected] })) + .toMatchObject({ ordinaryProviderRuns: 2, totalProviderRuns: 3 }); + expect(requested).toContain(`/api/heartbeat-runs/${input.notification.id}/workspace-operations`); + expect(collected).toMatchObject({ workspaceOperations }); + requested.length = 0; + await collectChatRunEvidence(api, input.notification); + expect(requested).not.toContain(`/api/heartbeat-runs/${input.notification.id}/workspace-operations`); + }); + + for (const [eventType, payload] of [ + ["item.completed", { kind: "fileChange", item: { type: "fileChange", changes: [{ path: "extra.txt" }] } }], + ["item.started", { kind: "commandExecution", item: { type: "commandExecution", command: "unexpected work" } }], + ["item.completed", { kind: "webSearch", item: { type: "webSearch" } }], + ["workspace.change.updated", { schema: "paperclip.workspace.diff.v1", files: [{ path: "extra.txt", operation: "add" }] }], + ["research.search.started", { query: "unexpected research" }], + ["item.completed", { kind: "dynamicToolCall", item: { type: "tool_use", id: "hidden", name: "create_task" } }], + ["item.completed", { kind: "unknown_provider_work" }], + ["item.completed", { kind: "agentMessage", item: { type: "fileChange" } }], + ["unrecognized.provider.work", {}], + ] as const) it(`rejects native work without a Paperclip tool event: ${eventType}/${"kind" in payload ? payload.kind : "event"}`, () => { + const input = completionRecording(); + input.completionEvidence.runEvidence[0]!.events.push({ companyId: "company", agentId: "agent", + runId: input.notification.id, eventType, seq: 4, + payload: { prpEvent: { schema: "paperclip.prp.event.v1", runId: input.notification.id, eventType, payload } } }); + expect(() => verifyChatCompletion(input.runs, input.completionEvidence)).toThrow(/Chat completion proof:/); + }); + + for (const [id, bound] of [["multi-repository", 2], ["clarify-reuse", 3], ["plan-handoff", 4]] as const) { + it(`accounts for the required verified notification after ${id}`, () => { + const input = completionRecording(id, bound); + const original = JSON.stringify(input); + expect(input.task.expectedRunCount).toBe(bound + 1); + expect(input.task.minimumExpectedRunCount).toBeUndefined(); + expect(() => assertChatFlowRunCount(input)).not.toThrow(); + expect(verifyChatCompletion(input.runs, input.completionEvidence)).toMatchObject({ ordinaryProviderRuns: bound, + totalProviderRuns: bound + 1, allRunIds: input.runs.map(r => r.id), notificationRunId: input.notification.id }); + expect(input.runs).toHaveLength(bound + 1); + expect(input.runs.at(-1)?.usageJson?.outputTokens).toBe(2071); + expect(JSON.stringify(input)).toBe(original); + }); + } + + const invalid: Array<[string, (input: ReturnType) => void]> = [ + ["missing required notification", f => { f.runs.pop(); }], + ["duplicate paid notification", f => { f.runs.push({ ...f.notification, id: "duplicate" }); }], + ["wrong source company", f => { f.completionEvidence.issue.companyId = "other-company"; }], + ["wrong child company", f => { f.completionEvidence.child.companyId = "other-company"; }], + ["cross-scope notification", f => { f.notification.companyId = "other-company"; }], + ["old conversation generation", f => { f.notification.contextSnapshot.conversationSessionGeneration = 1; }], + ["unknown automatic work", f => { f.notification.contextSnapshot.wakeReason = "unrelated_automation"; }], + ["two delivery receipts", f => { f.notification.contextSnapshot.chatCompletionDeliveryIds.push("other-delivery"); }], + ["wrong completed child", f => { f.notification.contextSnapshot.chatCompletionUpdates[0]!.id = "other-child"; }], + ["unfinished child", f => { f.completionEvidence.child.status = "in_review"; }], + ["never-started native work", f => { f.notification.processStartedAt = ""; }], + ["missing actual native completion", f => { f.notification.finishedAt = ""; }], + ["unaccounted provider work", f => { f.notification.usageJson.accountingReceiptReady = false; }], + ["missing intended reply", f => { f.completionEvidence.comments.length = 0; }], + ["duplicate completion replies", f => { f.completionEvidence.comments.push({ ...f.completionEvidence.comments[0]!, id: "other-reply" }); }], + ["deleted reply", f => { Object.assign(f.completionEvidence.comments[0]!, { deletedAt: "2026-11-04T21:36:16Z" }); }], + ["cross-generation reply", f => { f.completionEvidence.comments[0]!.conversationSessionGeneration = 1; }], + ["missing rendered result access", f => { f.completionEvidence.renderedLinks.length = 0; }], + ["foreign-origin task link", f => { f.completionEvidence.renderedLinks[0]!.href = "https://other.invalid/RUN/issues/RUN-2"; }], + ["foreign-company task route", f => { f.completionEvidence.renderedLinks[0]!.href = "/OTHER/issues/RUN-2"; }], + ["extra delegated work", f => { f.completionEvidence.tasks.push({ ...f.completionEvidence.child, id: "extra-child" }); }], + ["missing native result stream", f => { f.completionEvidence.runEvidence[0]!.events.length = 0; }], + ["cross-scope native events", f => { f.completionEvidence.runEvidence[0]!.events[0]!.companyId = "other-company"; }], + ["accepted result for another native run", f => { f.completionEvidence.runEvidence[0]!.events[2]!.payload.prpEvent.runId = "other-run"; }], + ["unfinished native tool", f => { f.completionEvidence.runEvidence[0]!.events.splice(1, 1); }], + ["tool completion before execution", f => { f.completionEvidence.runEvidence[0]!.events.reverse(); }], + ["missing workspace observation", f => { Object.assign(f.completionEvidence.runEvidence[0]!, { workspaceOperations: undefined }); }], + ["new task tool inside a notification", f => { + for (const e of f.completionEvidence.runEvidence[0]!.events.slice(0, 2)) e.payload.prpEvent.payload.name = "create_task"; + }], + ["additional workspace command", f => { f.completionEvidence.runEvidence[0]!.workspaceOperations.push({ + companyId: "company", heartbeatRunId: "notification", phase: "command", status: "succeeded", command: "unexpected work" }); }], + ]; + for (const [name, mutate] of invalid) it(`rejects ${name}`, () => { + const input = completionRecording(); mutate(input); + expect(() => assertChatFlowRunCount(input)).toThrow(/Chat completion proof:/); + }); + + it("keeps the existing ordinary-turn count exact", () => { + const missing = completionRecording(); missing.runs.splice(0, 1); + expect(() => assertChatFlowRunCount(missing)).toThrow(/Declared total provider-run bounds/); + const extra = completionRecording(); extra.runs.splice(0, 0, { ...extra.runs[0]!, id: "extra-ordinary" }); + expect(() => assertChatFlowRunCount(extra)).toThrow(/Declared total provider-run bounds/); + }); + + it("keeps every other case, profile and environment on its original total bound", () => { + for (const patch of [{ task: { id: "create-backlog", expectedRunCount: 2 } }, + { profileId: "runner-opencode" }, { environmentId: "daytona" }, { suiteId: "other-suite" }]) { + const input = { ...completionRecording(), ...patch }; + input.runs.push({ ...input.runs[0]!, id: "extra-run" }); + expect(() => assertChatFlowRunCount(input)).toThrow(/Declared total provider-run bounds/); + } + expect(needsChatCompletionProof("agent-chat", undefined, "runner-codex", "local")).toBe(false); + }); +}); diff --git a/server/src/onboarding-assets/first-task/skills/first-task/SKILL.md b/server/src/onboarding-assets/first-task/skills/first-task/SKILL.md index 9afe2d13692..61190126d1c 100644 --- a/server/src/onboarding-assets/first-task/skills/first-task/SKILL.md +++ b/server/src/onboarding-assets/first-task/skills/first-task/SKILL.md @@ -42,6 +42,8 @@ Work in this order. The tool's root fields are `idempotencyKey`, `interactionKind`, `title`, `prompt`, `continuationPolicy`, and `payload`, with optional `targetRevisionId` and `resolverPolicy` as advertised. Put `version` and `supersedeOnUserComment` inside `payload`; neither is a root argument. If validation rejects a proposal, correct its argument placement and keep the confirmation or checkbox approval kind. Switching it to a question form cannot authorize the proposed work. Wait for a successful saved card receipt before claiming you requested approval. + For ordinary first-task proposals that accept a reply in this conversation, omit the optional `resolverPolicy` and inherit the server/company default. Do not add `human_only` merely because you are asking the user to decide: `resolve-from-comment` records your interpretation as the responding agent and cannot resolve a human-only card. Preserve explicit user requirements for human-only approval, company restrictions, and governed-action requirements; omission does not override them. Never lower a saved card's policy or recreate it to bypass a denial. Restricted cards must keep their required human response controls. + ```json { "idempotencyKey": "first-task-proposal-confirmation", diff --git a/server/src/services/issue-thread-interactions.ts b/server/src/services/issue-thread-interactions.ts index 9000768a668..d5b0421ae08 100644 --- a/server/src/services/issue-thread-interactions.ts +++ b/server/src/services/issue-thread-interactions.ts @@ -2712,8 +2712,18 @@ export function issueThreadInteractionService( lockedCurrent.payload, issueContext.id, )?.key === "plan"; + // A response turn can reject its predecessor's proposal before replacing + // it. Requeueing here would revoke that responding run's tool ownership. + // Both identities are read under the issue/interaction locks above. + const activeResponseIsRevisingOlderProposal = + args.actor.agentId != null && + args.actor.runId != null && + args.actor.runId === issueContext.executionRunId && + lockedCurrent.sourceRunId != null && + lockedCurrent.sourceRunId !== issueContext.executionRunId; const shouldResumeReviewedIssue = issueContext.status === "in_review" && + !activeResponseIsRevisingOlderProposal && (lockedCurrent.continuationPolicy === "wake_assignee" || rejectedPlanNeedsRevision); if ( diff --git a/server/src/services/native-runtime/confirmation-comment-api.integration.test.ts b/server/src/services/native-runtime/confirmation-comment-api.integration.test.ts index a85d6dd5277..db2b9ada367 100644 --- a/server/src/services/native-runtime/confirmation-comment-api.integration.test.ts +++ b/server/src/services/native-runtime/confirmation-comment-api.integration.test.ts @@ -1,42 +1,179 @@ -import { createIssueThreadInteractionSchema } from "@paperclipai/shared"; import { randomUUID } from "node:crypto"; -import { eq } from "drizzle-orm"; +import { and, eq } from "drizzle-orm"; import { afterAll, beforeAll, describe, expect, it } from "vitest"; -import { issueComments, issueThreadInteractions, issues } from "@paperclipai/db"; +import { heartbeatRuns, issueComments, issues, issueThreadInteractions, nativeRunFinalizations } from "@paperclipai/db"; import { createLocalAgentJwt } from "../../agent-auth-jwt.js"; import { startRunnerApiTestServer } from "../../__tests__/helpers/runner-api-server.js"; +import { initializeRunIdentity } from "../run-identity.js"; import { issueThreadInteractionService } from "../issue-thread-interactions.js"; +import { PaperclipRunnerToolAuthority } from "./paperclip-runner-tool-authority.js"; describe("confirmation reply through the native API tool", () => { let server: Awaited>; const oldSecret = process.env.PAPERCLIP_AGENT_JWT_SECRET; beforeAll(async () => { process.env.PAPERCLIP_AGENT_JWT_SECRET = randomUUID(); server = await startRunnerApiTestServer(); }, 60_000); afterAll(async () => { await server?.close(); if (oldSecret === undefined) delete process.env.PAPERCLIP_AGENT_JWT_SECRET; else process.env.PAPERCLIP_AGENT_JWT_SECRET = oldSecret; }); - async function seed(mode: "standard" | "planning" | "ask" = "standard") { + async function proposal( + f: Awaited>, + idempotencyKey: string, + prompt: string, + resolverPolicy?: "human_only", + ) { + const result = await f.authority.execute({ tool: "request_human_input", callId: randomUUID(), arguments: { + idempotencyKey, interactionKind: "confirmation", title: "Approve child task", prompt, + continuationPolicy: "wake_assignee", ...(resolverPolicy === undefined ? {} : { resolverPolicy }), + payload: { version: 1, supersedeOnUserComment: false, acceptLabel: "Approve", rejectLabel: "Revise", rejectRequiresReason: true }, + } }); + const [card] = await server.db.select().from(issueThreadInteractions).where(and( + eq(issueThreadInteractions.companyId, f.companyId), eq(issueThreadInteractions.idempotencyKey, idempotencyKey), + )); + if (!card) throw new Error("Native proposal did not persist its confirmation"); + expect(result).toMatchObject({ disposition: "applied", interaction: { id: card.id, status: "pending" } }); + return card; + } + async function seed(mode: "standard" | "planning" | "ask" = "standard", resolverPolicy?: "human_only") { const f = await server.fixture({ conversation: true, disableWakeOnDemand: true, mode }); - const [issue] = await server.db.select().from(issues).where(eq(issues.id, f.issueId)); - const card = await issueThreadInteractionService(server.db).create(issue!, createIssueThreadInteractionSchema.parse({ kind: "request_confirmation", payload: { version: 1, prompt: "Write the welcome note?" } }), { agentId: f.agentId }); + const card = await proposal(f, "first-task-proposal-confirmation", "Approve creating one child task to write the welcome note?", resolverPolicy); const [comment] = await server.db.insert(issueComments).values({ companyId: f.companyId, issueId: f.issueId, authorType: "user", authorUserId: f.userId, body: "Yes, write it." }).returning(); const call = { tool: "call_api", callId: randomUUID(), arguments: { operationId: "POST /api/issues/{id}/interactions/{interactionId}/resolve-from-comment", pathParams: { id: f.issueId, interactionId: card.id }, body: { commentId: comment!.id, decision: "accept" } } }; return { f, card, comment: comment!, call }; } it("advertises and executes the real authenticated route, including retry", async () => { const { f, card, comment, call } = await seed(); + expect(card).toMatchObject({ requestedResolverPolicy: "anyone", effectiveResolverPolicy: "anyone", resolverPolicyProvenance: "inherited" }); const first = await f.authority.execute(call); - expect(first).toMatchObject({ status: 200, data: { deduplicated: false, interaction: { id: card.id, status: "accepted", result: { commentId: comment.id } } } }); + expect(first).toMatchObject({ status: 200, data: { deduplicated: false, interaction: { + id: card.id, status: "accepted", result: { outcome: "accepted", commentId: comment.id }, + resolvedByAgentId: f.agentId, resolvedByRunId: f.runId, resolvedByUserId: null, + } } }); const retry = await f.authority.execute({ ...call, callId: randomUUID() }); expect(retry).toMatchObject({ status: 200, data: { deduplicated: true } }); const snapshot = await f.snapshot(); expect(snapshot.activity.filter(row => row.action === "issue.thread_interaction_accepted")).toHaveLength(1); }); it("denies human-only resolution and foreign-message provenance through the same tool", async () => { - const { f, card, call, comment } = await seed(); - await server.db.update(issueThreadInteractions).set({ effectiveResolverPolicy: "human_only" }).where(eq(issueThreadInteractions.id, card.id)); + const { f, card, call, comment } = await seed("standard", "human_only"); + expect(card).toMatchObject({ requestedResolverPolicy: "human_only", effectiveResolverPolicy: "human_only", resolverPolicyProvenance: "explicit" }); expect(await f.authority.execute(call)).toMatchObject({ status: 403 }); + expect((await server.db.select().from(issueThreadInteractions).where(eq(issueThreadInteractions.id, card.id)))[0]).toMatchObject({ status: "pending", result: null }); const other = await seed(); const foreignAnswer = { ...other.call, arguments: { ...other.call.arguments, body: { ...other.call.arguments.body, commentId: comment.id } } }; expect(await other.f.authority.execute(foreignAnswer)).toMatchObject({ status: 422 }); }); + it("records acceptance on the revised native proposal without accepting its superseded predecessor", async () => { + const { f, card: original } = await seed(); + const revised = await proposal(f, "first-task-proposal-confirmation-revised", "Approve creating one child task to write the revised Sunday welcome note?"); + const [comment] = await server.db.insert(issueComments).values({ companyId: f.companyId, issueId: f.issueId, + authorType: "user", authorUserId: f.userId, body: "Yes, I accept the revised Sunday proposal. Please do it." }).returning(); + const call = { tool: "call_api", callId: randomUUID(), arguments: { + operationId: "POST /api/issues/{id}/interactions/{interactionId}/resolve-from-comment", + pathParams: { id: f.issueId, interactionId: original.id }, body: { commentId: comment!.id, decision: "accept" }, + } }; + expect(await f.authority.execute(call)).toMatchObject({ status: 409 }); + expect(await f.authority.execute({ ...call, callId: randomUUID(), arguments: { + ...call.arguments, pathParams: { ...call.arguments.pathParams, interactionId: revised.id }, + } })).toMatchObject({ status: 200, data: { interaction: { id: revised.id, status: "accepted", result: { commentId: comment!.id }, + resolvedByAgentId: f.agentId, resolvedByRunId: f.runId, resolvedByUserId: null } } }); + expect((await server.db.select().from(issueThreadInteractions).where(eq(issueThreadInteractions.id, original.id)))[0]).toMatchObject({ + status: "expired", result: { outcome: "superseded_by_newer_request", supersededByInteractionId: revised.id }, + }); + }); + it("retains a newer native response turn when it rejects an older proposal to revise it", async () => { + const f = await server.fixture({ conversation: true, disableWakeOnDemand: true }); + await server.db.update(issues).set({ conversationAgentId: null, conversationUserId: null, + conversationState: null }).where(eq(issues.id, f.issueId)); + const original = await proposal(f, "first-task-proposal-confirmation", "Approve the Saturday welcome note?"); + const nativeSessionId = randomUUID(); + await server.db.update(heartbeatRuns).set({ status: "succeeded", finishedAt: new Date(), nativeSessionId }) + .where(eq(heartbeatRuns.id, f.runId)); + const [comment] = await server.db.insert(issueComments).values({ companyId: f.companyId, issueId: f.issueId, + authorType: "user", authorUserId: f.userId, + body: "Change the note to our Sunday meetup. Please revise your proposal; I have not accepted it yet." }).returning(); + const runId = randomUUID(); + await server.db.insert(heartbeatRuns).values({ id: runId, companyId: f.companyId, agentId: f.agentId, + status: "running", responsibleUserId: f.userId, runtimeMode: "native", nativeIssueId: f.issueId, nativeSessionId, + invocationSource: "assignment", triggerDetail: "system", contextSnapshot: { issueId: f.issueId, commentId: comment!.id } }); + await initializeRunIdentity(server.db, { companyId: f.companyId, runId, issueId: f.issueId, + responsibleUserId: f.userId!, cause: "instruction" }); + const lockedAt = new Date(); + const lease = { runId, companyId: f.companyId, issueId: f.issueId, phase: "observed", attempt: 1, + leaseOwner: `response-${randomUUID()}`, leaseExpiresAt: new Date(lockedAt.getTime() + 20 * 60_000), + controllerBootId: randomUUID(), controllerPid: process.pid, controllerProcessStartedAt: lockedAt, + controllerGeneration: 1 }; + await server.db.insert(nativeRunFinalizations).values(lease); + await server.db.update(issues).set({ status: "in_review", executionRunId: runId, checkoutRunId: runId, + executionLockedAt: lockedAt, executionAgentNameKey: "api-eval-agent" }).where(eq(issues.id, f.issueId)); + const current = new PaperclipRunnerToolAuthority(server.db, { ...f.authority.binding, runId }); + await expect(current.execute({ tool: "get_task_context", callId: randomUUID(), arguments: {} })).resolves.toBeDefined(); + const rejected = await current.execute({ tool: "call_api", callId: randomUUID(), arguments: { + operationId: "POST /api/issues/{id}/interactions/{interactionId}/resolve-from-comment", + pathParams: { id: f.issueId, interactionId: original.id }, body: { commentId: comment!.id, decision: "reject", + reason: "User requested a revised proposal for the Sunday meetup." }, + } }); + expect((await server.db.select().from(issueThreadInteractions).where(eq(issueThreadInteractions.id, original.id)))[0]) + .toMatchObject({ status: "rejected", sourceRunId: f.runId, resolvedByRunId: runId, result: { commentId: comment!.id } }); + expect((await server.db.select().from(issues).where(eq(issues.id, f.issueId)))[0]) + .toMatchObject({ status: "in_review", executionRunId: runId, checkoutRunId: runId, + executionLockedAt: lockedAt, executionAgentNameKey: "api-eval-agent" }); + expect(rejected).toMatchObject({ status: 200 }); + await expect(current.execute({ tool: "get_task_context", callId: randomUUID(), arguments: {} })).resolves.toBeDefined(); + // The next attempted tool in the retained failure used this confirmation shape. + const next = await current.execute({ tool: "request_human_input", callId: randomUUID(), arguments: { + title: "Approve revised welcome-note task", + prompt: "Approve creating exactly one child task to write a two-sentence garden club welcome note for our free Sunday meetup and save it as a task document?", + payload: { version: 1, acceptLabel: "Approve", rejectLabel: "Revise", rejectRequiresReason: true, supersedeOnUserComment: false }, + idempotencyKey: "first-task-proposal-confirmation-garden-welcome-note-sunday-v2", + interactionKind: "confirmation", continuationPolicy: "wake_assignee", + } }); + expect(next).toMatchObject({ disposition: "applied", interaction: { status: "pending", sourceRunId: runId } }); + expect((await server.db.select().from(nativeRunFinalizations).where(eq(nativeRunFinalizations.runId, runId)))[0]) + .toMatchObject(lease); + expect((await f.snapshot()).activity.filter(row => row.action === "issue.thread_interaction_rejected")).toHaveLength(1); + }); + it("still requeues rejection of a proposal owned by the current response run", async () => { + const { f, card, call } = await seed(); + await server.db.update(issues).set({ status: "in_review", checkoutRunId: f.runId, + executionLockedAt: new Date() }).where(eq(issues.id, f.issueId)); + const token = createLocalAgentJwt(f.agentId, f.companyId, "paperclip_runner", f.runId, f.userId); + const response = await fetch(`${server.apiUrl}/api/issues/${f.issueId}/interactions/${card.id}/resolve-from-comment`, { + method: "POST", headers: { "Content-Type": "application/json", Authorization: `Bearer ${token}` }, + body: JSON.stringify({ ...call.arguments.body, decision: "reject", reason: "Revise this proposal." }), + }); + expect(response.status).toBe(200); + expect(await response.json()).toMatchObject({ interaction: { id: card.id, status: "rejected" } }); + expect((await server.db.select().from(issues).where(eq(issues.id, f.issueId)))[0]) + .toMatchObject({ status: "todo", executionRunId: null, checkoutRunId: null, executionLockedAt: null }); + }); + it("preserves human-only native completion-review rejection without ordinary continuation", async () => { + const f = await server.fixture({ conversation: true, disableWakeOnDemand: true }); + const service = issueThreadInteractionService(server.db); + const card = await service.create({ id: f.issueId, companyId: f.companyId }, { + kind: "request_confirmation", sourceRunId: f.runId, resolverPolicy: "human_only", continuationPolicy: "none", + payload: { version: 1, prompt: "Review the completed work.", + target: { type: "custom", key: "native_completion_review", revisionId: randomUUID() } }, + }, { agentId: f.agentId, runId: f.runId }); + await server.db.update(issues).set({ status: "in_review", checkoutRunId: f.runId, + executionLockedAt: new Date() }).where(eq(issues.id, f.issueId)); + expect(await service.rejectInteraction({ id: f.issueId, companyId: f.companyId }, card.id, + { reason: "The work needs revision." }, { userId: f.userId })) + .toMatchObject({ status: "rejected", effectiveResolverPolicy: "human_only", resolvedByUserId: f.userId }); + expect((await server.db.select().from(issues).where(eq(issues.id, f.issueId)))[0]) + .toMatchObject({ status: "todo", executionRunId: null, checkoutRunId: null, executionLockedAt: null }); + }); + it.each([false, true])("preserves human rejection requeueing with a newer response owner=%s", async (hasNewOwner) => { + const { f, card } = await seed(); + const runId = hasNewOwner ? randomUUID() : null; + if (runId) await server.db.insert(heartbeatRuns).values({ id: runId, companyId: f.companyId, + agentId: f.agentId, status: "running", runtimeMode: "native", nativeIssueId: f.issueId, + invocationSource: "assignment", triggerDetail: "system" }); + await server.db.update(issues).set({ status: "in_review", executionRunId: runId, + checkoutRunId: runId, executionLockedAt: runId ? new Date() : null }).where(eq(issues.id, f.issueId)); + const rejected = await issueThreadInteractionService(server.db).rejectInteraction( + { id: f.issueId, companyId: f.companyId }, card.id, { reason: "Please revise the proposal." }, { userId: f.userId }); + expect(rejected).toMatchObject({ status: "rejected", resolvedByUserId: f.userId }); + expect((await server.db.select().from(issues).where(eq(issues.id, f.issueId)))[0]) + .toMatchObject({ status: "todo", executionRunId: null, checkoutRunId: null, executionLockedAt: null }); + }); it("records a conversational decision in Plan mode without opening writes in Ask mode", async () => { const planning = await seed("planning"); expect(await planning.f.authority.execute(planning.call)).toMatchObject({ status: 200, data: { interaction: { status: "accepted" } } }); diff --git a/tests/runner-e2e/chat-completion-proof.ts b/tests/runner-e2e/chat-completion-proof.ts new file mode 100644 index 00000000000..e80d601d2a8 --- /dev/null +++ b/tests/runner-e2e/chat-completion-proof.ts @@ -0,0 +1,152 @@ +type Row = Record; + +export interface ChatCompletionEvidence { + companyId: string; + agentId: string; + companyPrefix: string; + baseURL: string; + issue: Row; + child: Row; + tasks: Row[]; + comments: Row[]; + runEvidence: Array<{ runId: string; events: Row[]; workspaceOperations?: Row[] }>; + renderedLinks: Array<{ commentId: string; href: string }>; +} + +const completionCases = new Set(["multi-repository", "clarify-reuse", "plan-handoff"]); +export function needsChatCompletionProof(suite: string, task: string | undefined, profile?: string, environment?: string) { + return suite === "agent-chat" && completionCases.has(task ?? "") + && profile === "runner-codex" && environment === "local"; +} + +const requireProof = (condition: unknown, code: string) => { + if (!condition) throw new Error(`Chat completion proof: ${code}`); +}; +const time = (value: unknown) => typeof value === "string" ? Date.parse(value) : NaN; +const scopeIssue = (run: Row) => run.nativeIssueId ?? run.contextSnapshot?.issueId; +const completionEventTypes = new Set([ + "legacy.case_handoff_cleanup_intent", "lifecycle", "run.startup.reuse", "run.performance.span", + "native.process_start_requested", "native.process_identity_recorded", "session.resumed", + "session.capabilities.updated", "session.goal.snapshot", "session.goal.cleared", "harness.diagnostic", + "provider.notice.recorded", "turn.submitted", "turn.accepted", "turn.started", "turn.completed", + "item.started", "item.delta", "item.completed", "tool.execution.started", "tool.execution.completed", + "run.result.proposed", "run.result.accepted", "run.terminal", "instruction_save", "run.presentation.resolved", +]); +const completionItemKinds = new Set([ + "thread_lineage", "model", "goal_cleared", "userMessage", "reasoning", "agentMessage", + "dynamicToolCall", "verification", "usage", +]); + +/** One required delivery is real paid work, not a row to hide from accounting. + * Only these three isolated native journeys use this independent proof. + * The catalog total includes this one turn; the caller keeps that bound exact. + */ +export function verifyChatCompletion(runs: readonly Row[], evidence: ChatCompletionEvidence | undefined) { + requireProof(evidence, "evidence_missing"); + const { companyId, agentId, issue, child, tasks, comments } = evidence!; + const generation = issue.conversationSessionGeneration; + requireProof(issue.companyId === companyId && issue.conversationAgentId === agentId + && Number.isSafeInteger(generation) && generation >= 0, "source_scope"); + requireProof(child.id !== issue.id && child.companyId === companyId && child.status === "done" + && !child.conversationAgentId && typeof child.assigneeAgentId === "string" + && Number.isFinite(time(child.completedAt)), "child_done"); + requireProof(tasks.length === 1 && tasks[0]?.id === child.id && tasks[0]?.companyId === companyId, "extra_work"); + requireProof(new Set(runs.map(r => r.id)).size === runs.length && runs.every(r => typeof r.id === "string" && r.id.length > 0 && r.companyId === companyId + && [issue.id, child.id].includes(scopeIssue(r)) && r.contextSnapshot?.issueId === scopeIssue(r) + && r.agentId === (scopeIssue(r) === issue.id ? agentId : child.assigneeAgentId) + && r.runtimeMode === "native" && r.status === "succeeded" && typeof r.nativeSessionId === "string" && r.nativeSessionId.length > 0 + && Number.isFinite(time(r.startedAt)) && Number.isFinite(time(r.processStartedAt)) + && Number.isFinite(time(r.finishedAt)) && time(r.finishedAt) >= time(r.startedAt) + && r.usageJson?.accountingReceiptReady === true + && !r.contextSnapshot?.conversationReset && !r.resultJson?.conversationReset), "actual_provider_runs"); + const notifications = runs.filter(r => r.contextSnapshot?.wakeReason === "chat_task_completed" + || r.contextSnapshot?.chatCompletionDeliveryIds?.length || r.contextSnapshot?.chatCompletionUpdates?.length); + requireProof(notifications.length === 1, "notification_count"); + const run = notifications[0]!, context = run.contextSnapshot; + requireProof(scopeIssue(run) === issue.id && run.agentId === agentId + && run.invocationSource === "automation" && run.triggerDetail === "system" + && context.wakeReason === "chat_task_completed" && context.wakeSource === "automation" + && context.conversationSessionGeneration === generation + && time(run.startedAt) >= time(child.completedAt), "notification_scope"); + requireProof(Array.isArray(context.chatCompletionDeliveryIds) && context.chatCompletionDeliveryIds.length === 1 + && typeof context.chatCompletionDeliveryIds[0] === "string" && context.chatCompletionDeliveryIds[0].length > 0 + && Array.isArray(context.chatCompletionUpdates) && context.chatCompletionUpdates.length === 1, "delivery_count"); + const update = context.chatCompletionUpdates[0]; + requireProof(update.id === child.id && update.status === "done" && update.identifier === child.identifier + && time(update.completedAt) === time(child.completedAt), "delivery_child"); + const childRuns = runs.filter(r => scopeIssue(r) === child.id); + requireProof(childRuns.length === 1 && time(childRuns[0]!.startedAt) <= time(child.completedAt), "child_execution"); + + const replies = comments.filter(c => c.createdByRunId === run.id); + requireProof(replies.length === 1, "reply_count"); + const reply = replies[0]!; + requireProof(reply.companyId === companyId && reply.issueId === issue.id && reply.authorAgentId === agentId + && reply.authorType === "agent" && !reply.deletedAt + && (reply.conversationSessionGeneration == null || reply.conversationSessionGeneration === generation) + && time(reply.createdAt) >= time(run.startedAt) && typeof reply.body === "string" && reply.body.trim(), "reply_scope"); + const link = evidence!.renderedLinks.find(candidate => { + if (candidate.commentId !== reply.id) return false; + try { + const url = new URL(candidate.href, evidence!.baseURL); + if (url.origin !== new URL(evidence!.baseURL).origin) return false; + const parts = url.pathname.split("/").filter(Boolean).map(decodeURIComponent); + const index = parts.indexOf("issues"); + return (index === 0 || index === 1 && parts[0] === evidence!.companyPrefix) + && parts.length === index + 2 && [child.id, child.identifier].filter(Boolean).includes(parts[index + 1]); + } catch { return false; } + }); + requireProof(link, "rendered_result_access"); + + const streams = evidence!.runEvidence.filter(e => e.runId === run.id); + requireProof(streams.length === 1 && streams[0]!.events.length > 0, "events_missing"); + const stream = streams[0]!; + requireProof(stream.events.every(e => typeof e.eventType === "string" && e.companyId === companyId && e.runId === run.id && e.agentId === agentId), "event_scope"); + // Native harness work need not use a Paperclip tool. Unknown event types and + // item kinds (commands, file changes, searches) cannot earn this allowance. + requireProof(stream.events.every(e => completionEventTypes.has(e.eventType)), "additional_native_work"); + const items = stream.events.filter(e => e.eventType.startsWith("item.")); + for (const event of items) { + const prp = event.payload?.prpEvent; + requireProof(prp?.schema === "paperclip.prp.event.v1" && prp.runId === run.id + && prp.eventType === event.eventType && completionItemKinds.has(prp.payload?.kind), "additional_native_work"); + if (prp.payload.kind !== "dynamicToolCall" && prp.payload.item != null) { + requireProof(prp.payload.item.type === prp.payload.kind, "additional_native_work"); + } + } + requireProof(stream.events.some(e => e.eventType === "run.result.accepted" + && e.payload?.prpEvent?.schema === "paperclip.prp.event.v1" && e.payload.prpEvent.runId === run.id + && e.payload.prpEvent.eventType === e.eventType), "accepted_result_missing"); + requireProof(Array.isArray(stream.workspaceOperations) && stream.workspaceOperations.every(o => o.phase === "workspace_finalize" + && o.heartbeatRunId === run.id && o.companyId === companyId && o.status === "succeeded" && !o.command), "additional_workspace_work"); + const allowedTools = new Set(["get_task", "list_task_documents", "get_task_document", "paperclip_finish"]); + const tools = stream.events.filter(e => e.eventType.startsWith("tool.execution.")); + const starts = new Map(), finishes = new Map(); + for (const event of tools) { + const prp = event.payload?.prpEvent, tool = prp?.payload; + requireProof(prp?.schema === "paperclip.prp.event.v1" && prp.runId === run.id && prp.eventType === event.eventType + && tool?.schema === "paperclip.tool.execution.v1" && allowedTools.has(tool.name) + && typeof tool.executionId === "string" && tool.executionId.length > 0, "additional_tool_work"); + const started = event.eventType === "tool.execution.started"; + requireProof(started || event.eventType === "tool.execution.completed", "tool_outcome"); + requireProof(tool.status === (started ? "running" : "completed"), "tool_outcome"); + const target = started ? starts : finishes; + requireProof(started || starts.get(tool.executionId) === tool.name, "tool_completion_order"); + requireProof(!target.has(tool.executionId), "duplicate_tool_execution"); + target.set(tool.executionId, tool.name); + } + requireProof(starts.size === finishes.size && [...starts].every(([id, name]) => finishes.get(id) === name) + && [...finishes.values()].filter(name => name === "paperclip_finish").length === 1, "tool_completion"); + for (const event of items) { + const payload = event.payload.prpEvent.payload; + if (payload.kind !== "dynamicToolCall") continue; + const item = payload.item; + requireProof(item && (event.eventType === "item.started" && item.type === "tool_use" + && allowedTools.has(item.name) && starts.get(item.id) === item.name + || event.eventType === "item.completed" && item.type === "tool_result" + && item.tool_use_id === item.id && finishes.has(item.id)), "unbound_native_tool"); + } + return { ordinaryProviderRuns: runs.length - 1, totalProviderRuns: runs.length, + allRunIds: runs.map(r => r.id), notificationRunId: run.id, + deliveryId: context.chatCompletionDeliveryIds[0] as string, childId: child.id as string, + replyId: reply.id as string, renderedResultHref: link!.href }; +} diff --git a/tests/runner-e2e/chat-flow.ts b/tests/runner-e2e/chat-flow.ts index 0a8808fb826..fb521b002e7 100644 --- a/tests/runner-e2e/chat-flow.ts +++ b/tests/runner-e2e/chat-flow.ts @@ -20,13 +20,22 @@ import { matchesRunCount, minimumRunCount } from "./run-count.js"; import { runChatCompletionUpdate } from "./completion-update-flow.js"; import { CHAT_FEATURE_SUITE } from "./chat-feature-cases.js"; import { runChatFeatureFlow } from "./chat-feature-flow.js"; +import { needsChatCompletionProof, verifyChatCompletion, type ChatCompletionEvidence } from "./chat-completion-proof.js"; -/** Hiring alone admits bounded, verified lifecycle notifications. Other suites keep their count contract. */ +export function retryChatCompletionObservation(error: unknown) { + return !(error instanceof Error && error.message.startsWith("Chat completion proof:")); +} + +/** Keep the ordinary-turn bound. Required completion delivery is separately proved real work. */ export function assertChatFlowRunCount(input: { - suiteId: string; task: Parameters[0]; runs: ChatRun[]; + suiteId: string; task: Parameters[0] & { id?: string }; runs: ChatRun[]; + profileId?: string; environmentId?: string; completionEvidence?: ChatCompletionEvidence; hiringEvidence?: HiringTemplateEvidence; hiringApiState?: unknown; }) { - expect(matchesRunCount(input.task, input.runs.length), "Declared total provider-run bounds").toBe(true); + const completion = needsChatCompletionProof(input.suiteId, input.task.id, input.profileId, input.environmentId) + ? verifyChatCompletion(input.runs, input.completionEvidence) : undefined; + expect(matchesRunCount(input.task, completion?.totalProviderRuns ?? input.runs.length), + completion ? "Declared total provider-run bounds with one proved completion turn" : "Declared total provider-run bounds").toBe(true); if (input.suiteId !== "hiring-templates") return; const accounting = gradeHiringTemplateTurns({ evidence: input.hiringEvidence ? { ...input.hiringEvidence, runs: input.runs } : undefined, @@ -299,6 +308,7 @@ export async function collectChatRunEvidence( api: Pick, run: ChatRun, allowAdmissionWait = false, + includeWorkspaceOperations = false, ) { const admissionEvents = allowAdmissionWait ? await collectRunEvents((afterSeq, limit) => api.get(`/api/heartbeat-runs/${run.id}/events?afterSeq=${afterSeq}&limit=${limit}`), @@ -320,6 +330,9 @@ export async function collectChatRunEvidence( events: admissionEvents ?? await collectRunEvents((afterSeq, limit) => api.get(`/api/heartbeat-runs/${run.id}/events?afterSeq=${afterSeq}&limit=${limit}`), ), + ...(includeWorkspaceOperations ? { + workspaceOperations: await api.get>>(`/api/heartbeat-runs/${run.id}/workspace-operations`), + } : {}), }; } @@ -390,6 +403,9 @@ export async function runChatFlow(input: ChatFlowInput) { const marker = execution.task.buildVisibleMarker(nonce); const draftMarker = chatMarker("DRAFT", nonce); const caseId = execution.task.id; + const completionAccounting = needsChatCompletionProof(execution.suite.id, caseId, execution.profile.id, execution.environment.id); + let completionChildId: string | undefined; + let completionEvidence: ChatCompletionEvidence | undefined; const stopCase = ["stop-new-resume", "stop-startup-new-resume"].includes(caseId); const interruptionCase = ["followup-while-running", "revise-while-running"].includes(caseId); const expectedStops = new Map(); @@ -429,12 +445,13 @@ export async function runChatFlow(input: ChatFlowInput) { response: await api.get(`/api/issues/${issue!.id}/recovery-actions?inspectSourceRunId=${stoppedForSessionReset}&inspectSourceActionId=${wait.recoveryActionId}`), }))), }); - const idle = async (minimumProviderRuns: number) => { + const idle = async (minimumProviderRuns: number, requireCompletion = false) => { const inconsistentIdle = createChatIdleFailureDetector(minimumProviderRuns); await pollUntil({ label: "chat turn settles to waiting", deadlineAt: Date.now() + 240_000, intervalMs: 1000, + retryOnError: retryChatCompletionObservation, load: async () => { const resolved = await api.get(chatPath); if (resolved) issue = resolved; @@ -462,7 +479,38 @@ export async function runChatFlow(input: ChatFlowInput) { if (admissionReady) verifyChatAdmissionWaitCoverage({ runs, runEvidence: proof.runEvidence, comments: proof.comments, issue: issue!, companyId: f.company.id, agentId: f.agent.id, stoppedRunId: stoppedForSessionReset, recoverySources: proof.sources }); } + let completionReady = !requireCompletion; + if (requireCompletion && providerRuns().length >= minimumProviderRuns + && runs.every(run => run.status === "succeeded")) { + const notifications = runs.filter(run => run.contextSnapshot?.wakeReason === "chat_task_completed"); + expect(notifications, "one required completion delivery").toHaveLength(1); + expect(completionChildId, "the existing delegated task must be retained").toBeTruthy(); + const [source, child, finalTasks, sourceComments] = await Promise.all([ + api.get(chatPath), + api.get(`/api/issues/${completionChildId}`), tasks(), + api.get(`/api/issues/${issue!.id}/comments?order=asc`), + ]); + const replies = sourceComments.filter(comment => comment.createdByRunId === notifications[0]!.id); + const renderedLinks = (await Promise.all(replies.map(async reply => { + const hrefs = await page.locator(`[id=${JSON.stringify(`comment-${reply.id}`)}] a[href]`) + .evaluateAll(elements => elements.map(element => element.getAttribute("href")!)); + return hrefs.map(href => ({ commentId: reply.id, href })); + }))).flat(); + // Wait for the real reply to render inside this existing idle deadline. + // Never extend the scenario or retry a provider to obtain the proof. + if (replies.length && renderedLinks.some(link => link.href.includes("/issues/"))) { + const companyPrefix = f.company.issuePrefix; + if (!companyPrefix) throw new Error("Chat completion proof: company_prefix_missing"); + completionEvidence = { companyId: f.company.id, agentId: f.agent.id, + companyPrefix, baseURL: api.baseURL, issue: source, child, + tasks: finalTasks, comments: sourceComments, renderedLinks, + runEvidence: [await collectChatRunEvidence(api, notifications[0]!, false, true)] }; + verifyChatCompletion(runs, completionEvidence); + completionReady = true; + } + } return { + completionReady, admissionReady, resolved: Boolean(resolved), status: resolved?.status, @@ -477,6 +525,7 @@ export async function runChatFlow(input: ChatFlowInput) { reject: (state) => state.failure ?? inconsistentIdle(state), accept: (state) => !state.failure && + state.completionReady && state.admissionReady && state.resolved && state.providerRunCount >= minimumProviderRuns && @@ -939,6 +988,7 @@ export async function runChatFlow(input: ChatFlowInput) { const children = await tasks(); expect(children).toHaveLength(1); const child = children[0]!; + if (completionAccounting) completionChildId = child.id; await pollUntil({ label: `execution task ${child.id} completes`, deadlineAt: Date.now() + 240_000, @@ -1083,7 +1133,7 @@ export async function runChatFlow(input: ChatFlowInput) { projects, }); } - await idle(minimumRunCount(execution.task)); + await idle(minimumRunCount(execution.task), completionAccounting); if (execution.suite.id === "hiring-templates") { // Refresh the whole consistent evidence generation for the final guard. hiringEvidence = await refreshHiringEvidence!(); @@ -1096,11 +1146,13 @@ export async function runChatFlow(input: ChatFlowInput) { if (admissionAccounting) admissionWaits = verifyChatAdmissionWaitCoverage({ runs, runEvidence: admissionRunEvidence!, comments: finalComments!, issue: issue!, companyId: f.company.id, agentId: f.agent.id, stoppedRunId: stoppedForSessionReset, recoverySources: admissionWaitSources }); assertChatFlowRunCount({ suiteId: execution.suite.id, task: execution.task, + profileId: execution.profile.id, environmentId: execution.environment.id, completionEvidence, // Hiring must account for every actual company run, including unexpected resets. - runs: execution.suite.id === "hiring-templates" ? runs : providerRuns(), + runs: execution.suite.id === "hiring-templates" || completionAccounting ? runs : providerRuns(), hiringEvidence, hiringApiState: execution.suite.id === "hiring-templates" ? hiringEvidence!.turnApiState : undefined, }); + if (completionAccounting) await input.evidence("chat-completion-proof.json", verifyChatCompletion(runs, completionEvidence)); for (const run of providerRuns()) { expect(run.runtimeMode).toBe(execution.profile.expectedRuntimeMode); expect(run.status).toBe(