mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-08 01:57:31 +01:00
fix(e2e): grade only started runs and bound the clarification readback (#19335)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Product E2E journeys grade agent work in the QA staging smoke plan > - On the current QA deployment, two journeys failed after the product had already done the requested work > - protected-actions counted a wake that the control plane retired before it started as a bound run, and failed its runtime binding check > - question-answer-resume waited for its saved document in a view that no longer exists. The wait lasted 27 minutes, until the case deadline, and starved the run queue > - This pull request grades only runs that started provider work, and reads the document where the UI now shows it, with a bounded wait > - The benefit is that both journeys measure the product, and a UI readback failure costs about two minutes instead of a whole worker ## Linked Issues or Issue Description **What happened?** - protected-actions: the task succeeded with two runs. A third automation wake for the same agent and task was cancelled before it started, with the reason "Agent cannot start work while completed runs await accounting". Its `startedAt` was null, its `resultJson.cancellation` had `source: "control_plane"` and `expected: true`, and its `executionRecovery.providerWorkStarted` was false. That run has no runtime binding, so `validateAgentTaskState` failed with `agent_run_binding_mismatch`. - question-answer-resume: the agent saved the requested document and the task was done by 19:32. The journey then navigated to `#document-<key>` and waited until 19:59 for an element that never appeared. With the task chat shell and Artifacts V1, documents render in the properties pane's Artifacts tab. The run reported `journey_operation_failed` and `evidence_collection_timeout`, and the 60-minute job ended before four other cases could run. **Expected behavior** - A wake retired before admission is kept as evidence but is not graded as provider work. - The saved document is read in its current view, and a UI readback failure ends quickly. **Steps to reproduce** QA staging smoke launch 3074 on appcb8df178c8. The stack-side run records are described above. **Paperclip version or commit** master at03eb49053f. ## What Changed - `agent-flows.ts`: add `retiredBeforeProviderStart(run)`. The run must have status `cancelled` and `startedAt === null`, a control-plane cancellation with `expected: true`, and `executionRecovery.providerWorkStarted === false`. `validateAgentTaskState` still checks such a run's company, agent and task binding and counts it toward the run limit, then skips the runtime checks. - `clarification-flow.ts`: after reload, open the Artifacts tab with the existing `openDocumentAuthoring` helper before reading the document. task-dependency-resume uses the same path and passes on this deployment. The UI readback is capped at 2 minutes after the API returned the saved document. Agent work time is not changed. - `agent-evidence.test.ts`: the retired wake is accepted. Nine variants still fail: a started run, a missing `startedAt`, a failed status, an unexpected cancellation, an operator source, `providerWorkStarted: true`, missing recovery evidence, another agent, and another task. The run limit still applies. ## Verification - `npx vitest run --config tests/runner-e2e/vitest.config.ts tests/runner-e2e/product-journeys`: 46 files passed. - Negative control: without the skip, the new test fails. - `clarification-readback.spec.ts` (browser support): 6 tests passed. - `tsc -p tests/runner-e2e/tsconfig.json` shows no errors in the changed files. - Not verified live yet. QA uses the pinned harness, so the pin must move to a commit that contains this change. ## Risks - The exclusion is narrow, and every field it requires is positive evidence that the wake did no work. Any started or unexpected extra run still fails. - The readback cap is stricter, not looser. It applies only after the API has confirmed the document. Agent work and answer timing still use the existing deadlines. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, browser, code editing). Extended reasoning was used. ## 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 - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
131c0c4609
commit
b4cc2d30d2
3 files changed
+78
-5
No files matched your search
@@ -1,5 +1,5 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { fixtureReplyStoredOnce, validateAgentTaskState } from "./agent-flows.js";
|
||||
import { agentTaskSettled, fixtureReplyStoredOnce, startedTaskRuns, validateAgentTaskState } from "./agent-flows.js";
|
||||
import { SLUGIFY_REVISION } from "../everyday-cases.js";
|
||||
import { activeNativeTurnStarted, nativeCancellationHasNoLaterWork, nativeStopAcknowledged, nativeWorkEvents } from "../stop-redirect-flow.js";
|
||||
import { parseTargetConfig } from "./catalog.js";
|
||||
@@ -75,6 +75,48 @@ describe("deployed agent journey evidence", () => {
|
||||
s.taskRuns[0]!.companyId = "other";
|
||||
expect(() => validateAgentTaskState(context, "owned-task", s, 3, ["run"])).toThrow();
|
||||
});
|
||||
it("ignores only a bound wake the control plane retired before provider work started", () => {
|
||||
// Observed on QA: an automation wake cancelled pre-start while completed runs awaited accounting.
|
||||
const retired = (s: ReturnType<typeof state>) => {
|
||||
(s.taskRuns as Array<Record<string, any>>).push({ id: "retired", companyId: "qa-company", agentId: s.taskRuns[0]!.agentId, issueId: "owned-task", status: "cancelled",
|
||||
startedAt: null, contextSnapshot: {}, resultJson: {
|
||||
executionRecovery: { kind: "accounting_wait", providerWorkStarted: false },
|
||||
cancellation: { source: "control_plane", expected: true, initiator: { type: "system" } } } });
|
||||
return s.taskRuns.at(-1)! as Record<string, any>;
|
||||
};
|
||||
const s = state(); retired(s);
|
||||
expect(() => validateAgentTaskState(context, "owned-task", s, 3)).not.toThrow();
|
||||
expect(s.taskRuns).toHaveLength(2);
|
||||
expect(() => validateAgentTaskState(context, "owned-task", s, 1)).toThrow("agent_task_binding_mismatch");
|
||||
const variants: Array<(run: Record<string, any>) => void> = [
|
||||
run => { run.startedAt = "2026-11-05T19:17:00Z"; },
|
||||
run => { delete run.startedAt; },
|
||||
run => { run.status = "failed"; },
|
||||
run => { run.resultJson.cancellation.expected = false; },
|
||||
run => { run.resultJson.cancellation.source = "operator"; },
|
||||
run => { run.resultJson.executionRecovery.providerWorkStarted = true; },
|
||||
run => { delete run.resultJson.executionRecovery; },
|
||||
run => { run.agentId = "other"; },
|
||||
run => { run.issueId = "other"; },
|
||||
];
|
||||
for (const change of variants) { const v = state(); change(retired(v)); expect(() => validateAgentTaskState(context, "owned-task", v, 3)).toThrow(); }
|
||||
});
|
||||
it("settles a completed task without counting a wake retired before provider start", () => {
|
||||
const run = (id: string, extra: Record<string, any> = {}): Record<string, any> => ({ id, status: "succeeded", startedAt: "2026-11-05T19:15:17Z", ...extra });
|
||||
const retired = run("retired", { status: "cancelled", startedAt: null, resultJson: {
|
||||
executionRecovery: { kind: "accounting_wait", providerWorkStarted: false },
|
||||
cancellation: { source: "control_plane", expected: true, initiator: { type: "system" } } } });
|
||||
const task = (taskRuns: Array<Record<string, any>>, status = "done") => ({ currentIssue: { id: "owned-task", status },
|
||||
taskRuns, comments: [], interactions: [], attachments: [] }) as unknown as Parameters<typeof agentTaskSettled>[0];
|
||||
expect(agentTaskSettled(task([run("first"), run("second"), retired]), 2)).toBe(true);
|
||||
expect(startedTaskRuns(task([run("first"), run("second"), retired]))).toHaveLength(2);
|
||||
// Negative controls: a started or unexpected extra run, missing work, open work or an unfinished task.
|
||||
expect(agentTaskSettled(task([run("first"), run("second"), { ...retired, startedAt: "2026-11-05T19:17:19Z" }]), 2)).toBe(false);
|
||||
expect(agentTaskSettled(task([run("first"), run("second"), { ...retired, resultJson: { ...retired.resultJson, cancellation: { ...retired.resultJson.cancellation, expected: false } } }]), 2)).toBe(false);
|
||||
expect(agentTaskSettled(task([run("first"), retired]), 2)).toBe(false);
|
||||
expect(agentTaskSettled(task([run("first"), run("second", { status: "running" })]), 2)).toBe(false);
|
||||
expect(agentTaskSettled(task([run("first"), run("second"), retired], "in_progress"), 2)).toBe(false);
|
||||
});
|
||||
it("requires explicit paid run limits for a mixed deployment campaign", () => {
|
||||
const config = { version: 1, id: "qa", mode: "existing-cloud", adapterModule: "/private/provider.mjs",
|
||||
executionEnvironment: "daytona", journeys: ["task-create", "ask-question", "plan-revise-accept", "build-revise", "protected-actions", "create-skill-studio", "stop-redirect"],
|
||||
|
||||
@@ -42,6 +42,29 @@ export function fixtureReplyStoredOnce(before: Row[], after: Row[], userId: stri
|
||||
return added.length === 1 && typeof added[0].body === "string" && added[0].body.replaceAll("\\_", "_") === message;
|
||||
}
|
||||
|
||||
/**
|
||||
* A wake the control plane retired before admission, for example while completed
|
||||
* runs await accounting. It never started provider work, so it has no runtime
|
||||
* binding to grade. Started, unexpected or user cancellations do not qualify.
|
||||
*/
|
||||
export function retiredBeforeProviderStart(run: Row) {
|
||||
const result = run.resultJson;
|
||||
return run.status === "cancelled" && run.startedAt === null
|
||||
&& result?.cancellation?.source === "control_plane" && result.cancellation.expected === true
|
||||
&& result.executionRecovery?.providerWorkStarted === false;
|
||||
}
|
||||
|
||||
/** Runs that count toward a journey's expected work; retired wakes stay in the raw state and run limit. */
|
||||
export function startedTaskRuns(state: Pick<TaskState, "taskRuns">) {
|
||||
return state.taskRuns.filter(run => !retiredBeforeProviderStart(run));
|
||||
}
|
||||
|
||||
/** The task is done after exactly `count` started runs, with nothing active or pending. */
|
||||
export function agentTaskSettled(state: TaskState, count: number) {
|
||||
return state.currentIssue.status === "done" && startedTaskRuns(state).length === count && state.taskRuns.every(run => !active(run))
|
||||
&& state.interactions.every(row => row.status !== "pending");
|
||||
}
|
||||
|
||||
/** Owned-task observations only: other work in a persistent QA company cannot affect this grade. */
|
||||
export function validateAgentTaskState(context: JourneyContext, issueId: string, state: TaskState, maxRuns: number, allowedCancelled: string[] = []) {
|
||||
const agent = context.agent!;
|
||||
@@ -51,6 +74,8 @@ export function validateAgentTaskState(context: JourneyContext, issueId: string,
|
||||
const provider = run.runnerProfileJson?.nativeExecutionInput?.provider;
|
||||
const boundIssueId = run.issueId ?? run.nativeIssueId ?? run.contextSnapshot?.issueId ?? run.contextSnapshot?.taskId;
|
||||
if (run.companyId !== context.company.id || run.agentId !== agent.id || boundIssueId !== issueId) throw new JourneyError("failed", "agent_run_binding_mismatch");
|
||||
// The retired wake stays in the raw record and in the run limit above.
|
||||
if (retiredBeforeProviderStart(run)) continue;
|
||||
if (!active(run) && (run.runtimeMode !== "native" || run.contextSnapshot?.paperclipEnvironment?.id !== context.environmentId
|
||||
|| provider?.kind !== agent.provider || provider?.model !== agent.model)) throw new JourneyError("failed", "agent_run_binding_mismatch");
|
||||
if (!["queued", "running", "succeeded"].includes(run.status) && !(run.status === "cancelled" && allowedCancelled.includes(run.id))) throw new JourneyError("failed", "agent_run_failed");
|
||||
@@ -103,8 +128,7 @@ export async function runAgentJourney(id: AgentJourney, context: JourneyContext,
|
||||
return state;
|
||||
};
|
||||
const settled = (issue: Row, count: number) => waitFor(options.deadlineAt, () => read(issue), state =>
|
||||
state.currentIssue.status === "done" && state.taskRuns.length === count && state.taskRuns.every(run => !active(run))
|
||||
&& state.interactions.every(row => row.status !== "pending"), "agent_task_not_completed");
|
||||
agentTaskSettled(state, count), "agent_task_not_completed");
|
||||
const finalReply = async (issue: Row, state: TaskState, marker?: string) => {
|
||||
const runs = state.taskRuns.filter(run => run.status === "succeeded");
|
||||
if (!runs.length || runs.some(run => !Number.isFinite(Date.parse(run.finishedAt)))) throw new JourneyError("failed", "final_run_time_missing");
|
||||
@@ -195,7 +219,7 @@ export async function runAgentJourney(id: AgentJourney, context: JourneyContext,
|
||||
const fixture = everydayTasks.find(task => task.id === `service-${phase}`)!;
|
||||
const issue = await create(`-${phase}`, `${fixture.buildPrompt(nonce)}\nUse only the connected service named "${service.name}". Do not use any other service.`);
|
||||
const pending = await waitFor(options.deadlineAt, () => read(issue), state =>
|
||||
state.taskRuns.length === 1 && !state.taskRuns.some(active) && state.interactions.some(row => row.status === "pending"), "protected_action_not_requested");
|
||||
startedTaskRuns(state).length === 1 && !state.taskRuns.some(active) && state.interactions.some(row => row.status === "pending"), "protected_action_not_requested");
|
||||
check(`${phase}-waited-for-decision`, pending.currentIssue.status !== "done" && service.invocationCount() === callsBefore);
|
||||
await open(issue); await context.page.reload();
|
||||
const decision = await decideProtectedActionThroughUi({ page: context.page, rows: pending.interactions as any,
|
||||
|
||||
@@ -8,9 +8,11 @@ import { answerResumeQuestionThroughUi } from "../question-resume-ui.js";
|
||||
import { createTaskThroughUi } from "../user-actions.js";
|
||||
import { validateAgentTaskState } from "./agent-flows.js";
|
||||
import { clarificationQuestionFailure, gradeClarificationEvidence, type ClarificationReadback } from "./clarification-evidence.js";
|
||||
import { openDocumentAuthoring } from "./task-document-flow.js";
|
||||
import { JourneyError, type JourneyContext, type JourneyEvidence, type TargetSession } from "./types.js";
|
||||
|
||||
type Row = Record<string, any>;
|
||||
export const DOCUMENT_READBACK_MS = 120_000;
|
||||
interface Options { deadlineAt: number; directory: string; capture(id: string, label: string, file: string): Promise<void> }
|
||||
|
||||
export async function waitForClarificationDocument(visible: Locator, answer: string, deadlineAt: number) {
|
||||
@@ -108,8 +110,13 @@ export async function runClarificationJourney(context: JourneyContext, session:
|
||||
const href = `/${encodeURIComponent(context.company.issuePrefix)}/issues/${encodeURIComponent(issue.identifier ?? issue.id)}#document-${encodeURIComponent(document.key)}`;
|
||||
await context.page.goto(new URL(href, context.api.origin).href, { waitUntil: "domcontentloaded" });
|
||||
await context.page.reload({ waitUntil: "domcontentloaded" });
|
||||
// With the task chat shell and Artifacts V1, saved documents render in the
|
||||
// properties pane's Artifacts tab, not the center column. Open it as a user would.
|
||||
await openDocumentAuthoring(context);
|
||||
const visible = context.page.locator(`[id=${JSON.stringify(`document-${document.key}`)}]:visible, [id=${JSON.stringify(`side-panel-content-document:${document.key}`)}]:visible`);
|
||||
await waitForClarificationDocument(visible, scenario.answer, options.deadlineAt);
|
||||
// The API already returned the saved document, so this is a bounded UI readback,
|
||||
// not agent work time: a missing view must not hold the worker until the case deadline.
|
||||
await waitForClarificationDocument(visible, scenario.answer, Math.min(options.deadlineAt, Date.now() + DOCUMENT_READBACK_MS));
|
||||
const persisted = await context.api.get<Row>(`/api/issues/${issue.id}/documents/${encodeURIComponent(document.key)}`);
|
||||
readback = { issueId: persisted.issueId, key: persisted.key, revisionId: persisted.latestRevisionId, body: persisted.body, visibleBody: await visible.innerText() };
|
||||
await options.capture("reloaded", "Saved answers after reload", "clarification-reloaded.png");
|
||||
|
||||
Reference in new issue
Block a user