Files
PaperClipAI/tests/runner-e2e/completion-updates.ts
T
DottaandClaude Opus 5.5 7afb57c022 test(runner-e2e): repair restart-cell harness drift (#19538)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work
> - The Runner Product E2E suite proves that agent work survives a
control-plane restart
> - A restart chaos campaign found three restart cells that fail in the
test harness, not in the product
> - A harness failure hides the real restart result, so the campaign
cannot classify these cells
> - This pull request repairs the harness drift in those cells and names
the one failure the harness cannot prevent
> - The benefit is that these cells now test restart behavior, or fail
with a clear cause

## Linked Issues or Issue Description

Refs #14811 (connector setup became one screen). Refs #19475
(completion-update restart fixture).

**What happened?**
1. `lifecycle-baseline.runner-codex.local.tool-review-restart` clicked a
"Save and continue" button. #14811 removed that button. The cell failed
during setup. After that fix, the cell also failed after the restart:
the expanded review card does not survive the post-restart reload, so
"Approve & run" was not visible.
2.
`local-session-integrity.runner-acpx-claude.local.structured-question-restart-resume`
failed with `page.goto: net::ERR_ABORTED` after the controller restart.
The old document's Vite client reloads when the replacement server
starts, and this aborts the test navigation. The code tolerated only
`TimeoutError`.
3.
`completion-updates.runner-acpx-claude.local.handoff-completion-restart`
waited for the source agent to run an instructed wait command. Claude
replied without the command (2/2 attempts), so the restart never
occurred. The cell then failed with a generic timeout.

**Expected behavior**
Each cell reaches its restart boundary and tests restart behavior. If
the boundary cannot be reached, the cell names the cause.

**Steps to reproduce**
Run `pnpm test:e2e:runner -- --max-parallel 1 --max-automatic-retries 0
--id <cell>` for each cell above on `master`.

## What Changed

- `tests/runner-e2e/connection-reviews.ts`: accept the one-screen "Check
link" step and the older "Save and continue" step. This uses the same
regex approach as `connection-flow.ts`.
- `tests/runner-e2e/runner.spec.ts` (tool review): click "Review
request" after the restart reload, on the document that acts on the
card.
- `tests/runner-e2e/chat-restart.ts`: add `gotoAfterServerRestart`. It
retries only `net::ERR_ABORTED`, within the caller's deadline and at
most 5 attempts. It still accepts `TimeoutError`. It rethrows all other
errors. The structured-question restart uses it. The existing sentinel
and durable-state assertions still prove the new document.
- `tests/runner-e2e/completion-updates.ts` and
`completion-update-flow.ts`: add `completionRestartGateSkipped` and
`completionRestartGateTimeoutDetail`. The gate poll stops at once with
`restart boundary not reached (agent skipped gate)` when no completion
turn is active and the turn that carries this task's Done facts
(`chatCompletionUpdates`, as in `completionDelivery`) succeeded or
published its reply before the gate reports `waiting`. A failed or
cancelled turn is not blamed on the agent, because delivery can retry
it; the timeout detail names it instead. The post-restart chat
navigation now uses `gotoAfterServerRestart`.
- A fixture-level gate is not possible without a product hook. The
native run finalizer publishes the completion reply inside its own
transaction, through the issue service closure. The isolated-server
entrypoint can patch only class prototypes and HTTP responses, and the
reply uses no tool call or HTTP request.
- `tests/runner-e2e/README.md`: document the skipped-gate failure.
- Docs: use `pnpm --filter @paperclipai/paperclip-runner run
build:runner-binaries` in `tests/runner-e2e/README.md`,
`tests/restart-chaos/README.md`, `tests/restart-chaos/main.ts` and
`doc/DEVELOPING.md`.
- No product code changes.

## Verification

- Unit: `pnpm exec vitest run --config tests/runner-e2e/vitest.config.ts
tests/runner-e2e/chat-restart.test.ts
tests/runner-e2e/completion-updates.test.ts
tests/runner-e2e/connection-reviews.test.ts` (36 passed).
- Typecheck: `node scripts/typecheck-test-projects.mjs tests/runner-e2e`
passed.
- Live cells, local, `--max-parallel 1 --max-automatic-retries 0`,
macOS, load average 35–65:
- `lifecycle-baseline.runner-codex.local.tool-review-restart`: PASS
(first run found the review-card reload drift, fixed in the second
commit).
-
`local-session-integrity.runner-acpx-claude.local.structured-question-restart-resume`:
PASS. A first attempt failed before the restart with `PRP command
session.open timed out` at load ~36 (environment).
-
`completion-updates.runner-acpx-claude.local.handoff-completion-restart`:
FAIL as designed, twice (before and after the review fix), with `restart
boundary not reached (agent skipped gate): completion turn … ended
succeeded without the reference gate reporting waiting`. Claude skipped
the wait both times, so this cell still does not exercise a restart.

## Risks

- Low risk. Test harness and docs only.
- The navigation retry could hide a real abort. It is limited to
`net::ERR_ABORTED`, 5 attempts and the deadline, and the test still
asserts that a new document loaded.
- The ACPX Claude completion restart cell still does not exercise a
restart when the agent skips the gate. It now reports this cause by
name.

## Model Used

- Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with
tool use (shell, file edit, browser test runs).

## Checklist

- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [x] All Paperclip CI gates are green
- [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-11-06 12:59:54 -06:00

123 lines
7.7 KiB
TypeScript

/** Mechanical delivery/access evidence, not a prose-quality or truthfulness judge.
* Keep replies verbatim for semantic review; never accept a marker alone as output.
*/
type Row = Record<string, any>;
export type CompletionObservation = {
sourceId: string;
worker: Row;
documents: Row[];
comments: Row[];
runs: Row[];
marker: string;
/** Synthetic user request and released brief, never inferred from the reply. */
fixtureRequest?: string;
/** Only other tasks explicitly delegated by this isolated fixture. */
relatedTasks?: Array<{ task: Row; documents: Row[] }>;
renderedLinks?: Array<{ commentId: string; href: string }>;
};
export const completionReviewRubric = [
"Does the final source-thread answer accurately say the requested work is finished?",
"Does it describe the saved result rather than repeat an earlier handoff promise?",
"Can the user access that result without asking another question?",
"Does it avoid inventing verification, publication, or other work not in the evidence?",
"Does it avoid repeated completion announcements or redundant acknowledgements, while allowing corrections and updates about different tasks?",
] as const;
export const COMPLETION_RESTART_GATE_SKIPPED = "restart boundary not reached (agent skipped gate)";
type GateState = { sourceId: string; workerId: string; ready: string; runs: Row[]; comments: Row[] };
const completionTurns = (state: GateState) => state.runs.filter(r => r.contextSnapshot?.issueId === state.sourceId &&
r.contextSnapshot?.wakeReason === "chat_task_completed");
/** Correlated as completionDelivery does: the turn received this task's Done facts. */
const carriesWorker = (run: Row, workerId: string) => Array.isArray(run.contextSnapshot?.chatCompletionUpdates) &&
run.contextSnapshot.chatCompletionUpdates.some((u: Row) => u.id === workerId);
/**
* The restart cell's boundary is the source provider waiting at its reference
* gate. The completion reply is published inside the run finalizer's own
* transaction, which an isolated-server fixture cannot hold without a product
* hook, so reaching the boundary depends on the agent running the wait. Once
* no completion turn is active and this task's correlated turn has succeeded
* or published its reply without the gate ever reporting "waiting", the
* boundary can no longer be reached: report it by name instead of waiting for
* a generic timeout. A failed or cancelled turn is not evidence that the agent
* skipped the gate, and delivery may retry it; completionRestartGateTimeoutDetail
* names it if the wait then times out.
*/
export function completionRestartGateSkipped(state: GateState): string | undefined {
if (state.ready === "waiting") return undefined;
const turns = completionTurns(state);
if (turns.some(r => ["queued", "running"].includes(String(r.status)))) return undefined;
const correlated = turns.filter(r => carriesWorker(r, state.workerId));
const reply = state.comments.find(c => c.authorAgentId && correlated.some(r => r.id === c.createdByRunId));
if (reply) return `${COMPLETION_RESTART_GATE_SKIPPED}: agent reply ${reply.id} was published without the reference gate reporting waiting`;
const succeeded = correlated.find(r => r.status === "succeeded");
if (succeeded) return `${COMPLETION_RESTART_GATE_SKIPPED}: completion turn ${succeeded.id} ended succeeded without the reference gate reporting waiting`;
return undefined;
}
/** Names completion turns that ended unsuccessfully before the gate when the wait times out. */
export function completionRestartGateTimeoutDetail(state: GateState | undefined): string | undefined {
if (!state || state.ready === "waiting") return undefined;
const ended = completionTurns(state).filter(r => !["queued", "running", "succeeded"].includes(String(r.status)));
if (!ended.length) return undefined;
return `restart boundary not reached: completion turn(s) ${ended.map(r => `${r.id} ended ${r.status}`).join(", ")} before the reference gate reported waiting`;
}
/** The start time exists only in the brief released after the source is idle. */
export function completionOutputUsesReleasedBrief(body: string): boolean {
return /\b(?:10\s*[:.h]\s*30|ten[-\s]+thirty|half[\s-]+past[\s-]+(?:ten|10))\b/i.test(body);
}
export function completionDelivery(observation: CompletionObservation) {
const { worker, documents, comments, runs, sourceId, marker } = observation;
const completedAt = Date.parse(worker.completedAt);
const outputs = documents.filter(d => d.issueId === worker.id &&
!new Set(["plan", "summary", "proposal"]).has(String(d.key ?? "").toLowerCase()) && typeof d.body === "string" && d.body.includes(marker));
const responses = comments.filter(c => c.issueId === sourceId && c.authorAgentId &&
Number.isFinite(completedAt) && Date.parse(c.createdAt) >= completedAt &&
runs.some(r => r.id === c.createdByRunId && r.agentId === c.authorAgentId &&
r.contextSnapshot?.issueId === sourceId && r.status === "succeeded"));
responses.sort((a, b) => a.createdAt.localeCompare(b.createdAt) || a.id.localeCompare(b.id));
const latestResponse = responses.at(-1);
const correlated = responses.filter(c => runs.some(r => r.id === c.createdByRunId &&
Array.isArray(r.contextSnapshot?.chatCompletionUpdates) &&
r.contextSnapshot.chatCompletionUpdates.some((update: Row) => update.id === worker.id && update.status === "done")));
const normalize = (text: string) => text.replace(/\s+/g, " ").trim();
// Delivery remains valid if a subsequent clarification omits the same link.
// Keep the latest reply separately for semantic review of the whole exchange.
const deliveries = correlated.map(response => {
const body = String(response.body ?? "");
const links = (observation.renderedLinks ?? []).filter(link => link.commentId === response.id).map(link => link.href);
const resultLinks = links.filter(link => {
try {
const url = new URL(link, "http://fixture.invalid");
const parts = url.pathname.split("/").map(decodeURIComponent);
const index = parts.indexOf("issues");
return index >= 0 && [worker.id, worker.identifier].filter(Boolean).includes(parts[index + 1]);
} catch { return false; }
});
const quotedOutput = outputs.some(d => normalize(d.body).length >= 60 && normalize(body).includes(normalize(d.body)));
return { response, resultLinks, quotedOutput };
});
const delivered = deliveries.filter(d => d.resultLinks.length > 0 || d.quotedOutput).at(-1);
const final = delivered?.response ?? latestResponse;
const resultLinks = delivered?.resultLinks ?? [];
const quotedOutput = delivered?.quotedOutput ?? false;
return {
checks: [
{ id: "completion-worker-done", passed: worker.status === "done" && Number.isFinite(completedAt), detail: "The delegated task is durably Done, not merely a successful run" },
{ id: "completion-output-saved", passed: outputs.length > 0, detail: "The worker saved a non-plan deliverable containing the requested reference" },
{ id: "completion-source-response", passed: Boolean(final), detail: "The originating thread has a run-attributed agent reply after task completion" },
{ id: "completion-source-correlated", passed: correlated.length > 0, detail: "A reply run received authoritative completion facts for this task; a late initial handoff reply does not qualify" },
{ id: "completion-result-access", passed: Boolean(final) && (resultLinks.length > 0 || quotedOutput), detail: "That reply links to the completed task/output or includes the actual saved output" },
],
response: final ?? null,
latestResponse: latestResponse ?? null,
responses,
resultLinks,
quotedOutput,
semanticReview: { status: "required", rubric: completionReviewRubric },
};
}