mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-11 05:11:49 +01:00
## Thinking Path > - Paperclip manages AI agents and their tasks. > - A task can outlive a provider process or a server restart. > - Legacy recovery treated unknown tool outcomes as a permanent execution hold. > - That hold could also reject a later user message. > - A conversation turn can use prior history without replaying prior tool calls. > - This pull request lets supported conversation adapters continue within the existing retry budget. > - Users can send a new message after automatic attempts stop. ## Linked Issues or Issue Description **What happened?** A server restart could interrupt a local ACP run and leave its task behind a permanent recovery hold. A later user message could be cancelled before the provider answered. The immediate recovery path could also create a successor outside the durable failure counter. **Expected behavior** Continue with a bounded new conversation turn. Preserve a compatible provider session or use full task context when it is unavailable. Do not replay recorded tools. When automatic attempts stop, allow a new user request through the normal execution gates. **Steps to reproduce** 1. Start a task with a local conversation adapter. 2. Restart the server while the provider is working. 3. Let the previous run become interrupted. 4. Send a follow-up message and observe the recovery hold on the old behavior. Related work: Refs #13075 for durable task recovery. Refs #12946 for retry-limit and checkout-lock handling. This change routes conversation recovery through the existing bounded scheduler. ## What Changed - Mark supported local conversation failures for continuation. Keep native-runner and non-conversation recovery rules. - Carry an interruption notice into the next turn. Retain stopped ACP session history even when a write outcome is unknown. - Clear unavailable ACP sessions so the next bounded attempt can use full task context. - Route immediate failure recovery through the same durable scheduler as process-loss recovery. Release only the predecessor checkout when its retry takes ownership. - Retire obsolete conversation holds using immutable run evidence, in bounded batches with an activity record. Preserve outcome evidence and do not wake historical tasks. - Block actual admission and Resume while a predecessor process or environment lease is still active. Keep the original interruption notice after a rejected wake. Preserve the upstream blocked-wake waiting contract: bounded retry planning can happen during cleanup, while deferred messages and execution remain gated. - Add subprocess and database regression tests. Update the execution contract. - Add the current thread-status field to the native recovery provider fixture so its damaged-journal test reaches the intended boundary. Tolerate an already-exited fixture process during test cleanup while still asserting both processes terminate. ## Verification - Workspace typecheck passed: `pnpm -r typecheck`. - Build passed: `pnpm build`. - Module boundaries passed: `pnpm check:module-boundaries`. - Focused tests passed: 293 recovery/session/dispatch tests, 66 retry and response-gate tests, and 37 native-session tests. Some suites overlap. - Tests cover interrupted writes, missing sessions, concurrent retries, restart persistence, pending questions and approvals, execution gates, and historical holds. - Built the Rust test executables with `pnpm --filter @paperclipai/paperclip-runner build:rust` for native-runner verification. - Full Vitest coverage verified locally using the repository’s general and serialized shards, with focused reruns for failures and files not reached after a shard stopped. The ownership-gate regression is fixed and the complete affected server shard passes (1,390 tests). Local parallel runs also hit temporary-directory, resource, and timing failures; those suites pass with canonical temporary paths and sequential reruns. No test timeouts were increased. - Final merged-branch regression run: 577 tests pass across process recovery, retry scheduling, liveness, durable chat, wake-queue application/adapter, dispatch, continuation, native sessions, and task chat. Earlier focused verification also passed 19 native control tests. Token gates and whitespace validation pass. - Browser verification passed all three ACP Stop/continue/pause scenarios, including a rerun after merging the upstream waiting behavior: `PAPERCLIP_E2E_PORT=3397 pnpm test:e2e tests/e2e/acp-stop-continuation.spec.ts`. The interrupted-write case verifies that follow-up completes without a repeated write. - Final-head [CI run 34625037394](https://github.com/paperclipai/paperclip/actions/runs/34625037394) passed on `06ac4bd9d150f8b209a96e5fd609c696958794a0`: all 31 reported checks are green, including server/workspace suites, all browser shards, native runner verification, build, typecheck, release dry run, and aggregate gates. The two conditional Storybook checks were skipped. Greptile reviewed this exact commit at 5/5; all review threads are resolved. ## Risks - A new model turn can choose to repeat an action. Paperclip does not replay recorded tool calls and does not certify unknown action outcomes. - Conversation adapters now stop after their retry budget instead of requiring action reconciliation. Explicit Stop, pause, dependency, approval, budget, and ownership gates remain in force. - No schema migration or dependency changes. Historical holds are folded without changing task status or waking work. ## Model Used OpenAI GPT-6 through Codex, with reasoning, repository tools, code execution, and test execution. The session does not expose a more specific model build ID or context-window size. ## 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 --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
109 lines
6.3 KiB
TypeScript
109 lines
6.3 KiB
TypeScript
import fs from 'node:fs/promises';
|
|
import os from 'node:os';
|
|
import path from 'node:path';
|
|
import { fileURLToPath } from 'node:url';
|
|
import { createHash } from 'node:crypto';
|
|
import { afterEach, expect, it } from 'vitest';
|
|
import type { AdapterExecutionContext } from '../types.js';
|
|
import { createAcpxEngineExecutor } from './execute.js';
|
|
import { sessionCodec } from './session-codec.js';
|
|
const fixture = fileURLToPath(new URL('../../../../scripts/mcp-fixtures/servers/acp-stop-agent.mjs', import.meta.url));
|
|
const roots: string[] = [];
|
|
afterEach(async () => { await Promise.all(roots.splice(0).map(root => fs.rm(root, { recursive: true, force: true }))); });
|
|
async function setup(tool?: string) {
|
|
const root = await fs.mkdtemp(path.join(os.tmpdir(), 'paperclip-acp-stop-'));
|
|
roots.push(root);
|
|
const abort = new AbortController();
|
|
let ready!: () => void;
|
|
const started = new Promise<void>(resolve => { ready = resolve; });
|
|
const ctx = {
|
|
runId: 'stop-test', agent: { id: 'agent', companyId: 'company' }, runtime: {},
|
|
config: { agent: 'custom', agentCommand: `${JSON.stringify(process.execPath)} ${JSON.stringify(fixture)}`, mode: 'persistent',
|
|
stateDir: path.join(root, 'state'), cwd: root, graceSec: 5,
|
|
env: { PAPERCLIP_STOP_FIXTURE_ROOT: root, ...(tool ? { PAPERCLIP_STOP_FIXTURE_TOOL: tool } : {}) } },
|
|
context: {}, signal: abort.signal,
|
|
onLog: async (_stream: string, text: string) => { await fs.appendFile(path.join(root, 'logs'), text); if (text.includes('Waiting for Stop.')) ready(); },
|
|
} as unknown as AdapterExecutionContext;
|
|
return { root, ctx, abort, started, execute: createAcpxEngineExecutor() };
|
|
}
|
|
it('stops an actual ACP process and resumes its established session with the new request', async () => {
|
|
const { root, ctx, abort, started, execute } = await setup('read');
|
|
ctx.authToken = 'first-run-test-token';
|
|
const running = execute(ctx);
|
|
await started;
|
|
abort.abort(new Error('Operator Stop'));
|
|
const result = await running;
|
|
expect(result.resultJson?.executionCancellation).toMatchObject({ state: 'acknowledged', forced: false });
|
|
expect(result.executionRecovery).toMatchObject({ kind: 'interrupted', sessionPreserved: true });
|
|
const params = sessionCodec.serialize(result.sessionParams ?? null);
|
|
expect(params?.interruptedCheckpoint).toBe(true);
|
|
const next = await execute({ ...ctx, runId: 'follow-up', authToken: 'follow-up-test-token', signal: undefined, context: { prompt: 'List recent Drive files' },
|
|
runtime: { ...ctx.runtime, sessionParams: params } });
|
|
expect(next.exitCode, JSON.stringify(next)).toBe(0);
|
|
const prompts = (await fs.readFile(path.join(root, 'prompts'), 'utf8')).trim().split('\n').map(line => JSON.parse(line));
|
|
expect(prompts).toHaveLength(2);
|
|
expect(prompts[1].sessionId).toBe(prompts[0].sessionId);
|
|
const launches = (await fs.readFile(path.join(root, 'run-env'), 'utf8')).trim().split('\n').map(line => JSON.parse(line));
|
|
expect(launches.map(launch => launch.runId)).toEqual(['stop-test', 'follow-up']);
|
|
expect(launches[1].tokenHash).toBe(createHash('sha256').update('follow-up-test-token').digest('hex'));
|
|
});
|
|
it('continues after an interrupted write without replaying that write', async () => {
|
|
const { root, ctx, abort, started, execute } = await setup('write');
|
|
const running = execute(ctx);
|
|
await started;
|
|
await new Promise(resolve => setTimeout(resolve, 100));
|
|
abort.abort();
|
|
const result = await running;
|
|
expect(result.resultJson?.executionCancellation).toMatchObject({ state: 'acknowledged' });
|
|
expect(result.executionRecovery).toBeUndefined();
|
|
const before = await fs.readFile(path.join(root, 'writes'), 'utf8');
|
|
const next = await execute({ ...ctx, runId: 'follow-up', signal: undefined, context: { prompt: 'What happened?' },
|
|
runtime: { ...ctx.runtime, sessionParams: sessionCodec.serialize(result.sessionParams ?? null) } });
|
|
expect(next.exitCode, JSON.stringify(next)).toBe(0);
|
|
expect(await fs.readFile(path.join(root, 'writes'), 'utf8')).toBe(before);
|
|
expect((await fs.readFile(path.join(root, 'completed'), 'utf8')).trim()).toBe('follow-up');
|
|
}, 15000);
|
|
it('does not dispatch a provider when Stop precedes startup', async () => {
|
|
const { root, ctx, abort, execute } = await setup();
|
|
abort.abort(new Error('Stopped before startup'));
|
|
expect(await execute(ctx)).toMatchObject({ errorCode: 'cancelled', executionRecovery: { kind: 'bootstrap', providerWorkStarted: false } });
|
|
await expect(fs.access(path.join(root, 'prompts'))).rejects.toThrow();
|
|
});
|
|
|
|
it.each(['missing session', 'changed configuration'])('starts a new turn when the interrupted session cannot resume: %s', async (change) => {
|
|
const { root, ctx, abort, started, execute } = await setup();
|
|
const running = execute(ctx);
|
|
await started;
|
|
abort.abort();
|
|
const result = await running;
|
|
expect(result.executionRecovery?.kind).toBe('interrupted');
|
|
if (change === 'missing session') await fs.rm(path.join(root, 'session'));
|
|
let next = await execute({ ...ctx, signal: undefined,
|
|
config: change === 'changed configuration' ? { ...ctx.config, env: { ...(ctx.config.env as object), SETTING: 'changed' } } : ctx.config,
|
|
runtime: { ...ctx.runtime, sessionParams: sessionCodec.serialize(result.sessionParams ?? null) },
|
|
});
|
|
if (change === 'missing session') {
|
|
expect(next.clearSession, JSON.stringify(next)).toBe(true);
|
|
next = await execute({ ...ctx, runId: 'fresh-follow-up', signal: undefined,
|
|
runtime: { ...ctx.runtime, sessionParams: null } });
|
|
}
|
|
expect(next.exitCode, JSON.stringify(next)).toBe(0);
|
|
expect((await fs.readFile(path.join(root, 'prompts'), 'utf8')).trim().split('\n')).toHaveLength(2);
|
|
});
|
|
|
|
it('keeps the Stop deadline active after cancellation returns until provider exit', async () => {
|
|
const { ctx, abort, started, execute } = await setup();
|
|
ctx.config.graceSec = 1;
|
|
ctx.config.env = { ...(ctx.config.env as object), PAPERCLIP_STOP_FIXTURE_HANG_ON_CLOSE: '1' };
|
|
let providerPid: number | undefined;
|
|
ctx.onSpawn = async ({ pid }) => { providerPid = pid; };
|
|
const running = execute(ctx);
|
|
await started;
|
|
abort.abort();
|
|
const result = await running;
|
|
expect(result.resultJson?.executionCancellation).toMatchObject({ state: 'acknowledged', forced: true });
|
|
expect(result.executionRecovery).toBeUndefined();
|
|
expect(providerPid).toBeTypeOf('number');
|
|
expect(() => process.kill(providerPid!, 0)).toThrow(expect.objectContaining({ code: 'ESRCH' }));
|
|
}, 10000);
|