mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-07 06:16:06 -10:00
DottaandClaude Opus 5.5
a4050187ec
## 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]>
71 lines
3.9 KiB
Plaintext
71 lines
3.9 KiB
Plaintext
---
|
|
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).
|