Files
PaperClipAI/packages/adapter-utils
DottaandClaude Opus 5.5 09241442ea fix(server): let a native reviewer approve its review stage (#20373)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work
> - Execution policies can require an independent review stage before a
task is done
> - A native (Runner) agent that is the active reviewer had no way to
record its decision
> - `PATCH status: "done"` is refused for a native run that owns the
task (#18411), and `paperclip_finish` accepts the report but leaves the
stage pending
> - The reviewer then searches the API until a deadline, and the task
stays in review with nobody able to decide it
> - This pull request records the final-stage approval on `PATCH`
without releasing the run's lock, and lets the run's own finish report
set the final status
> - The benefit is that an agent reviewer can actually approve its
review stage, in one short turn

## Linked Issues or Issue Description

Refs #18411 (added the native status guard). Related: #20264 (explicit
stage-decision operation, still open; this change is narrower and
compatible with it).

**What happened?**

A native reviewer agent was woken as the active `review` participant. It
read the deliverable and wrote its review in about 100 seconds. It then
called `paperclip_finish` with `done`. The tool answered "Completion
report accepted; the task's configured review stage is still pending".
It then tried `PATCH /api/issues/{id}` with `status: "done"` and a
comment. The route refused it with
`native_run_status_reported_by_finish`. The agent searched the API for 2
more minutes and found no other path. The stage stayed pending. When the
run ends with `done`, the status arbiter sees the pending stage and
keeps the task in review (`governed_gate_pending`), so the stage can
never resolve.

**Expected behavior**

The active review/approval participant can approve its stage from a
native run. The task reaches `done` after the approval. The implementer
still cannot approve. A run that is not the active participant still
cannot change the task status.

**Steps to reproduce**

1. Create a task with an execution policy that has one `review` stage
with a native agent participant.
2. Move the task to `in_review` so the stage is pending and the reviewer
agent wakes.
3. In the reviewer run, call `PATCH /api/issues/{id}` with `{ "status":
"done", "comment": "Approved: ..." }`.
4. Observe `409 native_run_status_reported_by_finish`. Call
`paperclip_finish` with `done` and observe that the stage stays pending.

## What Changed

- `PATCH /api/issues/:id`: when the requester is the native run that
holds the task lock and is the active participant, a `status: "done"`
request on the final stage now records the approval decision
(`executionState` completed, decision row, activity). The route removes
`status` from the write, so the task stays `in_review` and the run keeps
its execution lock. The run's `paperclip_finish` report then sets `done`
through the normal status arbiter.
- An intermediate-stage approval (which would hand the task to the next
participant) and every other status change from such a run keep the
existing `native_run_status_reported_by_finish` refusal.
- `paperclip_finish` from the active participant (`done` or
`needs_review`) while its stage is still pending is now rejected with
the exact decision path, instead of being accepted with "stage still
pending".
- The native finalizer now treats the final-stage approval that the run
itself recorded (read from the `issue_execution_decisions` row, bound to
this run and agent) as the completion authority for its clean `done`
report. Before this, a reviewer that cited free-text evidence had its
report graded as incomplete, and the task was reopened as `in_progress`
with a new wake for the reviewer. Failed verification, blocking
remaining work, attention requests, other dispositions, and a
still-pending stage keep the old outcome.
- The reviewer/approver wake text names the decision path (`PATCH` with
`status` and a required decision comment) and says a completion report
does not record the decision. A Runner reviewer that needs changes is
told to report them with `paperclip_block`, because a changes request
from the run that owns the task is a reassignment that stops that run.
- The locked review-policy recheck treats the deferred approval as a
verdict, so a policy changed to `human_only` before the locked write
still refuses it.

## Verification

- `pnpm exec vitest run
server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts
server/src/__tests__/issue-execution-policy-routes.test.ts
server/src/__tests__/issue-comment-reopen-routes.test.ts
server/src/__tests__/issue-execution-policy.test.ts
packages/adapter-utils/src/server-utils.test.ts`: 5 files, 595 tests
pass.
- `pnpm exec vitest run
server/src/services/native-runtime/native-completion-feedback.test.ts`:
29 tests pass (embedded PostgreSQL), including the decision-row binding
for `executionStageApprovedByRun`.
- `pnpm exec vitest run
server/src/services/native-runtime/status-arbiter.test.ts
server/src/services/native-runtime/native-run-finalizer-telemetry.test.ts
server/src/services/native-runtime/native-stale-owner-completion.test.ts`:
72 tests pass.
- New route tests fail without the route change (409 instead of 200/422)
and pass with it. The new feedback test fails without the feedback
change ("Completion report accepted" instead of a rejection).
- `npx tsc --noEmit -p server` and `-p packages/adapter-utils`: pass.
- A route test changes the review policy to `human_only` between the
first check and the locked write; the deferred approval is refused
(403).
- Live full-stack chat eval `task-coordination.independent-review`
(Runner, OpenCode) with this change: passed in GitHub Actions run
42705964209 (eval branch with the first two commits cherry-picked). The
reviewer run recorded its approval 24 s after its review brief was
released, reported `done` 14 s later, and the finalizer set the task to
`done` with reason `execution_stage_approved`; total reviewer run 73 s.
The earlier run 42702224713 exposed the finalizer evidence issue that
the second commit fixes.

## Risks

- Low to medium. The change is limited to one request shape: a native
run, holding the lock, that is the active participant of a pending final
stage, sending `status: "done"`. All other requests keep the old guard.
- Between the approval and the finish report, the task shows `in_review`
with a completed `executionState`. If the run fails before it reports,
the existing native recovery owns the task, as for any other run.
- Previous-release story: no persisted shape changes. A reviewer run
that started before the deploy could not have recorded a deferred
approval (the old route refused it), so the new finalizer input is false
for it and the old outcome applies.
- Intermediate-stage approvals and changes requests from native runs are
still refused by the route. A Runner reviewer reports required changes
with `paperclip_block`, which keeps the stage pending for a person. A
self-service changes request needs the deferred handoff semantics in
#20264.

## Model Used

- Anthropic Claude Opus 5.5 (`claude-opus-5-5`), 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
- [ ] All Paperclip CI gates are green
- [ ] 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 <noreply@anthropic.com>
2026-11-09 17:11:42 -06:00
..

@paperclipai/adapter-utils

Shared utilities for Paperclip adapters: process spawning, environment injection, sandbox/SSH transport, workspace sync, and the round-trip helpers that move code between the local execution-workspace cwd and wherever the agent actually runs.

For the adapter-author guide see docs/adapters/creating-an-adapter.md and the in-repo notes at packages/adapters/AUTHORING.md.

Sandbox bridge deadlines

Command-managed bridge control operations (queue reads, input delivery, setup, and cleanup) have a host-enforced deadline of at most 30 seconds per shell command. A shorter configured timeout still applies. These operations cannot inherit the agent run's hours-long lifetime or wait forever on a provider that ignores its timeout. Long-lived agent session commands keep their own limits.

A control timeout reports a transport failure. It does not prove that the remote command stopped and does not replay an uncertain write. The process session closes failed input delivery and uses its existing shutdown path; normal execution settlement must still verify termination.

Native archive transfer deadlines

Native syncIn and syncOut calls accept an optional minimumTimeoutMs budget. The server validates this budget and extends the worker RPC deadline. Providers can honor this field for file transfers without changing their configuration. Daytona uses the larger of this minimum and its configured timeout. Other providers keep their existing behavior. The minimum is limited to 30 minutes and does not change sandbox lifetimes, stored settings or the timeout for other commands.

The remote-workspace archive path uses the verified capture size for downloads and the local archive size for uploads. Large archives get one second per MiB plus 60 seconds, capped at 30 minutes. Small archives keep the existing provider timeout when that calculation is no greater than five minutes. Archive size, checksum, confinement and host space checks still apply.

An exact retained-sandbox resume keeps its recorded provider timeout and RPC overhead. It uses the normal one-minute fallback only when no timeout was recorded. A five-minute cold restore must not be cut off by that fallback. Resume still targets the recorded allocation and does not acquire a replacement.

No-remote-git contract

The local execution-workspace cwd is the only persistence boundary across runs. No adapter may depend on a git remote for cross-run state.

Adapters that run the agent on a different host should use the SSH round-trip helpers in src/ssh.ts:

  • prepareWorkspaceForSshExecution({ spec, localDir, remoteDir }) — bundles the local cwd (tracked files, dirty edits, untracked additions, and the git history needed to reconstruct it) to remoteDir before the run starts. Runs with no git remote configured.
  • restoreWorkspaceFromSshExecution({ spec, localDir, remoteDir, ... }) — syncs the remote cwd back into localDir after the run, including any new commits the agent created. Also runs with no git remote configured.

prepareRemoteManagedRuntime in src/remote-managed-runtime.ts wraps both calls for adapters that want a per-run remote workspace and an automatic restoreWorkspace() finally hook.

The invariant is pinned by the no-remote-git contract case in src/ssh-fixture.test.ts, which asserts that a remote-only commit propagates to the local worktree through the prepare → restore round-trip with no git remote configured at any point. Do not regress that test.