Commit Graph
3 Commits
Author SHA1 Message Date
Nicky LeachandPaperclip 6d72243d8a fix(runtime): recover task creation after a lost acquire reply (#18692)
## Thinking Path

> - Paperclip manages agent execution and the resources that execution
owns.
> - A retained environment task can need a replacement before the next
turn.
> - Core records the creation attempt before it asks the provider to
acquire it.
> - A lost reply can leave that journal without the resource ID needed
for cleanup.
> - This change lets an opted-in provider recover the identity from that
recorded attempt.
> - Core still requires the existing exact-task termination proof before
it releases ownership.

## Linked Issues or Issue Description

Refs #18405 and #18658. This is the separate lost-reply follow-up from
their recovery audit.

**Draft status:** Both prior review findings are fixed in
`a66520c8d4e06e0eec6ccb5446190ace0ca3003f`. Fresh standard Greptile is
5/5 with no unresolved threads. Current-head CI is green (53 successful
checks/statuses including Snyk, three expected skips). Apex completed
5/5 on this exact head at 2026-11-03T00:54:59Z (request 6592407130),
with no unresolved threads. The review cycle is complete; the PR remains
draft for human merge review.

**What happened?**

If a replacement task's acquire reply is never received, the creation
journal can reach manual reconciliation without a resource ID. Startup
cleanup cannot complete, and ordinary Stop cannot release the run owner.

**Expected behavior**

When the selected provider supports creation inspection, Core should
retain enough binding information to inspect the recorded attempt and
continue verified cleanup. An inspected identity must not count as
termination proof.

**Steps to reproduce**

1. Use a retained task whose provider supports creation inspection.
2. Lose the replacement acquire reply before Core records its resource
ID.
3. Restart cleanup and inspect the existing creation journal.
4. Confirm that the exact task is stopped and its termination receipt is
saved before cancellation releases the owner.

**Paperclip version or commit**

Current head `a66520c8d4e06e0eec6ccb5446190ace0ca3003f`, based on master
`3a7bb4dac62dbd7a34a55deb665c55df310bdf64`. Tested with isolated
PostgreSQL fixtures.

**Deployment mode**

Local source tests with controlled provider responses. No deployment or
live model request.

## What Changed

- Save the selected inspection capability and prior opaque binding in
the existing creation journal. Acquire through that same worker so a
worker replacement cannot change the recorded contract.
- Make failed opted-in creations due through the existing cleanup
notification and saved retry deadline.
- Add optional SDK observation field `creationLease`. Use it only to
recover the requested creation's identity and binding, then follow the
existing Stop, status, completion and receipt checks.
- Keep old entries and providers without the recorded opt-in under their
existing reconciliation policy.
- Add actual PostgreSQL recovery, cancellation and worker-replacement
regressions. Document the provider contract.

## Verification

- Current correction: 53 distinct targeted cases pass (20 real-worker
IPC, 12 task-recovery PostgreSQL and 21 generic-sandbox cases), with
server typechecking. The IPC selection was rerun after the final
late-observation ordering refinement; the other tested sources were
unchanged.
- An assembled live-child IPC and PostgreSQL case proves cleanup and
cancellation release the owner despite the original worker never
replying and remaining alive. Accepted host work, unrelated acquisitions
and late observations remain accounted for. Removing the recovery
handoff and restoring the old drain snapshot produce distinct assertion
failures.
- The unsupported-host control skips all 12 PostgreSQL cases without
setup; removing the guard fails the controlled setup. These tests do not
constitute live VM/provider qualification.
- Initial head `5db6882` passed an exact reference-tree comparison
against master plus the reviewed follow-up patch, 97 targeted cases and
server typechecking. Those checks predate the final inspection
correction.
- Previous head `59f87fa` passed 32 targeted cases (11 task-recovery and
21 generic-sandbox cases) and server typechecking. Both `pending` and
`needs_reconciliation` observations without `creationLease` preserve
ownership and get a future retry deadline. Restoring the previous guard
makes both new cases fail.
- The reproduced failure was a wrong generic cleanup shape that could
resolve its journal while leaving the owner held—not a demonstrated
false ownership release. The correction reuses the existing
missing-identity guard before the generic decoder and leaves generic
sandbox behavior unchanged.
- Earlier affected selection: 72 cases passed on the original follow-up
base. This overlaps the final selection and is not an additional count
of unique final-head tests.
- Two isolated negative controls restore the missing task inspection and
redirected-worker behaviors. Both fail at the expected recovery and
dispatch assertions without changing production source files.
- The PostgreSQL cases verify one cleanup lease, both late-result
orders, recovery after the old journal deadline, ordinary legacy
behavior, retained ownership on missing or mismatched identity, and
cancellation acknowledgement after exact termination proof.
- The tests use actual database and run-control services with controlled
worker callbacks. They do not prove live IPC or production recovery
timing.
- A compatible provider is needed for the new recovery path. No normal
successful-startup database or network call is added. No ordinary TTFT
improvement is claimed; the repair removes an indefinite ownership hold
for supported lost replies. No full local CI suite was run; GitHub runs
the full regression gates.

## Risks

The provider must recover the identity for the recorded creation
operation and preserve its selected binding. `creationLease` does not
authorize a new turn or prove that a process stopped. Older journal
entries do not infer a new identity after an upgrade. Unavailable
original machine bindings or credentials retain the existing uncertainty
behavior; this change does not repair machine or credential loss. No
schema migration, deployment or ROM qualification is included.

## Model Used

OpenAI Codex, GPT-6 family. Exact model ID and context window are not
exposed in this session. Tool-assisted implementation, targeted tests
and independent subagent review.

## 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>
2026-11-02 17:45:07 -08:00
Nicky LeachandPaperclip a9b18e1cbe fix(runner): preserve cleanup ownership across startup failures (#18658)
## Thinking Path

> - Paperclip controls which run owns an agent task.
> - A plugin can continue accepted work after Core stops waiting.
> - A late task-creation reply can therefore arrive after startup
cancellation.
> - Core must keep that work owned until its resource identity and
cleanup outcome are known.
> - This PR supplies the cleanup and recovery support before shared
startup is enabled.
> - The dependent activation PR can then reuse transport data within a
bounded startup context.

## Linked Issues or Issue Description

Refs #18405. This is the independently mergeable prerequisite for that
stacked activation PR.

**What existing behavior does this improve?**

Cleanup of explicitly recorded startup work that outlives its caller.

**Current behavior**

Caller timeout, worker exit, or failed cleanup-intent persistence can
leave work without a complete recovery path.

**Proposed behavior**

Retain the cleanup obligation, use existing Stop and recovery queues,
and release ownership only after the exact task and accepted work have
stopped. Ordinary calls keep their existing startup payloads and
budgets. This PR does not create startup contexts or advertise transport
reuse.

**Reason and benefit**

Separate the failure-handling contract from activation so each can be
reviewed on its own. This support PR has **no standalone TTFT speedup**.

## What Changed

- Add the startup cleanup marker, exact ownership checks, verified
completion, and acknowledgement helpers.
- Connect recorded cleanup obligations to existing Stop, deadline
reconciliation, restart recovery, and retry scheduling.
- Retain typed cleanup responsibility when a database write fails. Keep
the nonterminal run from being treated as ordinary completed cleanup.
- Observe late acquisition results and save resource identities before
retiring them. Retain real host writes and received results across
worker exit or persistence failure.
- Queue abandoned replacement tasks through the existing creation
journal and cleanup machinery. Require the replacement's own Stop proof.
- Accept an explicit startup context in the internal bridge for lifetime
tracking. Automatic context creation, marker production, capability
advertisement, transport capture, and executor activation belong to
#18405.
- Add PostgreSQL and worker IPC regressions, including legacy payload
assertions.

## Verification

Initial split source `8fb482d8be9e70101b0cfdd0830f41cf5b8e3cf4` had tree
`95508bd1859e991670cad8afd4de8d2167009ac5`. **190 distinct targeted
tests pass**, along with server typechecking. The selected checks cover
real PostgreSQL cleanup/ownership, worker IPC, the bridge and creation
journal, ordinary/distributed Stop, restart and deadline recovery,
idle-sleep behavior, and the unchanged legacy warm-task suite.
Independent recovery and worker reviews found no blocking issue.

The first correction, `b6342d42042601b376da38dc8e1dba9ca80e9539`, fixes
the replacement cleanup finding with the existing startup-cleanup
marker, scoped to contextual abandoned task creations. The replacement
was never prepared, so preparation-stop proof is not required. No
dispatch is fabricated and no normal-startup query is added. The actual
queue/sweep/driver/receipt/cancellation regression fails before the fix
and passes after it: void, running or foreign-task status retains
ownership; exact-task stopped status permits completion even when
Release returns void. **64 affected targeted cases and server
typechecking pass**. Independent review found no blocker.

Current source is `f53af72af76f263096c3c2e788506e6faef9474f`. It fixes
both recovery findings from the prior head's Apex 3/5: orphan recovery
now preserves pending or completed-but-not-cancelled cleanup through its
existing final-write fence, and exact late identities can reach verified
cleanup after reconciliation or restart. The regression selection went
from four failing assertions to six passing cases. Affected selections
cover 217 distinct cases; the exact final six-case regression and server
typecheck pass. Independent reviewers found no blocker in either fix.
Current-head standard review is 5/5 and all CI checks pass (54
successes, three expected skips, plus Snyk). Apex also completed at
**5/5** on this exact head, with zero unresolved threads. The requested
independent availability audit is complete on this head. It found an
activation-dependent operability gap: an acquisition response that is
never received can leave an unknown creation identity with no supported
task inspection/reconciliation path, while cleanup keeps the run owner
reserved. This does not occur in ordinary lower-only calls. The contract
and recovery policy need follow-up before activation; the external
review scores do not resolve that availability tradeoff.

The split preserves the accepted implementation at `21db7b3851` after
its clean merge with frozen master
`946735d03a55c1fb4cc5434d6bf1d87742daa9df`. The initial dependent
activation PR restored exact aggregate tree
`f68c6895f87c7b93ae03e6324d955d2659a99dce`. Both subsequent cleanup
corrections are integrated into upper head
`27978f294a02da3992585c7f4daa73cd8fcf99f8`; its current aggregate tree
is `a0b66c33f5e22745f034f45d24d213f5c210cf16`. Lower-only payload tests
cover the absence of capability advertisement and transport projection.

Only targeted local tests and typechecking are used. Full regression
coverage runs in GitHub CI. No deployment or ROM qualification is part
of this change.

The lower audit covered connected worker, creation-journal, Stop,
restart, deadline and orphan-recovery paths. A new isolated
real-PostgreSQL characterization reproduced the unknown-ID hold through
actual recovery, cleanup verification and run-control cancellation. It
models a lost acquisition response; it is not a live provider or
complete IPC test. No source edits were made by the audit. The
upper/composition audit remains required after its review cycle.

## Risks

- Recording a replacement result now commits with the existing cleanup
notification. This conditional replacement path adds
transaction/notification responses; common retained startup is
unchanged.
- This adds cleanup and recovery state handling. A missing receipt or
permanently inaccessible old backend can keep ownership held
indefinitely. Temporary outages retry; an ordinary retry is not evidence
that a deleted or replaced backend stopped.
- A caller timeout does not undo work already accepted by a plugin.
Unknown resource creation remains unresolved until its identity/outcome
is established. Current task creation has no supported
inspect-by-operation or operator reconciliation API for a permanently
lost identity; repeated Stop alone cannot resolve it.
- The support code understands records produced by the dependent
activation PR. It does not enable the shared startup policy by itself.
- Existing legacy callers must retain their behavior; context-only
automatic replacement cleanup remains explicitly scoped.

## Model Used

OpenAI Codex, GPT-6 family. The exact model ID and context window are
not exposed in this session. Tool-assisted implementation, targeted
testing, and independent subagent review.

## Checklist

- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used, with available version and
capability details
- [x] I have checked ROADMAP.md and confirmed this is the requested
split of existing work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have described the issue with the required fields
- [x] I have not referenced internal issues or private links
- [x] My branch name describes the change and contains no internal
ticket ID
- [x] Targeted local tests pass on this source
- [x] I have added or updated relevant tests
- [x] I have updated relevant documentation
- [x] I have documented the risks
- [x] All current-head GitHub CI gates are green
- [x] Current-head standard Greptile and Apex are 5/5 with no unresolved
findings
- [x] I will address all review comments before requesting merge

---------

Co-authored-by: Paperclip <noreply@paperclip.ing>
2026-11-02 14:13:00 -08:00
Nicky LeachandPaperclip f8a888a072 feat(runner): define bounded startup lifetime contracts (#18490)
## Thinking Path

> - Paperclip manages AI agents and their work.
> - Native sessions and plugin workers have separate startup lifetimes.
> - A caller timeout does not prove that a worker or an accepted host
write has stopped.
> - Shared startup code needs an exact deadline and an actual settlement
contract.
> - This PR adds those contracts and tests them before any production
caller uses them.
> - The dependent integration in #18405 will use the contracts with
verified cleanup.
> - This split lets reviewers assess the lifetime rules separately from
runtime activation.

## Linked Issues or Issue Description

Refs #18405. This PR extracts the foundation from that change. The
integration will depend on this PR.

**Problem or motivation**

A bounded startup must distinguish the caller's wait from actual worker
completion. A late worker reply or an accepted host request can outlive
the original RPC promise. A shorter timeout must not discard that
responsibility or authorize a replacement run.

**Proposed solution**

Add one Core execution context, optional Runner startup deadlines, and
worker settlement and retirement APIs. Test the contracts with real
child-process IPC. Keep production context creation, transport
projection, capability negotiation, durable cleanup, and Stop/recovery
integration in #18405.

**Alternatives considered**

One combined PR mixed the lifetime contract with its activation and
cleanup policy. Activating a shorter deadline before the complete
transport and cleanup path is unsafe. Splitting only the SDK
declarations would not provide a useful behavioral review boundary.

**Roadmap alignment**

This supports the existing plugin, sandbox execution, and bounded
recovery capabilities. It adds no new product workflow, endpoint,
migration, provider or runtime dependency.

## What Changed

- Add `NativeExecutionContext`. It retains the selected input, binds one
transport target, rejects work after cancellation or expiry, and tracks
actual worker settlement. Expiry alone does not complete pending work.
- Add worker startup groups. A caller timeout retains the group's
responsibility. Retirement revokes obsolete invocation scopes, waits for
accepted host handlers, and retires only the verified group's
bookkeeping. Each cleanup wait is bounded; an unfinished handler remains
pending for a later retry.
- Add an optional Runner startup signal and absolute deadline for
session open and retained attachment. Clean up abort listeners after
synchronous failures. Existing callers keep their default timeout path.
- Expose the remaining confirmed controller-lease budget without a new
query or renewal timer.
- Add optional SDK context and preparation-retirement fields. No
production bridge sends or advertises them in this slice.
- Keep the required lease mock update with the new `remainingMs()`
contract so this foundation typechecks independently.

Production activation files, including the native executor and
environment task bridge, match the current master base. No production
caller constructs `NativeExecutionContext` or passes the new Runner
startup option. This PR adds no durable cleanup producer and claims **no
standalone TTFT improvement**. The dependent integration must enable the
context, transport and verified cleanup together.

## Verification

Initial split verification on
`8ebc8a2f9c6c02e90f920356138b3742a4067b28`, based on
`9a3f395737e7a92faf99971459979061d423531a`:

- From `server`: `../node_modules/.bin/vitest run
src/services/native-runtime/native-execution-context.test.ts
src/services/native-runtime/native-execution-context-worker.test.ts
src/services/native-runtime/native-execution-lease.test.ts` — **17
passed**. Includes real child IPC, caller timeout with late reply,
worker death, accepted host-handler draining, obsolete cleanup
invocation rejection, unrelated work preservation, context scope,
expiry, and confirmed lease budget.
- From `packages/paperclip-runner`: `../../node_modules/.bin/vitest run
src/native-session-runtime.test.ts` — **188 passed**. Includes existing
callers without the new option and new
synchronous-failure/expired-deadline cases for fresh and retained
sessions.
- Direct TypeScript checks for `packages/plugins/sdk/tsconfig.json` and
`packages/paperclip-runner/tsconfig.json` — passed. Built only their
shared/eval TypeScript prerequisites.
- From `server`: `../node_modules/.bin/tsc --noEmit` — passed.
- Checked the partition against the reviewed combined candidate. The
twelve selected files match exactly. The thirteenth file carries only
the required test-mock update. The production executor, bridge and
target wiring remain equal to the base.
- `git diff --check`, added-line secret/private-data scan, and
added-code-comment scan — passed. No generated files or run artifacts
are committed.
- The first old-base split passed its focused tests but failed server
typechecking because the unchanged executor test mock lacked
`remainingMs()`. The original failure is retained. Moving that one-line
mock update into this foundation resolved the split dependency; the
final current-master checks above pass.
- Full local CI suites, Rust builds, deployments and provider calls were
not run, as requested. GitHub CI will provide full regression coverage.
Current-head GitHub CI, standard review and Apex are complete; exact
acceptance is recorded below.

Review follow-up `d7635f4d280dab757a3666703737bcda955c9361` fixes three
reproduced lifetime edge cases. A retiring startup group rejects new
calls and shares one retirement promise; failed bounded retirement
remains retryable. Transport identity uses a fixed field tuple rather
than object-property order. Finishing a context rejects pending lookup
waits without aborting an already-started backend or pretending the
worker completed. All three new regressions failed before the fix; the
updated foundation selection passes **20 tests**, including actual IPC
and failed-retirement retry. Server typechecking and added-line checks
pass. These replace the earlier 17-case selection, rather than adding 20
unique cases to it. The original failures and passing reruns are
retained.

Apex follow-up `cb8860bab5ab9d6357a352e11f276be0d0cdb26d` keeps removed
worker handles reachable while they own startup calls, accepted host
handlers or retirement work. Drain/retirement includes active and
retained handles; completion callbacks remove settled handles. Stop and
replacement remove only the exact handle they observed. This adds no
database/network operation, polling loop or generic shutdown wait. The
SDK, Runner, context and transport bridge are unchanged.

Seven additional IPC regressions failed on the prior manager and pass
after the fix, covering removal/replacement, Stop-all, host-handler
settlement, bounded retirement failure/retry, pruning, and replacement
ownership. The updated foundation selection passes **27 tests**, with
server typechecking passing; these supersede the earlier 20-case
selection. A separate real-PostgreSQL/child-IPC characterization uses
the integration's actual SDK host, storage registry and run-control path
with the exact corrected manager bytes. Before the fix, cleanup released
run ownership while an accepted host write was blocked, then the late
write restored obsolete metadata. After the fix, ownership and cleanup
remain pending until the write completes; cleanup then removes the stale
metadata before releasing ownership. The original failure, corrected
result and source snapshots are preserved. Final head
`cb8860bab5ab9d6357a352e11f276be0d0cdb26d` passes GitHub CI (53
successful checks, 2 expected skips, Snyk success), standard Greptile
**5/5** and fresh Apex **5/5**. All review threads are resolved and the
PR is conflict-free.

## Risks

The shared Runner helper now observes synchronous operation failures
through its asynchronous cleanup path. The affected runtime file's 188
tests pass, but full regression coverage still depends on CI.

The new lifetime APIs require honest settlement signals. A future caller
must not treat a deadline, worker crash or Stop intent as verified
retirement. Production activation remains dependent on the complete
integration and its cleanup tests in #18405. This foundation introduces
no shorter production startup budget or new cleanup policy by itself.

The context's transport projection is process-local. It must not be
persisted or used as a general authorization cache. No production caller
supplies it in this PR. There is no database migration, new dependency,
or deployment requirement for merging this foundation.

## Model Used

OpenAI Codex, based on GPT-6, assisted with repository inspection, the
split, tests and command execution. The exact serving model ID and
context-window size were not exposed in this session.

## 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 and contains no internal issue
identifier
- [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 — this
description documents the inactive contracts; operational activation
documentation remains with #18405
- [x] I have considered and documented any risks above
- [x] All Paperclip CI gates are green on `cb8860bab5`
- [x] Current-head standard Greptile and Apex are 5/5 with no unresolved
findings
- [x] I will address all Greptile and reviewer comments before
requesting merge

---------

Co-authored-by: Paperclip <noreply@paperclip.ing>
2026-11-02 08:22:42 -08:00