mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-04 02:32:07 +01:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Agent adapters run agent sessions through the ACPX engine > - The ACPX engine handled one run attempt as a long implicit procedure > - That shape made resource ownership, cleanup order, and failure behavior hard to verify > - This pull request gives the attempt a coordinator, a typed resource ledger, separate run sites, and explicit turn and settlement sequences > - The benefit is clear ownership, one cleanup path, safer session reuse, and testable failure behavior ## Linked Issues or Issue Description **What existing behavior does this improve?** The ACPX engine manages startup, turn execution, session reuse, and cleanup inside one large run procedure. **Current behavior** The run procedure owns several resources through implicit control flow. Cleanup and session reuse behavior depend on lane-specific branches and error paths. **Proposed behavior** The coordinator owns the run attempt. A typed ledger records six resources and their states. Host and sandbox run sites own lane-specific acquisition. Turn and settlement sequences expose typed outcomes. The engine emits allowlisted phase telemetry. **Reason and benefit** Explicit ownership makes cleanup and failure behavior easier to inspect. The fault matrix and characterization tests protect the external result while the refactor reduces hidden control flow. **Breaking changes** None to the public adapter contract. The host warm-save path now closes and relaunches the runtime because a transferred runtime could retain a run-scoped credential. A cold session-handshake failure now closes the created runtime. **Additional context** This pull request contains the ACPX engine lifecycle refactor, its tests, and the lifecycle document. ## What Changed - Add a run coordinator for startup, turn execution, settlement, and result reproduction. - Add a typed resource ledger with open, sealed, and consumed states. - Add host and sandbox run sites for lane-specific resource acquisition. - Replace separate runtime maps with a generic session reuse store. - Split session fingerprint identity from the outer session key. - Add typed turn and settlement sequences with one cleanup owner. - Add a closed allowlist for phase telemetry. - Add characterization tests and a 17-case fault matrix. - Add `doc/acp-run-lifecycle.md`. ## Verification - `npx vitest run packages/adapter-utils/src/acpx-engine/` passes 18 files and 286 tests at the submitted commit. - `pnpm --filter @paperclipai/adapter-utils typecheck` reports 0 errors at the submitted commit. - Run the full pull request checks after GitHub starts CI. - Run Greptile review after the pull request opens. ## Risks - The refactor changes internal control flow across the ACPX engine. - Host warm-save behavior now closes and relaunches the runtime. - Settlement changes the handling of a cold session-handshake failure from a leak to a close. - The characterization baselines and fault matrix reduce the risk of an external behavior change. > Paperclip is the open source app people use to manage AI agents for work > The adapter layer runs agent sessions through the ACPX engine > The engine needs explicit lifecycle ownership for reliable cleanup > This pull request adds coordinator-owned phases and a typed resource ledger > The result makes lifecycle behavior easier to test and review ## Model Used OpenAI GPT-5 Codex. Exact model ID: GPT-5. The model used tool execution, repository inspection, and code review support. The implementation author supplied the submitted code. ## 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>
185 lines
6.6 KiB
TypeScript
185 lines
6.6 KiB
TypeScript
import { describe, expect, it } from "vitest";
|
|
import { runAttempt, type RunPlan, type SettlementReason } from "./run-coordinator.js";
|
|
import { createRunResourceLedger } from "./run-resource-ledger.js";
|
|
import type {
|
|
AcquiredRunResources,
|
|
PreTurnFailedCause,
|
|
ReadyRunResources,
|
|
StartupReady,
|
|
StartupResult,
|
|
TurnCompletion,
|
|
} from "./run-contracts.js";
|
|
|
|
// The coordinator owns the run routing table. These tests drive it with fakes
|
|
// for the four steps (startup, turn, settle, reproduce) and assert the order and
|
|
// the routing, never the engine's real run state.
|
|
|
|
const readyResources = {} as ReadyRunResources;
|
|
|
|
function readyStartup(): StartupReady {
|
|
return { kind: "ready", resources: readyResources, context: {} as StartupReady["context"] };
|
|
}
|
|
|
|
function preTurnCause(): PreTurnFailedCause {
|
|
return { kind: "pre_turn_failed", phase: "handshake", error: new Error("handshake boom") };
|
|
}
|
|
|
|
function finalizedTurn(): TurnCompletion {
|
|
return { kind: "finalized" };
|
|
}
|
|
|
|
// A minimal plan whose steps push their names into `order`, so a test reads the
|
|
// exact sequence the coordinator ran.
|
|
function makePlan(
|
|
order: string[],
|
|
overrides: Partial<RunPlan<string>> & { ledger?: AcquiredRunResources } = {},
|
|
): RunPlan<string> {
|
|
const ledger = overrides.ledger ?? createRunResourceLedger();
|
|
return {
|
|
ledger,
|
|
startup: overrides.startup ?? (async (): Promise<StartupResult> => {
|
|
order.push("startup");
|
|
return readyStartup();
|
|
}),
|
|
rollbackStartup: overrides.rollbackStartup ?? ((consumed) => {
|
|
order.push(`rollback:${consumed.entries().length}`);
|
|
}),
|
|
recordDisposition: overrides.recordDisposition ?? ((report) => {
|
|
order.push(`disposition:${report.records.map((r) => `${r.id}=${r.disposition}`).join(",")}`);
|
|
}),
|
|
runTurn: overrides.runTurn ?? (async (): Promise<TurnCompletion> => {
|
|
order.push("turn");
|
|
return finalizedTurn();
|
|
}),
|
|
settle: overrides.settle ?? (async (reason: SettlementReason) => {
|
|
order.push(`settle:${reason.kind}`);
|
|
}),
|
|
reproduceResult: overrides.reproduceResult ?? (async (outcome) => {
|
|
order.push(`reproduce:${outcome.kind}`);
|
|
return "result";
|
|
}),
|
|
};
|
|
}
|
|
|
|
describe("ACPX run coordinator", () => {
|
|
it("test_startup_throw_replays_startup_rollback_entries_and_rethrows", async () => {
|
|
const order: string[] = [];
|
|
// Fill a partial ledger with two startup_rollback entries that record their
|
|
// release order when the rollback replays them.
|
|
const ledger = createRunResourceLedger();
|
|
const released: string[] = [];
|
|
ledger.register({
|
|
id: "staged_runtime",
|
|
payload: {
|
|
stagedRuntime: {} as never,
|
|
disposeStaged: async () => {
|
|
released.push("staged_runtime");
|
|
},
|
|
},
|
|
scope: "startup_rollback",
|
|
});
|
|
ledger.register({
|
|
id: "control_bridge",
|
|
payload: { stop: async () => { released.push("control_bridge"); } } as never,
|
|
scope: "startup_rollback",
|
|
});
|
|
|
|
const boom = new Error("build boom");
|
|
const plan = makePlan(order, {
|
|
ledger,
|
|
startup: async (): Promise<StartupResult> => {
|
|
order.push("startup");
|
|
throw boom;
|
|
},
|
|
// Replay every claimed entry through its own release, newest first.
|
|
rollbackStartup: async (consumed) => {
|
|
for (const entry of [...consumed.entries()].reverse()) {
|
|
if (entry.id === "staged_runtime") {
|
|
await (entry.payload as { disposeStaged: () => Promise<void> }).disposeStaged();
|
|
} else if (entry.id === "control_bridge") {
|
|
await (entry.payload as { stop: () => Promise<void> }).stop();
|
|
}
|
|
}
|
|
order.push("rollback");
|
|
},
|
|
});
|
|
|
|
// The coordinator rethrows the original startup error.
|
|
await expect(runAttempt(plan)).rejects.toBe(boom);
|
|
// It replayed the startup_rollback entries through the partial ledger...
|
|
expect(released).toEqual(["control_bridge", "staged_runtime"]);
|
|
// ...recorded a disposition report that marks each claimed entry finalized...
|
|
expect(order).toEqual([
|
|
"startup",
|
|
"rollback",
|
|
"disposition:staged_runtime=finalized,control_bridge=finalized",
|
|
]);
|
|
// ...and never ran the turn or the settlement.
|
|
expect(order).not.toContain("turn");
|
|
expect(order.some((step) => step.startsWith("settle"))).toBe(false);
|
|
});
|
|
|
|
it("test_settle_path_returns_error_result_after_settlement", async () => {
|
|
const order: string[] = [];
|
|
const cause = preTurnCause();
|
|
const plan = makePlan(order, {
|
|
startup: async (): Promise<StartupResult> => {
|
|
order.push("startup");
|
|
return { kind: "settle", cause, resources: createRunResourceLedger().takeForSettlement() };
|
|
},
|
|
settle: async (reason) => {
|
|
expect(reason.kind).toBe("pre_turn");
|
|
order.push("settle");
|
|
},
|
|
reproduceResult: async (outcome) => {
|
|
expect(outcome.kind).toBe("pre_turn");
|
|
order.push("reproduce");
|
|
return "error-result";
|
|
},
|
|
});
|
|
|
|
const result = await runAttempt(plan);
|
|
expect(result).toBe("error-result");
|
|
// The settle path never runs the turn; it settles first, then reproduces.
|
|
expect(order).toEqual(["startup", "settle", "reproduce"]);
|
|
});
|
|
|
|
it("test_ready_path_runs_turn_then_settles_then_reproduces_result", async () => {
|
|
const order: string[] = [];
|
|
const plan = makePlan(order);
|
|
|
|
const result = await runAttempt(plan);
|
|
expect(result).toBe("result");
|
|
// The ready path runs the turn, settles, then reproduces — in that order.
|
|
expect(order).toEqual(["startup", "turn", "settle:turn", "reproduce:turn"]);
|
|
});
|
|
|
|
it("test_coordinator_has_no_catch_around_the_turn", async () => {
|
|
// `runTurn` is contracted never to reject. This test proves the coordinator
|
|
// holds no catch around the turn: an (illegal) turn rejection propagates out
|
|
// of `runAttempt` unchanged, and neither settlement nor result reproduction
|
|
// runs. A catch around the turn would swallow the rejection and route it into
|
|
// settlement, so this rejection-propagation is the structural proof.
|
|
const order: string[] = [];
|
|
const boom = new Error("turn must never reject");
|
|
const settleCalls: number[] = [];
|
|
const reproduceCalls: number[] = [];
|
|
const plan = makePlan(order, {
|
|
runTurn: async (): Promise<TurnCompletion> => {
|
|
throw boom;
|
|
},
|
|
settle: async () => {
|
|
settleCalls.push(1);
|
|
},
|
|
reproduceResult: async () => {
|
|
reproduceCalls.push(1);
|
|
return "unreachable";
|
|
},
|
|
});
|
|
|
|
await expect(runAttempt(plan)).rejects.toBe(boom);
|
|
expect(settleCalls).toHaveLength(0);
|
|
expect(reproduceCalls).toHaveLength(0);
|
|
});
|
|
});
|