diff --git a/tests/runner-e2e/FIXTURES.md b/tests/runner-e2e/FIXTURES.md index d1571426ec0..990ec383670 100644 --- a/tests/runner-e2e/FIXTURES.md +++ b/tests/runner-e2e/FIXTURES.md @@ -100,7 +100,7 @@ canonical Plan revision, capture its pending UI, approve in the browser, and prove exactly two successful runs. `warm_three_turn` provides exactly two browser follow-up messages, preserves one project/execution-workspace scope, verifies host file contents after every turn, and finishes within three -ten-minute turn deadlines. Native warm turns 1 and 2 create a real confirmation card with `request_human_input` before reporting `needs_review`; the pending card supplies the required review gate. A disposition string alone does not create that gate. Turn 3 reports Done without another card. +ten-minute turn deadlines. Native turns 1 and 2 include an actionable human review in the completion report's `attentionRequests`. Paperclip creates the review gate from that report. An explicit question-tool wait yields the turn and suppresses its final prose, so it is not interchangeable with this completion-review fixture. Turn 3 reports Done without another review. Every selected case runs in its own isolated Paperclip process, and independent cases may run concurrently. Follow-up turns inside one case retain their shared diff --git a/tests/runner-e2e/catalog.test.ts b/tests/runner-e2e/catalog.test.ts index c4eb357793f..c6772a0680f 100644 --- a/tests/runner-e2e/catalog.test.ts +++ b/tests/runner-e2e/catalog.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest"; -import { createIssueThreadInteractionSchema } from "../../packages/shared/src/validators/issue.js"; +import { normalizePrpResultSignals } from "../../packages/paperclip-runner/src/protocol/result-normalization.js"; import { connectionReviewSuite, runnerEnvironments, @@ -26,27 +26,16 @@ import { } from "./selectors.js"; describe("runner E2E catalog", () => { - it("gives native warm turns a real review card before reporting needs_review", () => { + it("supplies an actionable human review in native warm completion examples", () => { const prompts = [daytonaWarmContinuityTask.buildPrompt("nonce"), ...daytonaWarmContinuityTask.buildFollowupMessages!("nonce")]; for (const [index, prompt] of prompts.entries()) { - const match = prompt.match(/call request_human_input with (\{.*\})\. Reuse/); - if (index === 2) { - expect(match).toBeNull(); - continue; - } - expect(match, `turn ${index + 1} must create its review interaction`).not.toBeNull(); - const request = JSON.parse(match![1]); - const interaction = createIssueThreadInteractionSchema.parse({ - kind: "request_confirmation", idempotencyKey: request.idempotencyKey, - title: request.title, summary: request.prompt, continuationPolicy: request.continuationPolicy, - payload: { ...request.payload, prompt: request.prompt }, - }); - expect(interaction).toMatchObject({ - kind: "request_confirmation", continuationPolicy: "wake_assignee", - payload: { target: { type: "custom", revisionId: `nonce-T${index + 1}` }, rejectRequiresReason: true }, - }); - expect(request.interactionKind).toBe("confirmation"); - expect(prompt.indexOf("call request_human_input with")).toBeLessThan(prompt.indexOf("then call paperclip_finish")); + const match = prompt.match(/attentionRequests:(\[.*?\]),evidence:/); + expect(match).not.toBeNull(); + const signals = normalizePrpResultSignals({ attentionRequests: JSON.parse(match![1]) }); + expect(signals.ignoredAttentionRequests).toEqual([]); + expect(signals.actionableAttentionRequests).toHaveLength(index === 2 ? 0 : 1); + if (index < 2) expect(signals.actionableAttentionRequests[0]).toMatchObject({kind:"review", ownerClass:"human"}); + expect(prompt).not.toContain("call request_human_input"); } }); diff --git a/tests/runner-e2e/catalog.ts b/tests/runner-e2e/catalog.ts index bbceb51d7a9..0686733664b 100644 --- a/tests/runner-e2e/catalog.ts +++ b/tests/runner-e2e/catalog.ts @@ -811,22 +811,10 @@ function warmTurnInstructions(turn: 1 | 2 | 3, nonce: string) { ); const marker = warmTurnMarker(turn, nonce); const finalTurn = turn === 3; - const nativeReviewRequest = { - interactionKind: "confirmation", - idempotencyKey: `daytona-warm-review-T${turn}-${nonce}`, - title: `Warm continuity turn ${turn}`, - prompt: `Is this warm continuity task ready to complete after turn ${turn}?`, - continuationPolicy: "wake_assignee", - payload: { - version: 1, - acceptLabel: "Approve completion", - rejectLabel: "Continue work", - rejectRequiresReason: true, - allowDeclineReason: true, - supersedeOnUserComment: false, - target: { type: "custom", key: `daytona_warm_turn_${turn}`, revisionId: `${nonce}-T${turn}`, label: `Warm continuity turn ${turn}` }, - }, - }; + const reviewRequests = finalTurn ? [] : [{ + kind: "review", ownerClass: "human", + summary: `Review the verified turn ${turn} workspace file and decide whether this task is ready to complete or needs another revision.`, + }]; const legacyCompletion = finalTurn ? `In a legacy runner, make exactly one public-API completion write after verification: PATCH /api/issues/$PAPERCLIP_TASK_ID with {"status":"done","comment":"${marker}"}. Include Authorization and X-Paperclip-Run-Id. Do not POST a separate comment.` : `In a legacy runner, after verification POST exactly one request_confirmation to /api/issues/$PAPERCLIP_TASK_ID/interactions with {"kind":"request_confirmation","idempotencyKey":"daytona-warm-review-T${turn}-${nonce}","resolverPolicy":"human_only","title":"Warm continuity turn ${turn}","summary":"Review completed warm continuity turn ${turn}.","continuationPolicy":"wake_assignee","payload":{"version":1,"prompt":"Is this warm continuity task ready to complete after turn ${turn}?","acceptLabel":"Approve completion","rejectLabel":"Continue work","rejectRequiresReason":true,"allowDeclineReason":true,"supersedeOnUserComment":false,"target":{"type":"custom","key":"daytona_warm_turn_${turn}","revisionId":"${nonce}-T${turn}","label":"Warm continuity turn ${turn}"}}}. Capture the returned interaction id. Then make exactly one issue PATCH with {"status":"in_review","comment":"${marker}","reviewInteractionId":""}. Include Authorization and X-Paperclip-Run-Id on both writes. If the issue PATCH fails, retry only that PATCH and never create another interaction. Do not POST a separate comment. After both writes succeed, end the response and heartbeat immediately; do not wait or poll because the reviewer action will start the next turn.`; @@ -836,8 +824,7 @@ function warmTurnInstructions(turn: 1 | 2 | 3, nonce: string) { ? `Create ${file} with exactly this one line followed by a newline: ${lines[0]}` : `Before changing anything, read ${file} and verify its content is exactly ${lines.slice(0, -1).join("\\n")} followed by a newline. Then append exactly ${lines.at(-1)} followed by a newline.`, `After the write, verify ${file} contains exactly these lines, once each and in order: ${lines.join(" | ")}.`, - ...(!finalTurn ? [`In a native runner, after verification and before reporting needs_review, call request_human_input with ${JSON.stringify(nativeReviewRequest)}. Reuse its idempotencyKey on a retry; do not create another card. The pending card is the review gate; do not report done while it is waiting.`] : []), - `In a native runner, use the current turn completion revision and criterion IDs (CURRENT_TURN_REVISION below is a placeholder, not a literal), then call paperclip_finish exactly once with {reportedWorkDisposition:"${finalTurn ? "done" : "needs_review"}",summary:"${marker}",completionClaim:{contractRevision:CURRENT_TURN_REVISION,objectiveSatisfied:true,criteria:[{criterionId:"objective",status:"satisfied",evidenceRefs:[]}],remainingWork:[]},evidence:[],verification:[{commandOrCheck:"read ${file}",status:"passed"}]}. Wait for that tool call to succeed, then emit exactly ${marker} once as the complete user-facing final response.`, + `In a native runner, use the current turn completion revision and criterion IDs (CURRENT_TURN_REVISION below is a placeholder, not a literal), then call paperclip_finish exactly once with {reportedWorkDisposition:"${finalTurn ? "done" : "needs_review"}",summary:"${marker}",completionClaim:{contractRevision:CURRENT_TURN_REVISION,objectiveSatisfied:true,criteria:[{criterionId:"objective",status:"satisfied",evidenceRefs:[]}],remainingWork:[]},attentionRequests:${JSON.stringify(reviewRequests)},evidence:[],verification:[{commandOrCheck:"read ${file}",status:"passed"}]}. Wait for that tool call to succeed, then emit exactly ${marker} once as the complete user-facing final response.`, legacyCompletion, `In a legacy runner, the PATCH comment is the complete visible response. After its 2xx response, finish silently: do not print, echo, or emit ${marker} again as assistant text.`, `Do not include ${marker} in any other visible response or write. Do not recreate, truncate, reorder, or duplicate prior lines.`, diff --git a/tests/runner-e2e/matchers.ts b/tests/runner-e2e/matchers.ts index 650fd907f4d..90b0f0bdb6e 100644 --- a/tests/runner-e2e/matchers.ts +++ b/tests/runner-e2e/matchers.ts @@ -166,3 +166,20 @@ export async function evaluateMatchers( matchers.map((matcher) => evaluateMatcher(matcher, observation)), ); } + + +export function persistedFinalRunMessage( + comments: Array<{ id: string; createdByRunId?: string | null; body?: string | null }>, + run: { id: string; resultJson?: Record | null }, +): string { + const runComments = comments.filter(comment => comment.createdByRunId === run.id); + const decision = run.resultJson?.presentationDecision; + const selectedId = decision && typeof decision === "object" && !Array.isArray(decision) + ? (decision as Record).commentId : null; + // Read the persisted comment selected for the user, not a finish summary or + // attachment preparation message. Missing selected evidence must still fail. + if (typeof selectedId === "string") { + return runComments.find(comment => comment.id === selectedId)?.body ?? ""; + } + return runComments.map(comment => comment.body ?? "").join("\n"); +} diff --git a/tests/runner-e2e/runner.spec.ts b/tests/runner-e2e/runner.spec.ts index 1523fa36706..c93b6a8d758 100644 --- a/tests/runner-e2e/runner.spec.ts +++ b/tests/runner-e2e/runner.spec.ts @@ -15,7 +15,7 @@ import { classifyFailure } from "./failure-classifier.js"; import { runnerE2EServerControlPaths } from "./harness-env.js"; import { setupConnectionReview } from "./connection-reviews.js"; import { setupLiveFixtures, type LiveFixtureValues } from "./live-fixtures.js"; -import { evaluateMatcher, type MatcherResult } from "./matchers.js"; +import { evaluateMatcher, persistedFinalRunMessage, type MatcherResult } from "./matchers.js"; import { acceptedPlanSessionResetFailures, hasTerminalMalformedPlanConfirmation, @@ -1707,10 +1707,7 @@ for (const execution of executions) { const message = agentComments .map((comment) => comment.body ?? "") .join("\n"); - const finalRunMessage = agentComments - .filter((comment) => comment.createdByRunId === finalRun.id) - .map((comment) => comment.body ?? "") - .join("\n"); + const finalRunMessage = persistedFinalRunMessage(agentComments, finalRun); const pendingInteractions = terminal.interactions.filter( (interaction) => interaction.status === "pending", ); diff --git a/tests/runner-e2e/support.test.ts b/tests/runner-e2e/support.test.ts index 655a3c4195c..3be21db56e6 100644 --- a/tests/runner-e2e/support.test.ts +++ b/tests/runner-e2e/support.test.ts @@ -18,7 +18,7 @@ import { } from "./harness-env.js"; import { runnerExecutionById, runnerMatrix } from "./catalog.js"; import { assertEmbeddedDatabaseIsolation } from "./instance-isolation.js"; -import { evaluateMatchers } from "./matchers.js"; +import { evaluateMatchers, persistedFinalRunMessage } from "./matchers.js"; import { assertSecretFree, findSecretLeak, @@ -1310,3 +1310,24 @@ describe("runner E2E macOS shared-memory cleanup", () => { ]); }); }); + + +describe("persisted final response selection", () => { + const comments = [ + { id: "attachment-comment", body: "Prepared file for this response.", createdByRunId: "run-1" }, + { id: "reply", body: "FINAL", createdByRunId: "run-1" }, + { id: "other-run", body: "unrelated", createdByRunId: "run-2" }, + ]; + const run = { id: "run-1", resultJson: { presentationDecision: { commentId: "reply" } } }; + it("grades the real final comment independently from an attachment's preparation comment", () => { + expect(persistedFinalRunMessage(comments, run)).toBe("FINAL"); + }); + it("fails closed when the selected final comment is missing or belongs to another run", () => { + expect(persistedFinalRunMessage(comments.slice(0, 1), run)).toBe(""); + expect(persistedFinalRunMessage(comments, { ...run, resultJson: { presentationDecision: { commentId: "other-run" } } })).toBe(""); + }); + it("keeps legacy fallback and does not replace absent visible text with a summary", () => { + expect(persistedFinalRunMessage(comments, { id: "run-1" })).toBe("Prepared file for this response.\nFINAL"); + expect(persistedFinalRunMessage([], { id: "run-1", resultJson: { summary: "FINAL" } })).toBe(""); + }); +});