mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-09 07:10:34 +01:00
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>
This commit is contained in:
1 parent
fd4b197f83
commit
7afb57c022
12 files changed
+233
-27
No files matched your search
+1
-1
@@ -3425,7 +3425,7 @@ successful warm-session retention or admit another provider turn.
|
||||
Run the credential-free real-process restart suite with:
|
||||
|
||||
```sh
|
||||
pnpm --filter @paperclipai/paperclip-runner build:runner-binaries
|
||||
pnpm --filter @paperclipai/paperclip-runner run build:runner-binaries
|
||||
pnpm exec vitest run server/src/services/native-runtime/native-runner-restart-recovery.integration.test.ts
|
||||
pnpm --filter @paperclipai/paperclip-runner exec vitest run src/live/runnerd-codex-transport-adoption.test.ts -t 'adopts a live runner'
|
||||
```
|
||||
|
||||
@@ -12,7 +12,7 @@ T-oracle.
|
||||
|
||||
```sh
|
||||
pnpm install --frozen-lockfile
|
||||
pnpm --filter @paperclipai/paperclip-runner build:runner-binaries # native runner (debug)
|
||||
pnpm --filter @paperclipai/paperclip-runner run build:runner-binaries # native runner (debug)
|
||||
set -a; source ~/.secrets; set +a # provider keys, process env only
|
||||
node cli/node_modules/tsx/dist/cli.mjs tests/restart-chaos/main.ts --env local \
|
||||
--adapters codex,claude,opencode --concurrency 4 --tasks 8 --faults K1,K2,K4 --seed 42
|
||||
|
||||
@@ -53,7 +53,7 @@ function preflight(plan: WavePlan) {
|
||||
if (missing.length) throw new Error(`Missing credentials ${missing.join(", ")}. Run: set -a; source ~/.secrets; set +a`);
|
||||
const runner = process.env.PAPERCLIP_RUNNER_BINARY
|
||||
?? path.join(repositoryRoot, "packages/paperclip-runner/runner/target/debug/paperclip-runnerd");
|
||||
if (!existsSync(runner)) throw new Error(`Runner binary missing at ${runner}. Run: pnpm --filter @paperclipai/paperclip-runner build:runner-binaries`);
|
||||
if (!existsSync(runner)) throw new Error(`Runner binary missing at ${runner}. Run: pnpm --filter @paperclipai/paperclip-runner run build:runner-binaries`);
|
||||
// Same provider bundle preparation as tests/runner-e2e/verified-provider-bundles.setup.mjs.
|
||||
const build = spawnSync(process.execPath, [path.join(repositoryRoot, "packages/paperclip-runner/scripts/build-verified-provider-entrypoints.mjs")],
|
||||
{ cwd: repositoryRoot, stdio: "inherit" });
|
||||
|
||||
@@ -686,7 +686,7 @@ so a content mismatch cannot suppress the communication evidence.
|
||||
The source thread is observed for 120 seconds. The probe retains a later
|
||||
correction even if an earlier reply already passes delivery and access. A later
|
||||
clarification does not erase an earlier accessible delivery.
|
||||
The busy cell holds a separate source reply open until the worker finishes; the multiple cell delegates two notes and requires one completion per task. The restart cell holds the source provider at a fixture reference gate, then restarts the server after durable Done but before publication and releases the gate. The gate makes the interruption boundary observable and prevents a fast successful reply from racing the restart assertion. The onboarding cell records its naturally occurring timing.
|
||||
The busy cell holds a separate source reply open until the worker finishes; the multiple cell delegates two notes and requires one completion per task. The restart cell holds the source provider at a fixture reference gate, then restarts the server after durable Done but before publication and releases the gate. The gate makes the interruption boundary observable and prevents a fast successful reply from racing the restart assertion. Reaching the gate depends on the source agent running the instructed wait: the completion reply is published inside the run finalizer's own transaction, so the isolated-server entrypoint cannot hold it without a product hook. If no completion turn is still active and the turn that received this task's Done facts has succeeded or published its reply before the gate reports `waiting`, the cell fails at once with `restart boundary not reached (agent skipped gate)` rather than a generic timeout. That failure is agent compliance (the restart was never exercised), not restart robustness. A completion turn that fails or is cancelled before the gate is not blamed on the agent, because delivery can retry it; if the wait then times out, the timeout names that turn and its status as `restart boundary not reached`. After the restart, the chat navigation retries an abort caused by the old document's reload. The onboarding cell records its naturally occurring timing.
|
||||
|
||||
The mechanical oracle requires a run-attributed source reply after durable
|
||||
completion, plus the actual saved output or a navigable task/output link.
|
||||
@@ -839,7 +839,7 @@ runner binaries:
|
||||
```bash
|
||||
pnpm install
|
||||
pnpm exec playwright install chromium
|
||||
pnpm --filter @paperclipai/paperclip-runner build:runner-binaries
|
||||
pnpm --filter @paperclipai/paperclip-runner run build:runner-binaries
|
||||
```
|
||||
|
||||
List cells without loading credentials or starting Paperclip:
|
||||
|
||||
@@ -0,0 +1,54 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { gotoAfterServerRestart, isAbortedNavigation } from "./chat-restart.js";
|
||||
|
||||
const aborted = () => new Error("page.goto: net::ERR_ABORTED at http://127.0.0.1:1/RUN/issues/RUN-1");
|
||||
const timeout = () => Object.assign(new Error("page.goto: Timeout 30000ms exceeded."), { name: "TimeoutError" });
|
||||
function page(outcomes: Array<Error | undefined>) {
|
||||
const calls: string[] = [];
|
||||
return {
|
||||
calls,
|
||||
goto: async (url: string) => {
|
||||
calls.push(url);
|
||||
const outcome = outcomes.shift();
|
||||
if (outcome) throw outcome;
|
||||
return null;
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
describe("navigation after a server restart", () => {
|
||||
it("retries a navigation the old document's reload aborted", async () => {
|
||||
const p = page([aborted(), aborted(), undefined]);
|
||||
await gotoAfterServerRestart(p as never, "/issue", { deadlineAt: Date.now() + 10_000, retryDelayMs: 0 });
|
||||
expect(p.calls).toEqual(["/issue", "/issue", "/issue"]);
|
||||
});
|
||||
|
||||
it("keeps tolerating an unsettled lifecycle timeout without retrying", async () => {
|
||||
const p = page([timeout()]);
|
||||
await gotoAfterServerRestart(p as never, "/issue", { deadlineAt: Date.now() + 10_000, retryDelayMs: 0 });
|
||||
expect(p.calls).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("rethrows other navigation errors immediately", async () => {
|
||||
const p = page([new Error("page.goto: net::ERR_CONNECTION_REFUSED"), undefined]);
|
||||
await expect(gotoAfterServerRestart(p as never, "/issue", { deadlineAt: Date.now() + 10_000, retryDelayMs: 0 }))
|
||||
.rejects.toThrow("ERR_CONNECTION_REFUSED");
|
||||
expect(p.calls).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("stops retrying aborts at the attempt bound and at the deadline", async () => {
|
||||
const bounded = page([aborted(), aborted(), aborted(), undefined]);
|
||||
await expect(gotoAfterServerRestart(bounded as never, "/issue", { deadlineAt: Date.now() + 10_000, retryDelayMs: 0, maxAttempts: 3 }))
|
||||
.rejects.toThrow("ERR_ABORTED");
|
||||
expect(bounded.calls).toHaveLength(3);
|
||||
const late = page([aborted(), undefined]);
|
||||
await expect(gotoAfterServerRestart(late as never, "/issue", { deadlineAt: 0, retryDelayMs: 0 })).rejects.toThrow("ERR_ABORTED");
|
||||
expect(late.calls).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("recognizes only Chromium's aborted-navigation error", () => {
|
||||
expect(isAbortedNavigation(aborted())).toBe(true);
|
||||
expect(isAbortedNavigation(new Error("net::ERR_ABORTED_SOMETHING"))).toBe(false);
|
||||
expect(isAbortedNavigation("net::ERR_ABORTED")).toBe(false);
|
||||
});
|
||||
});
|
||||
@@ -5,3 +5,37 @@ export async function restartChatServer(page: Page, restart: () => Promise<void>
|
||||
await page.goto("about:blank", { waitUntil: "commit", timeout: 30_000 });
|
||||
await restart();
|
||||
}
|
||||
|
||||
/** Chromium reports a navigation another navigation replaced as net::ERR_ABORTED. */
|
||||
export function isAbortedNavigation(error: unknown): boolean {
|
||||
return error instanceof Error && /\bnet::ERR_ABORTED\b/.test(error.message);
|
||||
}
|
||||
|
||||
/**
|
||||
* Open `url` on a page whose document predates a server restart. The old
|
||||
* document's Vite client reloads itself when the replacement server comes up;
|
||||
* if that reload lands while this navigation is in flight, Chromium aborts one
|
||||
* of them with net::ERR_ABORTED. Retry only that abort, within the caller's
|
||||
* deadline and a bounded attempt count, and rethrow every other error. A
|
||||
* timeout still resolves: the replacement server can commit and render a fresh
|
||||
* document while its lifecycle stays unsettled, so callers must assert the new
|
||||
* document and durable state explicitly afterwards.
|
||||
*/
|
||||
export async function gotoAfterServerRestart(
|
||||
page: Pick<Page, "goto">,
|
||||
url: string,
|
||||
input: { deadlineAt: number; maxAttempts?: number; retryDelayMs?: number; now?: () => number },
|
||||
): Promise<void> {
|
||||
const now = input.now ?? Date.now;
|
||||
const maxAttempts = input.maxAttempts ?? 5;
|
||||
for (let attempt = 1; ; attempt += 1) {
|
||||
try {
|
||||
await page.goto(url, { waitUntil: "commit" });
|
||||
return;
|
||||
} catch (error) {
|
||||
if (error instanceof Error && error.name === "TimeoutError") return;
|
||||
if (!isAbortedNavigation(error) || attempt >= maxAttempts || now() >= input.deadlineAt) throw error;
|
||||
}
|
||||
await new Promise(resolve => setTimeout(resolve, input.retryDelayMs ?? 500));
|
||||
}
|
||||
}
|
||||
@@ -6,7 +6,8 @@ import { pollUntil, type RunnerApi } from "./api.js";
|
||||
import { sendChatMessage, readChatOutputDocument, collectChatRunEvidence, type ChatFlowInput, type ChatRun } from "./chat-flow.js";
|
||||
import { arm as armDocumentGate, clear as clearDocumentGate, release as releaseDocumentGate, waitUntilHeld } from "./context-comment-gate.js";
|
||||
import { publishChatBrief, workspaceChatBrief } from "./chat-stories.js";
|
||||
import { completionDelivery, completionOutputUsesReleasedBrief, type CompletionObservation } from "./completion-updates.js";
|
||||
import { gotoAfterServerRestart } from "./chat-restart.js";
|
||||
import { completionDelivery, completionOutputUsesReleasedBrief, completionRestartGateSkipped, completionRestartGateTimeoutDetail, type CompletionObservation } from "./completion-updates.js";
|
||||
|
||||
type Row = Record<string, any>;
|
||||
export async function observeCompletionUpdate(input: {
|
||||
@@ -232,10 +233,15 @@ export async function runChatCompletionUpdate(context: {
|
||||
load: async () => {
|
||||
const runs = await context.allRuns();
|
||||
return { runs, ready: await readFile(completionWait!.at(taskDirectory(context.issue().id)).ready, "utf8").catch(() => ""),
|
||||
comments: await api.get<Row[]>(`/api/issues/${context.issue().id}/comments?order=asc`),
|
||||
sourceCwds: runCwd(runs, context.issue().id) };
|
||||
},
|
||||
accept: state => state.ready === "waiting" && state.runs.some(r => r.contextSnapshot?.issueId === context.issue().id &&
|
||||
r.contextSnapshot?.wakeReason === "chat_task_completed" && r.status === "running") });
|
||||
r.contextSnapshot?.wakeReason === "chat_task_completed" && r.status === "running"),
|
||||
// The gate depends on the agent running the instructed wait; name a
|
||||
// skipped gate rather than timing out (see completionRestartGateSkipped).
|
||||
reject: state => completionRestartGateSkipped({ sourceId: context.issue().id, workerId: task!.id, ...state }),
|
||||
timeoutDetail: state => completionRestartGateTimeoutDetail(state && { sourceId: context.issue().id, workerId: task!.id, ...state }) });
|
||||
const beforeRestart = await api.get<Row[]>(`/api/issues/${context.issue().id}/comments?order=asc`);
|
||||
const workerState = await api.get<Row>(`/api/issues/${task!.id}`);
|
||||
expect(beforeRestart.filter(c => c.authorAgentId && c.createdAt >= workerState.completedAt), "restart must precede completion publication").toEqual([]);
|
||||
@@ -245,7 +251,8 @@ export async function runChatCompletionUpdate(context: {
|
||||
await input.evidence("completion-restart-boundary.json", { worker: await api.get(`/api/issues/${task!.id}`), runs: reportingRun.runs, sourceReferenceGateReady: true });
|
||||
await input.restart();
|
||||
await publishBrief(completionWait!, context.issue().id, "The organizer is ready to receive the saved result. Report the completed work now.");
|
||||
await page.goto(`/${f.company.issuePrefix}/chats/${f.agent.id}`, { waitUntil: "commit" });
|
||||
// The old document's Vite client can reload and abort this navigation.
|
||||
await gotoAfterServerRestart(page, `/${f.company.issuePrefix}/chats/${f.agent.id}`, { deadlineAt: input.deadlineAt });
|
||||
}
|
||||
for (const [index, item] of delegated.entries()) {
|
||||
await pollUntil({ label: "delegated note completed", deadlineAt: Date.now() + 180_000, intervalMs: 1000,
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { completionDelivery, completionOutputUsesReleasedBrief, type CompletionObservation } from "./completion-updates.js";
|
||||
import { COMPLETION_RESTART_GATE_SKIPPED, completionDelivery, completionOutputUsesReleasedBrief, completionRestartGateSkipped, completionRestartGateTimeoutDetail, type CompletionObservation } from "./completion-updates.js";
|
||||
import { runnerMatrix } from "./catalog.js";
|
||||
|
||||
const output = "Welcome to our free Friday garden meetup for beginners. Join us at 10:30; reference GARDEN123.";
|
||||
@@ -82,3 +82,45 @@ describe("completion-update delivery oracle", () => {
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe("completion restart gate", () => {
|
||||
const turn = (status: string, id = "turn", workerId: string | null = "worker") => ({ id, status,
|
||||
contextSnapshot: { issueId: "source", wakeReason: "chat_task_completed", ...(workerId ? { chatCompletionUpdates: [{ id: workerId, status: "done" }] } : {}) } });
|
||||
const base = { sourceId: "source", workerId: "worker", runs: [] as Record<string, any>[], comments: [] as Record<string, any>[] };
|
||||
it("keeps waiting while the completion turn is pending or running at the gate", () => {
|
||||
expect(completionRestartGateSkipped({ ...base, ready: "" })).toBeUndefined();
|
||||
expect(completionRestartGateSkipped({ ...base, ready: "", runs: [turn("queued", "turn", null)] })).toBeUndefined();
|
||||
expect(completionRestartGateSkipped({ ...base, ready: "", runs: [turn("running")] })).toBeUndefined();
|
||||
expect(completionRestartGateSkipped({ ...base, ready: "waiting", runs: [turn("succeeded")] })).toBeUndefined();
|
||||
});
|
||||
it("names a correlated completion turn that succeeded without reaching the gate", () => {
|
||||
expect(completionRestartGateSkipped({ ...base, ready: "", runs: [turn("succeeded")] }))
|
||||
.toBe(`${COMPLETION_RESTART_GATE_SKIPPED}: completion turn turn ended succeeded without the reference gate reporting waiting`);
|
||||
});
|
||||
it("keeps waiting while a later completion turn is still active", () => {
|
||||
expect(completionRestartGateSkipped({ ...base, ready: "", runs: [turn("succeeded", "first"), turn("queued", "retry", null)] })).toBeUndefined();
|
||||
});
|
||||
it("does not blame the agent for a turn that failed before the gate, and names it on timeout", () => {
|
||||
const state = { ...base, ready: "", runs: [turn("failed")] };
|
||||
expect(completionRestartGateSkipped(state)).toBeUndefined();
|
||||
expect(completionRestartGateTimeoutDetail(state))
|
||||
.toBe("restart boundary not reached: completion turn(s) turn ended failed before the reference gate reported waiting");
|
||||
expect(completionRestartGateTimeoutDetail({ ...state, ready: "waiting" })).toBeUndefined();
|
||||
expect(completionRestartGateTimeoutDetail(undefined)).toBeUndefined();
|
||||
});
|
||||
it("names a correlated agent reply and ignores unrelated replies", () => {
|
||||
const comments = [
|
||||
{ id: "handoff", authorAgentId: "lead", createdByRunId: "initial" },
|
||||
{ id: "user", authorUserId: "board" },
|
||||
];
|
||||
expect(completionRestartGateSkipped({ ...base, ready: "", runs: [turn("failed")], comments })).toBeUndefined();
|
||||
expect(completionRestartGateSkipped({ ...base, ready: "", runs: [turn("failed")],
|
||||
comments: [...comments, { id: "reply", authorAgentId: "lead", createdByRunId: "turn" }] }))
|
||||
.toBe(`${COMPLETION_RESTART_GATE_SKIPPED}: agent reply reply was published without the reference gate reporting waiting`);
|
||||
});
|
||||
it("ignores other conversations, other tasks and ordinary chat turns", () => {
|
||||
const other = { id: "o", status: "succeeded", contextSnapshot: { issueId: "other", wakeReason: "chat_task_completed", chatCompletionUpdates: [{ id: "worker" }] } };
|
||||
const ordinary = { id: "u", status: "succeeded", contextSnapshot: { issueId: "source", wakeReason: "chat_message" } };
|
||||
expect(completionRestartGateSkipped({ ...base, ready: "", runs: [other, ordinary, turn("succeeded", "t2", "another-worker")] })).toBeUndefined();
|
||||
});
|
||||
});
|
||||
@@ -23,6 +23,47 @@ export const completionReviewRubric = [
|
||||
"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);
|
||||
|
||||
@@ -0,0 +1,19 @@
|
||||
import { readFile } from "node:fs/promises";
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { CONNECTOR_SETUP_NEXT_STEP } from "./connection-reviews.js";
|
||||
|
||||
describe("connection review setup", () => {
|
||||
it("accepts the one-screen Check link step and the older Save and continue step", () => {
|
||||
expect(CONNECTOR_SETUP_NEXT_STEP.test("Check link")).toBe(true);
|
||||
expect(CONNECTOR_SETUP_NEXT_STEP.test("Save and continue")).toBe(true);
|
||||
expect(CONNECTOR_SETUP_NEXT_STEP.test("Continue")).toBe(false);
|
||||
expect(CONNECTOR_SETUP_NEXT_STEP.test("Checking…")).toBe(false);
|
||||
});
|
||||
|
||||
it("matches a label the current connector setup actually renders", async () => {
|
||||
const source = await readFile(new URL("../../ui/src/features/connections/ConnectionSetupFlow.tsx", import.meta.url), "utf8");
|
||||
const labels = [...source.matchAll(/"(Check link|Save and continue)"/g)].map(match => match[1]!);
|
||||
expect(labels.length).toBeGreaterThan(0);
|
||||
expect(labels.every(label => CONNECTOR_SETUP_NEXT_STEP.test(label))).toBe(true);
|
||||
});
|
||||
});
|
||||
@@ -3,6 +3,10 @@ import { expect, type Page } from "@playwright/test";
|
||||
import type { RunnerApi } from "./api.js";
|
||||
import { startReviewProvider } from "../fixtures/connection-review-provider.js";
|
||||
|
||||
/** The button after the URL step: Check link today, Save and continue before #14811. */
|
||||
export const CONNECTOR_SETUP_NEXT_STEP = /^(Save and continue|Check link)$/i;
|
||||
const CHECK_LINK = /^Check link$/i;
|
||||
|
||||
/** Uses the same production connection and permission screens as the fixture browser suite. */
|
||||
export async function setupConnectionReview(input: {
|
||||
page: Page;
|
||||
@@ -62,8 +66,15 @@ export async function setupConnectionReview(input: {
|
||||
.getByPlaceholder("https://example.com/actions")
|
||||
.fill(provider.url);
|
||||
await page.getByRole("button", { name: "Continue", exact: true }).click();
|
||||
await page.getByRole("button", { name: "Save and continue" }).click();
|
||||
await page.getByRole("button", { name: /Check link/i }).click();
|
||||
// #14811 made connector setup one screen: Continue now leads straight to
|
||||
// Check link. Accept the older intermediate "Save and continue" step too,
|
||||
// as connection-flow.ts does, so either layout reaches the link check.
|
||||
const next = page
|
||||
.getByRole("button", { name: CONNECTOR_SETUP_NEXT_STEP })
|
||||
.first();
|
||||
await expect(next).toBeVisible();
|
||||
if (!CHECK_LINK.test((await next.textContent()) ?? "")) await next.click();
|
||||
await page.getByRole("button", { name: CHECK_LINK }).click();
|
||||
await expect(
|
||||
page.getByRole("heading", { name: /is ready/i }),
|
||||
).toBeVisible();
|
||||
|
||||
@@ -64,7 +64,7 @@ import { armResponseTiming } from "./response-timing.js";
|
||||
|
||||
import { projectFirstTaskResult, runFirstTaskFlow, setupFirstTaskFixtures } from "./first-task-flow.js";
|
||||
import { runChatFlow } from "./chat-flow.js";
|
||||
import { restartChatServer } from "./chat-restart.js";
|
||||
import { gotoAfterServerRestart, restartChatServer } from "./chat-restart.js";
|
||||
import { matchesRunCount, minimumRunCount } from "./run-count.js";
|
||||
import { createHash, randomBytes } from "node:crypto";
|
||||
import { mkdir, readFile, rename, writeFile } from "node:fs/promises";
|
||||
@@ -1489,11 +1489,13 @@ for (const execution of executions) {
|
||||
await pollUntil({ label: "governed waiting turn", deadlineAt, load: loadTaskState, accept: state => state.taskRuns.length === 1 && state.taskRuns.every(run => TERMINAL_RUN_STATUSES.has(run.status)) });
|
||||
await captureScreenshot("tool-review-pending", "Connection review awaiting a human", "tool-review-pending.png");
|
||||
await page.getByRole("button", { name: "Dismiss Approve tool action" }).click();
|
||||
await page.getByRole("button", { name: "Review request", exact: true }).click();
|
||||
if (execution.task.toolReviewDecision === "restart") {
|
||||
await restartIsolatedPaperclipServer({ api, requestId: `tool-review-${nonce}`, deadlineAt });
|
||||
await page.reload();
|
||||
}
|
||||
// Open the review card on the document that will act on it: an
|
||||
// expanded card does not survive the post-restart reload.
|
||||
await page.getByRole("button", { name: "Review request", exact: true }).click();
|
||||
if (execution.task.toolReviewDecision === "always") {
|
||||
await page.getByRole("button", { name: "Approval options", exact: true }).click();
|
||||
await page.getByRole("menuitem", { name: "Always allow", exact: true }).click();
|
||||
@@ -1602,20 +1604,16 @@ for (const execution of executions) {
|
||||
(key) => Reflect.set(window, key, true),
|
||||
documentSentinel,
|
||||
);
|
||||
try {
|
||||
await page.goto(
|
||||
`/${encodeURIComponent(issuePrefix)}/issues/${encodeURIComponent(issue.identifier ?? issue.id)}`,
|
||||
// The replacement Vite server can commit and render a fresh
|
||||
// document while its navigation lifecycle remains unsettled.
|
||||
// The sentinel and explicit assertions below prove the new
|
||||
// document and durable state even when Playwright times out.
|
||||
{ waitUntil: "commit" },
|
||||
);
|
||||
} catch (error) {
|
||||
if (!(error instanceof Error) || error.name !== "TimeoutError") {
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
// The replacement Vite server can commit and render a fresh document
|
||||
// while its navigation lifecycle remains unsettled, and the old
|
||||
// document's Vite client can reload and abort this navigation. The
|
||||
// sentinel and explicit assertions below prove the new document and
|
||||
// durable state either way.
|
||||
await gotoAfterServerRestart(
|
||||
page,
|
||||
`/${encodeURIComponent(issuePrefix)}/issues/${encodeURIComponent(issue.identifier ?? issue.id)}`,
|
||||
{ deadlineAt },
|
||||
);
|
||||
await expect(
|
||||
page
|
||||
.getByRole("radio", {
|
||||
|
||||
Reference in new issue
Block a user