diff --git a/doc/evals.md b/doc/evals.md index 45a196b3461..b1e2ee306cf 100644 --- a/doc/evals.md +++ b/doc/evals.md @@ -224,3 +224,11 @@ without that record, the helper will refuse to overwrite an existing prefix. Verify the public page and its links after publication. This manual refresh does not add a scheduled workflow. Preserve the measurement date when choosing a newer rendering of the same campaign. + +Remaining native chat boundaries are in the explicit-only +`agent-chat-qualification` suite: active task reassignment, user Retry after +verified worker process loss, and multi-turn answers grounded in actual task +records. See the [workflow and qualification limits](../tests/runner-e2e/README.md#remaining-native-agent-chat-qualification). +The 26 native `first-task` cells exercise onboarding before native selection +becomes the UI default. Live results and semantic answer reviews must accompany +any qualification claim; catalog presence alone is not a pass. diff --git a/server/src/__tests__/agent-live-run-routes.test.ts b/server/src/__tests__/agent-live-run-routes.test.ts index f08952ad7da..28919226bde 100644 --- a/server/src/__tests__/agent-live-run-routes.test.ts +++ b/server/src/__tests__/agent-live-run-routes.test.ts @@ -1007,6 +1007,19 @@ describe("agent live run routes", () => { }); }); + it("refuses a quarantined native retry before queuing another failed attempt", async () => { + const fixture = createFailedChatRetryDb(false); + mockHeartbeatService.getRun.mockResolvedValue({ ...selectedRun, + runtimeMode: "native", errorCode: "native_session_cleanup_quarantined" }); + const res = await requestApp(await createApp(fixture.db, { + type: "board", userId: "operator", source: "session", companyIds: ["company-1"], + }), url => request(url).post(`/api/agents/${routeAgentId}/wakeup`).send(retryBody)); + expect(res.status).toBe(409); + expect(res.body).toMatchObject({ error: expect.stringContaining("cleanup and reconciliation") }); + expect(mockHeartbeatService.wakeup).not.toHaveBeenCalled(); + expect(mockChatRunRetries.prepareFailedChatRunRetry).not.toHaveBeenCalled(); + }); + it("retries a task for an operator without agent-creation permission", async () => { const fixture = createFailedChatRetryDb(false); mockHeartbeatService.getRun.mockResolvedValue({ ...selectedRun, contextSnapshot: { diff --git a/server/src/routes/agents.ts b/server/src/routes/agents.ts index af0e058631a..5b4888018eb 100644 --- a/server/src/routes/agents.ts +++ b/server/src/routes/agents.ts @@ -5778,6 +5778,11 @@ export function agentRoutes( if (!["failed", "timed_out"].includes(failedRun.status)) { throw conflict("Only a failed run can be retried."); } + if (failedRun.runtimeMode === "native" && failedRun.errorCode === "native_session_cleanup_quarantined") { + throw conflict("The stopped native session requires cleanup and reconciliation before a new attempt.", { + code: "native_session_cleanup_quarantined", + }); + } const failedContext = asRecord(failedRun.contextSnapshot) ?? {}; const issueId = typeof failedContext.issueId === "string" diff --git a/server/src/services/execution-projection.test.ts b/server/src/services/execution-projection.test.ts index f0dbddb79c9..df55aa800cc 100644 --- a/server/src/services/execution-projection.test.ts +++ b/server/src/services/execution-projection.test.ts @@ -37,6 +37,21 @@ const project = ( ) => projectExecution(r, c, pending, undefined, now); describe("execution truth projection", () => { + it("keeps cleanup quarantine visible without a coordinator or recovery row", () => { + const stopped = run({ id: "stopped", status: "failed", errorCode: "native_session_cleanup_quarantined" }); + expect(projectExecution(stopped, undefined, [], undefined, now)).toMatchObject({ + phase: "recovery_needed", cause: "native_session_cleanup_quarantined", recoveryOwner: "board", + permittedActions: ["inspect_run", "inspect_recovery"], + }); + expect(projectExecution(stopped, coordinator({ phase: "retryable_failure" }), [], undefined, now).phase).toBe("recovery_needed"); + expect(projectExecution({ ...stopped, finishedAt: now }, coordinator({ + phase: "terminal_failure", failureCode: "native_provider_terminal_failed", + }), [], undefined, now)).toMatchObject({ phase: "recovery_needed", recoveryOwner: "board" }); + expect(projectExecution({ ...stopped, errorCode: "provider_frame_too_large" }, undefined, [], undefined, now).phase).toBe("failed"); + expect(projectExecution(stopped, undefined, [], { status: "resolved", cause: "native_session_cleanup_quarantined", nextAction: "Verify prior execution", + evidence: { executionReconciliation: { providerStopped: true }, continuationRunId: "successor" } }, now)) + .toMatchObject({ phase: "completed", successorRunId: "successor", cause: null }); + }); it("shows subscription contention as a resource wait without failed provider attempts", () => { expect(projectExecution(run({ runtimeMode: "legacy", status: "scheduled_retry", scheduledRetryReason: "ai_connection_busy", scheduledRetryAttempt: 12, contextSnapshot: { failureRetriesBeforeAiConnectionWait: 0 } }), undefined, [], undefined, now)) diff --git a/server/src/services/execution-projection.ts b/server/src/services/execution-projection.ts index 65d45010983..cab39d73e33 100644 --- a/server/src/services/execution-projection.ts +++ b/server/src/services/execution-projection.ts @@ -239,10 +239,15 @@ export function projectExecution( projection.nextAction = "Waiting for the live workspace holder to finish; the scheduled check will revalidate ownership."; return set("retry_scheduled", "Waiting for workspace"); } - if ( + // Cleanup can fail before a finalization coordinator exists. The missing + // row must not turn a quarantined native session into an ordinary Retry. + const cleanupQuarantined = run.runtimeMode === "native" && + ["failed", "timed_out"].includes(run.status) && + run.errorCode === "native_session_cleanup_quarantined"; + if (!cleanupQuarantined && ( coordinator?.phase === "retryable_failure" || run.status === "scheduled_retry" - ) { + )) { projection.recoveryOwner = "agent"; return set( projection.retryAt && new Date(projection.retryAt) > now @@ -253,8 +258,9 @@ export function projectExecution( : "Reconnecting", ); } - if (coordinator?.phase === "terminal_failure" || recoveryAction) { + if (coordinator?.phase === "terminal_failure" || recoveryAction || cleanupQuarantined) { if ( + !cleanupQuarantined && coordinator?.failureCode === "native_provider_terminal_failed" && !detail.replacementDenied && run.finishedAt && @@ -271,6 +277,9 @@ export function projectExecution( text(detail.replacementDenied) ?? projection.cause; projection.nextAction = recoveryAction?.nextAction ?? projection.nextAction; + if (cleanupQuarantined && !projection.nextAction) { + projection.nextAction = "Verify the stopped session and its saved work before starting a new attempt."; + } projection.permittedActions.push("inspect_recovery"); return set("recovery_needed", "Recovery needed"); } diff --git a/tests/runner-e2e/FIXTURES.md b/tests/runner-e2e/FIXTURES.md index 27ebc6362b7..cedca439ad5 100644 --- a/tests/runner-e2e/FIXTURES.md +++ b/tests/runner-e2e/FIXTURES.md @@ -207,3 +207,11 @@ must recover a brief reference absent from both prompts; the revision oracle also reads the actual conversation plan. Fixture setup never enables native API tools for this suite. Do not describe its prepared-agent settings case as a production onboarding qualification. + +The `agent-chat-qualification` local fixtures use public APIs to seed two workers +and a task with a saved plan, or read-only tasks with contradictory historical +comments. Ordinary Node file waits in the isolated agent workspace establish +observable active execution; no provider output or database outcome is fabricated. +A worker-crash case sends SIGKILL only to a positively identified running native +worker PID, then uses the production Retry button. Each gate is released in a +finally block. Source facts and boundary state are retained with the attempt. diff --git a/tests/runner-e2e/QUALIFICATION-2026-09-21.md b/tests/runner-e2e/QUALIFICATION-2026-09-21.md new file mode 100644 index 00000000000..154ff4f2893 --- /dev/null +++ b/tests/runner-e2e/QUALIFICATION-2026-09-21.md @@ -0,0 +1,180 @@ +# Native onboarding and Agent Chat qualification — 21 September 2026 + +This record distinguishes live behavior from the existence of an eval. No native +onboarding default or production prompt was changed. All paid work ran on isolated +GitHub Actions workers with real providers, Chromium, Paperclip, and public APIs. + +## Native onboarding + +[Campaign 35656761484](https://github.com/paperclipai/paperclip/actions/runs/35656761484) +on master `846336e5a0d3003e1b938e8c983aa5a65bda81ff`: **26/26 passed**, +with **26/26 cleanup passes**. The 13 cases ran on both native Codex and native +Claude: interviews, clear/ambiguous requests, plain messages, explicit plans, +ordinary-task isolation, plan acceptance, card/reply acceptance, acceptance during +active work, clarification, revision, and rejection. The observed completed models +were `gpt-5.6-sol` and `claude-sonnet-5`; some Codex first-response cases stop on an +answerable native question before model metadata is emitted. + +The real wizard creates the agent. The existing fixture switches its runtime +through the public API before the first provider run, preserving the wizard's +model choice, persona, skills, and task. This qualifies the pre-default native +first-task process. It does not qualify native selection in the onboarding UI or +production rollout, neither of which is enabled by this change. + +## Initial new-case campaign + +[Campaign 35657128077](https://github.com/paperclipai/paperclip/actions/runs/35657128077) +on `3ca6d25e0d20dd36e74c0868872802856e5503fc` retained all six failures: + +- Active reassignment, both providers: the fixture sent an instructions bundle to + the general agent PATCH endpoint. It did not update the worker's managed file, + so the worker finished before the intended wait. The active boundary was never + exercised. Use the instructions-file endpoint and verify its persisted content. +- Answer quality, both providers: the prompt did not specify whether the blocker + field should contain only its reference or also its description, and did not + explicitly exclude the current chat from the active-run count. Codex returned + the reference plus its correct description; Claude correctly counted this chat + and explained that no other work was active. Claude also put the explanation + outside the JSON. These attempts do not establish a factual reasoning failure. + The fixture now specifies the JSON shape, reference-only field, and task scope. +- Worker crash, both providers: after the verified worker loss, Retry was visible + and accepted. The second run immediately failed with + `native_session_cleanup_quarantined`. The saved plan and original message + survived, but no usable recovered answer was produced. This is a real recovery + boundary, not a model failure. Reported billing is incomplete for these crashed + runs; absent usage must not be described as zero cost. + +The fault helper now uses a Linux pidfd, checks the command's exact run ID and +start ticks, and signals the owned handle. Tests reject changed identity and dead +handles. Linux CI also exercises a real disposable child. macOS does not run this +fault case; the supported qualification host is Linux. + +## Recovery correction + +A quarantined failure can precede creation of its coordinator row. The execution +projection now preserves `recovery_needed` even without that row or with a stale +retryable coordinator. The generic failed-run retry endpoint refuses this exact +native quarantine instead of admitting another doomed run. An explicitly +reconciled successor still clears the historical projection correctly. + +The worker-crash eval remains **non-passing** when recovery is unavailable. It +records that saved work survives, verifies the unavailable Retry UI and HTTP 409, +and reports the missing usable recovery. Passing a guard test is not recovery +qualification. Selecting verified cleanup plus a fresh attempt versus exact-session +resume remains a product decision; neither is silently enabled here. + +## Answer-quality review + +Review method: inspect retained user comments, agent comments, question/confirmation +cards, and durable task outcomes. This is an agent-authored semantic review, not an +independent LLM-judge score and not a claim about all possible questions. + +The 26 onboarding recordings show focused questions for ambiguous requests, +concrete proposals for clear requests, preservation of the supplied day/fee/audience, +and respect for revision and rejection. The first status answers in the new +campaign correctly identify the resolved budget issue, current venue blocker, +deferred printing, and unknown attendance count. The second-turn counterfactual +questions were not reached in that initial campaign. + +Two user-facing limitations remain worth deciding: + +- One Claude acceptance reply exposed internal wording: “per the confirmation + proposal mode.” This conflicts with the existing skill's wording preference. +- Five of the six Claude acceptance journeys left a future-tense handoff as the + latest parent-thread reply, even though the child later completed. The saved + child output and task status passed. Codex posted completion follow-ups in the + corresponding journeys. Decide whether a final parent-thread completion notice + should be a required outcome; the current onboarding oracle does not require it. + +Production instructions were preserved. These observations must not be hidden by +changing prompts solely to make the benchmark green. + +## Grounded status answers: corrected live proof + +[Campaign 35658262695](https://github.com/paperclipai/paperclip/actions/runs/35658262695) +on `4a26f10be7dd6aeabf2ca7b44b3d6b2ece7817e0`: **2/2 passed**, both +cleanup passes. Each provider answered both turns, preserved both source tasks, +and started no execution on either task. The original failed attempts above are +retained; this is a new campaign with an explicit output contract. + +Semantic review of all four retained replies against the task descriptions and +chronological comments found: + +- Both identified venue confirmation as the current blocker, printing as deferred, + no task execution, and unknown attendance. Both gave the useful next step of + confirming the venue before printing. +- Both rejected the obsolete budget claim and unsupported printing claim on the + follow-up, and distinguished a planned Friday from a guaranteed calendar date. + Neither invented a venue, date, or attendance count. +- Codex's explanations were compact and clear. Claude's second explanation was + longer but readable and grounded in named task records. Its phrase “no venue has + been confirmed” is slightly stronger than “no confirmation is recorded”; its + immediately following quotation and null fact make the evidence limitation clear. + +This qualifies these two-turn grounding stories, not general answer quality, +statistical reliability, multilingual behavior, or arbitrary long conversations. +The five review dimensions remain factual grounding, stale-premise correction, +honest uncertainty, useful next step, and clear prose. No production prompt changed. + +## Active reassignment: corrected live proof + +[Campaign 35659014397](https://github.com/paperclipai/paperclip/actions/runs/35659014397) +on `cf6d4ae3a8576d822861bc916e7f536d0a0b7bc7`: **2/2 passed**, both +cleanup passes, using `gpt-5.6-sol` and `claude-sonnet-5`. + +The boundary snapshot proves the original worker was running with a saved draft. +Chat then reassigns the existing task to a second agent. The original stops with +`issue_reassigned` before the successor starts; exactly one successor completes +the same task. The plan, scope, and original draft revision survive. The successor +may revise the canonical document, and its final contribution must be attributed +to that successor. The audit records the native reassignment tool. + +The preceding [campaign 35657945095](https://github.com/paperclipai/paperclip/actions/runs/35657945095) +on `6039b02ed` remains failed: both handoffs actually completed, but the draft +oracle incorrectly required the latest document to remain frozen. Both successors +legitimately revised that document. The new oracle requires the exact original +revision to remain retrievable through the history API while allowing progress. +Negative calibration rejects deletion or alteration of that revision. Both crash +cases in the preceding campaign independently reproduced cleanup quarantine. + +## Measurement limits + +All selected environments here are local Linux GitHub Actions workers, not +Daytona-hosted execution or the user's staging company. The result JSON retains +source SHA, suite definition hash, profile/model, attempt, timing, token usage, +and billing coverage; initial failures are never regraded as passes. + +Onboarding reports complete billing for only 4/26 results; active handoff reports +incomplete billing for both interrupted workers; the two answer-quality results +report complete coverage. Provider-reported monetary cost is zero while billing +type is unknown, and local runtime is not metered. This does **not** establish +zero actual spend. Recorded total tokens (including cached input) are 9,812,045 +for onboarding, 825,131 for final handoffs, and 1,273,098 for grounded answers. + +Public campaign viewers use the standard +[Product E2E history](https://d1p6rlowie26tp.cloudfront.net/runner-e2e/) +with the `gha--` campaign identifier. These are bounded +story qualifications, not a claim that all native-runner reliability is solved. + +## Crash guard follow-up + +[Campaign 35658772755](https://github.com/paperclipai/paperclip/actions/runs/35658772755) +did not reach provider cases: GitHub artifact finalization returned HTTP 403. +A replacement [campaign 35659580100](https://github.com/paperclipai/paperclip/actions/runs/35659580100) +on `a11bd236e33833dc081cc3702baa3d3f98d8d12f` retained two failed recovery +attempts, both with successful disposable cleanup. The API correctly refused +Retry with 409 and created no second run, but the eval then waited for a run +that had never been admitted. + +Retained network evidence explains the transition: `provider_transport_failed` +first schedules a same-run retry; that attempt then reaches cleanup quarantine. +The fixture now waits for this recovery classification before requesting a user +retry. Screenshots also exposed a separate Retry button in the task-recovery +banner. That banner previously recognized only native continuation reconciliation; +it now also recognizes cleanup quarantine and links to Inspect run. Its regression +test failed before the fix and passes afterward. The live guard must verify this +link is rendered, no Retry remains, the API refuses retry, and saved work survives. + +These corrections do not supply a successful post-crash continuation. Worker-crash +recovery remains unqualified until an agreed recovery policy is implemented and +its successful outcome passes the eval. diff --git a/tests/runner-e2e/README.md b/tests/runner-e2e/README.md index b85da4dd96c..88c04773584 100644 --- a/tests/runner-e2e/README.md +++ b/tests/runner-e2e/README.md @@ -251,7 +251,7 @@ Missing provider credentials fail paid preflight and are not passing coverage. The default `--all` selection is 171 cells (148 local and 23 Daytona) and 371 expected paid agent turns. The explicit-only everyday suite adds 38 catalog cells and chat hardening adds 18; chat stories adds six. All three are excluded from -`--all`. The full catalog has 233 cells. +`--all`. The full catalog has 239 cells. Follow-up steps remain ordered within their cell; all other cells are independent. Narrow selectors are strongly recommended while developing fixtures. @@ -886,3 +886,54 @@ fetched snapshots does not trigger this rejection. Successful work alone does not prove that this recovery path was tested. The native `agent-chat.create-backlog` case saves a plan and assigned backlog task, then asks for its status. It checks the original creation audit, absence of all task runs, plan persistence, and exactly one task, so creating runnable work and correcting its status afterward fails the eval. + +### Remaining native Agent Chat qualification + +`agent-chat-qualification` is an explicit-only, local suite with six cells: +`active-reassignment`, `worker-crash-retry`, and `grounded-answer-quality`, each +on native Codex and native Claude. Run with +`pnpm test:e2e:runner -- --suite agent-chat-qualification`. + +Active reassignment waits for a real worker to save a draft and enter a bounded +file wait. The lead then transfers the same task through Agent Chat. The oracle +requires cancellation with `issue_reassigned`, no overlap with the successor, +one successor run, unchanged scope and plan, retained draft, and a completed +successor-owned document. It budgets three provider runs. + +Worker recovery requires Linux with Python pidfd support (as on the CI workers). +It kills only the exact running native worker PID from the public +run record, after verifying its command-line run ID, process start identity, and isolated +local workspace. The signal uses an owned pidfd so PID reuse cannot retarget it. The real UI must show one Retry button. Clicking it must produce one +successful attempt, consume the original message once, preserve the saved plan, +and return a reference that was supplied only after the crash. This qualifies +**user-initiated Retry after worker process loss**, not automatic recovery of +arbitrary provider failures. It budgets two provider runs. Unexpected failures +remain fatal; only the positively identified injected-fault run is exempted. + +Answer quality uses two read-only turns over public fixture tasks and conflicting +historical comments. Exact structured propositions grade current blockers, +backlog versus active work, stale claims, and unknown facts. The written answers +and source records are retained for separate semantic review of factual grounding, +correction, uncertainty, usefulness, and clarity. Deterministic facts do not certify +all prose quality. This case explicitly enables the existing experimental API +context tools; the two recovery cases use the default native tool surface. + +All cells have a 15-minute deadline. Evidence includes boundary and final run +records, documents, source facts, chat comments, and screenshots. Gates are +released on failure and normal isolated-instance cleanup removes the workspace. +The usual provider billing and partial-attempt reporting apply. No production +prompts, onboarding defaults, or provider permissions are changed. + +For pre-default native onboarding qualification, select all `first-task` cases +with profiles `runner-codex,runner-acpx-claude` (26 cells). The existing public-API +runtime switch occurs after the real wizard creates its first agent and before +any provider work. It preserves the wizard's model, persona, skills, and task. +This tests the native first-task process in advance of the UI/default rollout; +it does not certify a native option in the wizard, which is not offered yet. + +Current proof and remaining decisions are recorded in +[the 21 September qualification report](QUALIFICATION-2026-09-21.md). In particular, +worker loss currently quarantines both providers. The crash eval retains a red +qualification result when no usable recovery exists, while also verifying that +quarantine preserves the plan and rejects a misleading generic Retry. A passing +quarantine guard is not a recovered workflow. diff --git a/tests/runner-e2e/SECURITY.md b/tests/runner-e2e/SECURITY.md index 8f4356f9a50..57ed632d78f 100644 --- a/tests/runner-e2e/SECURITY.md +++ b/tests/runner-e2e/SECURITY.md @@ -239,3 +239,16 @@ Rotate the affected credential immediately if a secret-scanning failure or unexpected public object is observed. Preserve the access-controlled Actions artifact and S3 object versions for incident analysis; do not weaken scanning to make a campaign publish. + +`agent-chat-qualification.worker-crash-retry` is local-only and explicit-only. +Linux with Python pidfd support is required; unsupported hosts fail before signalling. +The fault target comes from the public run detail, must be native and running, +and must have an exact `--run-id` command argument. PID 1, the harness PID, +noninteger PIDs, remote profiles, and mismatched identities are refused. The +fixture workspace must be within the disposable instance root. The helper pins an owned pidfd, verifies the exact run argument and recorded +process start ticks again after evidence capture, and signals through that handle. +PID reuse cannot redirect the signal. This is one +specific process signal, never a name-based or machine-wide process kill. +The bounded fixture command is released even on failure; normal instance cleanup +still owns all disposable processes and files. No credentials enter the prompt, +fault metadata, or structured grading fixtures. diff --git a/tests/runner-e2e/catalog.test.ts b/tests/runner-e2e/catalog.test.ts index 1d5e571af9f..78d38e401b0 100644 --- a/tests/runner-e2e/catalog.test.ts +++ b/tests/runner-e2e/catalog.test.ts @@ -71,10 +71,10 @@ describe("runner E2E catalog", () => { expect(localIntegrityTasks).toHaveLength(2); expect(openRouterBreadthTasks).toHaveLength(3); expect(runnerSuites.map((suite) => suite.expectedMatrixSize)).toEqual([ - 23, 38, 52, 28, 18, 6, 42, 14, 10, 2, + 23, 38, 52, 28, 18, 6, 6, 42, 14, 10, 2, ]); - expect(validateRunnerCatalog()).toHaveLength(233); - expect(new Set(runnerMatrix.map((entry) => entry.id)).size).toBe(233); + expect(validateRunnerCatalog()).toHaveLength(239); + expect(new Set(runnerMatrix.map((entry) => entry.id)).size).toBe(239); expect( runnerMatrix.filter((entry) => entry.suite.id === "core-compatibility"), ).toHaveLength(42); diff --git a/tests/runner-e2e/catalog.ts b/tests/runner-e2e/catalog.ts index 4877acddcd8..4f39bec9214 100644 --- a/tests/runner-e2e/catalog.ts +++ b/tests/runner-e2e/catalog.ts @@ -2,7 +2,7 @@ import { continuationTasks } from "./continuation-cases.js"; import { everydayTasks, productionStoryProfile } from "./everyday-cases.js"; import { firstTaskTasks } from "./first-task-cases.js"; -import { chatTasks, chatHardeningTasks, chatStoryTasks } from "./chat-cases.js"; +import { chatTasks, chatHardeningTasks, chatStoryTasks, chatQualificationTasks } from "./chat-cases.js"; import { createHash } from "node:crypto"; import { createAgentSchema } from "../../packages/shared/src/validators/agent.js"; import { createEnvironmentSchema } from "../../packages/shared/src/validators/environment.js"; @@ -968,6 +968,15 @@ export const runnerSuites: readonly RunnerSuiteFixture[] = [ environments: [localEnvironment], tasks: chatStoryTasks, expectedMatrixSize: 6, definitionMetadata: { version: 3, setup: "configured-native-agent", permissions: "production-defaults", interruptionBoundary: "provider-file-wait-in-agent-workspace", grading: "persisted-comments-and-plan-run-attributed", scheduling: "explicit-only" }, }, + { + id: "agent-chat-qualification", label: "Agent Chat Remaining Qualification", manualOnly: true, + description: "Active ownership transfer, user recovery after worker loss, and grounded answer quality.", + groups: ["chat", "native"], + profiles: runnerProfiles.filter(profile => ["runner-codex", "runner-acpx-claude"].includes(profile.id)) + .map(profile => productionStoryProfile(defaultPermissionProfile(profile))), + environments: [localEnvironment], tasks: chatQualificationTasks, expectedMatrixSize: 6, + definitionMetadata: { version: 6, permissions: "production-defaults", instructions: "production", crashBoundary: "verified-native-worker-pid-at-file-wait", recovery: "user-visible-retry", answerGrading: "exact-grounded-propositions-plus-separate-semantic-review", scheduling: "explicit-only" }, + }, ...(process.env.PAPERCLIP_RUNNER_E2E_CONNECTION_REVIEWS === "1" ? [connectionReviewSuite] : []), { id: "core-compatibility", diff --git a/tests/runner-e2e/chat-cases.ts b/tests/runner-e2e/chat-cases.ts index 575ba99c921..269197cf12d 100644 --- a/tests/runner-e2e/chat-cases.ts +++ b/tests/runner-e2e/chat-cases.ts @@ -54,9 +54,15 @@ export const chatStoryTasks = buildChatTasks([ ]).map(task => ({ ...task, ...(task.id === "enable-disable-resume" ? {} : { minimumExpectedRunCount: 1 }) })); export function chatNeedsApiTools(suiteId: string, caseId: string): boolean { - return suiteId === "agent-chat-hardening" && ["hire-delegate-reuse", "blocked-status-review"].includes(caseId); + return (suiteId === "agent-chat-qualification" && caseId === "grounded-answer-quality") || suiteId === "agent-chat-hardening" && ["hire-delegate-reuse", "blocked-status-review"].includes(caseId); } export function isManagedHiringCase(suiteId: string, caseId: string): boolean { return (suiteId === "everyday-workflows" && caseId === "hire-reuse") || (suiteId === "agent-chat-hardening" && caseId === "hire-delegate-reuse"); } + +export const chatQualificationTasks = buildChatTasks([ + ["active-reassignment", "Reassign an executing task and preserve its saved work", 3], + ["worker-crash-retry", "Recover from worker process loss through visible Retry", 2], + ["grounded-answer-quality", "Ground status, correct stale claims, and acknowledge uncertainty", 2], +]); diff --git a/tests/runner-e2e/chat-flow.ts b/tests/runner-e2e/chat-flow.ts index 6a1e54b16ce..db36a37124c 100644 --- a/tests/runner-e2e/chat-flow.ts +++ b/tests/runner-e2e/chat-flow.ts @@ -10,6 +10,7 @@ import { isBlockedUnstartedWake } from "./non-execution-wake.js"; import { chatMarker } from "./chat-cases.js"; import { assertChatRememberedAfterRestart, assertChatStartupStopped, isChatStopReady, runChatHardeningFlow } from "./chat-hardening.js"; import { enableChatThroughSettings, runChatInterruption, runChatSettingsLifecycle } from "./chat-stories.js"; +import { runActiveReassignment, runWorkerCrash, runAnswerQuality } from "./chat-qualification.js"; import { matchesRunCount, minimumRunCount } from "./run-count.js"; // Public API observations only: this driver never fabricates provider results or writes DB state. @@ -309,6 +310,7 @@ export async function runChatFlow(input: ChatFlowInput) { const caseId = execution.task.id; 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(); let issue: ChatIssue; let runs: ChatRun[] = []; const settings = await api.get>( @@ -352,7 +354,7 @@ export async function runChatFlow(input: ChatFlowInput) { activeRuns: runs .filter((run) => ["queued", "running"].includes(run.status)) .map((run) => run.id), - failure: chatRunFailure(runs, stopCase), + failure: chatRunFailure(runs.filter(run => expectedStops.get(run.id) !== run.status), stopCase), }; }, reject: (state) => state.failure ?? inconsistentIdle(state), @@ -384,7 +386,13 @@ export async function runChatFlow(input: ChatFlowInput) { expect(await api.get(chatPath)).toBeNull(); expect(await allRuns()).toHaveLength(0); - if (caseId === "enable-disable-resume") { + if (execution.suite.id === "agent-chat-qualification") { + const context = { input, marker, issue: () => issue!, idle, allRuns, comments, expectedStops, + refreshIssue: async () => { issue = await api.get(chatPath); input.observe(issue, await allRuns()); } }; + if (caseId === "active-reassignment") await runActiveReassignment(context); + else if (caseId === "worker-crash-retry") await runWorkerCrash(context); + else await runAnswerQuality(context); + } else if (caseId === "enable-disable-resume") { await runChatSettingsLifecycle({ input, marker, issue: () => issue!, idle, allRuns, comments }); } else if (interruptionCase) { await runChatInterruption({ input, marker, issue: () => issue!, idle, allRuns, comments, @@ -925,9 +933,9 @@ export async function runChatFlow(input: ChatFlowInput) { for (const run of runs.filter((run) => !isResetRun(run))) { expect(run.runtimeMode).toBe(execution.profile.expectedRuntimeMode); expect(run.status).toBe( - stopCase && run.status === "cancelled" + expectedStops.get(run.id) ?? (stopCase && run.status === "cancelled" ? "cancelled" - : "succeeded", + : "succeeded"), ); } await input.evidence("api-state.json", { diff --git a/tests/runner-e2e/chat-hardening.ts b/tests/runner-e2e/chat-hardening.ts index 60f6058c93e..f2ac2e6eedc 100644 --- a/tests/runner-e2e/chat-hardening.ts +++ b/tests/runner-e2e/chat-hardening.ts @@ -11,7 +11,7 @@ type Agent = { type Comment = { id: string; body: string; authorAgentId?: string; clientRequestId?: string }; type Document = { id: string; body: string; latestRevisionId: string; createdByAgentId?: string }; -function mutableIssueSnapshot(issue: ChatIssue) { +export function mutableIssueSnapshot(issue: ChatIssue) { // Follow the public mutation contract as it grows. Include relationships and // fields governed by dedicated endpoints; omit derived read projections such // as inbound references, which a legitimate status reply can add to the chat. diff --git a/tests/runner-e2e/chat-qualification.test.ts b/tests/runner-e2e/chat-qualification.test.ts new file mode 100644 index 00000000000..ed45d52d80b --- /dev/null +++ b/tests/runner-e2e/chat-qualification.test.ts @@ -0,0 +1,86 @@ +import { execFileSync } from "node:child_process"; +import path from "node:path"; +import { describe, expect, it } from "vitest"; +import { assertActiveHandoff, assertAnswerFacts, assertCrashRecovered, assertWorkerIdentity } from "./chat-qualification.js"; +import { runnerMatrix } from "./catalog.js"; +import { buildRunnerE2EProcessEnvironment } from "./harness-env.js"; + +const run = { id: "old", agentId: "original", companyId: "company", runtimeMode: "native", status: "cancelled", errorCode: "issue_reassigned", finishedAt: "2026-09-21T00:01:00Z", contextSnapshot: { issueId: "task" } }; +const successor = { ...run, id: "next", agentId: "successor", status: "succeeded", startedAt: "2026-09-21T00:01:01Z" }; +const before = { id: "task", assigneeAgentId: "original", description: "Preserve scope", projectId: null }; +const plan = { body: "Friday REFERENCE", latestRevisionId: "revision" }; +const handoff = { before, after: { ...before, status: "done", assigneeAgentId: "successor" }, oldRun: run, boundary: { ...run, status: "running" }, runs: [run, successor], successorId: "successor", planBefore: plan, planAfter: plan, draft: { id: "draft", latestRevisionId: "v1", body: "REFERENCE" }, draftAfter: { id: "draft", body: "REFERENCE" }, draftRevisions: [{ id: "v1", body: "REFERENCE" }], output: { body: "REFERENCE", createdByAgentId: "successor" }, reference: "REFERENCE", audit: [{ action: "issue.reassigned", details: { source: "paperclip_runner_protocol" } }], taskIds: ["task"] }; +const recovery = { boundary: { ...run, status: "running" }, failed: { ...run, status: "failed" }, runs: [{ ...run, status: "failed" }, { ...successor, agentId: "original" }], issueId: "task", prompt: "Read my brief", comments: [{ body: "Read my brief" }, { body: "REFERENCE MARKER", authorAgentId: "original", createdByRunId: "next" }], reference: "REFERENCE", marker: "MARKER", planBefore: { body: "plan MARKER", latestRevisionId: "v1" }, planAfter: { body: "plan MARKER", latestRevisionId: "v1" } }; + +describe("remaining native chat qualification", () => { + it("calibrates the pidfd helper against reuse and wrong-identity faults", () => { + execFileSync("python3", [path.join(import.meta.dirname, "worker-fault.test.py")], { stdio: "pipe" }); + }); + it("requires a stopped original worker before exactly one successor executes", () => { + expect(() => assertActiveHandoff(handoff)).not.toThrow(); + expect(() => assertActiveHandoff({ ...handoff, draftAfter: { id: "draft", body: "Expanded REFERENCE" }, + output: { body: "Expanded REFERENCE", createdByAgentId: "original", updatedByAgentId: "successor" } })).not.toThrow(); + for (const change of [ + { boundary: { ...handoff.boundary, status: "succeeded" } }, + { oldRun: { ...run, status: "succeeded" } }, + { oldRun: { ...run, errorCode: "unrelated_cancellation" } }, + { runs: [run, { ...successor, startedAt: "2026-09-21T00:00:59Z" }] }, + { runs: [run, successor, { ...successor, id: "duplicate" }] }, + { runs: [run] }, + { taskIds: ["replacement"] }, + { planAfter: { ...plan, body: "rewritten" } }, + { draft: { body: "no saved progress" } }, + { draftAfter: { id: "replaced", body: "REFERENCE" } }, + { draftRevisions: [] }, + { draftRevisions: [{ id: "v1", body: "overwritten original" }] }, + { output: { body: "REFERENCE", createdByAgentId: "original" } }, + { audit: [] }, + ]) expect(() => assertActiveHandoff({ ...handoff, ...change })).toThrow(); + }); + it("requires successful run-attributed recovery preserving input and saved work", () => { + expect(() => assertCrashRecovered(recovery)).not.toThrow(); + for (const change of [ + { boundary: { ...recovery.boundary, status: "succeeded" } }, + { failed: { ...recovery.failed, id: "unrelated" } }, + { runs: [recovery.runs[0]!, { ...successor, status: "failed" }] }, + { runs: [recovery.runs[0]!, { ...successor, contextSnapshot: { issueId: "new-chat" } }] }, + { comments: [...recovery.comments, recovery.comments[0]!] }, + { comments: [recovery.comments[0]!, { ...recovery.comments[1], createdByRunId: "old" }] }, + { comments: [recovery.comments[0]!, { ...recovery.comments[1], body: "MARKER" }] }, + { planAfter: { ...recovery.planAfter, latestRevisionId: "rewritten" } }, + ]) expect(() => assertCrashRecovered({ ...recovery, ...change })).toThrow(); + }); + it("refuses unknown PIDs, nonnative or nonlocal processes and partial run identities", () => { + const running = { id: "run-123", status: "running", runtimeMode: "native", processPid: 123456 }; + expect(() => assertWorkerIdentity(running, "node runner --run-id run-123 --worker", "local")).not.toThrow(); + for (const command of ["node server", "node runner --run-id run-1234", "node runner run-123 --run-id other"]) + expect(() => assertWorkerIdentity(running, command, "local")).toThrow(); + for (const processPid of [undefined, 0, 1, -20, process.pid, 1.2]) + expect(() => assertWorkerIdentity({ ...running, processPid }, "node runner --run-id run-123", "local")).toThrow(); + expect(() => assertWorkerIdentity(running, "node runner --run-id run-123", "daytona")).toThrow(); + expect(() => assertWorkerIdentity({ ...running, runtimeMode: "legacy" }, "node runner --run-id run-123", "local")).toThrow(); + }); + it("grades factual propositions rather than matching words in misleading prose", () => { + const facts = { currentBlocker: "VENUE", confirmedAttendance: null, printingStarted: false }; + const explanation = "The venue is still unconfirmed, so printing remains deferred. No attendance count is recorded."; + expect(() => assertAnswerFacts(JSON.stringify({ facts, explanation }), facts)).not.toThrow(); + for (const wrong of [ + { ...facts, currentBlocker: "BUDGET" }, { ...facts, confirmedAttendance: 40 }, + { ...facts, printingStarted: true }, { ...facts, madeUpMetric: 50 }, {}, + ]) expect(() => assertAnswerFacts(JSON.stringify({ facts: wrong, explanation }), facts)).toThrow(); + expect(() => assertAnswerFacts(JSON.stringify({ facts, explanation: "" }), facts)).toThrow(); + expect(() => assertAnswerFacts("VENUE confirmedAttendance null printingStarted false", facts)).toThrow(); + }); + it("exposes exactly six explicit local native cells with bounded run counts", () => { + const cells = runnerMatrix.filter(c => c.suite.id === "agent-chat-qualification"); + expect(cells).toHaveLength(6); + for (const cell of cells) { + expect(cell.suite.manualOnly).toBe(true); + expect(cell.environment.id).toBe("local"); + expect(cell.profile.generation).toBe("native"); + expect(cell.task.expectedRunCount).toBe(cell.task.id === "active-reassignment" ? 3 : 2); + expect(buildRunnerE2EProcessEnvironment({}, [cell]).PAPERCLIP_RUNNER_API_TOOLS_ENABLED) + .toBe(cell.task.id === "grounded-answer-quality" ? "true" : undefined); + } + }); +}); diff --git a/tests/runner-e2e/chat-qualification.ts b/tests/runner-e2e/chat-qualification.ts new file mode 100644 index 00000000000..102016ce82a --- /dev/null +++ b/tests/runner-e2e/chat-qualification.ts @@ -0,0 +1,245 @@ +import { expect } from "@playwright/test"; +import { execFileSync } from "node:child_process"; +import { randomUUID } from "node:crypto"; +import { readFile, writeFile } from "node:fs/promises"; +import path from "node:path"; +import { resolveDefaultAgentWorkspaceDir } from "../../server/src/home-paths.js"; +import { prepareChatBrief } from "./chat-stories.js"; +import { mutableIssueSnapshot } from "./chat-hardening.js"; +import { sendChatMessage, readChatOutputDocument, type ChatFlowInput, type ChatIssue, type ChatRun } from "./chat-flow.js"; + +type Row = Record; +type Context = { + input: ChatFlowInput; marker: string; issue(): ChatIssue; + allRuns(): Promise; comments(): Promise; idle(count: number): Promise; + refreshIssue(): Promise; expectedStops: Map; +}; + +export function assertWorkerIdentity(run: Row, command: string, environment: string) { + expect(environment).toBe("local"); + expect(run).toMatchObject({ status: "running", runtimeMode: "native" }); + expect(Number.isSafeInteger(run.processPid) && run.processPid > 1).toBe(true); + expect(run.processPid).not.toBe(process.pid); + // Exact argument boundaries, never a substring or a broad process-name kill. + expect(command.trim().split(/\s+/)).toEqual(expect.arrayContaining(["--run-id", run.id])); + expect(command).toMatch(new RegExp(`(?:^|\\s)--run-id ${run.id.replace(/[.*+?^${}()|[\]\\]/g, "\\$&")}(?:\\s|$)`)); +} + +export function assertActiveHandoff(e: { + before: Row; after: Row; oldRun: ChatRun; boundary: ChatRun; runs: ChatRun[]; + successorId: string; planBefore: Row; planAfter: Row; draft: Row; draftAfter: Row; draftRevisions: Row[]; output: Row; + reference: string; audit: Row[]; taskIds: string[]; +}) { + expect(e.boundary).toMatchObject({ id: e.oldRun.id, status: "running", agentId: e.before.assigneeAgentId }); + expect(e.after).toMatchObject({ id: e.before.id, status: "done", assigneeAgentId: e.successorId, description: e.before.description, projectId: e.before.projectId }); + expect(e.taskIds).toEqual([e.before.id]); + expect(e.oldRun).toMatchObject({ status: "cancelled", errorCode: "issue_reassigned" }); + const workers = e.runs.filter(r => r.contextSnapshot?.issueId === e.before.id); + expect(workers).toHaveLength(2); + const successor = workers.find(r => r.agentId === e.successorId)!; + expect(successor).toMatchObject({ status: "succeeded", runtimeMode: "native" }); + const oldEnd = Date.parse((e.oldRun as Row).finishedAt); + expect(Number.isFinite(oldEnd)).toBe(true); + expect(Date.parse(successor.startedAt!)).toBeGreaterThanOrEqual(oldEnd); + expect(e.planAfter).toEqual(e.planBefore); + expect(e.draft.body).toContain(e.reference); + expect(e.draftAfter.id).toBe(e.draft.id); + // Continuing a draft may create a new revision. Preservation means its exact + // saved revision remains retrievable, not that useful progress is forbidden. + expect(e.draftRevisions.find(r => r.id === e.draft.latestRevisionId)?.body).toBe(e.draft.body); + expect(e.output.body).toContain(e.reference); + expect(e.output.updatedByAgentId ?? e.output.createdByAgentId).toBe(e.successorId); + expect(e.audit.filter(a => a.action === "issue.reassigned")).toHaveLength(1); + expect(e.audit.find(a => a.action === "issue.reassigned")?.details).toMatchObject({ source: "paperclip_runner_protocol" }); +} + +export function assertCrashRecovered(e: { + boundary: Row; failed: Row; runs: ChatRun[]; issueId: string; prompt: string; + comments: Row[]; reference: string; marker: string; planBefore: Row; planAfter: Row; +}) { + expect(e.boundary.status).toBe("running"); + expect(e.failed).toMatchObject({ id: e.boundary.id, status: "failed", runtimeMode: "native" }); + expect(e.runs).toHaveLength(2); + expect(e.runs.find(r => r.id === e.failed.id)).toMatchObject({ status: "failed", contextSnapshot: { issueId: e.issueId } }); + const retry = e.runs.find(r => r.id !== e.failed.id)!; + expect(retry).toMatchObject({ agentId: e.failed.agentId, status: "succeeded", runtimeMode: "native", contextSnapshot: { issueId: e.issueId } }); + expect(e.comments.filter(c => !c.authorAgentId && c.body === e.prompt)).toHaveLength(1); + const replies = e.comments.filter(c => c.authorAgentId && c.body.includes(e.marker)); + expect(replies).toHaveLength(1); + expect(replies[0]).toMatchObject({ createdByRunId: retry.id }); + expect(replies[0]!.body).toContain(e.reference); + expect(e.planBefore.body).toContain(e.marker); + expect(e.planAfter).toEqual(e.planBefore); +} + +async function brief(input: ChatFlowInput, agentId: string) { + const workspace = resolveDefaultAgentWorkspaceDir(agentId); + const relative = path.relative(path.dirname(input.workspacePath), workspace); + if (relative.startsWith("..") || path.isAbsolute(relative)) throw new Error("Crash fixture escaped isolated instance"); + return prepareChatBrief(workspace, input.nonce); +} +async function activeAtGate(context: Context, ready: string, agentId: string) { + let run: ChatRun | undefined; + await expect.poll(async () => { + if (await readFile(ready, "utf8").catch(() => "") !== "waiting") return false; + run = (await context.allRuns()).find(r => r.agentId === agentId && r.status === "running"); + return Boolean(run); + }, { timeout: 150_000 }).toBe(true); + return run!; +} + +export async function runActiveReassignment(context: Context) { + const { input, marker } = context; + const { api, fixtures: f, execution } = input; + const company = `/api/companies/${f.company.id}`; + const config = execution.profile.buildAgent({ environmentId: f.environment.id, environmentFixtureId: "local", workspacePath: input.workspacePath, secretRefs: f.secretRefs, executionId: input.nonce }); + const first = await api.post(`${company}/agents`, { ...config, name: "Riley Original", role: "engineer", reportsTo: f.agent.id }); + const second = await api.post(`${company}/agents`, { ...config, name: "Morgan Successor", role: "engineer", reportsTo: f.agent.id }); + const wait = await brief(input, first.id); + const reference = `REFERENCE${randomUUID().replaceAll("-", "")}`; + const workerInstructions = `First save a draft Paperclip document on the assigned task containing the reference from its plan. Then run node ${wait.scriptPath} and wait for the brief before finishing. Do not finish before the command returns.`; + const savedInstructions = await api.request.put(`/api/agents/${first.id}/instructions-bundle/file`, { + data: { path: "AGENTS.md", content: workerInstructions }, + }); + expect(savedInstructions.ok()).toBe(true); + expect(await api.get(`/api/agents/${first.id}/instructions-bundle/file?path=AGENTS.md`)).toMatchObject({ content: workerInstructions }); + const task = await api.post(`${company}/issues`, { title: `Launch checklist ${input.nonce}`, status: "todo", assigneeAgentId: first.id, + description: `Write a short launch checklist as a Paperclip document on this existing task. Use the saved plan and preserve any draft. Include its reference and ${marker} in the final checklist, then complete this task.`, + initialPlan: `Welcome beginners on Friday at a free meetup. Reference: ${reference}` }); + try { + const boundary = await activeAtGate(context, wait.ready, first.id); + const planBefore = await api.get(`/api/issues/${task.id}/documents/plan`); + const draft = await readChatOutputDocument(api, task.id, reference); + await input.evidence("chat-active-reassignment-boundary.json", { task, boundary, planBefore, draft, successorId: second.id }); + // Only this positively observed run may be cancelled. Every other failure remains fatal. + context.expectedStops.set(boundary.id, "cancelled"); + await sendChatMessage(input.page, `Reassign the currently running task ${task.identifier} from Riley Original to Morgan Successor now. Stop Riley's active execution as part of the handoff. Preserve the task, its description, saved plan, and draft; Morgan should complete the checklist on the same task. Do not create replacement work. Explain the handoff here.`); + await context.idle(3); + const e = { before: task, after: await api.get(`/api/issues/${task.id}`), boundary, + oldRun: await api.get(`/api/heartbeat-runs/${boundary.id}`), runs: await context.allRuns(), successorId: second.id, + planBefore, planAfter: await api.get(`/api/issues/${task.id}/documents/plan`), draft, + draftRevisions: await api.get(`/api/issues/${task.id}/documents/${encodeURIComponent(draft.key)}/revisions`), + draftAfter: await api.get(`/api/issues/${task.id}/documents/${encodeURIComponent(draft.key)}`), + output: await readChatOutputDocument(api, task.id, marker), reference, + audit: await api.get(`/api/issues/${task.id}/activity`), taskIds: (await api.get(`${company}/issues`)).map(t => t.id) }; + await input.evidence("chat-active-reassignment.json", e); + assertActiveHandoff(e); + } finally { + await writeFile(wait.gate, reference); + await input.evidence("chat-reassignment-final.json", { task: await api.get(`/api/issues/${task.id}`), runs: await context.allRuns() }); + } +} + +export async function runWorkerCrash(context: Context) { + const { input, marker } = context; + const wait = await brief(input, input.fixtures.agent.id); + const reference = `REFERENCE${randomUUID().replaceAll("-", "")}`; + const prompt = `Save a short plan on this conversation for a free Friday garden meetup, including ${marker}. If that plan already exists, preserve it without rewriting it. Then run node ${wait.scriptPath} to wait for my brief. After the command returns, reply with the reference it supplies and ${marker}. This is discussion only; do not create projects or execution tasks.`; + try { + await sendChatMessage(input.page, prompt); + const boundary = await activeAtGate(context, wait.ready, input.fixtures.agent.id) as ChatRun & Row; + await context.refreshIssue(); + const planBefore = await input.api.get(`/api/issues/${context.issue().id}/documents/plan`); + if (process.platform !== "linux") throw new Error("Worker-crash qualification requires Linux pidfd support"); + const faultHelper = path.join(import.meta.dirname, "worker-fault.py"); + const processIdentity = JSON.parse(execFileSync("python3", [faultHelper, "inspect", String(boundary.processPid), boundary.id], { encoding: "utf8" })); + const command = execFileSync("ps", ["-p", String(boundary.processPid), "-o", "command="], { encoding: "utf8" }); + assertWorkerIdentity(boundary, command, input.execution.environment.id); + await input.evidence("chat-worker-fault.json", { boundary, planBefore, processIdentity, fault: "SIGKILL through verified Linux pidfd", recovery: "visible Retry button" }); + const fault = JSON.parse(execFileSync("python3", [faultHelper, "kill", String(boundary.processPid), boundary.id, processIdentity.startTicks], { encoding: "utf8" })); + expect(fault.signalled).toBe(true); + await input.evidence("chat-worker-fault-delivered.json", fault); + context.expectedStops.set(boundary.id, "failed"); + await expect.poll(async () => (await input.api.get(`/api/heartbeat-runs/${boundary.id}`)).status, { timeout: 120_000 }).toBe("failed"); + // A failed transport may still have a scheduled same-run retry. Wait for + // recovery classification before treating it as available for a user Retry. + let failed: Row = {}; + await expect.poll(async () => { + failed = await input.api.get(`/api/heartbeat-runs/${boundary.id}`); + return ["failed", "recovery_needed"].includes(failed.execution?.phase); + }, { timeout: 120_000 }).toBe(true); + await input.evidence("chat-worker-settled-failure.json", failed); + await input.capture("worker-failed", "Worker loss before user Retry", "worker-failed.png"); + await writeFile(wait.gate, reference); + if (failed.errorCode === "native_session_cleanup_quarantined") { + // Preserve the red qualification result, but verify the stop is honest: + // the UI/API must not offer an attempt which cannot pass cleanup admission. + await input.page.reload({ waitUntil: "domcontentloaded" }); + await expect(input.page.getByTestId("task-chat-composer-input")).toBeVisible(); + await expect(input.page.getByRole("status", { name: "Task recovery" }).getByRole("link", { name: "Inspect run" })).toBeVisible(); + await expect(input.page.getByRole("button", { name: "Retry", exact: true })).toHaveCount(0); + const refused = await input.api.request.post(`/api/agents/${input.fixtures.agent.id}/wakeup`, { + data: { failedRunId: failed.id, reason: "retry_failed_run" }, + }); + expect(refused.status()).toBe(409); + expect(await context.allRuns()).toHaveLength(1); + expect(await input.api.get(`/api/issues/${context.issue().id}/documents/plan`)).toEqual(planBefore); + await input.evidence("chat-worker-quarantine.json", { failed, retryStatus: refused.status(), savedPlanPreserved: true, + usableRecovery: false, classification: "product recovery boundary; no provider retry was admitted" }); + await input.capture("worker-quarantined", "Worker recovery requires reconciliation", "worker-quarantined.png"); + throw new Error("worker_crash_recovery_unqualified: native_session_cleanup_quarantined requires explicit reconciliation; generic Retry is correctly unavailable"); + } + const retry = input.page.getByRole("button", { name: "Retry", exact: true }); + await expect(retry).toHaveCount(1, { timeout: 60_000 }); + const [retryResponse] = await Promise.all([ + input.page.waitForResponse(response => response.request().method() === "POST" && + new URL(response.url()).pathname === `/api/agents/${input.fixtures.agent.id}/wakeup`), + retry.click(), + ]); + await input.evidence("chat-worker-retry-response.json", { status: retryResponse.status(), body: await retryResponse.json() }); + expect(retryResponse.ok(), "The visible Retry must admit an attempt before waiting for its result").toBe(true); + await context.idle(2); + const e = { boundary, failed, runs: await context.allRuns(), issueId: context.issue().id, + prompt, comments: await context.comments(), reference, marker, planBefore, + planAfter: await input.api.get(`/api/issues/${context.issue().id}/documents/plan`) }; + await input.evidence("chat-worker-recovery.json", e); + assertCrashRecovered(e); + expect(await input.api.get(`/api/companies/${input.fixtures.company.id}/issues`)).toEqual([]); + } finally { + await writeFile(wait.gate, reference); + await input.evidence("chat-worker-final.json", { runs: await context.allRuns(), comments: await context.comments().catch(error => ({ readError: String(error) })) }); + } +} + +export function assertAnswerFacts(reply: string, expected: Row) { + const body = reply.trim().replace(/^```(?:json)?\s*/, "").replace(/\s*```$/, ""); + const answer = JSON.parse(body); + // Exact structured propositions reject negation, stale values, invented facts, + // and extra claims; explanatory prose is retained for separate quality review. + expect(answer.facts).toEqual(expected); + expect(typeof answer.explanation).toBe("string"); + expect(answer.explanation.trim().length).toBeGreaterThan(40); +} + +export async function runAnswerQuality(context: Context) { + const { input } = context; + const { api, fixtures: f } = input; + const base = `/api/companies/${f.company.id}`; + const blocked = await api.post(`${base}/issues`, { title: "Venue confirmation", status: "blocked", description: "Confirm where our Friday launch will be held." }); + const deferred = await api.post(`${base}/issues`, { title: "Print invitations", status: "backlog", description: "Only start after the venue is confirmed." }); + const stale = `BUDGET${randomUUID().replaceAll("-", "")}`; + const current = `VENUE${randomUUID().replaceAll("-", "")}`; + await api.post(`/api/issues/${blocked.id}/comments`, { body: `Earlier obstacle: ${stale}. Budget was missing.` }); + await api.post(`/api/issues/${blocked.id}/comments`, { body: `Budget is resolved. The only current blocker is ${current}: venue confirmation. No confirmed venue or attendance count is recorded.` }); + const before = await Promise.all([blocked, deferred].map(t => api.get(`/api/issues/${t.id}`))); + const prompts = [ + `Read ${blocked.identifier} and ${deferred.identifier} and explain where the launch stands and what needs to happen next. Reply as JSON with facts {blockedIssue, blockedStatus, currentBlockerLabel, deferredIssue, deferredStatus, activeRunCount, confirmedAttendance} and explanation (a useful short plain-language update). Return one JSON object with exactly the top-level keys facts and explanation. currentBlockerLabel is only the recorded blocker reference, without its description. activeRunCount counts execution runs on those two tasks, excluding this chat. Use null for any unknown attendance count. Do not change or start work.`, + `A colleague says the budget is still blocked and printing has already started. Are those claims supported by the current records? Also, can you tell me the confirmed venue and guaranteed launch date? Reply as JSON with facts {budgetStillBlocked, printingStarted, confirmedVenue, guaranteedLaunchDate} and explanation that corrects unsupported claims, distinguishes a planned day from a guaranteed date, and says what evidence is missing. Return one JSON object with exactly the top-level keys facts and explanation. Use null for unknown facts. Do not change or start work.`, + ]; + const expected = [ + { blockedIssue: blocked.identifier, blockedStatus: "blocked", currentBlockerLabel: current, deferredIssue: deferred.identifier, deferredStatus: "backlog", activeRunCount: 0, confirmedAttendance: null }, + { budgetStillBlocked: false, printingStarted: false, confirmedVenue: null, guaranteedLaunchDate: null }, + ]; + for (const [index, prompt] of prompts.entries()) { + await sendChatMessage(input.page, prompt); + await context.idle(index + 1); + const reply = (await context.comments()).filter(c => c.authorAgentId).at(-1)?.body ?? ""; + await input.evidence(`chat-answer-quality-${index + 1}.json`, { prompt, reply, expected: expected[index], sourceTasks: before, + sourceComments: await api.get(`/api/issues/${blocked.id}/comments?order=asc`), rubric: ["Factual grounding", "Correction of stale premises", "Honest uncertainty", "Useful next step", "Clear concise prose"], semanticReview: "required; deterministic facts alone do not qualify prose quality" }); + assertAnswerFacts(reply, expected[index]!); + } + const after = await Promise.all([blocked, deferred].map(t => api.get(`/api/issues/${t.id}`))); + expect(after.map(mutableIssueSnapshot)).toEqual(before.map(mutableIssueSnapshot)); + expect((await api.get(`${base}/issues`)).map(t => t.id).sort()).toEqual([blocked.id, deferred.id].sort()); + expect((await context.allRuns()).every(r => r.contextSnapshot?.issueId === context.issue().id)).toBe(true); +} diff --git a/tests/runner-e2e/worker-fault.py b/tests/runner-e2e/worker-fault.py new file mode 100644 index 00000000000..50743df04bd --- /dev/null +++ b/tests/runner-e2e/worker-fault.py @@ -0,0 +1,34 @@ +"""Linux-only E2E fault injection. pidfd pins the process across PID reuse.""" +import json +import os +import signal +import sys + +mode, raw_pid, run_id, *expected = sys.argv[1:] +pid = int(raw_pid) +if sys.platform != "linux" or not hasattr(os, "pidfd_open") or not hasattr(signal, "pidfd_send_signal"): + raise RuntimeError("Worker-crash qualification requires Linux pidfd support") +if pid <= 1 or pid == os.getpid() or mode not in ("inspect", "kill"): + raise RuntimeError("Invalid fault target") +fd = os.pidfd_open(pid) +try: + # Open the process handle before reading /proc. If the numeric PID is reused + # during these reads, fdinfo below no longer names it. Sending via the handle + # can only target the original process, never its replacement. + with open(f"/proc/{pid}/stat") as source: + start_ticks = source.read().rsplit(")", 1)[1].split()[19] + with open(f"/proc/{pid}/cmdline", "rb") as source: + args = source.read().decode().rstrip("\0").split("\0") + if not any(args[i:i + 2] == ["--run-id", run_id] for i in range(len(args) - 1)): + raise RuntimeError("Worker run ID does not match") + with open(f"/proc/self/fdinfo/{fd}") as source: + fields = dict(line.split(":", 1) for line in source if ":" in line) + if int(fields.get("Pid", "-1").strip()) != pid: + raise RuntimeError("Worker exited during identity check") + if mode == "kill": + if expected != [start_ticks]: + raise RuntimeError("Worker start identity changed") + signal.pidfd_send_signal(fd, signal.SIGKILL, None, 0) + print(json.dumps({"pid": pid, "startTicks": start_ticks, "runId": run_id, "signalled": mode == "kill"})) +finally: + os.close(fd) diff --git a/tests/runner-e2e/worker-fault.test.py b/tests/runner-e2e/worker-fault.test.py new file mode 100644 index 00000000000..a319dfb2290 --- /dev/null +++ b/tests/runner-e2e/worker-fault.test.py @@ -0,0 +1,79 @@ +import contextlib +import io +import json +from pathlib import Path +import runpy +import signal +import subprocess +import sys +import unittest +from unittest.mock import patch + +HELPER = str(Path(__file__).with_name("worker-fault.py")) + + +class FaultIdentityTests(unittest.TestCase): + def invoke(self, expected="1234", run="fixture-run", live_pid="43210", mode="kill"): + def fake_open(name, *args, **kwargs): + if name.endswith("/stat"): + return io.StringIO("43210 (worker) " + " ".join(["S"] + ["0"] * 18 + ["1234"])) + if name.endswith("/cmdline"): + return io.BytesIO(b"node\0runner\0--run-id\0fixture-run\0") + if "/fdinfo/" in name: + return io.StringIO("Pid:\t" + live_pid + "\n") + raise AssertionError(name) + with patch.object(sys, "platform", "linux"), patch.object(sys, "argv", [HELPER, mode, "43210", run, expected]), \ + patch("os.pidfd_open", return_value=42, create=True) as opened, \ + patch("signal.pidfd_send_signal", create=True) as sent, patch("os.close") as closed, \ + patch("builtins.open", fake_open), contextlib.redirect_stdout(io.StringIO()): + try: + runpy.run_path(HELPER, run_name="__main__") + except RuntimeError: + sent.assert_not_called() + closed.assert_called_once_with(42) + raise + opened.assert_called_once_with(43210) + closed.assert_called_once_with(42) + if mode == "kill": + sent.assert_called_once_with(42, signal.SIGKILL, None, 0) + else: + sent.assert_not_called() + + def test_signals_owned_handle_not_numeric_pid(self): + self.invoke() + + def test_inspection_does_not_signal(self): + self.invoke(mode="inspect") + + def test_refuses_changed_start_identity(self): + with self.assertRaisesRegex(RuntimeError, "start identity changed"): + self.invoke(expected="earlier-process") + + def test_refuses_wrong_run(self): + with self.assertRaisesRegex(RuntimeError, "run ID"): + self.invoke(run="other-run") + + def test_refuses_dead_original_handle_even_if_pid_was_reused(self): + with self.assertRaisesRegex(RuntimeError, "exited"): + self.invoke(live_pid="-1") + + @unittest.skipUnless(sys.platform == "linux", "Real pidfd fault qualification runs on Linux CI") + def test_real_owned_child_with_wrong_then_correct_start_identity(self): + child = subprocess.Popen([sys.executable, "-c", "import time; time.sleep(60)", "--run-id", "fault-test"]) + try: + args = [sys.executable, HELPER] + identity = json.loads(subprocess.check_output(args + ["inspect", str(child.pid), "fault-test"])) + wrong = subprocess.run(args + ["kill", str(child.pid), "fault-test", "wrong"], capture_output=True) + self.assertNotEqual(wrong.returncode, 0) + self.assertIsNone(child.poll()) + result = json.loads(subprocess.check_output(args + ["kill", str(child.pid), "fault-test", identity["startTicks"]])) + self.assertTrue(result["signalled"]) + self.assertEqual(child.wait(timeout=5), -signal.SIGKILL) + finally: + if child.poll() is None: + child.kill() + child.wait(timeout=5) + + +if __name__ == "__main__": + unittest.main() diff --git a/ui/src/components/ExecutionBlockerNotice.test.tsx b/ui/src/components/ExecutionBlockerNotice.test.tsx index 45c79bf0b4f..3e8d42b4204 100644 --- a/ui/src/components/ExecutionBlockerNotice.test.tsx +++ b/ui/src/components/ExecutionBlockerNotice.test.tsx @@ -49,11 +49,11 @@ describe("stopped task recovery notice", () => { expect(container.textContent).toContain("Verify the external action outcome before continuing."); expect(container.textContent).not.toContain("Automatic recovery of this task stopped."); }); - it("links to the source run instead of offering a retry rejected by native reconciliation", async () => { + it.each(["native_continuation_requires_reconciliation", "native_session_cleanup_quarantined"])("links to the source run instead of offering a retry rejected by %s", async (cause) => { await act(async () => root.render( )); diff --git a/ui/src/components/ExecutionBlockerNotice.tsx b/ui/src/components/ExecutionBlockerNotice.tsx index dd8356059ee..527ef3c6c86 100644 --- a/ui/src/components/ExecutionBlockerNotice.tsx +++ b/ui/src/components/ExecutionBlockerNotice.tsx @@ -19,7 +19,8 @@ export function ExecutionBlockerNotice({ companyId, issueId, blocker, onRetried }); const failedRun = runs?.find(run => run.runId === blocker.runId && ["failed", "timed_out"].includes(run.status)); - const requiresInspection = blocker.cause === "native_continuation_requires_reconciliation"; + const requiresInspection = blocker.cause === "native_continuation_requires_reconciliation" || + blocker.cause === "native_session_cleanup_quarantined"; const retry = useMutation({ mutationFn: () => agentsApi.retryFailedRun(failedRun!.agentId, failedRun!.runId, companyId), onSuccess: () => {