mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-07 06:16:06 -10:00
docs(rules): make upgrade safety part of run and recovery design (#19827)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Native runs save session, checkpoint, lease, and launch state, and recovery resumes that state later > - Deploys and restarts often land between the save and the resume, so one release resumes state that another release saved > - On 2026-11-06/07, staging deployed four times. Most of 182 failed runs were saved by one release and resumed by another (#19462, #19659, #19755) > - These refusals showed as the generic retryable `native_session_interrupted`. Retries repeated them, and recovery replaced a thread (now prevented by #19733). Remote-only code also shipped without a test that applied Daytona confinement (#19743) > - This pull request makes upgrade safety a required part of how we design, fix, and review run and recovery code > - The benefit is that authors, agents, and Greptile check the release cutover before a change ships, not after a deploy breaks saved runs ## Linked Issues or Issue Description Refs #19462, Refs #19516, Refs #19622, Refs #19659, Refs #19733, Refs #19743, Refs #19755 **Issue type** Missing contributor guidance for run and recovery code. **Where is the issue?** `.agents/skills/fix-failed-paperclip-run/`, `rules/`, `AGENTS.md`, and `doc/execution-semantics.md`. **What's wrong?** No rule or skill step asks how state saved by the previous release is handled. Fixes added one error code at a time. Saved-identity refusals used a generic retryable code. **Suggested fix** Add a scoped repository rule and additive skill and contract text that require a previous-release story, permanent named refusal codes, and cutover tests. ## What Changed - Add `rules/upgrade-safe-run-state.mdc`. It covers native runtime, recovery, session preparation, runner, ACPX engine, and sandbox image paths, and the `packages/db` schema for native run, checkpoint, session, and run identity state. It requires authors to state how previous-release state is handled. It requires a permanent, named code for unknown or changed saved identity, a class-level fix, an old-saved/new-resumed regression, a deploy-lands-mid-run case where relevant, and a remote-confinement test for sandbox paths. - Register the rule in `.greptile/files.json` with the same scope as its `globs`. - Link the rule from the "Scoped repository rules" section of `AGENTS.md`. - `fix-failed-paperclip-run` skill: step 1 asks if a deploy or restart separated save from resume, by comparing served revisions. Step 2 asks this early. Step 4 adds a "Persisted run identity across releases" safety row and an upgrade-safety paragraph. - `references/failure-patterns.md`: add "Saved run state meets a different release". - `doc/execution-semantics.md`: add §9.4 "Release compatibility of saved run state". ## Verification - Docs, skills, and rules only. No product code changes. - Confirm that each rule glob matches real paths, for example `server/src/services/native-runtime/`, `server/src/services/recovery/`, `server/src/services/heartbeat/session-preparation.ts`, and `packages/adapter-utils/src/acpx-engine/`. - Confirm that `.greptile/files.json` parses and that its new `scope` array equals the rule's `globs`. - `rules/README.md` names no lint check for rules. I did not edit maps, so `node scripts/check-maps.mjs` does not apply. ## Risks - Low risk. The change adds guidance only. - The rule scope includes all of `packages/paperclip-runner/**`. Greptile can apply the rule to runner docs or UI files. The rule's exception section says that changes with no persisted shape or identity need no cutover test. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, with tool use and code execution in Claude 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 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <[email protected]>
This commit is contained in:
1 parent
f2d8a51027
commit
a4050187ec
6 files changed
+134
No files matched your search
@@ -29,6 +29,10 @@ available tools and records before asking the user for missing information:
|
||||
architecture, adapter, model/effort, execution workspace, trust mode, and credential
|
||||
source. Trace effective values back through task overrides, inherited settings,
|
||||
defaults, and saved session state. Redact credential values.
|
||||
- Whether a deploy or restart separated save from resume. Compare the served
|
||||
revision and runner/provider pack versions that saved the session, checkpoint,
|
||||
lease, or launch input with those that resumed it. Failures clustered after
|
||||
a deploy usually mean the new release refuses state the old one saved.
|
||||
- Which endpoint is authorized: verified fix and green PR, or also rollout and
|
||||
recovery. Preserve the requested work and success criteria throughout.
|
||||
|
||||
@@ -67,6 +71,10 @@ Separate these questions:
|
||||
3. Why did recovery or continuation fail to complete the already-authorized work?
|
||||
4. Which later errors are consequences, misleading summaries, or independent bugs?
|
||||
|
||||
Answer early whether the failing step resumed state saved by a different release.
|
||||
If it did, compare what the old release persisted with what the new one requires
|
||||
before pursuing provider, model, or capacity explanations.
|
||||
|
||||
Trace the causal value or transition to its producer and shared contract. Verify
|
||||
the hypothesis against a counterexample or a reproduction. Check logs before and
|
||||
after the apparent error: a recovered authorization warning may not be fatal, and
|
||||
@@ -137,12 +145,25 @@ assumptions into observable regression cases, not just assurances in the PR:
|
||||
| Waiting or task disposition | Real approval/budget/credential gates still stop work; actionable work has a durable wake; recovery is bounded; no recursive recovery tree. |
|
||||
| Input, tool, or credential plumbing | Company and trust boundaries hold; intentional configuration reaches execution without ambient secret forwarding; distinct tools still dispatch correctly; sensitive logging remains redacted. |
|
||||
| Packaging, compatibility, or session identity | Relevant local/sandbox, platform, fresh/resumed, and old/new artifact combinations work; obsolete saved values are handled by a contract rather than a list of incident exceptions. |
|
||||
| Persisted run identity across releases | State saved by the previous release resumes on the new one, or stops with a permanent, named code that recovery does not retry. A deploy landing mid-run follows the same contract. |
|
||||
| Performance or lifecycle | Warm reuse remains warm where intended; queueing and finalization do not regress; configured deadlines retain their meaning. |
|
||||
|
||||
Select cases that exercise the actual blast radius. Do not create an unrelated
|
||||
platform matrix. Test the sibling success path and the negative case that the old
|
||||
guard protected. Keep reproduction and safety tests through later revisions.
|
||||
|
||||
Upgrade safety is part of every repair that touches persisted run identity:
|
||||
launch arguments, provider/runner pack digests, leases, listeners, session or
|
||||
checkpoint schema, and workspace authority. Each must either carry forward across
|
||||
releases or fail with a permanent, named code, never a generic retryable one such
|
||||
as `native_session_interrupted`. A retry cannot change saved identity; it repeats
|
||||
the refusal or replaces saved state. Add an old-saved/new-resumed regression using
|
||||
state shaped by the previous release, and a deploy-lands-mid-run case when the run
|
||||
can be in flight at the cutover. Prefer one class-level rule at the shared
|
||||
admission or recovery boundary over another per-error-code exception. For remote
|
||||
sandbox paths, test with the provider's confinement rules applied; a local test
|
||||
without them does not prove the remote path.
|
||||
|
||||
With the red established and the safety assumptions identified, make the repair
|
||||
and run the same regression green. Run the relevant safety cases and existing
|
||||
tests. Refactor only while they stay green. Investigate unexpected failures
|
||||
|
||||
@@ -22,6 +22,23 @@ Repeat an interruption to prove the recovery budget and genuine stops still work
|
||||
strings does not define compatibility. Disabling warm reuse may hide the failure
|
||||
while imposing a new startup cost on every turn.
|
||||
|
||||
## Saved run state meets a different release
|
||||
|
||||
**Mechanism:** One release saves a session, checkpoint, lease, or launch input;
|
||||
a deploy or restart lands; another release resumes it. Launch arguments, a pack
|
||||
digest, a listener, or workspace authority changed in between, so the new code
|
||||
refuses the saved state. The refusal surfaces as a generic retryable interruption,
|
||||
and recovery retries or replaces saved state over a refusal that cannot change.
|
||||
|
||||
**Prove:** Save state with the previous release's shape, then resume with the
|
||||
current code. Also land the restart while the run is in flight. Assert the state
|
||||
carries forward, or recovery stops once with a permanent, named code and keeps the
|
||||
saved state. For remote sandboxes, apply the provider's workspace confinement.
|
||||
|
||||
**Trap:** Adding one more code to a non-retryable list fixes one identity, not the
|
||||
class. A fresh local session and a test run outside the sandbox's confinement both
|
||||
skip the cutover the incident depended on.
|
||||
|
||||
## A successful result gets stranded during finalization
|
||||
|
||||
**Mechanism:** The provider completed useful work, but result persistence, artifact
|
||||
|
||||
@@ -102,6 +102,24 @@
|
||||
"ui/src/**/*.tsx",
|
||||
"ui/src/**/use*.ts"
|
||||
]
|
||||
},
|
||||
{
|
||||
"path": "rules/upgrade-safe-run-state.mdc",
|
||||
"description": "Upgrade safety for persisted run, session, launch, and pack state: state saved by the previous release resumes or fails with a permanent, named code, with old-saved/new-resumed and mid-run deploy tests.",
|
||||
"scope": [
|
||||
"server/src/services/native-runtime/**",
|
||||
"server/src/services/recovery/**",
|
||||
"server/src/services/heartbeat/session-preparation.ts",
|
||||
"server/src/services/heartbeat/run-state.ts",
|
||||
"server/src/services/runner-revision-mismatch.ts",
|
||||
"server/src/services/sandbox-provider-runtime.ts",
|
||||
"packages/paperclip-runner/**",
|
||||
"packages/adapter-utils/src/acpx-engine/**",
|
||||
"docker/daytona-runner/**",
|
||||
"packages/db/src/schema/native_*.ts",
|
||||
"packages/db/src/schema/agent_task_sessions.ts",
|
||||
"packages/db/src/schema/run_identity_contexts.ts"
|
||||
]
|
||||
}
|
||||
]
|
||||
}
|
||||
@@ -45,6 +45,10 @@ For ACPX provider profiles, their release attestations and pins, the provider-pa
|
||||
contract registry, and ACPX driver and provider code under
|
||||
`packages/paperclip-runner/`, read
|
||||
[`rules/acpx-profile-identity.mdc`](rules/acpx-profile-identity.mdc).
|
||||
For native runtime, recovery, session preparation, runner, ACPX engine, and
|
||||
provider-pack or sandbox image code that persists or resumes run state, and for
|
||||
the native run, checkpoint, and session schema in `packages/db`, read
|
||||
[`rules/upgrade-safe-run-state.mdc`](rules/upgrade-safe-run-state.mdc).
|
||||
See [`rules/README.md`](rules/README.md) for rule authoring and Greptile setup.
|
||||
|
||||
## 3. Repo Map
|
||||
|
||||
@@ -1744,6 +1744,10 @@ Status-only operational recovery can update task liveness, clear bad status, rec
|
||||
|
||||
Automatic retries that can continue source work use the agent's configured model. This includes failed source-work retries, process-loss retries, transient or scheduled retries, max-turn continuations, source-assignee continuations, assigned-todo dispatch recovery, and any run that can update repo files, task documents, plans, work products, or attachments. When status-only recovery determines that actual work remains, it must hand back to a worker run before source work or persistent deliverable updates resume.
|
||||
|
||||
### 9.4 Release compatibility of saved run state
|
||||
|
||||
Deploys and restarts routinely separate saving run state from resuming it. Saved launch configuration, provider and runner pack identity, leases and listeners, session or checkpoint schema, and workspace authority must either carry forward to the next release or be refused with a permanent, named error code. Recovery does not retry that refusal or replace the saved state over it, and it is never reported as a generic retryable interruption such as `native_session_interrupted`. See [`rules/upgrade-safe-run-state.mdc`](../rules/upgrade-safe-run-state.mdc).
|
||||
|
||||
## 10. Startup and Periodic Reconciliation
|
||||
|
||||
Startup recovery and periodic recovery are different from normal wakeup delivery.
|
||||
|
||||
@@ -0,0 +1,70 @@
|
||||
---
|
||||
description: Run, session, launch, and pack state saved by one release must resume on the next or fail with a permanent, named code - never a generic retryable one.
|
||||
globs: "server/src/services/native-runtime/**,server/src/services/recovery/**,server/src/services/heartbeat/session-preparation.ts,server/src/services/heartbeat/run-state.ts,server/src/services/runner-revision-mismatch.ts,server/src/services/sandbox-provider-runtime.ts,packages/paperclip-runner/**,packages/adapter-utils/src/acpx-engine/**,docker/daytona-runner/**,packages/db/src/schema/native_*.ts,packages/db/src/schema/agent_task_sessions.ts,packages/db/src/schema/run_identity_contexts.ts"
|
||||
alwaysApply: false
|
||||
---
|
||||
|
||||
# Upgrade-safe run state
|
||||
|
||||
## Scope and purpose
|
||||
|
||||
This rule covers native runtime, recovery, session preparation, runner, ACPX
|
||||
engine, provider-pack, and sandbox image code, and the database schema for
|
||||
native run, checkpoint, session, and run identity state. Deploys and restarts happen while
|
||||
runs are saved or in flight, so state written by one release is routinely
|
||||
resumed by the next. Code here must treat that cutover as a normal path, not an
|
||||
edge case.
|
||||
|
||||
Persisted run identity includes:
|
||||
|
||||
- launch configuration and protected launch arguments;
|
||||
- provider and runner pack identity, such as profile versions, command digests,
|
||||
and pack contract digests;
|
||||
- execution leases, owners, and runtime API listeners;
|
||||
- saved session, thread, and checkpoint schema;
|
||||
- workspace authority, paths, and sandbox confinement.
|
||||
|
||||
## Required behavior
|
||||
|
||||
- **State the previous-release story.** A change that adds, removes, or changes
|
||||
persisted run or session state, launch configuration, or pack identity must
|
||||
say in the PR how state saved by the previous release is handled: it carries
|
||||
forward, it is migrated, or it is refused.
|
||||
- **Refuse permanently and by name.** When saved identity is unknown or no longer
|
||||
accepted, map it to a permanent, named error code that recovery does not
|
||||
retry. Never report it as a generic retryable code such as
|
||||
`native_session_interrupted`. A retry cannot change saved identity; it repeats
|
||||
the refusal, and a replacement can destroy saved state.
|
||||
- **Keep saved state on refusal.** A permanent refusal leaves the saved session,
|
||||
checkpoint, and workspace intact and hands the run on with that named reason.
|
||||
- **Fix the class.** Prefer one admission or recovery rule that classifies
|
||||
release-incompatible state over adding single error codes to a list.
|
||||
- **Test the cutover.** Add an old-saved/new-resumed regression that saves state
|
||||
in the previous release's shape and resumes it with the new code. When a run
|
||||
can be in flight at the cutover (preparing, attaching, holding a lease or
|
||||
listener), add a case where the deploy or restart lands mid-run.
|
||||
- **Apply remote confinement in tests.** Remote-sandbox paths need a test that
|
||||
applies the provider's workspace confinement rules. A local test without them
|
||||
does not prove the remote path.
|
||||
|
||||
## Exceptions
|
||||
|
||||
- A deliberate reset of saved sessions (for example an ACPX profile version
|
||||
bump under [`acpx-profile-identity.mdc`](acpx-profile-identity.mdc)) is
|
||||
allowed when the PR names which saved state it resets and why, and the reset
|
||||
is reported with a named code rather than as an interruption.
|
||||
- Refactors, comments, and logging that change no persisted shape or identity
|
||||
need no cutover test.
|
||||
|
||||
## Review criteria
|
||||
|
||||
- Does the PR say how state saved by the previous release is handled?
|
||||
- Does changed or unknown saved identity map to a permanent, named code instead
|
||||
of a generic retryable one?
|
||||
- Is there an old-saved/new-resumed regression, and a deploy-lands-mid-run case
|
||||
where the run can be in flight?
|
||||
- Is the fix class-level, rather than one more per-code exception?
|
||||
- Do remote-sandbox paths have a test that applies the provider's confinement?
|
||||
|
||||
The release-compatibility contract is stated in
|
||||
[`doc/execution-semantics.md`](../doc/execution-semantics.md#94-release-compatibility-of-saved-run-state).
|
||||
Reference in new issue
Block a user