mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-21 16:27:18 +02:00
fix(paperclip-runner): deliver the shutdown settlement event past the terminal-turn gate (#14668)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The OpenCode driver maps provider events to runtime request events > - A provider turn can end before a pending runtime request receives its answer > - The consumer reads one turn's events and stops at that turn's terminal event > - A settlement event that arrives after that terminal event never reaches the consumer > - This pull request settles the request inside its own turn, before the terminal event > - The benefit is reliable request settlement without weakening late-frame protection ## Linked Issues or Issue Description **What happened?** A pending runtime request stayed open after an OpenCode turn failed through `session.error`. Session shutdown then dropped its settlement event as a late provider frame. **Expected behavior** The driver must deliver `runtime_request.expired` with the original `turnId` and `itemId`, inside the same single read pass the consumer performs on that turn. **Steps to reproduce** 1. Start an OpenCode turn that creates a native runtime request. 2. Leave the request pending and fail the turn through `session.error`. 3. Close the session and inspect the emitted events. **Paperclip version or commit** `22d41c6081f05658e0d7c8485d0f22f35af4a79e` **Deployment mode** Built from source with the OpenCode driver test fixture. ## What Changed - Settle a pending runtime request as soon as its own turn goes terminal, before the terminal turn event. - Add an optional `bypassTerminalTurnGate` parameter to the OpenCode session emitter, and set it on the settlement emit. - Keep a settlement loop in session close as a fallback for a request whose turn never went terminal. - Add a fixture trigger and a regression test that reads one turn in a single pass. - Keep the late provider frame gate unchanged for every provider event path. ## Verification - Run `pnpm vitest run packages/paperclip-runner/src/drivers/opencode/opencode-server-driver.test.ts`. - Run `tsc -p tsconfig.json --noEmit` in `packages/paperclip-runner`. - Run `tsc -p tsconfig.surfaces.json --noEmit` in `packages/paperclip-runner`. - Confirm that the regression test receives `runtime_request.expired` with the original identifiers. - Confirm that the late-frame tests still report dropped provider frames. ## Risks Four call sites now reach the settlement path: session close and the three terminal turn paths (completed, cancelled, and failed). At the three terminal turn paths the turn is still the active turn, so the gate admits the settlement event with or without the parameter. Session close is the only place where the parameter changes the result of the gate, and only for a request whose turn already ended. Every provider event path keeps the existing terminal-turn gate. The known driver test failure is pre-existing and does not touch this change. ## Model Used Claude Sonnet 5, with code execution and test assistance. ## 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 the changed tests locally; the known pre-existing failure remains documented above - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation, or no documentation change applies - [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 have addressed all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
3c561642b4
commit
af5c2d101c
3 files changed
+159
-12
No files matched your search
@@ -1022,6 +1022,87 @@ describe("OpenCodeServerDriver", () => {
|
||||
await session.close({ reason: "test" });
|
||||
});
|
||||
|
||||
it("delivers the settlement event for a runtime request whose turn fails through the same single-pass consumer that reads the turn", async () => {
|
||||
await chmod(fixture, 0o755);
|
||||
const root = await mkdtemp(
|
||||
join(tmpdir(), "paperclip-opencode-pending-then-fail-"),
|
||||
);
|
||||
const workspace = await mkdtemp(
|
||||
join(tmpdir(), "paperclip-opencode-pending-then-fail-workspace-"),
|
||||
);
|
||||
roots.push(root, workspace);
|
||||
const driver = new OpenCodeServerDriver({
|
||||
model: "openrouter/deepseek/deepseek-v4-flash-0731",
|
||||
runtimeDirectory: root,
|
||||
command: fixture,
|
||||
environment: {
|
||||
PATH: process.env.PATH,
|
||||
OPENROUTER_API_KEY: "fixture-key",
|
||||
},
|
||||
});
|
||||
const session = await driver.openSession({
|
||||
runId: "run-pending-then-fail",
|
||||
normalizedSessionId: "pending-then-fail",
|
||||
workingDirectory: workspace,
|
||||
});
|
||||
|
||||
const { turnId } = await session.startTurn({
|
||||
message: { role: "user", text: "pending-request-then-turn-fails" },
|
||||
});
|
||||
// The fixture asks a native question, which leaves a runtime request
|
||||
// pending, then fails the same turn through `session.error` without
|
||||
// ever resolving that request. `collectTurnEvents` reads exactly one
|
||||
// pass and stops at the turn's terminal event, the same rule the
|
||||
// production consumer applies (see its doc comment above). The
|
||||
// settlement event must arrive inside that same pass: a production
|
||||
// consumer that stops at `turn.failed` never opens a second read
|
||||
// afterward, so a settlement event that only `close()` produced later
|
||||
// would never reach it.
|
||||
const turnEvents = await collectTurnEvents(session.events());
|
||||
expect(
|
||||
turnEvents.some(
|
||||
(event) => event.eventType === "runtime_request.created",
|
||||
),
|
||||
).toBe(true);
|
||||
const settlementIndex = turnEvents.findIndex(
|
||||
(event) => event.eventType === "runtime_request.expired",
|
||||
);
|
||||
const terminalIndex = turnEvents.findIndex(
|
||||
(event) => event.eventType === "turn.failed",
|
||||
);
|
||||
expect(settlementIndex).toBeGreaterThanOrEqual(0);
|
||||
expect(terminalIndex).toBeGreaterThanOrEqual(0);
|
||||
// The settlement fact must precede the turn's terminal event, or a
|
||||
// consumer that stops reading at that terminal event misses it.
|
||||
expect(settlementIndex).toBeLessThan(terminalIndex);
|
||||
expect(turnEvents[settlementIndex]).toMatchObject({
|
||||
turnId,
|
||||
itemId: "question-native-1",
|
||||
});
|
||||
expect(turnEvents[terminalIndex]).toMatchObject({ turnId });
|
||||
expect(
|
||||
turnEvents.some(
|
||||
(event) =>
|
||||
event.eventType === "harness.diagnostic" &&
|
||||
event.payload.code === "opencode_late_terminal_turn_event_dropped" &&
|
||||
event.payload.turnId === turnId,
|
||||
),
|
||||
).toBe(false);
|
||||
// The request already settled with the turn; nothing is left pending
|
||||
// for `close()` to settle a second time.
|
||||
expect(session.pendingRuntimeRequests?.()).toHaveLength(0);
|
||||
|
||||
await session.close({ reason: "test" });
|
||||
const closeEvents = await collectTurnEvents(session.events());
|
||||
expect(
|
||||
closeEvents.some((event) =>
|
||||
["runtime_request.expired", "runtime_request.cancelled"].includes(
|
||||
event.eventType,
|
||||
),
|
||||
),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("keeps the session usable after a cancelled turn so the next turn on the same session still completes", async () => {
|
||||
await chmod(fixture, 0o755);
|
||||
const root = await mkdtemp(
|
||||
|
||||
@@ -864,22 +864,48 @@ class OpenCodeHarnessSession implements HarnessSession {
|
||||
if (this.#closed) return;
|
||||
this.#closed = true;
|
||||
this.#abort.abort();
|
||||
for (const { request } of this.#pendingRuntimeRequests.values()) {
|
||||
this.#emit(
|
||||
request.input === undefined
|
||||
? "runtime_request.cancelled"
|
||||
: "runtime_request.expired",
|
||||
request.input === undefined
|
||||
? harnessRuntimeRequestOutcome(request, { reason: "session_closed" })
|
||||
: harnessRuntimeInputExpiredOutcome(request, "provider_process_lost"),
|
||||
{ turnId: request.turnId, itemId: request.itemId },
|
||||
);
|
||||
}
|
||||
this.#pendingRuntimeRequests.clear();
|
||||
// Settle whatever is still pending, including a request whose turn
|
||||
// already went terminal without the driver observing it (see
|
||||
// `#settlePendingRuntimeRequestsForTurn`). Bypass the terminal-turn
|
||||
// gate so each settlement event still reaches the consumer instead of
|
||||
// getting dropped as a late frame.
|
||||
for (const requestId of [...this.#pendingRuntimeRequests.keys()])
|
||||
this.#settlePendingRuntimeRequest(requestId);
|
||||
this.#events.close();
|
||||
await this.#runtime.close();
|
||||
}
|
||||
|
||||
// A request can outlive its own turn: the turn can fail or get cancelled
|
||||
// while the request is still pending, which clears `#activeTurnId`
|
||||
// without settling the request. Settle any such request the moment its
|
||||
// owning turn goes terminal, before emitting the terminal turn event
|
||||
// itself, so the settlement event reaches the consumer within the same
|
||||
// turn it belongs to instead of waiting for a later, separate close.
|
||||
#settlePendingRuntimeRequestsForTurn(turnId: string): void {
|
||||
const requestIds = [...this.#pendingRuntimeRequests]
|
||||
.filter(([, pending]) => pending.request.turnId === turnId)
|
||||
.map(([requestId]) => requestId);
|
||||
for (const requestId of requestIds)
|
||||
this.#settlePendingRuntimeRequest(requestId);
|
||||
}
|
||||
|
||||
#settlePendingRuntimeRequest(requestId: string): void {
|
||||
const pending = this.#pendingRuntimeRequests.get(requestId);
|
||||
if (!pending) return;
|
||||
this.#pendingRuntimeRequests.delete(requestId);
|
||||
const { request } = pending;
|
||||
this.#emit(
|
||||
request.input === undefined
|
||||
? "runtime_request.cancelled"
|
||||
: "runtime_request.expired",
|
||||
request.input === undefined
|
||||
? harnessRuntimeRequestOutcome(request, { reason: "session_closed" })
|
||||
: harnessRuntimeInputExpiredOutcome(request, "provider_process_lost"),
|
||||
{ turnId: request.turnId, itemId: request.itemId },
|
||||
{ bypassTerminalTurnGate: true },
|
||||
);
|
||||
}
|
||||
|
||||
async dispatchTool(call: {
|
||||
tool: string;
|
||||
callId: string;
|
||||
@@ -1508,6 +1534,10 @@ class OpenCodeHarnessSession implements HarnessSession {
|
||||
{ ...workspace, complete: true },
|
||||
{ turnId, itemId: `${turnId}:workspace` },
|
||||
);
|
||||
// Settle any request this turn never answered before the terminal
|
||||
// event, so a consumer that stops reading at that terminal event still
|
||||
// observes the settlement.
|
||||
this.#settlePendingRuntimeRequestsForTurn(turnId);
|
||||
// Emit while this turn is still `#activeTurnId`; the gate in `#emit`
|
||||
// drops any frame whose turnId is not the active turn, so nulling it
|
||||
// first would make `#emit` drop this very event.
|
||||
@@ -1530,6 +1560,10 @@ class OpenCodeHarnessSession implements HarnessSession {
|
||||
// card. Preserve the provider fact as a cancelled terminal event; the
|
||||
// native session loop independently commits the authoritative yielded
|
||||
// result when this abort followed a governed wait.
|
||||
// Settle any request this turn never answered before the terminal
|
||||
// event, so a consumer that stops reading at that terminal event
|
||||
// still observes the settlement.
|
||||
this.#settlePendingRuntimeRequestsForTurn(turnId);
|
||||
this.#emit(
|
||||
"turn.cancelled",
|
||||
{
|
||||
@@ -1559,6 +1593,10 @@ class OpenCodeHarnessSession implements HarnessSession {
|
||||
},
|
||||
{ turnId, itemId: `${turnId}:session-error` },
|
||||
);
|
||||
// Settle any request this turn never answered before the terminal
|
||||
// event, so a consumer that stops reading at that terminal event still
|
||||
// observes the settlement.
|
||||
this.#settlePendingRuntimeRequestsForTurn(turnId);
|
||||
this.#emit(
|
||||
"turn.failed",
|
||||
{ status: "failed", error: bounded(properties.error ?? properties) },
|
||||
@@ -1829,8 +1867,10 @@ class OpenCodeHarnessSession implements HarnessSession {
|
||||
eventType: PrpEvent["eventType"],
|
||||
payload: Record<string, unknown>,
|
||||
refs: { turnId?: string; itemId?: string } = {},
|
||||
options?: { bypassTerminalTurnGate?: boolean },
|
||||
): void {
|
||||
if (
|
||||
!options?.bypassTerminalTurnGate &&
|
||||
eventType !== "harness.diagnostic" &&
|
||||
refs.turnId !== undefined &&
|
||||
refs.turnId !== this.#activeTurnId
|
||||
|
||||
@@ -483,6 +483,32 @@ const server = createServer(async (request, response) => {
|
||||
});
|
||||
return;
|
||||
}
|
||||
if (
|
||||
String(parsedPrompt.message ?? "").includes(
|
||||
"pending-request-then-turn-fails",
|
||||
)
|
||||
) {
|
||||
pendingQuestion = nativeQuestion();
|
||||
emit({
|
||||
type: "question.asked",
|
||||
id: "event-question-pending-then-fail",
|
||||
properties: pendingQuestion,
|
||||
});
|
||||
setTimeout(() => {
|
||||
emit({
|
||||
type: "session.error",
|
||||
id: "event-session-failed-with-pending-request",
|
||||
properties: {
|
||||
sessionID: session.id,
|
||||
error: {
|
||||
name: "ProviderError",
|
||||
message: "The fake provider failed with a request still pending.",
|
||||
},
|
||||
},
|
||||
});
|
||||
}, 10);
|
||||
return;
|
||||
}
|
||||
if (
|
||||
String(parsedPrompt.message ?? "").includes("late-straggler-source")
|
||||
) {
|
||||
|
||||
Reference in new issue
Block a user