fix: preserve run outcomes when agent file cleanup is deferred (#14945)

## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work.
> - The heartbeat service records each agent turn and releases its
working files.
> - A turn can save its work and finish before instruction-copy cleanup
runs.
> - A cleanup exception can replace that completed result with an
adapter failure.
> - This also loses result accounting and can prevent environment lease
release.
> - This pull request records a cleanup warning and keeps the original
run outcome.
> - The existing recovery sweep retries cleanup from the durable
working-copy record.

## Linked Issues or Issue Description

Related cleanup and lock work: #14866 and #14869. Related, distinct
work: #14695 retains warm-process files; #12021 handles provider-process
SIGTERM after a terminal result.

**What happened?**

An agent saved its plan, posted a comment, and requested approval. The
provider completed its turn. Instruction-copy cleanup then timed out on
a directory lock. Its exception escaped a `finally` block and replaced
the provider result, so the completed turn showed `Run failed`.

**Expected behavior**

Keep the provider outcome, usage, cost, saved work, and pending
approval. Record a cleanup warning and let the existing recovery sweep
retry. A real provider failure must keep its original error. A failed
file save must keep its failed-save receipt.

**Steps to reproduce**

1. Complete a legacy adapter turn that saves work and requests approval.
2. Make instruction-copy release throw a directory-lock timeout.
3. Read the run result. Before this change, the cleanup error replaces
the provider outcome.

The new heartbeat tests reproduce the failure without a live provider or
external service.

**Paperclip version or commit**

The regression reproduces on `c83df091b1a5207375eaf23466bb5c62e4e1518e`.
This branch is rebased onto `cf8ad63c80`.

**Deployment mode**

Server-managed agent execution with persistent instruction working
copies.

## What Changed

- Catch instruction-copy release failures in both heartbeat teardown
paths. Stop repeating a failed cleanup attempt within the same run.
- Write a sanitized `instruction_cleanup` warning. A warning-write
failure also preserves the run result.
- Test successful, failed, and throwing providers; both teardown paths;
warning-write failure; accounting; approval state; and execution-control
release.
- Extend the held-lock test to prove a fresh recovery worker removes the
deferred copy and preserves its failed-save receipt.
- Document deferred cleanup and the run-log event.

## Verification

- Red proof: all five new heartbeat regression cases fail with the
original release calls.
- At head `20bea4f431c916d2f5db1970213aab85f5daa34c`, all 417 tests
passed across heartbeat process recovery, agent directory working
copies, and directory merge locks.
- Full local `pnpm -r typecheck`, `pnpm build`, and `git diff --check`
passed.
- [GitHub
CI](https://github.com/paperclipai/paperclip/actions/runs/37031119795)
passed at this head. All 53 reported checks passed; the two Storybook
checks were correctly skipped. This includes general and serialized
tests, browser shards, runner verification, build, typecheck, and the
canary dry run.
- Greptile reviewed this head with 5/5, no code comments, and no
unresolved review threads. The branch has no merge conflicts.
- The local `pnpm test:run` attempt was stopped after it reported eight
failures in unchanged suites. Four Slack/AgentMail cases selected an
unrelated ancestor skills directory and failed with `ENOENT`; the two
Slack cases passed with a temporary local skill-root link, which was
then removed. Three company-skill cases reproduced macOS `EACCES` errors
when renaming read-only cache directories. One gateway case passed when
rerun alone. No full local-suite pass is claimed; the complete CI test
jobs passed.

## Risks

- Cleanup errors now leave recovery work pending. The durable
working-copy record remains available for the existing retry sweep.
- This change preserves provider failures and failed-save receipts. It
does not claim that unsaved file edits were saved.
- Native instruction reservation errors retain their existing behavior
because they guard process ownership.
- No schema, lockfile, workflow, API, or UI changes.

## Model Used

OpenAI Codex, based on GPT-6, with reasoning, repository tools, and code
execution. The exact backend model ID and context-window size are not
exposed by 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 (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>
This commit is contained in:
Devin FoleyandPaperclip authored and GitHub committed 2026-10-02 14:16:18 -07:00
1 parent d9b64ee28e
commit d6d88b9de2
5 files changed
+117 -2

No files matched your search

+6
View File
@@ -180,6 +180,12 @@ unavailable local copy can still contain uncollected edits; this cleanup path
preserves those bytes even if local stop proof arrives later.
Deferred cleanup retries after a
delay so one blocked copy does not prevent other copies from being cleaned.
If releasing a run's instruction copy fails, the run records a cleanup warning
and leaves the durable copy for the recovery sweep. Cleanup does not replace the
provider's result, discard usage accounting, or prevent environment lease release.
It does not claim that unsaved agent-file changes were saved; collection failures
keep their separate failed-save receipts. A run attempts failed cleanup only once
before handing it to recovery, rather than repeating the lock wait in teardown.
Re-preparing an existing run uses the same lock as cleanup and rechecks its
receipt under that lock. Preparing a new run keeps its separate admission path.
Missing stop proof or lost remote bytes produce a visible diagnostic, never a
+7
View File
@@ -357,3 +357,10 @@ Successful checkpoints can include `checkpointStats`: `scannedEntries`,
capture, not cumulative traffic or an atomic snapshot of background writers.
They contain no file contents. The receipt remains in the instance run log;
it adds no Paperclip Telemetry or OpenTelemetry export.
If instruction-copy release throws, `instruction_cleanup` records a warning
with payload `{ "state": "deferred" }`. The existing working-copy recovery sweep
retries cleanup. This event preserves the run outcome and does not claim a file
save; `instruction_save` remains authoritative for collection. The event contains
no raw exception, host path, file contents, or lock-owner metadata. Failure to
write the warning must not replace the provider outcome or stop lease release.