mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-12 14:19:00 +01:00
76994c50275dbc005cc248e11189a38baa21719f
3473
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 DottaandClaude Opus 5.5
|
76994c5027 |
fix(native): bound failover backup restores by inactivity, not total time (#19906)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - A native per-turn session that continues an earlier conversation first restores its failover backup from durable storage. This happens before the coordinator claim, so no provider has started. > - That restore ran under a fixed 300 s total deadline (`AbortSignal.timeout(300_000)`). The deadline covers the download and the local decrypt, digest, extraction and provider-artifact steps. > - On staging, restore time for resumed sessions grew during the day. It reached 300 s, and then every resumed run of one review agent failed with the generic "The operation was aborted". Fresh sessions were not affected. > - A failure at this point also enters the saved-work reconciliation path, so the user request stopped. > - This pull request bounds each durable read by inactivity instead of total time. A slow transfer continues while bytes arrive. A read that stops sending bytes fails with a named code. > - The benefit is that resumed sessions no longer fail only because their backup is large or the store is slow. ## Linked Issues or Issue Description Refs #19894 (adds the same idle-deadline rule for workspace snapshot reads; this PR covers the separate native failover-backup reads). **What happened?** On staging, a PR-review agent had 17 runs fail in about 80 minutes. Each failure had the message "The operation was aborted" and the code `native_continuation_requires_reconciliation`. Every failing run resumed an existing session (`sessionIdBefore` set, per-turn lifecycle). Every fresh-session run in the same window succeeded. In each failing run, the error came 304–309 s after the `environment.startup` span, and no `native.coordinator.claim` span was recorded. The run result had `executionRecovery: { kind: "bootstrap", providerWorkStarted: false }`. Earlier resumed runs of the same agent show the time from `environment.startup` to `native.coordinator.claim` growing from about 30 s to about 200 s before the failures started. The only fixed 300 s deadline on that path is the failover-backup restore in `restoreDurableHarnessForExecution`. **Expected behavior** A resumed session restores its backup when the store keeps sending bytes, even when the total time is more than five minutes. A read that stalls fails with a named, diagnosable code. A failed restore publishes no partial backup and keeps the saved backup. **Steps to reproduce** 1. Persist a native failover backup with provider artifacts. 2. Restore it through a storage reader that sends bytes slowly, so the full restore takes longer than the deadline. 3. Before this change, the restore fails with "The operation was aborted". A reader that stops sending bytes, with no caller deadline, waits indefinitely. ## What Changed - `native-harness-backup-storage.ts`: add `downloadIdleTimeoutMs` to `restoreDurableNativeHarnessBackup` and `verifyDurableNativeHarnessBackup`. When set, each durable `getObject` (the backup object and the provider artifacts) runs under an idle timer. Each received chunk renews the timer. The timer is active only while a read is in flight, so local decrypt, digest, extraction and copy are never timed. On expiry the whole restore aborts and throws `native_harness_storage_download_stalled`. Caller cancellation keeps its own error. - `verifyRecoverableNativeHarnessBackup` and the executor's `restoreDurableHarnessForExecution` use the five-minute idle limit (`NATIVE_HARNESS_DOWNLOAD_IDLE_TIMEOUT_MS`) instead of `AbortSignal.timeout(300_000)`. - The whole restore or verification keeps a 30-minute outer limit. It fails with `native_harness_storage_restore_deadline_exceeded`, so a hung local step stays bounded and named. - The executor passes the startup Stop signal (`providerPackSourceAbort`) into the restore, so Stop (also a Stop that came before the restore began) ends it. Only a startup that already completed does not reuse its signal. The restore checks the signal after the local copy and before it publishes `current`. - The wrapped storage is a `Proxy` that replaces only `getObject` and forwards every other member to the real service. - `doc/DEVELOPING.md`: document the idle limit and the named code. Previous-release story (`rules/upgrade-safe-run-state.mdc`): no persisted shape changes. Backups, artifact rows, receipts and checkpoints saved by the previous release restore unchanged. A deploy that lands during a restore stops the process. The new process starts the restore again from the saved durable backup under the new limit. The existing hash, size, encryption and atomic publication checks are unchanged. ## Verification Run from `server/`: - `pnpm exec vitest run src/services/native-runtime/native-harness-backup-download-deadline.integration.test.ts` — 4 tests passed (embedded PostgreSQL, local-disk storage, an OpenCode backup with provider artifacts). The tests are in a focused suite, so the existing 990-line storage suite does not grow. - `pnpm exec vitest run src/services/native-runtime/native-harness-backup-storage.integration.test.ts` — 68 existing tests passed. - New: "keeps restoring a backup whose download outlasts the idle window while bytes keep arriving". With the former total-deadline policy, the same slow transfer fails with "The operation was aborted" and publishes nothing (the staging message). With the idle limit, the restore completes after more than twice the window, restores the session and the OpenCode launch artifacts, and verification also completes. - New: "names a download that stops sending bytes and publishes no partial backup". It checks restore and verify, checks that no `current` backup is published, and then restores the same saved backup successfully. - New: "stops at the outer bound during local copy and publishes nothing". - New: "lets Stop end a progressing download with its own reason, including a Stop before it starts". - RED: with the production files reverted to `origin/master`, the stall test does not finish and times out after 10 s, because no stall is detected. The other two tests pass on the old code, because the total deadline was the executor's caller policy. The first test reproduces that policy directly. - `pnpm exec vitest run` on `native-harness-online-preservation`, `native-harness-filesystem-independence` and `native-controller-migration` integration suites passed. `native-warm-checkpoint-ownership.integration.test.ts` fails locally on master as well, because the local Rust `paperclip-runnerd` binary is not built. - `node scripts/typecheck-layers.mjs` passed for all layers. Not verified: a live resumed run on staging with this build. The staging evidence shows where the time goes (before the coordinator claim, on resumed sessions only) but does not show which restore step was slowest. The fix removes the total deadline for every step on that path. ## Risks - A restore that progresses very slowly can now take longer than five minutes. Before, it failed. The run still stops on caller cancellation. - A local step that hangs (for example a stalled disk) is now cut off by the 30-minute outer limit instead of 5 minutes. A stalled local writer also pauses the read through backpressure, so it can be reported as `native_harness_storage_download_stalled`. - A failure before the coordinator claim still enters the existing saved-work reconciliation path. This PR does not change that classification. It removes the cause of these failures. ## Model Used - Provider: Anthropic. Model: Claude Opus 5.5 (`claude-opus-5-5`), with tool use (shell, file edits, read-only browser for API inspection) 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> |
||
|
DottaandPaperclip
|
57ae2fc309 |
perf(workspaces): reduce snapshot stream scheduling overhead (#19900)
Use bounded 1 MiB filesystem buffers during workspace archive hashing, encryption, and restore. Preserve integrity, ownership, cancellation, and snapshot format guarantees. Co-Authored-By: Paperclip <noreply@paperclip.ing> |
||
|
DottaandPaperclip
|
d7f02c51b4 |
fix(skills): scope file protection to admitted selections (#19904)
Allow unused skill deletion when unstarted admission entries have no retained execution. Limit unknown legacy selections to skills that could have existed when the validated immutable run finished. Co-Authored-By: Paperclip <noreply@paperclip.ing> |
||
|
Devin FoleyandPaperclip
|
c9959814ba |
fix(workspaces): cancel stalled snapshot reads and preserve restore refusals (#19894)
Cancel encrypted snapshot reads after five minutes without incoming bytes. Renew the deadline on progress, preserve caller cancellation, settle the physical request and private scratch, and retain saved snapshots for normal retry admission. Keep Node warnings out of server-owned helper refusal labels. Verified both complete owner suites (145 tests), the prior unchanged storage and transport coverage, server typechecks, final-head CI and Greptile 5/5. The earlier CI cancellation-signal assertion failure was reproduced locally and corrected with checks for abort state, reason, stream closure and cleanup. Co-Authored-By: Paperclip <noreply@paperclip.ing> |
||
|
DottaandClaude Opus 5.5
|
988a4b6749 |
feat(recovery): ownable-kind registry and report-only ownership reconciler (#19861)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A control-plane restart leaves in-progress records owned by dead processes, and about 31 per-subsystem proofs decide when to release them > - The durable ownership plan (section C) asks for one registry of ownable record kinds and one reconciler > - Phase 1 must measure first: how many in-progress records exist, who owns them, and which invariants they break, with no behavior change > - This pull request registers 46 kinds from the Phase 0 inventory's section C and adds a report-only pass to the existing native finalization reconciler > - The benefit is per-kind invariant-violation counts from chaos waves and staging. That data unlocks Phase 2 (moving kinds onto one settle path) and the removal of the company-wide admission gate ## Linked Issues or Issue Description Refs #19831 (plan, section C and Phase 1). Refs #19846 (Phase 0 inventory, section C). Stacked on #19853 (the prover) and #19851 (one process identity). **Problem** No single place lists the control plane's in-progress records or checks the plan's invariants. Each subsystem releases its own records by its own rules, so nobody can count how many records break "owned", "settled" or "visible next action". **Expected behavior** One report-only pass lists every registered kind, classifies each owner as gone, alive or unprovable, and records each invariant violation once. It changes no record and stays quiet when nothing is in progress. **Impact** Phase 2 moves kinds onto the shared settle path in the order these counts show. Phase 3 removes the admission gate once the I3 data exists. ## What Changed - New `server/src/services/recovery/ownership-registry.ts`: 46 `OwnableKind` entries (`id`, `obligation`, `storage`, `listInProgress`, `settle: null`, `invariantChecks`). Each record names its owner as a process stamp, a fence token, a run, an operator, or none. The skill-cache lock is registered with a named coverage gap because its lock files live in per-skill roots. - New `server/src/services/recovery/ownership-invariant-report.ts`: one pass classifies every owner with `proveOwnerGone` (or the owning run's status), then runs: - I1 owned: an in-progress record with no owner; - I2 settled: an owner proven gone on two consecutive passes while the record is still in progress; - I3 admission independence: terminal runs whose accounting outlives the 10 min closure, which hold company admission; - I4 exactly-once intents: wake requests ended `skipped`/`failed` on a temporary refusal (heuristic until section E); - I7 visible next action: an agent-assigned `in_progress` issue with no live run, no pending wake and no open recovery action for 5 min. - I5, I6 and I8 are reported as named coverage gaps; I8 comes in the speed-budget PR. - Violations are written once per process to the activity log as `recovery.invariant_violation` (`actorId: ownership-reconciler`, `details.mode: report_only`), insert-only with no live-event or plugin fan-out. Nothing goes to Telemetry (AGENTS.md §5.7). - `native-finalization-reconciler.ts`: `registerOwnershipReconciliationWork` runs the pass in the shared delivery-work scheduler (new queue `ownership-invariant-report`). There is no second loop. - `index.ts` registers it after residual recovery and does not await its first pass, so readiness is not delayed. ### Sleep-aware trigger (rules/sleep-aware-background-work.mdc) - **Trigger:** one pass at registration (startup reconstruction from durable truth). After that, a settled `active-execution-recovery` signal, which run lifecycle, controller renewal and process-evidence writes already emit. Signals are coalesced to at most one pass per 30 s. No producer gained a write. - **Pending predicate and next deadline:** a 30 s follow-up only while the last pass saw a changing in-progress set, or an owner proven gone that needs the second I2 pass. - **Quiet:** an empty set or the same stuck set twice sets no deadline. The test asserts `nextWakeAt() === null` after an idle pass. - **Turn path:** the pass never runs before `turn.start`. Stamps read only cached identity. ## Verification - `npx vitest run src/__tests__/ownership-reconciliation-report.test.ts` (embedded Postgres, in `server/`): 4 tests pass: - every kind is registered once with no settle path; - verdicts per class: alive (this process's claim), gone (lease of a failed run), unprovable (no owner; another PID space); - each rule fires on its fixture, and I2 fires only on the second pass; - every touched table is byte-identical before and after (report-only); - activity rows are written once per violation, and none are written with `record: false`; - the worker is quiet when idle and schedules a follow-up only while records change. - `npx vitest run src/__tests__/delivery-work-coordinator.test.ts src/services/owner-proof.test.ts src/__tests__/server-startup-feedback-export.test.ts`: pass. - `npx tsc --noEmit -p server`: no errors. ## Risks - No record changes, so no settle risk. The risk is load: one pass runs about 46 small indexed or `limit 201` queries, at most once per 30 s while runs are active, and none when idle. - I4 matches refusal text with a heuristic. It can over- or under-count until section E gives wake requests closed terminal reasons. It is labelled as such in the code. - Violations dedupe per process, so a restart writes a still-open violation again (once per boot). Chaos reports count distinct `(rule, entityId)`. - Previous-release story (`rules/upgrade-safe-run-state.mdc`): no persisted run state changes. The pass reads rows of any age, and a missing field classifies as unprovable. - `heartbeat_runs` and `native_run_finalizations` record no host or PID space, so foreign-boot owners there classify as unprovable (`missing_identity`). This is the inventory's finding 2, and the report makes it countable. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, in Claude Code with tool use and code execution. ## 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> |
||
|
|
12681d1611 |
fix(sentry): stop resending retained task interruption failures (#19041)
Keep task-state Sentry reports within the recent saved-change window while retaining reports for newly failed recovery and delayed Retry preparation. Include the safe run error code without changing event grouping. Verified 402 relevant local tests, every server typecheck layer, all final-head CI checks, and Greptile 5/5 with no unresolved review threads. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Co-Authored-By: Paperclip <noreply@paperclip.ing> |
||
|
Devin FoleyandPaperclip
|
25273aa69b |
fix(onboarding): start the agent when the opening question is answered (#19886)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work.
> - A new company starts with an onboarding first task. The task opens
with a greeting and an opening question card that the assignee agent
posts.
> - Onboarding does not wake the agent when it creates the task. The
user's answer must start the agent.
> - When the user clicks an answer, run dispatch cancels the agent's run
0.4 seconds later with `issue_not_in_progress`. Answer continuations
only start on `in_progress` or `in_review` tasks, and the onboarding
task is still `todo`.
> - The user sees nothing until an unrelated bounded retry starts the
agent about two minutes later. This is the first thing every new user
does.
> - This pull request lets an answer start a `todo` task when the task
has never started and no run asked the question.
> - The benefit is that the agent starts immediately after the first
answer. The protection for parked tasks does not change.
## Linked Issues or Issue Description
**What happened?**
I created a new company and clicked an option on the onboarding opening
question ("What would you like to do?"). The agent's run was cancelled
immediately with `issue_not_in_progress`: "Cancelled because
resolved-interaction continuation issue is no longer in_progress
(current status: todo) before the queued run could start". The task
stayed in `todo`. About two minutes later, a `transient_failure` bounded
retry of the cancelled run started the agent through the normal checkout
path.
**Expected behavior**
The agent starts as soon as the user answers the opening question.
**Steps to reproduce**
1. Create a new company and hire the first agent through onboarding.
2. Open the onboarding first task.
3. Click an option on the opening question card and submit.
4. Look at the agent's runs. The first run is `cancelled` with
`issue_not_in_progress`.
**Paperclip version or commit**
`master` at `86bc3db6938`. The same code is on `00ad9503fe3`.
**Relevant logs or output**
```
heartbeat_runs: status=cancelled error_code=issue_not_in_progress
"Cancelled because resolved-interaction continuation issue is no longer in_progress (current status: todo) before the queued run could start"
two minutes later: "Scheduled bounded retry 1/2" (retryReason transient_failure)
```
Related: #13068 added the onboarding opening question. #13318 made
answer continuations require `in_progress` or `in_review`.
## What Changed
- New `server/src/services/unstarted-task-question-answer.ts`.
`isUnstartedTaskQuestionAnswer` returns true only when all of these are
true:
- the interaction is an answered question (`ask_user_questions`, status
`answered`) on this task, and it has no `sourceRunId` (no run asked it);
- the task is `todo`;
- the task's `startedAt` is null.
It reads only server-owned rows. `startedAt` is never cleared, so a task
that started and was later moved back to `todo` does not match.
- `run-dispatch/adapters/postgres.ts`: the staleness facts include
`isUnstartedTaskQuestionAnswer`. The adapter reads it under the issue
lock, and only for a resolved-interaction continuation on a `todo` task.
- `run-dispatch/domain/policy.ts`: `decideQueuedRunStaleness` lets such
a continuation start from `todo`. The other answer continuations still
require `in_progress` or `in_review`.
- `services/heartbeat.ts`: the final continuation checkout adds `todo`
to the expected statuses only for the same case. The task then moves to
`in_progress` in the same guarded update.
## Verification
- `heartbeat-stale-queue-invalidation.test.ts` (embedded Postgres):
- New test: an answer to a question that no run asked, on a
never-started `todo` task, starts the assignee. The task moves to
`in_progress` under the run's execution lock. With the fix reverted,
this test fails with the reported error (`cancelled`,
`issue_not_in_progress`).
- New test: three regression cases stay cancelled with
`issue_not_in_progress`:
- an answer on a task that started and was moved back to `todo`;
- an answer to a question that an agent run asked;
- a rejected confirmation that no run asked.
Without the kind and status check, the confirmation case fails.
- `policy.test.ts`: the new fact allows `todo`. The same fact on a
`backlog` task is still cancelled.
- `pnpm --filter @paperclipai/server typecheck`: exit 0.
- `npx vitest run` on these suites: 8 files, 497 tests passed.
- stale-queue invalidation
- run-dispatch policy
- onboarding first-task routes
- onboarding first-task admission
- question-response delivery
- issue-thread interaction routes
- heartbeat retry scheduling
- legacy continuation authority
## Risks
- Low risk. The change only affects answer continuations on `todo` tasks
that have never started, where the question has no source run. All other
continuations behave as before.
- The exception is not limited to the onboarding card. Any answered
question that no run asked, on a never-started `todo` task, can now
start the assignee. This matches what a plain comment wake does on a
`todo` task. Confirmations and other interaction kinds keep the existing
rule. For example, rejecting a confirmation that no run asked still does
not start the task.
- The final checkout does one extra indexed read. It only runs for
resolved-interaction continuations on `todo` tasks.
- I did not find which component scheduled the two-minute
`transient_failure` retry. After this change, onboarding does not depend
on that retry.
> For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and
discuss it in `#dev` before opening the PR. Feature PRs that overlap
with planned core work may need to be redirected — check the roadmap
first. See `CONTRIBUTING.md`.
## Model Used
- Claude Opus 5.5 (`claude-opus-5-5`) in Claude Code, with tool use
(shell, file edits, test runs). Investigation, fix, and tests were
AI-assisted, with human direction.
## 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
- [ ] I have updated relevant documentation to reflect my changes (no
docs describe this behavior)
- [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
---------
Co-authored-by: Paperclip <noreply@paperclip.ing>
|
||
|
DottaandClaude Opus 5.5
|
f439150cb3 |
fix(recovery): limit the null-host owner fallback to single-host servers and record each use (#19883)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - #19868 and #19871 made restart recovery judge a recorded pid only in the PID space that recorded it > - Rows saved before those host columns existed have no recorded host. They kept the previous local dead-pid proof, so in-flight native runs would not be stranded at upgrade > - On an ephemeral (multi-host) server that fallback is unsafe: the row may have been written on another host > - This pull request limits the fallback to single-host servers, records every use, and gives ephemeral servers a working board action > - The benefit is that single-host upgrades still recover on their own, multi-host servers never accept a cross-host proof, and the next release can remove the fallback with evidence ## Linked Issues or Issue Description **Bug description** After #19868 and #19871, a `native_run_finalizations` row with `controller_host_hash = null`, or a run with `process_host_hash = null`, still accepted "the pid is dead here" as proof on every server. That includes an ephemeral server, where the row may come from another host. Nothing recorded how often the fallback was used, so it had no safe removal path. **Expected behavior** - Single-host server: the fallback applies for one release, and each use writes a `legacy_host_unknown_dead_pid_proof` activity-log entry. - Ephemeral server (`PAPERCLIP_EPHEMERAL_SERVER=true`): the row is unprovable, and the board has an action to confirm the previous server stopped. - A mismatched host is always unprovable. Refs #19868, #19871, #19846 ## What Changed - `native-restart-recovery.ts` `evaluateNativeControllerTakeover`: - A coordinator row with `controllerHostHash: null` returns `controller_host_unrecorded` when `ephemeralServerEnabled()` is true. - Otherwise a pid proof from that row carries `legacyHostFallback: true`. - `operator_attested` is accepted as proof (`operator_attested_controller_stopped`). - Receipt-built owners, which have no host key, are unchanged. - New `recordLegacyHostFallbackProof` writes the activity-log entry. It is not a run-log event: run events are progress evidence for restart recovery's crash-loop credit, and the record must not change any recovery decision. It is called from restart recovery, the dead-controller result-lease release, native retirement, and remote warm-session recovery. - New `native-controller-host-recovery.ts`: - `requestNativeControllerStopConfirmation` (board-owned `native_controller_host_unverified` recovery action, idempotent, never displaces another active action). - `confirmNativeControllerStopped`. It verifies the exact recorded controller and stamps `operator_attested` with compare-and-set. It then resolves the action, wakes live-run recovery, and logs `issue.native_controller_stop_confirmed`. - Route `POST /issues/:id/recovery-actions/confirm-controller-stopped`. It is board-only and requires `runtime:confirm_owner_stop`. Generic `restored`/`false_positive` resolution of this cause is refused. A shared `confirmNativeControllerStoppedSchema` validates the body. - UI: `WorkspaceExportRecovery` reuses the "Confirm the previous server stopped" control for this cause, and the generic recovery card hides it. Adds the `issuesApi.confirmControllerStopped` client call. - `recovery/service.ts` orphan reaper: a null `process_host_hash` is judged only when not ephemeral. A successful use records the entry best-effort, after the terminal write. On an ephemeral server the run is left to its existing Stop control. - `ephemeralServerEnabled` moves to a leaf `ephemeral-server-mode.ts` (re-exported by `ephemeral-server-contract.ts`). Importing the contract module from recovery code created an import cycle in the runner child process. - The new route is registered in OpenAPI. - `doc/execution-semantics.md`: documents the host rule and the one-release sunset. ## Verification - `native-restart-recovery.test.ts`: null host on a single-host server is released with the flag. Null host on an ephemeral server is unprovable. A mismatched host is unprovable in both modes. An attested controller is accepted. Receipt owners are unchanged. - New `native-controller-host-recovery.test.ts` (embedded Postgres): - The fallback writes the activity-log entry and no run event. - Ephemeral mode creates exactly one board action. - A wrong run id is refused. - Confirmation stamps the attestation, after which takeover is allowed. The action resolves and the activity is logged. A second confirmation is refused. - `recovery-stale-issue-lock-sweep.test.ts`: on a single-host server a null host is terminalized and records one event. On an ephemeral server it is left alone with no event. The existing audit-failure case still clears the lock. - `WorkspaceExportRecovery.control.test.tsx`: the controller confirmation posts to `confirmControllerStopped` and never to the copyback endpoint. - Locally: these suites plus `remote-warm-session-recovery`, `native-controller-recovery.integration` and `native-startup-cleanup-recovery` (136 passed), and after the CI fixes `native-runner-restart-preparation` and `native-runner-restart-recovery` integration, `openapi-routes` and the suites above (256 passed); the UI recovery suites (52 passed); `node scripts/typecheck-layers.mjs` with no errors in the touched files; `pnpm check:token-gates` clean. ## Risks - Previous-release story: no schema change. Null-host rows saved by earlier releases recover as before on single-host servers, now with an event, and wait for the board on ephemeral servers. The next release removes the fallback; the event count shows whether any rows remain. - On an ephemeral server, a null-host orphaned run process is not reaped automatically. The operator uses Stop. That is the intended "unprovable" outcome. - A board confirmation is an attestation. It applies only to the exact recorded controller (boot id + pid) and only while the host is still unrecorded. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used 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 - [ ] 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> |
||
|
DottaandPaperclip
|
5a70999306 |
Add a versioned online workspace migration command (#19882)
## Thinking Path > - Paperclip preserves the files and recovery state produced by its agents. > - Canonical online migration can preserve selected local workspaces while unrelated work continues. > - Operators sometimes need a newer migration owner before the serving web process can take that release. > - The existing packaged maintenance entry requires stopped writers. > - This pull request adds a finite online entry from one coherent reviewed release tree. > - The entry delegates capture and publication to the existing owners and keeps tool and serving identities separate. ## Linked Issues or Issue Description Refs #19854, #19855 and #19872. The duplicate search found related owner fixes and no separate versioned online entry. **What happened?** The finite online workspace importer was available as a library in the serving image. The packaged all-phase migration entry required maintenance mode. **Expected behavior** Run the reviewed online owner from a separate complete release tree without replacing the web process, stopping unrelated work or changing its release policy. **Deployment mode** A managed instance with its original local volume, database, existing encryption key and shared S3 storage. ## What Changed - Add strict inspect, single, cohort and exact single-journal resume requests with explicit company, workspace, deadline and concurrency bounds. - Verify the private tool root and its own build stamp separately from the serving source commit and deployment. - Reuse the nonmutating full migration-manifest inspector and require the additive controller host-identity column before publication. - Hydrate existing configuration and key bindings, then use the original ESM database pool and canonical storage, source locks and migration owners. - Keep multi-member inspection with the optional canonical cohort preflight in a read-only transaction with denied storage. Releases without that owner refuse cohort operations; single import and resume remain available. - Register the optional runtime bundle entry and document the supported full-tree source mode, resource admission and reconciliation. - Add hash-bound old-writer/reader compatibility and encrypted restore tests. ## Verification Final local revision: `c9345b72f27aee1158047dccd0b1060f9a0be6df`, on master `fbe689938c3bf8c5849d2cd56b9da90029db9389`, macOS arm64 with Node 24.22.2. - Entry and runtime packaging: **34/34 PASS**. - Real PostgreSQL/filesystem compatibility: **6/6 PASS**, disposable database wrapper exit 0 and cleanup complete. The frozen old source lock owner contends with the new entry; unrelated work proceeds. The old authority and snapshot readers decrypt and restore newly published bytes. - Pending and uploaded journal cases use the unchanged old persisted format and preserve operation/snapshot IDs through resume. - Full layered server source and test typecheck: **PASS**. - Module boundaries and `git diff --check`: **PASS**. - Actual source-mode module import under Node 24 succeeds without starting the application. - A separate temporary full-old-producer fixture did not get past module setup: 90 seconds under Vitest and a 15-second direct Node control. Neither is counted as an executed old-producer test or a product failure. The smaller frozen old-writer/reader proof and old-format journal cases are the local evidence. - Hosted resume of the actual saved operation and Linux artifact qualification remain operational acceptance gates. This PR does not claim those ran. - Review regression: a real successful import returned a null operation ID (genuine RED). The repaired entry binds the exact returned snapshot to its succeeded migration journal after commit (GREEN). Committed or superseded snapshots are accepted; a missing readback requires reconciliation rather than a blind retry. - Final affected check: **40/40 PASS** across the entry, packaging and actual PostgreSQL compatibility suites; wrapper exit 0 and cleanup complete. Full layered typecheck and boundaries pass after the user-requested rebase and current Runner rebuild. - A second real postcommit database-read fault also has genuine RED/GREEN proof. The succeeded journal, saved authority and original bytes remain intact, and every readback error now preserves the reconcile-required warning. - Final-head [CI run 42404116001](https://github.com/paperclipai/paperclip/actions/runs/42404116001): **PASS**, with all 59 check contexts terminal (55 success and 4 intentional skips). Fresh Greptile review is **5/5** on `c9345b72f27aee1158047dccd0b1060f9a0be6df`, with no unresolved threads; check and thread pagination are complete. ## Risks - The tool must come from one authenticated complete release tree with its own dependencies. A copied bundle chunk can retain old absolute paths and is not a portable release. - Concurrency is not resource admission. The operator must check memory, CPU and scratch headroom and start with one import or two cohort members. - Schema must be prepared by the existing schema owner. This entry never applies DDL. - Cancellation awaits the canonical owner. An SSH interruption does not prove the process stopped; durable journals must be reconciled before retry. - The entry preserves remote obligations and source files. It does not prove enrollment readiness or zero-downtime operation. ## Model Used OpenAI GPT-6 through Codex, with reasoning, repository tools and code execution. The exact serving snapshot and context-window size are not exposed to this agent. ## 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> |
||
|
DottaandClaude Opus 5.5
|
0189fedffc |
fix(native): bound an idle warm local runner whose controller died (#19879)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - A warm runner (`lifecycleMode: warm`) keeps runnerd alive between runs so a follow-up turn starts fast. The follow-up default flip (next PR) makes warm the default for local Codex and OpenCode agents. > - The controller owns a warm runner's idle window. Runnerd ignores the idle timeout itself. > - A graceful controller shutdown suspends idle warm sessions. A controller that dies (SIGKILL, crash) does not. Local runners launch with no process reconnect grace, so the idle runner kept running until its 1-hour connection lease ran out, owned by nobody. > - This pull request gives warm local runners the 7-minute reconnect grace that remote runners already use. > - The benefit: idle runners are reaped by proof within a bounded time, never left orphaned, before warm becomes the default. ## Linked Issues or Issue Description Refs #19874 (warm completion contract), #19846 (TTFT baseline), #19852. **What existing behavior does this improve?** Ownership of an idle warm local runner after its controller dies. **Current behavior** `runnerProcessReconnectGraceMs` is `undefined` for local targets, so runnerd launches without `--reconnect-grace-ms`. An idle warm runner whose controller was killed stays alive until its connection lease (one hour) expires. The restart-chaos harness cannot show this: its K1 kills the controller's whole process tree, runner included. **Proposed behavior** `localRunnerProcessReconnectGraceMs(policy)` returns the remote grace (7 minutes) for a warm policy and `undefined` for per-turn. After the grace, runnerd records `transport_reconnect_grace_exceeded`, stops its provider, keeps durable state and exits. The next run resumes cold. **Reason and benefit** The restart plan requires idle runners to be reaped by proof, never orphaned. Seven minutes is far longer than a controller restart (13–20 s locally) and the long-outage fault holds (3–5 minutes), so a live turn still reattaches. ## What Changed - `native-runner-recovery-contract.ts`: `localRunnerProcessReconnectGraceMs`. - `native-session-executor.ts`: the local runnerd transport passes it as `runnerProcessReconnectGraceMs`. Remote targets keep `null` (lease-owned). - `runnerd-warm-idle-orphan.test.ts` (new): real runnerd and fake Codex. An idle warm runner, after its turn completed, whose controller detaches. - `doc/DEVELOPING.md`: the grace and its bound. ## Verification - `pnpm --filter @paperclipai/paperclip-runner exec vitest run src/live/runnerd-warm-idle-orphan.test.ts`: with a process grace, the idle runner exits with `transport_reconnect_grace_exceeded` and the next transport resumes the same provider session cold. With no grace (`null`, today's local launch), the runner is still alive after the controller left. - `cd server && npx vitest run src/services/native-runtime/native-runner-recovery-contract.test.ts`: warm gets the remote grace; per-turn keeps none; the grace outlasts a controller restart and a long outage. - `tsc --noEmit` passes for `server` and `packages/paperclip-runner`. - Local restart measurement with warm runners (K1/K2, codex, claude, opencode, n = 3 each): every interrupted run recovered, with recovery latency equal or lower than per-turn. The chaos waves with this change are in the default-flip PR. ## Risks - A warm local runner whose controller stays down for more than 7 minutes exits. A turn in progress then resumes through `resume_dead_runner`, the same path as a controller crash that kills the runner. Remote runners already behave this way. - Previous-release story (`rules/upgrade-safe-run-state.mdc`): no persisted state changes. A warm runner launched by the previous release keeps the launch arguments it started with (no grace) until it exits. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`) in Claude Code, with tool use (shell, file edit, test execution). ## 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> |
||
|
DottaandClaude Opus 5.5
|
ff53eb84db |
feat(recovery): add OwnerStamp and proveOwnerGone (gone | alive | unprovable) (#19853)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Restart recovery must decide when a dead process's in-progress record may be released > - About 31 sites decide "owner gone" today, with different rules for EPERM, PID reuse and other hosts (Phase 0 inventory, finding 1) > - The durable ownership plan (section A) asks for one owner stamp and one prover that returns `gone | alive | unprovable` > - This pull request adds that stamp and prover. Nothing calls it yet; the next PR uses it in a report-only reconciler > - The benefit is one tested definition of "owner gone" that later phases move each record kind onto ## Linked Issues or Issue Description Refs #19831 (plan, section A and Phase 1). Refs #19846 (Phase 0 inventory). Refs #19848 (Track 1: 1 s start-time gap and EPERM as alive). Stacked on #19851. **Problem** Each subsystem proves "owner gone" with its own rules. Some accept a dead PID on another host as proof. None returns "unprovable". **Expected behavior** One prover with one set of rules. A different host and an expired fence alone are never proof. "Unprovable" never releases a record. **Impact** This is the primitive that Phase 1's report-only reconciler and Phase 2's migrations use. ## What Changed - New `server/src/services/owner-proof.ts`: - `OwnerStamp`: a local stamp (`host`, `processSpace`, `bootId`, `pid`, `processStartedAt`, `processGroupId`, `generation`, `scope`) or a remote stamp (`fenceToken`, `generation`, `scope`, `provider`). - `proveOwnerGone(stamp, …)` returns `{ verdict: "gone" | "alive" | "unprovable", reason }`. - Local rules: this boot is alive; the stamp must name this host and PID space (host, kernel boot, PID namespace); the PID must be dead or reused (start-time gap of at least 1 s, via Track 1's `processStartsDefinitelyDiffer`); EPERM is alive; the process group must be empty or led by a newer process. - Remote rules: a superseded fence token or generation is gone; a provider stop proof is gone; a held fence is alive; a matching fence that only expired is unprovable. Process liveness is never consulted for a remote owner. - `currentLocalOwnerStamp()` reads only cached identity, so writing a stamp adds no OS call to the turn path. - `controller-process-identity.ts` exports `observedProcessStartedAt` and `UNKNOWN_PROCESS_SPACE` for the prover. - New `owner-proof.test.ts`: the proof matrix (EPERM, ESRCH, PID reuse inside and at 1 s, unreadable start time, a different host, a different or unknown PID space, process groups, a held, superseded, expired or provider-proved remote fence). ## Verification - `npx vitest run src/services/owner-proof.test.ts` (in `server/`): 20 tests pass. - `npx tsc --noEmit -p server`: no errors. ## Risks - Low risk. No caller uses the prover yet, so no behavior changes. - Previous-release story (`rules/upgrade-safe-run-state.mdc`): no persisted shape changes. Stamps from older rows that lack fields return `unprovable` (`missing_identity`), which never releases a record. - Track 1 is still fixing proofs in place. The prover reuses Track 1's helpers (`processExists`, `processStartsDefinitelyDiffer`), so a later Track 1 rule change applies here too. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, in Claude Code with tool use and code execution. ## 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> |
||
|
DottaandPaperclip
|
965ca91dd0 |
Capture legacy workspace cohorts under one source fence (#19873)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work.
> - Workspace migration preserves local files as authenticated portable
snapshots.
> - Workspaces that share Git storage require one complete source fence.
> - Independent imports contend on that fence and cannot safely run as a
batch.
> - This change adds an opt-in cohort owner that captures members
concurrently and publishes them atomically.
> - Existing source, recovery, snapshot and publication checks remain
authoritative.
## Linked Issues or Issue Description
**What existing behavior does this improve?**
The administrative remote-workspace migration owner for workspaces in
one shared physical source component. Related changes: #19688, #19854
and #19855.
**Current behavior**
Each independent import acquires the complete source fence. Parallel
independent calls therefore contend instead of overlapping archive work.
A caller cannot use a surrounding transaction and assume that a
successful savepoint confirms the outer commit.
**Proposed behavior**
An explicit `migrateRemoteWorkspaceCohort` call selects one to eight
leaves from one proven component. It owns the full
source/lifecycle/copyback fence, uses the original pool for bounded
archive work, validates every member, publishes serially, and returns
success only after the outer commit.
**Reason and benefit**
This supplies a correctness-gated path for a controlled two-member
hosted qualification. It makes no wall-clock speedup claim. No running
app call site or scheduler selects cohorts automatically.
**Breaking changes**
The existing single-import API retains its guards and rejects a journal
owned by a cohort. New cohort journals require the same complete member
set on resume.
## What Changed
- Extract the existing reservation, capture, validation and publication
stages for reuse by both owners.
- Add an opt-in cohort API with default concurrency two and a maximum of
eight. Every member must share the same complete physical/Git/alias
component; selected roots must be leaves relative to each other.
- Persist exact cohort and operation membership in the existing journal.
Compare all selected fingerprints with the aggregate proof before the
first reservation under the complete fence, and recheck each member when
reserving its journal. Refuse foreign journals, incomplete or mixed
replay sets, lost fences and changed sources. Keep all authorities and
activity events unpublished if the outer commit fails.
- Share the merged nested-child verifier inside one complete fence.
Queue its prepare calls, retain its two-child bound and authenticated
immutable proof, and recheck local bytes and database bindings on every
pass.
- Document the API, exact-membership resume protocol, resource bounds
and required hosted qualification.
## Verification
- Integration head `fce1896010`: 32/32 PostgreSQL/filesystem cases
across the cohort owner, the complete merged nested-verifier suite, and
five existing parent-import cases.
- Final source-proof repair: 19/19 cohort-owner cases pass, including
real first- and last-member metadata changes before reservation,
changed-source partial-resume refusal, mixed local/imported refusal with
storage forbidden, cancelled/failed/finished admission, a late finished
timestamp, complete fences, queued child-proof reuse, cold restore,
cancellation, commit failure and exact replay. All local database checks
use the canonical owned wrapper, one Vitest worker and Node 24.22.2 on
macOS arm64/PostgreSQL 18.1.
- Final rebased head is based on
`fbe689938c3bf8c5849d2cd56b9da90029db9389`, including current
source-owner, database-schema and Runner contract changes. The 19-case
owner suite, full server layered source/tests/test-tree typechecks,
module boundaries and diff checks are rerun on that base; required CI
supplies the native Runner qualification.
- Selection-only `node scripts/local-verify.mjs --base
|
||
|
DottaandClaude Opus 5.5
|
5a20fca9f7 |
fix(recovery): judge an orphaned run's pid only in the PID space that recorded it (#19871)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The orphan reaper (`terminalizeOrphanedRunningRun`) ends a `running` run whose recorded process is gone > - It judged "gone" with `kill(pid, 0)` on the local host, wherever the process actually ran: another host, another PID namespace, or a sandbox > - So a run whose process is alive elsewhere could be terminalized while it still works > - #19868 fixed the same gap for native coordinator takeover and the runtime-control sweep. This pull request finishes finding 2 for the reaper > - The benefit is that a dead local pid never proves an owner on another host is gone ## Linked Issues or Issue Description **Bug description** The Phase 0 ownership inventory (finding 2) names the orphan reaper `terminalizeOrphanedRunningRun` (`recovery/service.ts`) as a proof that accepts "the pid is dead here" for a process anywhere. `heartbeat_runs` recorded `process_pid` with no PID space, and a sandbox pid could collide with a local one. **Steps to reproduce** 1. A run records `process_pid` on host A, or in a sandbox. 2. On host B, that pid number is not alive. 3. B's stale-lock sweep terminalizes the run as `orphaned_running_run`. **Expected behavior** The process-death authority applies only in the PID space that recorded the pid. Issue-terminal authority is unchanged. Refs #19846, follows #19868 ## What Changed - Migration `0442`: add nullable `heartbeat_runs.process_host_hash`. - `heartbeat/run-lifecycle.ts` `persistHeartbeatRunProcessMetadata`, the one writer of `process_pid`, stores `currentProcessSpaceHash()` for a local pid and `"remote"` for a sandbox pid. - `recovery/service.ts`: the process-death authority runs only when `process_host_hash` is null (rows saved before this column, for one release) or `sameProcessSpace` matches. - Tests: the reaper leaves a dead pid alone for another host, a sandbox, and an unreadable space, and terminalizes it in this space. The writer stamps local and remote pids. ## Verification - `npx vitest run src/__tests__/recovery-stale-issue-lock-sweep.test.ts`: 46 passed. - Red before the fix: with the old `recovery/service.ts`, the three cross-space cases fail (the run is terminalized). Green after. - `pnpm db:generate` produced `0442_low_salo.sql` (one `ADD COLUMN`), and the snapshot set was pruned to five. `pnpm --filter @paperclipai/db check:migrations` passed. Server `tsc --noEmit` shows no errors in the changed files. ## Risks - Previous-release story: runs saved with `process_host_hash = null` keep the previous local proof for this release. That matches #19868, because stranding in-flight runs at upgrade would be a restart failure. New rows are strict. - A run whose process really lived on another host now stays `running` until issue-terminal authority, its controller lease, or another owner settles it. - No new I/O before `turn.start`. The hash is computed at module load, and the stamp rides the existing process-metadata update. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used 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 - [ ] 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> |
||
|
DottaandClaude Opus 5.5
|
00e627a23e |
test(logs): give the 4 MiB archive segment test a 60 s budget (#19877)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Native run logs migrate into a bounded object-storage archive (#19719) > - One integration test migrates a 4 MiB log and reads it back across a segment boundary against embedded PostgreSQL > - On loaded CI runners it exceeds Vitest's 15 s default, so `General tests (server (9/12))` fails on unrelated PRs > - This pull request gives that one test an explicit 60 s budget > - The benefit is that unrelated PRs stop failing on a timing-only timeout, and the test keeps its exact-bytes assertions ## Linked Issues or Issue Description Refs #19719 (the test's origin). **Problem** `native-log-archive.integration.test.ts > preserves exact bytes across the bounded archive segment boundary` times out at 15 s on CI. On 2026-11-07 it failed on at least eight unrelated branches, among them `fix/skill-cache-lock-owner-proof`, `chat-eval/master-candidate`, `fix/subtasks-own-workspace` and `feat/owner-gone-prover` (three attempts). **Expected behavior** The test passes when the code is correct, regardless of runner load. **Impact** It blocks merges on unrelated PRs and wastes reruns. ## What Changed - Give that single test an explicit 60 s timeout, with a comment that names the reason. No assertion changes. ## Verification - `npx vitest run src/services/native-log-archive.integration.test.ts` (in `server/`): 15 tests pass locally. - The CI shard `General tests (server (9/12))` should pass on this PR. ## Risks - Low risk. This is a test-only timeout change. A real hang still fails, after 60 s. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, in Claude Code with tool use and code execution. ## 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> |
||
|
DottaandClaude Opus 5.5
|
00ad9503fe |
feat(commentary): agents read only their own commentary unless granted commentary:view (#19842)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Agents can submit complaints and suggestions as commentary, and
people with commentary access read them on the Commentary page
> - When a person asks an agent in chat what agents have been
complaining about, the agent cannot read commentary and gets "Board
access required"
> - That response does not say where a person can see it, so the agent
can name the feed but cannot link it
> - Product rule: an agent cannot read other agents' commentary unless
it holds `commentary:view`, which is off by default; it may read its own
> - This pull request implements that rule and adds a remediation to the
denial naming the human page for the agent's own company
> - The benefit is a clear permission boundary and a working link for
the person instead of a dead end
## Linked Issues or Issue Description
**What happened?**
In the `activity.agent-feedback` chat eval, the agent correctly said it
could not read agent commentary and that only the person could open it,
but it gave no link, because the denial named no page.
**Expected behavior**
An agent denied a commentary read learns which page a person with
commentary access can open.
**Steps to reproduce**
1. As an agent, call `GET /api/companies/{companyId}/agent-commentary`.
2. Before this change, the 403 body is only `{ "error": "Board access
required" }`.
## What Changed
- `server/src/routes/agent-commentary.ts`:
- An agent with the `commentary:view` grant reads all commentary its
task access allows. The grant is not among the default agent grants.
- Without the grant, an agent may read its own entries: `?agentId=<its
own id>` on the list, or its own entry by ID.
- Without the grant, listing all commentary, filtering by another agent,
or reading another agent's entry returns 403 "Board access required".
The response's `remediation` names `/<issuePrefix>/activity/commentary`
for the agent's own company.
- A request for another company still fails on company access first,
with no page path.
- Board reads are unchanged: `commentary:view` for the user, plus task
read conditions.
- `agent-commentary.integration.test.ts`: one test covers the full-list
and other-agent denials with the page path, own-list and own-entry
reads, the cross-company denial, the grant allowing a full list, and
that `commentary:view` is not a default agent grant.
## Verification
- `npx vitest run src/__tests__/agent-commentary.integration.test.ts` in
`server`: 21 tests passed.
- The new test fails against the previous route.
- Negative control: with the previous route, the new test fails.
- Related commentary tests: `agent-commentary-reader`,
`agent-commentary-helper`, `commentary-permission-migration` passed.
`agent-commentary-skills` fails the same way with and without this
change (a missing skill file in a temporary directory locally).
- The chat eval rerun with this change is recorded in #19614.
## Risks
- Agents can now read their own commentary when they ask for it
explicitly; before, every agent read was denied.
- The denial's status code and message are unchanged; a `remediation`
string is added for same-company agents.
- There is no UI yet for granting `commentary:view` to an agent.
- An agent learns the company's issue prefix, which it already sees in
task identifiers.
> For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and
discuss it in `#dev` before opening the PR. Feature PRs that overlap
with planned core work may need to be redirected — check the roadmap
first. See `CONTRIBUTING.md`.
## 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
- [ ] 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>
|
||
|
DottaandClaude Opus 5.5
|
05e3309740 |
fix(accounting): settle a failed usage capture as unpriced at the closure bound (#19857)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Every finished run settles its usage into the cost ledger. Until it settles, budget admission makes new company work wait > - When usage capture fails (`accountingCaptureFailed=true`), accounting skips the run forever. It is also not "recovering", so it counts in `waitingRunCount` > - So one failed capture holds new company work until the UTC budget month rolls over > - This pull request bounds that wait with the existing accounting closure timeout. It then settles the run's known usage as unpriced, with provenance that names the capture failure > - The benefit is that admission recovers by itself, while unknown spend stays visible and still counts under a blocking unpriced-usage policy ## Linked Issues or Issue Description **Bug description** The Phase 0 ownership inventory (finding 3) found that runs with `accountingCaptureFailed=true` are excluded from accounting (`run-cost-accounting.ts` `actionableRunAccounting` and `accountRunCost`). They are also excluded from "recovering" in `budget-exposure.ts`, so they count as waiting until the month window rolls over. Nothing else releases them. **Steps to reproduce** 1. Finish a run whose usage capture failed (for example, a fence write failure or usage after a source handoff). 2. Wait any length of time and run `reconcileRunCosts`. 3. The run stays `costAccountingPending`, and `getInvocationBlock` keeps holding new company work. **Expected behavior** After a bounded wait, the run settles with its known usage marked unpriced, and held work resumes. A blocking unpriced-usage policy still treats the run as unknown spend. Refs #19846 ## What Changed - `run-cost-accounting.ts`: new `captureFailureBoundPassed`. It applies to a terminal capture-failed run whose native coordinator (if any) settled, after `ACCOUNTING_CLOSURE_TIMEOUT_MS`. This is the same bound used for missing closure evidence. - `actionableRunAccounting` selects these runs, and `readCostAccountingWork` schedules a wake at their bound. A past bound never schedules a wake, as before. - `accountRunCost`: a capture-failed run is never treated as pre-provider work. Before the bound it stays pending. After the bound it settles through the existing lost-receipt path as `receiptUnavailable` + `closureTimedOut`: known cost becomes a lower-bound estimate, otherwise the charge is unpriced, and the run gets `accountingClosureTimedOut: true`. - Provenance evidence and the `cost.accounting_closure_timed_out` activity say that the usage capture failed (`details.captureFailed: true`). - Tests: three new cases in `closed-native-receipt-accounting.test.ts`. `cost-data-quality.test.ts` now pins both sides of the bound for a board retry. ## Verification - `npx vitest run src/__tests__/closed-native-receipt-accounting.test.ts`: 54 passed. - `npx vitest run src/services/run-cost-accounting.test.ts src/__tests__/idle-accounting-work.test.ts src/__tests__/cost-data-quality.test.ts`: 40 passed. - Red before the fix: with the old `run-cost-accounting.ts`, all 3 new capture-failure cases fail. The run never settles, nothing schedules a wake, and work stays held. Green after. - Hard stop: "keeps a settled capture failure under the blocking unpriced-usage policy" shows that a blocking policy still blocks after settlement. Real observed spend (`budgetExposure`) counts toward the hard stop before and after. - Server `tsc --noEmit` shows no errors in the changed files. ## Risks - Behavior change: capture-failed runs no longer stay pending forever. The 2026-10-28 Cursor runner plan recorded "capture failures remain pending" as an interim rule. This PR keeps them pending through the closure bound and then settles them as unknown spend, never as free. A complete receipt that arrives later still reconciles through late-receipt handling. - A board retry of a capture-failed run past the bound now settles it. Before the bound it is still refused. - Previous-release story: no persisted shape changes. Existing capture-failed rows past the bound settle on the next reconcile pass. That can release work that was held, and under a blocking policy it records unknown spend that blocks. - No new waits or I/O before `turn.start`. This is background accounting only. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used 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 - [ ] 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> |
||
|
DottaandPaperclip
|
4d5265fe7e |
Preserve fully covered historical workspace sources (#19872)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Workspace migration must preserve local files while retaining unfinished recovery obligations. > - A shared Git source needs proof for its companion workspaces before import can publish. > - Recovery backfill can add a historical-source marker to an exhausted prepared-export hold. > - The hold owner rejected that marker even when every recorded allocation already belonged to its proven prepared source. > - This pull request accepts only that fully validated form through the existing source owners and publication fences. > - Unrelated work can continue while import saves the local files and keeps the remote recovery hold intact. ## Linked Issues or Issue Description Refs #19830. No separate public issue was found. A search for the marker found no open duplicate PR. **What happened?** A companion workspace with an exhausted prepared-export hold blocked the whole shared Git cohort after canonical recovery backfill added its historical-source marker. The prepared and historical source readers both accepted the same allocation. **Expected behavior** Local file preservation can proceed when all historical references are already covered by the proven prepared source. The recovery hold and remote obligations remain unchanged. Unknown or uncovered sources stay blocked. **Steps to reproduce** 1. Create a shared Git repository with a selected leaf and a companion workspace. 2. Give the companion a settled failed export, accepted result, exhausted recovery and blocked task. 3. Run canonical historical recovery backfill for its retained allocation. 4. Import the selected leaf through the normal online migration owner. **Paperclip version or commit** Reproduced on `4185e308659abf0887930e20049ca6e7cbfc0e81`. **Deployment mode** A source installation with legacy local Git workspaces and object storage. ## What Changed - Validate the exact historical marker schema, run and unique lease references against the already proven prepared source and declared allocations. - Reuse the canonical historical-source inspector and compare the same provider allocation and remote working directory. - Reject unknown fields, malformed markers and foreign, duplicate or uncovered references. - Keep the full marker in the existing hold fingerprint and retain the existing source, task, lifecycle, finalizer and publication checks. - Add real PostgreSQL/Git tests through shared-cohort inspection and the encrypted import/restore owners. ## Verification - The unchanged owner failed the actual selected-leaf importer regression with `online_workspace_source_scope_unproven`. The canonical prepared/historical reader controls passed before that refusal. - Production head `3e678e10fa` passed 41 focused PostgreSQL/Git cases across three files in 18.93 seconds after rebasing onto the new finalizer schema migration. This includes 14 new cases; 80 unrelated cases were filtered out. The final test-only review update passed both additional multi-allocation controls plus the original selected-leaf cold restore (3/3, 11.29 seconds); TypeScript passed again. - The positive uses the actual historical backfill producer. It cold-restores an encrypted selected-leaf snapshot with the original Git tree unavailable and keeps the peer's task, finalizer, result, allocation and recovery action unchanged. - The tests retain an unrelated active object-backed run, prove the peer finalizer lock is held, refuse bad bindings and reject late marker changes without discarding the uploaded snapshot. - Server TypeScript, module-boundary checks and `git diff --check` passed. The canonical PostgreSQL wrapper exited and cleaned up. - Final-head CI passed: 56 successful checks and four intentional skips, with no failures or pending jobs ([run](https://github.com/paperclipai/paperclip/actions/runs/42397090541)). Greptile reviewed `0cbf0a0255` at 5/5 with no open findings; GitHub reports CLEAN/MERGEABLE. ## Risks - The allowance applies only to the existing exhausted prepared-export hold. Other recovery forms and unknown or uncovered references remain blockers. - A marked hold adds database reads proportional to its already declared allocations during maintenance inspection. It adds no provider calls, payload download or recurring background work. - Import saves the current local files. It does not claim that unsaved remote files were recovered, that a provider stopped or that execution may resume. Existing holds and warnings remain authoritative. - Local tests use the real encryption and restore owners with a local object backend. Hosted backfill and normal-delivery qualification remain separate. ## Model Used OpenAI Codex, based on GPT-6, with reasoning, repository tools and code execution. The exact serving snapshot and context-window limit are 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 #` 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> |
||
|
DottaandClaude Opus 5.5
|
51c352ec2e |
fix(skills): release a runtime skill-cache lock whose holder provably exited (#19849)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Before a run starts, the server builds a read-only cache of each company skill under a cross-process file lock > - The lock records only a pid and a hostname, so a crash leaves a lock that nothing can release > - If the holder pid is dead on the same host, the next run fails and asks for manual cleanup. If the host name differs, the next run waits 60 s and then fails > - This is on the turn path, so one crash during a skill build breaks later runs that use that skill > - This pull request records the shared controller process identity in the lock and releases the lock at once when `controllerProcessExited` proves the holder is gone > - The benefit is that a crash during a skill build no longer blocks later runs, and the release never relies on lock age or another host ## Linked Issues or Issue Description **Bug description** The Phase 0 ownership inventory (finding 4) lists the runtime skill-cache lock (`runtime-skill-cache.ts`) as a record that nothing releases automatically. When a publisher crashes while it holds `<skillId>.lock`, every later preparation of that skill fails. **Steps to reproduce** 1. Start a skill cache build, then kill the server process while it holds the lock. 2. Restart and start a run that uses the same skill. 3. Preparation fails with "Runtime skill cache publisher exited; remove its stale publication lock before retrying". For a holder whose host name differs, it fails after 60 s with "publisher is busy". **Expected behavior** A lock whose holder has provably exited is released, and the build continues without waiting. A holder that cannot be proven gone is still waited for. Refs #19846 ## What Changed - `controller-process-identity.ts`: add `currentControllerProcessStamp()`. It returns this process's identity with no I/O, so the turn path gets no new wait. - `runtime-skill-cache.ts`: the lock owner file now also stores `controller` (boot id, pid, PID-space hash, start time if already known). `pid` and `host` stay so older readers still work. - New `runtimeSkillCacheLockHolderExited`: true only when `controllerProcessExited` proves the holder gone. Another host or PID namespace, an unknown space, a legacy lock without `controller`, a live pid, and this process all return false. - On proof, waiters race for one exclusive claim named by the stale holder's unique token (`<lock>.released-<token>`). Only the winner unlinks the lock, and only while it still has the stale inode. The claim is kept, so a late waiter does nothing. The waiter then retries the link right away instead of after the 20 ms poll. - A live holder is re-proved at most once a second. `controllerProcessExited` skips the start-time probe (`ps` on macOS) when no start time was recorded. - The busy error now names the lock path. The same-host "publisher exited" hard failure is removed. ## Verification - `npx vitest run src/__tests__/runtime-skill-cache.test.ts src/services/controller-process-identity.test.ts`: 28 passed, rebased on master after #19848. - New case "takes over a release claim whose claimer exited before removing the stale lock" covers the second Greptile finding. - New case "never removes a newer lock for a waiter that read a stale one late" covers the Greptile P1. - New case "releases a publication lock at once when its holder provably exited". On the old code it fails with "Runtime skill cache publisher exited; remove its stale publication lock before retrying". On the new code it builds in well under 5 s and leaves no lock. - New case "never treats age, another host, or a legacy holder as proof": a different host hash, a legacy `{pid, host}` lock, a live pid, this process, and malformed content all stay held. - Server `tsc --noEmit` shows no errors in the changed files. ## Risks - Previous-release story: a lock written by the previous release has no `controller` field. It is unprovable. The waiter waits 60 s and fails as before, and the error now names the path to remove. A new lock still carries `pid` and `host`, so a previous-release reader behaves as before. - Lock release race: while a stale lock exists, only the single claim winner can remove it, and the inode check cannot match a newer lock because the stale holder's owner file stays and pins that inode. A claim whose claimer crashed is itself released by the same proof (depth bounded at 2). - Each released lock leaves one small claim file and the dead holder's owner file next to it. - No new I/O before `turn.start`. The stamp is synchronous. The proof runs only when the lock is contended. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used 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 - [ ] 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> |
||
|
DottaandClaude Opus 5.5
|
738058553f |
fix(recovery): resume a continuation that never launched its runner in its own sandbox (#19850)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A chat follow-up on a remote native task runs as a continuation: it starts from its predecessor's saved provider session and, for a warm session, copies the predecessor's runner process identity onto the new run > - When the continuation fails before it asks for a runner of its own, its due same-run retry sees a recorded process identity and reattaches to "this run's" runner. That runner never existed, so reattachment always fails with `native_remote_recovery_lease_mismatch` and the task is blocked > - On staging, 34 OpenCode runs on 31 tasks failed this way in one evening. Each first attempt failed while staging its provider pack, recorded no runner start, and kept its own sandbox lease active > - This pull request proves that shape from durable state, retires the inherited identity, and resumes the run the way its failed attempt ran, in its own retained sandbox > - The benefit is that these retries recover automatically, without a second sandbox and without the unsaved-work hold, while every run that may hold its own provider work keeps verified reattachment ## Linked Issues or Issue Description Refs #19839, Refs #19399, Refs #19755, Refs #19779, Refs #19827 **What happened?** A remote OpenCode chat continuation failed during provider-pack staging. The run had no runner start event, no process identity event, no controller checkpoint, and no provider event of its own. Its saved session (no active turn, no pending request) and its process identity were inherited from the previous run. Its failure detail recorded `providerEventsExist: false`, and its own sandbox lease stayed active. The due same-run retry chose `reattach_remote_runner` because a process identity was recorded. Reattachment failed with `native_remote_recovery_lease_mismatch`, and the run ended in `native_continuation_requires_reconciliation`. **Steps to reproduce** 1. Continue a warm remote native session so the new run inherits the predecessor's runner process identity and saved session. 2. Fail the new run with a retryable error before it requests a runner (for example during provider-pack staging). 3. Let the same-run retry become due. Recovery requests `reattach_remote_runner`, and reattachment fails. **Expected behavior** The retry resumes the run the way its failed attempt ran, in the same sandbox, and continues the inherited provider session. A run with any provider work, runner start, or controller checkpoint of its own keeps verified reattachment. ## What Changed - New `server/src/services/native-runtime/native-inherited-runner-retry.ts`: - `retireInheritedRemoteRunnerIdentity` locks the issue, the coordinator, then the run (the same order as same-run resumption). It retires the recorded process identity only when all of these hold: the coordinator is a result-less, unowned `exact_checkpoint_resume` retry with `providerEventsExist: false`; the run has no runner start, process identity, adapter invocation, tool start, runner event, controller checkpoint, provider event or provider session of its own; the saved session has no active turn and no pending runtime request; and a settled predecessor of the same native session recorded the exact same process identity (PID, group, start time). - It records `nativeInheritedRunnerRetirement` on the run profile, with the run's own sandbox location when that lease is still active, and an `inherited_runner_never_launched` lifecycle event. - `readInheritedRunnerRetainedSandbox` returns that sandbox only while it still matches the run's saved remote location. - `heartbeat/recovery.ts`: a due remote retry calls the retirement first. It returns `retired`, `not_applicable` or `transient`. A retired run (including one a concurrent sweep already retired) goes to the ordinary same-run resume (`claimNativeSessionResumptions`), with every existing admission gate. A lock timeout or database error is `transient`: the sweep defers the run and checks again, and never falls back to reattachment. Any other run keeps verified reattachment. - `heartbeat/workspace-preparation.ts`: an ordinary same-run resume with a retired inherited runner acquires its retained sandbox through the existing exact-lease path (`reattachRemoteLease`), which validates and resumes that lease and never acquires a replacement. It acquires as an ordinary resume does only when the exact-lease failure proves the sandbox is unusable (`stage=environment_acquire|environment_resume`, `field=lease|lease_status|lease_expired|provider_lease_id`). Any other failure, such as a scope mismatch on a live lease, is kept and acquires nothing. - New `server/src/services/retained-sandbox-absent-error.ts`: a sandbox provider's resume reply with no provider lease and `expired: true` (the reply Daytona and other providers send when the sandbox is gone) now throws `RetainedSandboxAbsentError`. Every other mismatched reply keeps the existing error. The exact-lease reattachment in `environment-run-orchestrator.ts` maps it to `native_remote_recovery_lease_mismatch: stage=environment_resume field=lease_expired`, so the same-run resume above acquires as an ordinary resume when the provider has removed the retained sandbox. A transient resume failure is not treated as absence and never acquires another sandbox. - `native-restart-recovery.ts` exports the existing `hasNativeExecutionStartEvidence` so both paths use one definition of "execution reached a runner". - `doc/execution-semantics.md` documents the rule. ## Previous-release state - No schema change. The new profile field is additive. - A run saved by an older release without `providerEventsExist`, without a native session ID, or without a matching predecessor keeps verified reattachment. That path is unchanged. When it loses its runner, it still requests the user's Retry once (#19399). - A run that an older release left with its own runner start, controller checkpoint or provider event keeps verified reattachment, because its outcome may be ambiguous. - A profile without the new marker never changes acquisition. ## Verification - `npx vitest run server/src/services/heartbeat/recovery.test.ts -t "never launched its own runner|keeps verified reattachment"` (embedded PostgreSQL): - Staging shape (inherited session and process identity, no runner start of its own, saved remote workspace, active own lease, recorded PID). Before the fix: fails, because recovery requested reattachment. After the fix: passes. The PID fields are cleared, the marker names the predecessor and the retained lease, `executeRun` is called once with a plain `nativeLeaseOwner` claim, the run still has exactly one (active) lease, and no process is signalled. - Negatives, passing before and after: a provider turn of its own, a runner start of its own, a controller checkpoint of its own, no matching predecessor, a pending runtime request in the saved session, and a failure detail without `providerEventsExist`. Each keeps verified reattachment and gets no marker. - `npx vitest run server/src/services/heartbeat/workspace-preparation.test.ts -t "retained sandbox|retired inherited"`: - A retired inherited runner acquires its retained sandbox with `reattachRemoteLease`. Before: fails. After: passes. - When the retained sandbox is gone, it acquires as an ordinary resume. Before: fails. After: passes. - Without the marker, acquisition is unchanged (passes before and after). - A `nativeWorkspaceSync` reference also resumes its retained sandbox. A live lease's `lease_scope` failure is kept and acquires nothing. - `recovery.test.ts` also covers a second retirement call (returns `retired`, one event) and a held issue lock (defers; no reattachment, no dispatch, PID kept). - `environment-runtime.test.ts`: on exact resume, only a provider reply with no lease and `expired: true` (with or without a workspace sentinel mismatch) is `RetainedSandboxAbsentError`; a different lease or a reply without `expired` keeps the existing error. `environment-run-orchestrator.test.ts`: confirmed absence becomes `stage=environment_resume field=lease_expired` and acquires nothing. - Affected suites pass locally: `recovery.test.ts`, `workspace-preparation.test.ts`, `heartbeat-native-remote-failure-retention.test.ts`, `workspace-recovery-retry-copyback.test.ts`, `native-runner-restart-recovery.integration.test.ts`. `pnpm --filter @paperclipai/server typecheck` passes. ## Risks - **No second executor.** The rule applies only when this run never requested a runner, so no runner of this run exists. The predecessor must be settled, and its identity is matched exactly. The resume then takes the normal same-run claim, which holds the issue lock and admission gates. - **No repeated side effects.** The run has no provider event, turn or tool start of its own. A saved session with a pending runtime request or an active turn keeps verified reattachment. The resumed provider session goes through the existing provider recovery, which refuses a pending side-effecting call with a permanent, named code. - **Bounded.** The retirement takes effect once per run, because afterwards the run has no recorded identity. A contended retirement waits for the next sweep. Further failures use the existing three-attempt provider budget. - I did not run a live Daytona sandbox, or a full preparation through a real sandbox plugin, through this path. The orchestrator's exact-lease checks are covered by its own unit tests. The evidence is recovery and preparation tests on a real database with the staging run shape. ## Model Used - Claude Opus 5.5 (Anthropic), model ID `claude-opus-5-5`, run in Claude Code with tool use (shell, file editing, read-only browser access to staging). ## 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> |
||
|
Devin FoleyandClaude Opus 5.5
|
de82f93443 |
fix(storage): retry S3 reads that the provider times out with HTTP 408 (#19039)
Mark HTTP 408 on GET/HEAD as retryable inside the SDK retry middleware so the standard backoff, retry quota and three-attempt limit apply. Writes are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
68db7ed9e3 |
fix(recovery): never accept a dead local pid as proof for an owner on another host (#19868)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - After a restart, recovery decides that an in-progress record's owner is gone and takes it over > - Two of those proofs use `kill(pid, 0)` on the local host. A pid that is dead here says nothing about a process on another host or in another PID namespace > - So a controller or runtime-control owner that is still alive elsewhere can be taken over while it still runs > - This pull request records the owner's PID space (the existing `controller-process-identity` host hash) and judges a pid only in that same space > - The benefit is that a different host is never proof. Those records follow their existing lease or TTL instead ## Linked Issues or Issue Description **Bug description** The Phase 0 ownership inventory (finding 2) found three proofs that accept "the pid is dead here" as proof for an owner on another host: `evaluateNativeControllerTakeover` (`native_run_finalizations` has no host column), the runtime-control sweep (`workspace-operations.ts`), and the orphan reaper `terminalizeOrphanedRunningRun`. Only `controllerProcessExited` checked the host. **Steps to reproduce** 1. Run two control-plane processes on different hosts, or in different PID namespaces, against one database. 2. Let host A own a native finalization lease or a runtime-control operation. 3. On host B, run restart recovery. If A's pid number is not alive on B, B takes over the live owner. **Expected behavior** An owner whose PID space differs from this process's, or cannot be read, is unprovable. Its record waits for its lease, fence, or TTL. Refs #19846 ## What Changed - Migration `0441`: add nullable `native_run_finalizations.controller_host_hash`. - `controller-process-identity.ts`: export `currentProcessSpaceHash()` and `sameProcessSpace()`. An unreadable space is stored as `"unknown"`, which never matches. - `native-restart-recovery.ts`: `currentNativeControllerIdentity` carries `hostHash`, which both coordinator claim writes (`native-restart-recovery.ts`, `native-session-executor.ts`) store. `evaluateNativeControllerTakeover` returns `controller_host_unprovable` before any pid probe when the recorded space is not this one. `native-dead-controller-result-lease.ts` selects the column. - `workspace-operations.ts`: runtime-control owner stamps record `hostHash`. `ownerProcessProvenGone` requires the same PID space (EPERM = alive), otherwise the existing staleness TTL applies. - Not in this PR: the orphan reaper `terminalizeOrphanedRunningRun`. `heartbeat_runs` has no host for `process_pid`, and its writers are spread across legacy and native paths. It follows as a separate PR with its own column. ## Verification - `npx vitest run src/services/native-runtime/native-restart-recovery.test.ts src/__tests__/workspace-runtime-control-recovery.test.ts`: 55 passed. New cases cover another host and an unreadable space (dead and recycled pid) for takeover, plus another host, unreadable space, and legacy stamp for the sweep. The sweep cases also confirm that the TTL still reclaims after the heartbeat lapses. - Red before the fix: the 5 new cases fail on the old code. Green after. - `native-controller-recovery.integration.test.ts`, `native-failed-run-owner-release.test.ts`, `native-startup-cleanup-recovery.test.ts`: 71 passed. - `pnpm db:generate` produced `0441_lucky_chameleon.sql` (one `ADD COLUMN`) and pruned to five snapshots. `pnpm --filter @paperclipai/db check:migrations` passed. Server `tsc --noEmit` shows no errors in the changed files. ## Risks - Previous-release story: `native_run_finalizations` rows saved before this column have `controller_host_hash = null`. They keep the previous local dead-pid proof for this release. Making them unprovable would leave every native run in flight at the upgrade in `awaiting_evidence` with no TTL, because local takeovers have no lease fallback. Rows written by this release onward are strict. Runtime-control stamps without `hostHash` are strict now, because that path has a staleness TTL. - Owners built from other receipts (workspace finalization owner, workspace stop proof) pass no host and keep the previous behavior. Track 2's shared `OwnerStamp` should absorb them. - A new row on a host that really moved (redeploy onto a new machine) waits for its lease or an operator instead of being taken over by pid. That is the intended "different host is never proof" rule. - No new I/O before `turn.start`. The hash is computed once at module load. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used 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 - [ ] 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> |
||
|
DottaandPaperclip
|
4185e30865 |
Recover archived commits from original bare sources (#19855)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Workspace migration must preserve saved files before the web server releases its local volume. > - An archived Git checkout can retain a trusted branch-deletion receipt after its private clone and generated project cache are removed. > - The original repository can still exist at its recorded file URL, but the archive owner did not check that source. > - This pull request permits a narrow, verified original bare-repository source for the exact recorded commit. > - The existing encrypted snapshot and publication owners then preserve the workspace through their normal migration path. ## Linked Issues or Issue Description No public issue exists. Searches for the archived-source error and original bare Git source found no duplicate change. **What happened?** Migration reported `archived_git_source_unavailable` for an archived checkout even though its original bare repository still contained the exact saved commit. The private clone and generated cache were absent. **Expected behavior** The canonical owner can use the recorded original local repository when its full commit identity and source ownership are proven. Uncertain or changed sources remain blocked. **Steps to reproduce** 1. Create an isolated checkout from a local bare repository. 2. Close it through the normal cleanup owner and retain the successful branch-deletion receipt. 3. Remove the private clone and generated cache, while retaining the recorded original repository and full base revision. 4. Inspect the archived workspace for migration. **Paperclip version or commit** Reproduced on `84908d176b2800f4c792c4249c610ec0baebdd28`. **Deployment mode** A source installation with existing local Git workspaces and shared object storage. ## What Changed - Consider the exact configured file-URL repository only after the canonical primary source is definitively absent. - Require a matching source record and successful cleanup receipt, the exact full base/deleted commit, a contained bare SHA-1 repository, bounded standard configuration and alias-free metadata. - Restrict this source path to local Git transport and a minimal environment. Reconstruct the tree only in private migration scratch. - Retain existing source fingerprints, journal identity, encrypted upload/readback and final publication checks. Lock and recheck the configured source and cleanup record before publication. - Add real PostgreSQL/Git coverage for encrypted cold restore, lost-reply retry and refused source or publication changes. ## Verification - The unchanged owner failed the real four-object original-source regression with an ineligible inventory. - The review regressions failed before each repair: four returned-primary retry/publication cases and two actual production metadata/reaper cases. - Final head `8925519310` passed all 40 focused PostgreSQL/Git tests in 56.38 seconds. Four positive formats use canonical/legacy metadata and ordinary/verified-tip cleanup. Each imports an encrypted snapshot, removes the original file URI from reach, then authenticates and cold-restores exact files, executable mode and Git history. - The same file covers source/receipt/writer guards, changed sources before publication, authenticated lost-reply reuse and 18 returned-primary directory/alias/unreadable cases. Refused publication retains returned edits and uploaded ciphertext. - Server TypeScript, module-boundary checks and `git diff --check` passed on this revision. Canonical test wrappers exited and cleaned up. - An earlier focused run also passed 22 existing archived-Git/restore-point controls. `pnpm verify --explain` completed without executing extra suites. Required CI supplies final broad coverage. - Final head passed all 60 CI/review contexts: 56 successful checks and four expected skips. Greptile reviewed this exact head at 5/5, with no open review threads. - One unchanged archive segment-boundary test timed out on both framework attempts in the initial server shard. The one job-only retry passed with unchanged code and timeout. The test and its resolved local import graph match a green control revision. The original failures remain retained; the cause is unresolved. ## Risks - The fallback is intentionally narrow. Original repositories with a different full tip, unsupported configuration, aliases (including an aliased instance root), external object storage or more than 10,000 metadata entries remain blocked. - The first source check and final publication check add bounded local filesystem and Git work to this maintenance path. They add no recurring worker or network Git access. - Original repository files remain intact. Importing a retained workspace does not make its configured local file URL portable for future fresh project work. That separate configuration dependency still requires preservation and qualification before volume removal. - The local regression uses the real encryption and restore owners with a local object-storage backend. It does not claim a hosted backfill or a deployed migration. ## Model Used OpenAI Codex, based on GPT-6, with reasoning, repository tools and code execution. The exact serving snapshot and context-window limit are 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 #` 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> |
||
|
DottaandClaude Opus 5.5
|
a9ac2e2039 |
feat(native): split local runner startup into stage, launch and session-load spans (#19852)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - When a person messages an agent, the agent must start its turn fast. Time to first token (TTFT) and agent boot are first-class metrics. > - The Phase 0 baseline (#19846) found that local runner startup costs 1.6–17 s at p50 and up to 50 s at p95. It is the largest part of TTFT. > - The run log records that startup as one span (`runner.session.bootstrap` or `runner.session.resume`). We cannot see if the time goes to staging, to the `paperclip-runnerd` process launch, or to the provider session load. > - This pull request splits local runner startup into three sub-spans in the run log. > - The benefit: the follow-up work to cut follow-up-turn TTFT can measure each step before and after. ## Linked Issues or Issue Description Refs #19846 (TTFT baseline, finding 1 and gap "No sub-spans exist inside local runner startup"). Refs #19838. **What existing behavior does this improve?** The run-log performance spans for local native runner startup. **Current behavior** The local path records one `runner.session.bootstrap` or `runner.session.resume` span that covers runtime staging, the runner process launch and the provider handshake together. The remote path already records `runner.runtime.stage` and `runner.process.launch`; the local path does not. **Proposed behavior** The local path records three sequential child spans of `runner.session.startup`: `runner.runtime.stage`, `runner.process.launch` and `provider.session.load`. **Reason and benefit** The TTFT work needs to know which step dominates before it changes the runner lifecycle. Without these spans, a change to warm runner reuse cannot show which cost it removes. ## What Changed - `RunnerdCodexTransport` has a new optional `onStartupStep` callback. `#start` and `#resume` time three milestones and call it synchronously. A failing callback never changes startup. A nested startup (exact adoption from `thread/start`) keeps the outer clock. - `runner.runtime.stage`: launch preparation before the runner process exists. - `runner.process.launch`: spawn until runnerd accepts the run (`run.prepare`, or the recovery attachment on resume). An adopted live runner records no launch. This keeps the existing meaning in `summarizeStartupReuse`: a launch span means a fresh runner. - `provider.session.load`: `session.open` (or the recovery probe) until the provider identity is confirmed. - `recordNativeRunnerStartupStep` (`native-run-trace.ts`) records each step as a `run.performance.span` row with `mode` and `target: "local"`. It uses `trace.defer`, so the run-log write never delays `turn.start`. - The native executor passes the callback only for a local target. Remote launchers keep their own spans, so there are no duplicates. - `redaction.ts`: allowlist `runner.process.launch` and `provider.session.load` as native span names. - `doc/run-log-events.md`: new section "Local runner startup sub-steps". Data path (AGENTS.md §5.7): run log only, plus Observability spans when an operator sets an OTLP endpoint. The attributes (`mode`, `target`, `outcome`) are already in the closed span-attribute allowlist. No Telemetry change. ## Verification - `pnpm --filter @paperclipai/paperclip-runner exec vitest run src/live/runnerd-codex-transport-startup-steps.test.ts`: a real runnerd and the fake Codex fixture. Bootstrap and a cold resume each report stage, launch and session load in order, with `ok` outcomes and no overlap. A throwing sink does not change startup. - `cd server && npx vitest run src/services/native-runtime/native-run-trace.test.ts src/__tests__/redaction.test.ts`: the rows have the correct parent, offsets, durations and failed level. A stalled run-log sink does not block the caller. The rows settle before `task.provider_session.measured`. The names survive redaction. - The existing `runnerd-codex-transport*` suites pass (412 tests). Under host load (load average 16–27), the first full run had 7 timing failures in suites that do not set the callback. Each failing file passed when I ran it again alone. `runnerd-codex-transport-settlement` passed 3/3 for the failing case alone. - `tsc --noEmit` passes for `packages/paperclip-runner` and `server`. ## Risks - Low risk. Without `onStartupStep`, the transport behaves as before: the clock is not created and each milestone call is an optional-chain no-op. - More run-log rows: three per local runner startup. - The step boundaries are controller-side observations. `runner.process.launch` includes the runnerd authentication and the `run.prepare` acknowledgement, not only the OS spawn. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`) in Claude Code, with tool use (shell, file edit, test execution). ## 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> |
||
|
DottaandClaude Opus 5.5
|
e8a418cc4d |
fix(workspaces): settle an uncertain remote provisioning row once its sandbox is stopped (#19862)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A remote task workspace runs the project's provisioning commands in a sandbox. Each command has a `remote_workspace_provision` journal row, because an arbitrary command cannot be replayed after an ambiguous reply > - A row left `running` with `uncertain: true` blocks every later reservation of that workspace, and nothing ever settles it > - A crash or lost reply during provisioning therefore leaves the task's workspace unusable, with no next action > - This pull request settles such a row only when the earlier run's sandbox is provably stopped. Otherwise the 409 names the row, its run, and the operator's action > - The benefit is that a crashed provisioning attempt no longer locks the workspace forever, and the effect is never assumed to have succeeded ## Linked Issues or Issue Description **Bug description** The Phase 0 ownership inventory (finding 4) lists `remote_workspace_provision` rows left `running`/`uncertain` (`remote-workspaces.ts`) as a record with no automatic release. `reserveRemoteWorkspace` rejects every other run with `remote_workspace_provision_unverified`. A command that simply exited non-zero also stayed `uncertain`. **Steps to reproduce** 1. Start remote provisioning and lose the runner's reply (or have the command exit non-zero). 2. End the run and stop its sandbox. 3. Start another run on the task. Reservation fails with `remote_workspace_provision_unverified`, and the next attempt fails the same way. **Expected behavior** After the earlier sandbox is provably stopped, the row settles and the workspace can be reserved again. While that cannot be proven, the error says what to do. Refs #19846 ## What Changed - `remote-workspaces.ts` `reserveRemoteWorkspace`: for each uncertain provision row from another run, `provisionOwnerProvenStopped` requires two things. The row's run is terminal, and every environment lease of that run carries a provider stop or destroy receipt (`hasRemoteTerminationReceipt` or `hasRecoveredCleanupStopReceipt`). A proven row is settled as `failed` (`uncertain: false`, `settledBy: "owner_sandbox_stopped"`) and logged as `workspace.remote_provision_settled`. It is never marked succeeded. - If a later reservation check (for example a writer hold) rejects the attempt, the proven rows are re-proved and settled in their own transaction, so they do not block again. - An unprovable row still blocks. The 409 now carries `operationId` and `runId` and names the action: stop the earlier sandbox, or set the task's Workspace to "New isolated workspace". That picker option already exists in `IssueProperties`. - `provisionRemoteWorkspace`: a reported non-zero exit (not a timeout) is recorded as `failed` with its `exitCode` instead of staying uncertain. ## Verification - `npx vitest run src/services/remote-workspaces.integration.test.ts src/services/remote-workspace-save-concurrency.test.ts`: 126 passed. - New case "settles an uncertain provisioning command once its run's sandbox is provably stopped". A failed run alone still gets the 409 with `operationId`/`runId`. After a stop receipt, the row settles, and the activity is logged once. - New case "records a provisioning command that reported a failing exit as finished, not uncertain". - Red before the fix: with the old `remote-workspaces.ts`, both new cases fail. Green after. - Server `tsc --noEmit` shows no errors in `remote-workspaces.ts`. - Not run: a live browser check of the "New isolated workspace" path for this error. I confirmed in code that it reserves a different workspace, so it bypasses this check. ## Risks - A settled row means "effect unknown". The next writer provisions again in its own epoch, as every new writer already does. A partially applied, non-idempotent command could see its partial effect. That is the same exposure as any failed provisioning, and it happens only after the earlier sandbox has stopped, so the two commands never run at the same time. - Previous-release story: no persisted shape changes. Uncertain rows saved by the previous release are evaluated by the same proof on the next reservation. - No new waits before `turn.start`. The proof runs only when an uncertain row exists, inside the existing reservation transaction. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used 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 - [ ] 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> |
||
|
DottaandClaude Opus 5.5
|
fa86162790 |
refactor(recovery): one process boot id and a cached controller identity (#19851)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Restart recovery decides when a record owned by a dead server process may be released > - One server process minted three different boot UUIDs, one per module (`controller-process-identity.ts:13`, `native-restart-recovery.ts:284`, `legacy-controller-lease.ts:8`) > - The controller identity also ran `ps -o lstart` on macOS at every native claim, preparation check and lease renewal, which is on the path before `turn.start` > - The durable ownership plan (Phase 1, step 1) needs one process identity before it can add one owner stamp and one prover > - This pull request makes the three modules stamp one boot id and reads the process start time from the OS once > - The benefit is one meaning of "owned by this process" across tables, and no `ps` spawn on the turn path ## Linked Issues or Issue Description Refs #19831 (plan: `doc/plans/2026-11-07-durable-ownership-and-reconciliation.md`, Phase 1). Refs #19846 (Phase 0 inventory, finding 1, and the TTFT baseline row "Controller identity"). **Problem** One process has three boot ids, so the same fact ("this record is owned by this process") has three encodings. The uncached controller identity spawns `ps` on macOS on every claim and renewal. **Expected behavior** One boot id per process. The OS start-time read runs once per process. **Impact** Phase 1 needs one identity to build `OwnerStamp` and `proveOwnerGone` on. The turn path loses one subprocess spawn per claim and renewal. ## What Changed - `controller-process-identity.ts` owns the single process boot id. It adds `currentProcessStartedAtMs()`, which caches a successful OS start-time read for the life of the process and retries a failed read. It also exports `currentProcessSpaceHash()` for the coming prover. - `native-restart-recovery.ts`: `currentNativeControllerIdentity()` and `isCurrentNativeControllerProcess()` use the shared boot id and the cached start time. Its own `randomUUID()` boot id is removed. The fallback to the server-info start time is kept. - `legacy-controller-lease.ts`: `legacyControllerBootId` is now the shared boot id. - Tests in `native-controller-identity.test.ts`: one boot id across native claims, legacy leases and creation intents; one OS read across many claims and renewals; a failed read is retried, not cached. ## Verification - `npx vitest run src/services/native-runtime/native-controller-identity.test.ts src/services/legacy-controller-lease.test.ts src/__tests__/heartbeat-pending-cleanup-sweep.test.ts src/__tests__/sandbox-creation-operations.test.ts src/services/heartbeat/upgrade-readiness-post-result.test.ts src/services/heartbeat/recovery.test.ts` (in `server/`): 6 files pass. - `npx tsc --noEmit -p server`: no errors. - I checked every reader of the three ids. Each one compares a stored column only with the id of the module that writes that column (`heartbeat_runs.controller_boot_id` with the legacy id, `native_run_finalizations.controller_boot_id` with the native id, metadata `bootId` with the creation id). No reader compares the ids of two modules, so making them equal changes no decision inside one process. ## Risks - Low risk. No schema change and no persisted shape change. - Previous-release story (`rules/upgrade-safe-run-state.mdc`): rows saved by an older release carry that release's random per-module ids. They never equal the new process's id, exactly as a row from any earlier boot never did. They carry forward with no migration. - The start-time cache is correct because a process's own start time never changes. A failed read is not cached, so a transient `ps` failure does not pin the fallback value. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, in Claude Code with tool use and code execution. ## 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> |
||
|
DottaandPaperclip
|
54b83b15a3 |
Reuse authenticated nested workspace verification (#19854)
## Thinking Path > - Paperclip preserves the work that its agents produce. > - Portable workspace migration can save nested workspaces before their parent. > - The parent must prove that a committed child snapshot covers any files it omits. > - The existing owner downloads and restores the same immutable child archive before and after parent capture. > - The fresh local-file checks are required, but the repeated immutable archive check is avoidable within one live source fence. > - This pull request reuses only an authenticated archive proof with an unchanged reference and provider identity. > - It also bounds independent child branches at two workers while retaining every fresh local and Git check. ## Linked Issues or Issue Description Refs #19805 and #19688. A duplicate search found no separate fix for repeated child archive verification. **What existing behavior does this improve?** Online Git parent migration after nested workspaces have committed portable snapshots. **Subsystem affected** server/ — workspace migration and nested snapshot coverage. **Current behavior** Each parent verification pass downloads, authenticates and restores every child archive, then captures the child's current local tree. The second pass repeats the immutable download and restore. Independent child branches also run in sequence. A large parent can exhaust its existing budget before parent capture begins. **Proposed behavior** One private verifier belongs to the existing live source-owner transaction. It can reuse a completed archive proof only when the full snapshot reference, operation, child closure, ETag and byte length still match. Missing or weak ETags and non-cancellation HEAD errors keep full verification. Caller cancellation still stops work. A changed identity forces fresh authenticated readback. The owner still captures current local bytes and Git state in both passes. **Reason and benefit** Remove duplicate immutable work without treating an old local-file check as current authority. Two independent topmost branches can overlap. Recursive descendants stay serial within those branches, so the bound does not grow with depth. ## What Changed - Add a private per-fence verifier to the existing tree owner. It clears its proof map at the end of the import and refuses a different owner transaction. - Reuse only completed ciphertext, GCM, plaintext, and canonical restore verification with a matching fresh provider identity. - Retain all current database, operation, recursive coverage, local-file, Git and manifest checks on each pass. - Bound independent branches at two. Abort queued work on failure and await active peers before scratch cleanup or fence release. - Use the verifier only in the existing online Git importer. Keep the offline full-verification path and all capture deadlines unchanged. - Document resource admission, fallback behavior and the hosted acceptance limit. ## Verification - Genuine baseline RED through the real parent importer: child GET counts 2 + 2, canonical restores 4, maximum overlapping readbacks 1. The four fresh local child captures already pass. - Exact positive GREEN: child GET counts 1 + 1, canonical restores 2, maximum overlapping readbacks 2. All four fresh local child captures remain. - The fixture uses real encrypted archives and the canonical S3 provider's local HTTP HEAD parsing with actual payload-derived ETags. It uses fake fixture credentials only. - Final focused suite: **13/13 PASS**, 148.36 seconds, at `eaf17d3340`. This includes recursive max-two completion, failure/cancellation joining, identity/ref/content changes, same-fence alias reuse and stale-owner refusal. - Both affected existing migration suites: **256/256 PASS**, 201.84 seconds, at initial head `eaf17d3340`. - The HEAD-failure review repair has genuine two-case RED evidence and three passing failure/cancellation controls. Full layered source/test-tree typecheck and module boundaries pass at repaired head `763648adad`. Final exact-head controls: **21/21 PASS**, 86.63 seconds (all 16 new controls plus five existing parent cases). - The first extended recursive fixture correctly failed because it created a Git branch after taking an earlier snapshot. The fixture now creates all branches before any import; the source-change guard remains intact. - Exact-head CI: 56 success, four expected skips, and one neutral Paperclip Review. Required gates pass; Greptile is fresh 5/5. Its optional aggregate-handler cleanup recommendation is resolved with the intentional fail-closed rationale. - Local verification is macOS arm64 / Node26.4.0. The observed hosted parent still needs to complete within its unchanged budget after normal delivery. No hosted timing improvement is claimed here. ## Risks - Two simultaneous child verifications can use more I/O, memory and scratch space. Operators must count logical verifications, even in one process. - HEAD availability and provider identity are required for proof reuse. Providers without a usable ETag perform the full check. - Reuse lasts only for one live transaction. A retry, changed reference, changed provider identity, process exit or owner change requires fresh verification. - Faster verification does not waive changed child bytes or guarantee a specific hosted completion time. No provider turn, authority marker, source deletion or larger timeout is introduced. ## Model Used OpenAI GPT-6 through Codex, with repository and code-execution tools. The exact serving model revision and context-window size are not exposed to this agent. ## 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> |
||
|
DottaandClaude Opus 5.5
|
e7936fb1ad |
fix(heartbeat): gate process-loss retries on the shared failure budget (#19856)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - When a server restart loses a run's process, the orphan reaper decides whether to retry the run > - `doc/execution-semantics.md` says process loss uses the shared durable failure retry counter, and `enqueueProcessLossRetry` already schedules against that budget > - The reaper still guards on `processLossRetryCount < 1`, but nothing has written that column since the move to the shared budget, so the guard does nothing > - So an exhausted chain still says "retrying once" and skips the interaction continuation path, and a legacy row with a non-zero counter is refused a retry it is owed > - This pull request replaces the dead guard with `failedRunRetryBudgetSpent`, the budget that the retry actually spends > - The benefit is one restart budget, read and written in one place, with honest run messages ## Linked Issues or Issue Description **Bug description** The Phase 0 ownership inventory (finding 5) found that `processLossRetryCount` has no writer and that its only reader is the reaper guard at `heartbeat/recovery.ts:2675`. The guard always passes for new rows. For rows written by earlier releases, it refuses a retry even when the shared failure budget has room. **Steps to reproduce** 1. Seed a legacy run that a restart stopped before adapter dispatch. Spend its whole failure retry budget (`executionRetryAccounting.failureRetries = max`). 2. Run `reapOrphanedRuns()`. 3. The run fails with "Process lost -- server may have restarted; retrying once", but no retry is scheduled. **Expected behavior** An exhausted budget gives no promise of a retry. A legacy counter value does not block a retry that the shared budget allows. Refs #19846 ## What Changed - `heartbeat/recovery.ts`: `shouldRetry` now checks `!failedRunRetryBudgetSpent(run)` instead of `processLossRetryCount < 1`. When the budget is spent, the reaper falls through to `scheduleInteractionContinuationInfrastructureRetryIfEligible`, which is what the old guard intended. - `packages/db/src/schema/heartbeat_runs.ts`: a comment marks `process_loss_retry_count` as legacy. There is no schema or migration change. - Tests: the three cases that used `processLossRetryCount: 1` to mean "retry already spent" now spend the real failure budget. Two new cases cover a legacy counter (a retry is still scheduled) and a spent budget (no "retrying once", `retryEligible: false`, no retry run). ## Verification - `npx vitest run src/__tests__/heartbeat-process-recovery.test.ts -t "process-loss|failure budget|bypass unknown process outcomes|immediate terminal-run cleanup|paused-tree work|restart stopped before adapter dispatch|reached adapter dispatch"`: 7 passed. - Red before the fix: with the old `recovery.ts`, both new cases fail. Green after. - Server `tsc --noEmit` shows no errors in the changed files. - The full `heartbeat-process-recovery.test.ts` file (419 cases) stalled locally at 0% CPU after about 15 minutes. That run was on a worktree whose package builds were stale at first. I ran the focused subset instead, and required CI runs the full file. ## Risks - Low risk. The retry decision now follows the same budget that `enqueueProcessLossRetry` already enforced. A run with budget left behaves as before. - Previous-release story: rows saved with `process_loss_retry_count > 0` are no longer refused for that reason alone. The shared failure budget, which earlier releases also wrote, bounds them. The column stays, so earlier-release readers (`queue.ts` startup resume) keep working. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used 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 - [ ] 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> |
||
|
|
377bc87df1 |
feat(connectors): show and select personal AI account defaults (#19845)
Show a crown on each saved personal AI provider default. Add a provider-specific menu action to other eligible accounts. Share saved-state queries, block duplicate writes, and keep the saved crown until a write succeeds. Keep account details available for accessible grants. Show session lookup errors with Retry. Resolve list permissions once per request and index account lookups. Add UI and API regression coverage and account documentation. Co-Authored-By: Paperclip <noreply@paperclip.ing> Co-Authored-By: Paperclip <Paperclip-Paperclip@users.noreply.github.com> |
||
|
DottaandClaude Opus 5.5
|
824b25654c |
fix(ui): keep a Retry for retained-file holds whose source cannot be verified (#19840)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - When a native run finishes but its files cannot be saved, the task stays blocked behind a retained-file recovery hold. > - The server records that hold as blocked and retryable, and the task notice tells the user to "Retry to try again". > - The task page hides its own Retry for these holds and relies on the retained-file recovery panel instead. > - That panel only binds the recovery record when a read-only source inspection returns `verified`. > - Inspection can return `blocked` for normal reasons: a sandbox that predates ownership receipts (state saved by an older release), or a workspace that moved on after its files were recovered. > - Then neither surface offers an action, and "Check file recovery status" always answers "File recovery changed. Refresh the task", also after a reload. > - This pull request binds the record by its own identity, uses inspection only to gate the physical file-recovery request, and keeps the task Retry visible whenever the panel has no control of its own. ## Linked Issues or Issue Description Refs #18848 (the change that added the retained-file recovery panel and hid the generic Retry for these holds). **What happened?** A blocked task with an active `native_workspace_sync_out_retry_exhausted` hold showed "The saved files need recovery before work can continue", "The recorded source is unavailable or has changed", and "Saving the agent's files failed after automatic retries. Retry to try again". It showed no Retry and no recover button. "Check file recovery status" always answered "File recovery changed. Refresh the task to inspect the current recovery", and a reload did not change it. The hold was saved by an earlier release: `systemRecovery` exhausted with `allocation_ownership_ambiguous` / `service_required`, and `workspaceRecovery` blocked with `retryable: true`. The source inspection endpoint returned `status: "blocked"` for it. A second task whose files were already recovered showed the same "need recovery" and "has changed" text, because its inspection reported a reference mismatch after recovery. **Expected behavior** A retryable hold always shows a working Retry or recover control. A notice that says "Retry" has a Retry button. Status checks never loop on "changed, refresh". Recovered files show as recovered. When the source cannot be verified, the page names the reason and who must inspect it. **Steps to reproduce** 1. Use an active retained-export hold whose `workspaceRecovery` is blocked and retryable, and whose `systemRecovery` is exhausted. 2. Make the source inspection (`GET /api/issues/:id/recovery-actions?inspectSourceRunId=…&inspectSourceActionId=…`) return `status: "blocked"`. 3. Open the task as a board operator. 4. Before this change, the page shows no Retry and no recover button. "Check file recovery status" reports "File recovery changed. Refresh the task". **Paperclip version or commit** Observed on build `8812c19`. The regressions fail on master `0f5ce5050e` before the production edits. ## What Changed - `WorkspaceExportRecovery`: bind the recovery record by action, run and schema identity, not by inspection status. Inspection status now gates only the physical "Recover saved files" request (and the request re-checks it before POST). - `WorkspaceExportRecovery`: report through `onRecoveryControlChange` whether the panel offers its own control. It reports `true` only while the source is loading or verified and the files are not yet recovered, or while it shows the previous-server stop confirmation. - `IssueDetail`: hide the task Retry for retained-export holds only while the panel reports that it owns the control. Otherwise the existing task Retry (`POST /issues/:id/retry`) shows. The server renews a bounded recovery generation and re-verifies the source. - Show recovered files as recovered even when inspection no longer matches after recovery. - Name an unverifiable source in plain words (with the code for other reasons). State that Paperclip does not apply files it can't verify. - Recovery text never asks a person to inspect a sandbox or environment, and never names an owner or administrator as the one who must act. This covers the retained-file panel, the run source inspector, the copyback wait, and the native cleanup quarantine message. The execution blocker and the recovery action read model describe a spent recovery budget from its current state, so text saved by older releases ("If Retry keeps failing, inspect the run and its environment") is not shown. - Task Retry uses the existing `createUuid()` helper, so it works on plain-HTTP previews without `crypto.randomUUID`. An accepted Retry also refreshes the inspected file-recovery state. After a failed accepted request, the unverified-source reason stays visible; it hides only while recovery runs. - When the recovery record was replaced, "Check file recovery status" reloads the task detail instead of telling the user to refresh. - Tests: component regressions for an old-release hold, a recovered hold with a mismatched source, verified ownership, and a replaced record. A page regression proves Retry is shown and calls `retryTask` for unverified and stale sources. A server safety test proves that Retry for the old-release hold shape is accepted, opens one new bounded generation, queues file recovery again, keeps the hold active, and is idempotent. ## Verification - RED (master `0f5ce5050e` production files, new tests): 5 component tests fail. The failure output shows the exact staging text: "Check file recovery status", "File recovery changed. Refresh the task to inspect the current recovery.", "The recorded source is unavailable or has changed." 2 page tests (unverified and stale) fail with `expected [ 'Check file recovery status', … ] to include 'Retry'`. - GREEN: `cd ui && npx vitest run src/components/WorkspaceExportRecovery.control.test.tsx src/components/WorkspaceExportRecovery.test.tsx src/components/RunWorkspaceRecoverySurface.test.tsx src/components/ExecutionBlockerNotice.test.tsx src/pages/IssueDetail.test.tsx` passes 320 tests. - Server: `cd server && npx vitest run src/services/task-retry.test.ts` passes 81 tests, including the new old-release safety case. This is safety evidence, not a red: the server path was already correct, and the defect was in the UI. - Full UI suite: 11564 pass. 4 tests in `IssuesList.test.tsx` and `CompanyEnvironments.test.tsx` failed under full-suite load and pass in isolation (106/106). They are unrelated to this change. - `pnpm check:token-gates` is clean. `node scripts/run-tsc.mjs -p ui/tsconfig.json --noEmit` reports no errors in the touched files. The remaining errors are local artifacts that were not built (plugin SDK UI entry, generated Storybook catalog). - Staging was read only (GET requests). The inspect request did not loop in a fresh tab: two calls in 30 seconds. Most of the tab's request volume is periodic company issue-list refetches, which this change does not touch. ## Risks - Low risk. This is a UI-only behavior change. Server authorization, source verification, stop receipts, and the bounded recovery policy are unchanged and remain authoritative. - The task Retry is now visible for retained-export holds whose source does not verify. Retry renews one recovery generation. The worker re-runs the same source-identity checks, so Retry cannot bypass an ownership refusal. If the refusal is permanent (for example an ambiguous sandbox allocation), the hold returns to exhausted and the page names the reason. An automatic fallback for such sources is follow-up work. - Wording: a parametrized UI test renders every recovery state and asserts that no administrator, owner, or "inspect the environment" copy appears. A server test proves that a stored old-release message is replaced by state-derived text. Both failed before the change. - Previous-release story: the old-release hold shape (board-owned, `systemRecovery` revision 5 exhausted with `service_required`, `workspaceRecovery` blocked and retryable) is covered by component, page, and server tests. No persisted state shape changes. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Opus 5.5 (Anthropic), model ID `claude-opus-5-5`, used through Claude Code with tool use (repository search, file edits, shell, test execution, and a read-only browser). ## 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 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
630411af53 |
fix(recovery): require a 1 s start-time gap and treat EPERM as alive in owner proofs (#19848)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - After a restart, recovery code decides whether the process that owns an in-progress run is gone > - Two proofs compare process start times to the millisecond, but `ps -o lstart` on macOS and BSD reports whole seconds > - So a live owner can look like a reused PID, and recovery can release it while it still runs > - Signal-0 handling also disagrees: one check treats EPERM as dead, while all others treat it as alive > - This pull request puts one start-time rule and one EPERM rule in `controller-process-identity.ts` and uses them at every site the Phase 0 ownership inventory named > - The benefit is that no proof releases a live owner because of clock precision or a permission error ## Linked Issues or Issue Description **Bug description** The Phase 0 ownership inventory (finding 1) found inconsistent "owner gone" proofs. `native-failed-run-owner-release.ts` (`recordedProcessMayBeLive`, `runProcessMayBeLive`) and `native-session-executor.ts` (`verifiedRecoveryProcessIsAlive`) treat any start-time difference of 1 ms or more as a reused PID. `native-restart-recovery.ts` and `controllerProcessExited` require a gap of at least 1 s. `local-service-supervisor.ts` (`isPidAlive`, `isProcessGroupAlive`) treats EPERM as dead. **Steps to reproduce** 1. On macOS, record a live process start time with millisecond precision. 2. Run the abandoned-owner release pass, or the recovered-runner liveness check, against that live PID. 3. The pass releases the owner, or the check reports the runner as gone. **Expected behavior** A sub-second start-time gap is ambiguous and keeps the owner. EPERM means the process exists. Refs #19846 ## What Changed - `controller-process-identity.ts`: export `processExists` (EPERM = alive) and `processStartsDefinitelyDiffer` (gap >= 1 s; a missing time proves nothing). `controllerProcessExited` uses the new helper. - `native-failed-run-owner-release.ts`: the controller and run-process checks use the shared helpers instead of exact millisecond equality. - `native-session-executor.ts`: `verifiedRecoveryProcessIsAlive` accepts a start time within 1 s as the same runner. Its signal path uses the same check. - `native-restart-recovery.ts`: `definitivelyDifferentProcessStart` delegates to the shared helper. No behavior change. - `local-service-supervisor.ts`: `isPidAlive` and `isProcessGroupAlive` treat EPERM as alive. ## Verification - New `controller-process-identity.test.ts`: the 1 s boundary, EPERM and ESRCH for every signal-0 check, and a live spawned runner whose recorded start is 500 ms off. - `native-failed-run-owner-release.test.ts`: two new cases (controller and run process) with a live child and a recorded start 500 ms off. The owner stays `observed`. - Red before the fix: with the old call sites, all 4 new cases fail (`releaseAbandonedNativeOwners` returns 1 and releases the live owner; EPERM returns false; the live runner reads as gone). Green after the fix. - `npx vitest run src/services/controller-process-identity.test.ts src/services/native-runtime/native-failed-run-owner-release.test.ts src/services/native-runtime/native-restart-recovery.test.ts src/__tests__/local-service-supervisor.test.ts`: 59 passed. ## Risks - Low risk. The change only makes proofs stricter: a sub-second gap or EPERM now keeps an owner that the old code released. Such a row waits for the existing lease, TTL, or real process exit. - A recycled PID whose new process started within 1 s of the recorded one now stays held until it exits. That is the same rule the takeover path already used. - Previous-release story: no persisted shape changes. Rows saved by the previous release are evaluated with the stricter rule. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used 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 - [ ] 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> |
||
|
DottaandClaude Opus 5.5
|
e2d490c61b |
feat(observability): anchor native time to first token at the accepted wake (#19838)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - When someone messages an agent, the reply must start fast. The durable ownership plan (#19831) makes time to first token (TTFT) a hard invariant. > - Phase 0 of that plan records a TTFT baseline per adapter and environment, measured from the moment the message or wake is accepted. > - Today `task.time_to_first_token` starts at the wake comment, the answered question, or the run row's creation. A wake request that was accepted earlier (deferred, coalesced or retried) is not part of the measurement, and the span does not say which start it used. > - This pull request moves the start back to the earliest accepted input and names it on the span. > - The benefit is that the baseline and later budgets measure what the user waits for, with no new work before `turn.start`. ## Linked Issues or Issue Description Refs #19831 **What is the problem?** `task.time_to_first_token` does not include the time between an accepted wake request and the creation of its run row. It also does not record which input it started from. **What is the expected outcome?** The span starts at the earliest accepted input and carries an `anchor` attribute: `comment`, `question_response`, `wake_request` or `run_created`. **Why does it matter?** Phase 0 of the durable ownership plan sets TTFT budgets from this baseline. A start that skips queue time hides the latency that deferred wakes add. ## What Changed - `resolveNativeTimeToFirstTokenStart` (pure) in `native-run-trace.ts` picks the start and the anchor. `anchor` joins the closed span-attribute allowlist. - `readNativeRunWakeRequestedAtMs` reads `agent_wakeup_requests.requested_at` for the run. It never throws. - `native-session-executor.ts` calls it only after the first deliverable text, so it adds no wait before `turn.start`. The span now ends at the text commit time, not after the trace write. - Updated `doc/run-log-events.md` and `doc/observability.md`. ## Verification - `cd server && npx vitest run src/services/native-runtime/native-run-trace.test.ts src/services/native-runtime/native-run-wake-anchor.test.ts` (26 pass). - `cd server && npx vitest run src/services/native-runtime/native-session-executor.test.ts -t first` (41 pass). The existing TTFT test now asserts the anchor for each ingress. ## Risks - Low risk. This changes only the run-log and OTLP span. It is not Telemetry (AGENTS.md §5.7), and it does not change runtime behavior. - One indexed primary-key read per native dispatch, after the first text. A read failure drops the anchor and keeps the old start. - TTFT values can grow for deferred wakes. That is the intended correction. `startOffsetMs` can be negative when the wake request comes before the trace start. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), via Claude Code with tool use and code execution. ## 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 - [ ] 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> |
||
|
DottaandClaude Opus 5.5
|
84908d176b |
fix(recovery): name the failed check in native_remote_recovery_lease_mismatch (#19839)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Remote native runs execute in a sandbox, and recovery can reattach a run to its original sandbox runner > - Reattachment checks the saved lease, workspace, and runner identity in five places. Each failed check throws the same bare `native_remote_recovery_lease_mismatch` > - On staging, 34 OpenCode runs on 31 tasks failed with this bare error in one evening. The run records could not show which check failed, so the cause could not be confirmed from the run alone > - This pull request makes every producer name the failed step and field with closed values > - The benefit is that an operator and the next investigation can see which reattachment check failed, without new logging or identifiers in the error ## Linked Issues or Issue Description Refs #19399, Refs #19733, Refs #19755, Refs #19779 **What happened?** Remote same-run retries failed with `native_remote_recovery_lease_mismatch` and the task was blocked for reconciliation. The error is thrown by environment acquire, environment resume, workspace preparation, workspace sync, and remote runner verification. All of them used the same bare string. The run's `error` field and its run events did not show which check failed. **Steps to reproduce** 1. Start reattachment for a remote native run whose saved lease, workspace reference, or runner identity no longer matches. 2. Read the run's `error`. 3. It shows only `native_remote_recovery_lease_mismatch`. **Expected behavior** The error names the step and the field that differed, with no identifiers, and existing consumers of the code keep working. ## What Changed - New `server/src/services/native-runtime/remote-recovery-lease-mismatch.ts`: the code constant, a closed `stage` and `field` vocabulary, and `remoteRecoveryLeaseMismatch(stage, field)`. It builds `native_remote_recovery_lease_mismatch: stage=<stage> field=<field>`. - `environment-run-orchestrator.ts` (stages `environment_acquire` and `environment_resume`), `heartbeat/workspace-preparation.ts` (`workspace_preparation`), `native-workspace-sync.ts` (`workspace_sync`), and `verifyRemoteRunnerReattachment` in `native-session-executor.ts` (`runner_verification`) now throw the detailed form. Each check keeps the same condition and order. Only the message changes. - `restart-interrupted-turn.ts` uses the shared constant. It already matches the code exactly or as the `code: ` prefix, so automatic continuation after a reattachment loss is unchanged. No other consumer matches the code. - `doc/execution-semantics.md` documents the detail format. ## Previous-release state This change adds no persisted state and changes no saved identity. Runs that a previous release failed keep the bare `native_remote_recovery_lease_mismatch` in `error`. Runs failed by this release store the detailed form. The only consumer (`isReattachmentLoss` in `restart-interrupted-turn.ts`) accepts both forms, and `workspace-recovery-retry-copyback.test.ts` covers both. ## Verification - Exact-detail assertions in the producer tests: - `server/src/__tests__/environment-run-orchestrator.test.ts`: every scope, status, expiry, provider, lease-ID, cwd, plugin-task, and resume case asserts its stage and field. Against the previous producer, 21 of these cases fail (the message was the bare code). With this change, all 57 tests pass. - `native-session-executor.test.ts`: a replaced sandbox reports `runner_verification`/`provider_lease_id`. A missing durable runner identity reports `identity_run_id` before any sandbox read. - `native-workspace-sync.test.ts`, `heartbeat/workspace-preparation.test.ts`, `native-controller-recovery.integration.test.ts` assert their exact detail. - End to end on embedded PostgreSQL: `heartbeat-native-remote-failure-retention.test.ts` records `native_remote_recovery_lease_mismatch: stage=environment_resume field=lease` on the run when the provider no longer resumes the lease, and it still requests the user's Retry once. `workspace-recovery-retry-copyback.test.ts` adds a detailed-form row that still continues once. - The affected server test files pass locally. `pnpm --filter @paperclipai/server typecheck` passes. ## Risks - Low risk. Only the error text changes. Conditions, ordering, and fail-closed behavior are unchanged. - Logs and run errors show a longer message. The detail has closed values only, so it adds no identifiers or secrets. - A first version of this PR also changed remote retry selection. Review showed that a plain resume in a new sandbox hits the remote workspace's unsaved-work hold, and that a `destroy` disposition does not prove the old sandbox is gone. That change is removed. It needs a separate design. ## Model Used - Claude Opus 5.5 (Anthropic), model ID `claude-opus-5-5`, run in Claude Code with tool use (shell, file editing, read-only browser access to staging). ## 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> |
||
|
Dotta
|
86bc3db693 |
feat(runner): search and load unassigned company skills (#19676)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Native agents use tools to read reusable company skills. > - The existing native skill tools read only skills assigned to the agent. > - An agent can stop when a task needs another skill that is already in the company library. > - This pull request adds tools to search and read accessible company skills on demand. > - Agents can use the saved instructions without a persistent assignment change. ## Linked Issues or Issue Description **Problem or motivation** An agent cannot use an unassigned company skill through the standard native skill tools. A generic API workaround needs extra discovery and can return the full library. **Proposed solution** Add `search_skills` and `load_skill` to the native tool catalog. Search returns small pages of stored metadata. Load returns text from a saved version and lists its files. Both tools use the current run identity and existing company and project access checks. **Alternatives considered** Assign each skill before a read, or use generic API discovery. Both add work to a read that the company already permits. **Roadmap alignment** This extends the existing skills discovery path. It does not add a store, permission model or installation flow. Related: #15409 adds an assigned-skill loader to a different adapter. ## What Changed - Add bounded company skill search. Search does not synchronize sources or fetch repositories. - Add versioned text reads for SKILL.md and supporting files. Page text and file lists. Reject unsafe paths and binary files. - Recheck access after storage reads. Keep the existing closed Muse, skill-test and completion-review tool scopes. - Add integration tests with isolated PostgreSQL and real local package storage. Update the native tool count assertion and skill documentation. - Limit text files to 16 MiB before body reads. Accept saved file paths up to 4,096 characters plus the repository prefix. Return the response digest only on the first page. - Add optional byte limits to the package readers. Select only the requested legacy file in PostgreSQL. Cast the size comparison to `bigint` so existing callers can omit the limit. - Cache decoded text and its digest for one run. Limit the cache to 64 MiB and 64 files. Check access on each page and clear the cache when the run closes. - Read repository source files and wrapper inputs through verified warm reads or isolated recovery. These reads do not hold the company startup lock. ## Verification - Local verification tested tree `7ba5828387ca0a9c2ee99655818a28de1235a854`, published as commit `c9d04cba601541cca506f5b72aa6f7272a25379f`. The published tree matches that tested checkout. Current head `33fa3d3d4c7988288685ce7c6499d064e0cb9079` (tree `8daaa2f3264d1cd65a9b6d85dbb9d3729ac4a29d`) removes six comment lines from three source files; its executable code is unchanged from the locally tested revision. - Four native skill and authority Vitest suites: 72 passed and 7 existing sandbox checks skipped. All 20 skill-library integration tests passed. - Skill-library and package-store suites: 65 passed. This includes the 45 package-store cases and the same 20 integration cases. Ran with umask `022` for executable-file tests. - Server source and test typecheck layers passed in low-memory mode. The affected source cycle also passed a final separate check. - `git diff --check` and `node scripts/check-module-boundaries.mjs` passed. - Red/green evidence: the new default inline-read case reproduced PostgreSQL error `22003`. Two max-size paging cases reproduced extra body reads. Warm and cold repository reads timed out while another connection held the company startup lock. All five cases pass after the fixes. - Both 16 MiB paging cases check that the second page performs no additional body read. They also check that source-access revocation blocks a cached page. Warm and cold lock cases load both the generated wrapper and a supporting file while the startup lock remains held. - The first cold-read test attempt had an incorrect helper import. Corrected the test import, reproduced the lock timeout, then verified the fix. - [CI on the previous revision](https://github.com/paperclipai/paperclip/actions/runs/42319652795) passed typecheck, build and policy. Serialized shard 2 failed in the unchanged test `schedules bounded retries for failed accepted interaction continuation wakes`. The same case passed locally: 1 passed; 416 cases were excluded by the filter. The cause of the earlier CI failure remains unknown. [CI for |
||
|
DottaandPaperclip
|
b8e02fc2b4 |
fix(native): preserve bounded larger web inventories (#19832)
## Thinking Path > - Paperclip manages AI agents and preserves their work across server changes. > - Native harness preservation stores retained web-server files in authenticated archives. > - A retained skill can contain hundreds of valid local files. > - The WEB evidence owner rejected any directory or inventory above 128 entries. > - This blocked preservation of two 462-file skills even though their payload fit the existing byte limits. > - This pull request shares finite inventory bounds between capture and archive validation. > - It preserves every file and keeps the execution, identity, writer, and publication guards. ## Linked Issues or Issue Description Refs: #19814 **What happened?** WEB evidence preservation rejected a retained source with 927 local skill files. The source had settled execution ownership. The files fit the existing payload limits. The directory enumeration failed before archive publication because its entry limit was 128. **Expected behavior** Preserve a valid finite inventory within explicit file, payload, path, and metadata limits. Keep remote recovery obligations unchanged. **Steps to reproduce** 1. Retain two 462-file skills and one three-file skill under a settled native source. 2. Request canonical WEB evidence preservation. 3. The old owner rejects directory enumeration. The added integration test reproduces this refusal with real files and PostgreSQL. ## What Changed - Share the WEB limit through the existing controller archive owner: 10,000 total entries, 4 MiB of manifest metadata, and 4,096 bytes per path. - Apply the limits to filesystem capture, published binding validation, and archive descriptor validation. - Keep the existing 16 MiB per-file and 64 MiB payload bounds. Keep authority history at 128 entries and 32 epochs. - Add real archive and cold-restore coverage for 927 files, legacy manifest compatibility, and bound refusals. Document the limits. ## Verification - Genuine baseline RED: the new 927-file integration case reaches the original `captureWebEvidence.directoryNames` limit and fails before publication. - The initial setup attempt failed before test collection because the generated plugin SDK was absent. The required SDK build fixed setup; it was not counted as a regression result. - The first affected three-suite run passed 93 cases. The large roundtrip exceeded the generic 15-second case deadline. That failure is retained. The isolated large case passed with a 60-second case limit, but full CI later exceeded that limit while the other 4,577 tests in its shard passed. Both timeout failures are retained. The final fixture keeps all 927 files and 18.5 MB, uses bounded 16-way independent comparisons, and has a 120-second whole-case limit. The canonical preservation signal remains 60 seconds. No production deadline or assertion changed. - The 927-file case then passed: encrypted archive, authenticated cold restore, all bytes/hashes/modes, unchanged remote leases, and database-only validation with source volume absent and storage reads forbidden. - The final metadata repair adds a genuine RED case: an entry list below 4 MiB previously emitted a larger complete manifest without rejection. The owner now checks the complete serialized manifest. All ten focused cases passed after that repair. The final bounded-I/O fixture passed the same ten cases in 37.26 seconds, including the full 927-file roundtrip. All 93 other affected cases passed in the preceding three-suite run. Fresh required CI supplies broad final-head coverage. - Server TypeScript, module boundaries, and whitespace checks pass. Run the focused PostgreSQL checks from `server/` with the repository wrapper: ```sh node ../cli/node_modules/tsx/dist/cli.mjs ../packages/db/src/test-postgres-command.ts -- node ../node_modules/vitest/vitest.mjs run src/services/native-runtime/native-harness-filesystem-independence.integration.test.ts --testNamePattern 'preserves all 927|existing 128-entry WEB|WEB manifest beyond|oversized WEB filesystem' --hookTimeout 90000 --maxWorkers 1 --fsModuleCache ``` ## Risks The owner can hold more entries in memory. The fixed 64 MiB payload, 4 MiB metadata, path, and entry bounds constrain that cost. The archive format and old receipt bytes do not change. Older builds still have the 128-entry limit and cannot qualify a larger new receipt. This change does not settle remote recovery, grant execution authority, alter finalizer locks, or change publication CAS rules. No hosted preservation or deployment was performed for this PR. ## Model Used OpenAI `gpt-6.1-sol`, with code execution and repository tools. The context window size is not exposed. ## 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> |
||
|
DottaandPaperclip
|
061c84d280 |
fix: preserve local copies behind settled remote recovery holds (#19830)
## Thinking Path > - Paperclip manages AI agents and the work they save. > - Online migration must save legacy local files while other remote work continues. > - A failed native run can retain a remote recovery hold after its local writer has settled. > - The current source proof recognizes only some of those recovery producers. > - This blocks stable local files and their shared Git aliases behind an interrupted session or exhausted export recovery. > - This change admits two exact producer shapes under the existing physical and task locks. > - It preserves the failed run, accepted result, remote hold, retained allocations, and recovery history. ## Linked Issues or Issue Description Refs #19642, #19688, #19810. A public search found these related changes. This is a narrow correction to their local preservation proof. **What happened?** Online workspace import rejects a stable local source with `online_workspace_source_scope_unproven` when its prepared native source has an ambiguous session interruption hold or an exhausted file recovery hold. Shared Git aliases inherit the refusal. **Expected behavior** Save the stable local copy under the existing source locks. Keep remote recovery blocked until its own recovery owner supplies the required evidence. **Steps to reproduce** 1. Create a failed native run with an unowned terminal finalizer and an exact prepared allocation. 2. Keep either its board-owned interrupted-session failure block or its exhausted system recovery and blocked file recovery with an accepted result. 3. Import the local Git source or a shared alias through `migrateRemoteWorkspace`. 4. Before this change, both actual importer tests reject the source. With this change, both save and restore its files while retaining the hold. **Paperclip version or commit** Reproduced on master at `b2af45f0f1` with local PostgreSQL 18.1 on macOS arm64 and Node 26.4.0. ## What Changed - Reuse the exact prepared-source failure proof for ambiguous native session interruptions. - Admit an exhausted export hold only with its exact run, blocked task, unowned finalizer, accepted result, original allocation, and current recovery receipts. - Bind the complete accepted result into the publication fingerprint. Keep the existing source, alias, Git, finalizer, and task locks. - Add real importer, authenticated restore, refusal, and publication-race controls. Document the separate local and remote obligations. ## Verification - Baseline: the exhausted-export importer case failed at `remote_workspace_migration_blocked:online_workspace_source_scope_unproven`. - Review found that the first interruption fixture used the artifact-failure resume mode. The corrected fixture reads `nativeProviderRecoveryEvidence` and `nativeSessionFailureDisposition`. Its canonical ambiguous interruption case failed at the source guard before the cause-specific mode repair. - Fresh recorded metadata then showed that an interrupted source can have a boolean `providerSessionEstablished: false`. The final fixture derives that exact false+ambiguous state from a checkpoint without a provider identity or provider events. It failed against the intermediate shared-true guard. The final cause-specific proof passes it. The artifact-failure case still requires true+exact mode. - Both importer cases remove the originals and restore saved files through the real encrypted snapshot owner. They keep the failed run status, native source reference, result bytes, tasks, recovery actions, retained allocations, and unrelated active remote work unchanged. The existing migration owner adds only its local source recipe. - Final affected PostgreSQL suites: **107/107 passed**, 74.44 seconds, canonical shared PostgreSQL wrapper with one worker and cleanup complete. Controls reject the wrong interruption mode, a string session flag, unreleased owners, active controllers, stale fingerprints, changed allocations, changed recovery state, and changes during upload. Command from `server`: `node ../cli/node_modules/tsx/dist/cli.mjs ../packages/db/src/test-postgres-command.ts -- node ../node_modules/vitest/vitest.mjs run src/services/remote-workspace-online-source.test.ts src/services/online-legacy-workspace-recovery.test.ts --fsModuleCache --maxWorkers=1 --hookTimeout=90000`. - Final server source and all test typecheck layers passed. Module boundary check and `git diff --check` passed. - A first setup attempt could not load the unbuilt plugin SDK and ran zero tests. Building the SDK corrected the setup. An unchanged-baseline run repeated source refusals after a local edit used the wrong path. The first 105-case and 106-case passes predate the final producer correction and are superseded by the 107-case pass. Earlier failures and passes are retained separately. - A read-only probe loaded the exact committed proposed inspection code against three current stored holds in one repeatable-read snapshot. All three passed the actual function. It verified the owned accepted result and both interruption fields without changing source or provider state. This is applicability proof; an import must still acquire its locks and recheck after deployment. - The first final-head CI run failed browser shard 5 at `cloud-auth.spec.ts:34` with an APIResponse disposed during route teardown. After the workflow completed, one scoped failed-job retry passed that shard and its browser aggregate on the same head. The initial failure is retained. No code, test, timeout or assertion changed for that retry. - `verify --explain` returned 2 because the documentation path requires broad CI. Required CI passed on `2050fa058cabe75e3d8f64f1f6bdb4b9b2fd0873`. Fresh Greptile review is 5/5 with zero open threads. No hosted workspace, provider, task, or recovery state was changed for this PR. ## Risks The new proof could accept an inconsistent recovery producer. Exact company, run, task, allocation, action, result, and receipt checks keep uncertain sources blocked. Changed evidence during capture prevents publication. A saved local snapshot is not proof that remote files were recovered, and this change never creates such a receipt. ## Model Used OpenAI GPT-6 (Codex), with reasoning, tool use, and code execution. The runtime does not expose a more specific model ID or context window. ## 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> |
||
|
DottaandClaude Opus 5.5
|
a94881d1a7 |
fix(heartbeat): save a comment refused by a temporary hold and route post-quarantine input to its own gate (#19829)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A user's follow-up comment or Retry must be delivered once, eventually, even when a temporary company admission hold (accounting still settling) is active > - #19741 kept claimed runs queued through such a hold and woke held work when it cleared > - A restart-chaos wave still lost a comment, because its wake was refused before admission and written as a terminal skip > - The same wave left a Retry waiting forever for a stop record of a process that never existed > - This pull request saves the refused comment as a deferred receipt, and routes input after a pre-launch quarantine failure to the quarantine's own gate > - The benefit is that the comment is delivered after the hold, and the task asks for the action that actually unblocks it instead of offering a Retry that can never start ## Linked Issues or Issue Description Refs #19741 and #19702. **What happened?** In a restart-chaos wave, a failed run's accounting held the company for 600 s. 1. A user commented while the task's run was live. The enqueue budget gate refused the comment's wake before it reached the deferral behind the live run. Because it was a user request with no saved receipt, it was written as `skipped` (`budget.blocked`). The live run finished without it and the task ended `done`, so the comment was lost. 2. A Retry created a run that the native cleanup-quarantine fence refused before it requested any runner launch (`native_session_cleanup_quarantined`, no process events). A second Retry then waited with `process_identity_missing` ("The previous run has no verified stop record") for a process that never existed. The task kept offering Retry, and that Retry could never start. **Expected behavior** A comment refused by a temporary hold stays saved and is delivered once later. Input after a run that was refused before launch is handled by the quarantine gate: Retry is not offered (send a new message), and a new message gets the verified-stop check of the previous provider session. ## What Changed - `heartbeat/queue.ts`: when the enqueue budget gate refuses a user's comment wake and the block is temporary (not a reached hard stop, per `isBoardOwnedBudgetBlock`), the wake is saved as a `deferred_issue_execution` receipt. The receipt carries the comment ids and an `executionWait` of `budget.blocked`, and the gate notifies the deferred-receipt worker and throws `PersistedExecutionWaitRefusal`. It is not written as `skipped`. Normal promotion (when the live run ends) and #19741's hold-release wakeups deliver it. - `native-local-process-stop.ts`: new `nativeRunRefusedBeforeLaunch`. It returns true for a failed native run with `native_session_cleanup_quarantined`, no recorded process, and no launch-request or identity event. - `explicit-native-continuation.ts`: such a run passes the process-identity check, so the existing quarantine branch decides. A Retry gets `cleanup_quarantined` (shown with `canRetry: false` and "send a new message"). A new message gets `verifyStoppedNativeSessionForContinuation`. Any launch request or identity record keeps the original stop proofs. - `native-session-executor.ts` (`verifyStoppedNativeSessionForContinuation`): when the source has no process record and no launch event, the verifier relies on the previous owner's stop evidence. Before this change it required the source to share the owner's process identity, so a new message stayed held even after the owner stopped. A source that carries a process identity must still match the owner exactly. ## Verification Each new test fails on `master` and passes with this change, on embedded Postgres: - `server/src/__tests__/heartbeat-held-comment-wake.test.ts`: a comment whose wake the hold refuses at enqueue is saved with a `budget.blocked` wait and is not skipped. It is delivered exactly once after accounting settles. - `server/src/services/explicit-native-continuation.test.ts`: - A run refused by the quarantine before launch routes a new message to the quarantine's stop verification and a Retry to `cleanup_quarantined`. These failed on `master` with `process_identity_missing`. - Guards (green on both): a quarantined run that requested a launch, and another failure without a process record, still require `process_identity_missing`. - `server/src/services/quarantined-task-retry.test.ts`: a never-launched quarantined source is certified once the previous owner has stopped (red on `master`). It stays held if the source requested a launch or the owner is still running. All 31 tests pass, and the `stopped native conversation physical cleanup` suite passes (46). - Related suites pass locally (6 files, 620 tests): explicit native continuation, held comment wake, queued comments routes, comment wake batching, refused assignment wake, deferred receipt work. Two `deferred-execution-receipt-work` tests failed once under parallel load and passed when run alone. ## Risks - The saved-comment path applies only to user comment wakes refused by a temporary budget hold. Hard stops, system wakes, case handoffs, durable chat requests, and existing saved receipts are unchanged. - The pre-launch rule is limited to the quarantine failure code with no launch evidence. Every other missing process identity still requires the existing stop proof. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, run as an agent in Claude Code with tool use (shell, file editing, local test execution, kept restart-chaos instance inspection). ## 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> |
||
|
DottaandPaperclip
|
2e52dfa864 |
fix(slack): retain DM thread tasks after completion (#19486)
## Thinking Path > - Paperclip manages AI agents and their work. > - Slack conversations store that work in tasks and comments. > - A native thread must keep its task when work finishes. > - The server treated all DMs as linear conversations. > - A reply in a completed DM thread therefore started a new task without its earlier comments. > - This pull request uses the native thread identity for task continuity. > - People can continue the same work with its saved context. ## Linked Issues or Issue Description **What happened?** A reply in a Slack DM thread created a new task after the preceding task was Done or Cancelled. It also created a new task when the saved conversation was completed. The new task lacked earlier comments. **Expected behavior** A native Slack thread keeps one task in channels and DMs. Later replies retain earlier user and agent comments. Top-level DMs keep their existing task generation rules. **Steps to reproduce** 1. Start work in a Slack DM thread. 2. Mark its task Done. 3. Reply in the same thread. 4. Before this fix, the reply creates another task. **Paperclip version or commit** Reproduced from master at |
||
|
Nicky Leach
|
45c2bd94c6 | fix(native): name the interrupted attempt when an exact resume is refused (#19709) | ||
|
DottaandClaude Opus 5.5
|
8373ae905a |
fix(accounting): settle a never-started run whose recorder bound an empty fence map (#19826)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Budgets hold new work while a native run's final usage receipt is outstanding > - A run that never started a provider has nothing to account. #19700 added a rule that settles such a run at once. > - In restart chaos waves, Retry runs that failed before any provider start still held company admission for up to 10 minutes > - This pull request fixes the condition that stopped the rule from matching > - The benefit: a failed run that provably never started a provider settles at once and never holds other work ## Linked Issues or Issue Description Refs #19700, #19724. **What happened?** An OpenCode Retry run failed at once with `native_session_cleanup_quarantined`. It had no model, no usage receipts, no process start and no provider events, yet it held company admission for the full 600-second closure bound. Two more waves had the same shape, with holds of 309 and 324 seconds. **Root cause** `nativeProviderWorkNeverStarted` (`run-cost-accounting.ts`) required `usage_json.accountingReceiptSourceFences` to be absent. `createRunUsageRecorder` binds every run with that key, as an empty map `{}`, even when no earlier receipt source exists. So the rule never matched a run that had a usage recorder. The run then waited for the closure bound as a run whose receipt was still closing. The restart-evidence check for the unpriced-usage exemption (`budget-exposure.ts`, from #19724) had the same flaw: it treated the presence of the key as restart evidence. **Expected behavior** A failed native run with no receipts, no replaced receipt source and a process that never launched (or was verified stopped) settles at once with no charge. A run whose provider might still be spending keeps holding. ## What Changed - `run-cost-accounting.ts`: the never-started rule treats an empty fence map as "no replaced source". Only a non-empty map shows that provider work began. - `budget-exposure.ts`: restart evidence also requires a non-empty fence map. - `doc/SPEC-implementation.md`: documents the rule. ## Verification - Red then green, `closed-native-receipt-accounting.test.ts`: - "settles a quarantined Retry whose recorder bound an empty fence map at once". This is the exact chaos row: failed, `native_session_cleanup_quarantined`, terminal_failure finalization, receipt sequence 0, empty fence map, no receipts, no process events. - "does not count an empty fence map as restart evidence for a timed-out closure". - Existing tests still pass. A run with a recorded but unverified process still waits for the bound. The restart-derived exemptions still apply to runs with a non-empty fence map or a recovery transition. - Local: `closed-native-receipt-accounting` (51), the budget suites, `heartbeat-cost-accounting`, `adapter-cost-failure-paths`, `native-stop-accounting` and `native-session-executor` all pass. Server `tsc --noEmit` is clean. ## Risks - Low. The change only narrows what counts as a replaced receipt source. A run whose source was really replaced has a non-empty map and behaves as before. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), Claude Code agent with tool use and code execution. ## 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> |
||
|
Dotta
|
3112e4c955 |
Verify original Dot OAuth authority for signed Cloud requests (#19819)
## Thinking Path > - Paperclip manages AI agents and their work. > - Dot uses a user OAuth grant to act in one company. > - Cloud must use that original grant when it resolves a selected user's identity. > - A Cloud signature alone must not create user or board authority. > - The tenant's OAuth owner must check the token and its current restrictions. > - This pull request adds a signed private HTTP boundary for that check. > - The result contains only the checked identity fields. It grants no task or board access. ## Linked Issues or Issue Description **Subsystem affected** Server external-agent identity, public MCP OAuth, Cloud control verification, and HTTP logging. **Problem or motivation** Cloud needs a private way to resolve the original Dot token for the selected user, client, company and resource. The existing generic Cloud control route creates an operator actor. That route is not the right owner for this identity check. **Proposed solution** Add `POST /mcp/runner/oauth/cloud-principal`. Require the existing Cloud signature and replay verifier. Bind the whole request to a new, closed `mcp-dot:principal` action with a maximum 60-second lifetime. Authenticate the original token once through the normal Dot OAuth owner. Check current namespace authority before and after that call. **Alternatives considered** A Cloud-created grant or a board actor would change the authority owner. Neither is used. The private route reuses the existing OAuth and Cloud identity owners. **Roadmap alignment** This is a narrow boundary for the existing MCP and Cloud areas in `ROADMAP.md`. It adds no work queue, public hosted profile, or provider enablement. Related public work: Refs #14846, #19427, #19750 and #19776. Searches for the exact private path and action found no duplicate PR. The matching private Cloud caller has merged. The current App head passed its required checks and review. ## What Changed - Add an original-token principal reader. Keep the original client, selected user, company, resource, purpose, scope, membership and feature restrictions. - Add a 16 KiB raw JSON private route. Reject compressed, invalid UTF-8, malformed and unknown-field bodies. Bind an ordered whole-body digest to the signature and existing nonce fence. - Compare the current stack, claim, instance, origin and connector identity before and after OAuth. An absent owner or namespace fails closed. - Mount the route in normal `createApp` after hostname, runtime-claim and standby checks, before session/actor work. Do not add it to generic board-control routing. - Keep private request and response logs content-free, including raw buffers, headers, query data and reflected errors. - Add controlled HTTP/privacy tests and a real normal-App OAuth/Cloud-signer integration case. Document the machine-only contract and correct the existing `assistant-connections/client-use` access entry. No new capability ID or agent API entry is added. No schema, lockfile, workflow, Runner, Muse or useful-task fixture changes belong to this PR. ## Verification - Current `70c03593`: normal module-boundary policy and all seven App compiler programs passed. The public module factory has an explicit portable Express return contract. Whole HTTP/log controls passed again: 30/30. The existing real paired OAuth/Cloud-signer/private HTTP case then passed again on a fresh PostgreSQL fixture: 1/1, zero skipped. All known child processes and five database connections closed; the final independent SQL audit was empty. - The original remote policy failure rejected an App import of a module-internal path; dependent E2E/verify gates stopped on that failure. The fix delegates through the public module API. A first local declaration run reported TS2883 for its inferred return type; the explicit public type fixed it. Original logs remain preserved. No checker, compiler option, test body or authority check was weakened. - The normal full App graph passed on the integrated source. The later fixture-only repairs each passed the complete affected owning program:405 entries, including 403 source roots. The remaining programs and own dependency outputs carried only where their source was exact. - Whole private HTTP/log files passed:30 tests, zero failures or skips. The HTTP controls use a controlled OAuth owner with real signatures. The log controls use the actual HTTP logger. - The paired Cloud source passed its normal strict build and whole 26 signer/client/namespace and credential-error controls. These are controlled caller tests, not provider tests. - One normal-App paired case passed with a dedicated PostgreSQL database. It used real DCR 201, PKCE, consent, token exchange and a persisted unpaired grant; actual signed metadata publication and a committed Cloud receipt; the real Cloud signer/client and actual App HTTP; exactly-once authentication; refresh; current namespace restrictions; and revocation. The Cloud registry/provider were the existing controlled in-memory/Fake fixtures. Recorded inbound Hosts matched, with zero metadata overflow. No grant, readiness or mirror ACK was fabricated. - The original principal helper's 21 controlled tests and two real-OAuth tests passed before the HTTP slice. Those three files remained unchanged through integration. They are separate source-scoped evidence, not an extra combined run. - Preserved failures: a helper import and test typing error were corrected before the passed checks. An earlier suite failure did not serialize its hook error and remains unexplained. Later runs captured a missing normal managed-config initialization and HTTP 404 from the loopback mapper. The fixture now calls the normal initialization owners and uses the existing Host-preserving canonical HTTP adapter. All original authority assertions and deadlines remain. - Reproduce the local files with `pnpm --filter @paperclipai/server exec vitest run src/modules/external-agents/cloud-principal-http.test.ts src/modules/external-agents/cloud-principal-log.test.ts --maxWorkers=1 --no-file-parallelism`. The real integration file is `server/src/__tests__/cloud-principal-normal-app.postgres.test.ts`; it requires its explicit dedicated test DB and an own compiled matching Cloud checkout. - The normal `node scripts/check-maps.mjs` check passed for 35 areas and 192 capability IDs. This validates map structure, not business acceptance. The later edits are documentation/map only; all executed business source and own outputs from the paired case remained exact. - Current-head remote CI passes: 59 successful checks and three intentional skips. Greptile reviewed this exact head at 5/5 with no findings or review threads. The prior Ponytail review was Ship 5/5 at the earlier head; no current-head Ponytail turn is present, which is nonblocking under the authorized review policy. The three full-repository commands were not repeated locally; the focused normal App graph and affected tests above passed, and required remote CI supplies the broad gates. No deployed, provider-capacity, cold-wake or full hosted lifecycle result is claimed. ## Risks - This is a security boundary. Signature, current namespace and original OAuth restrictions must continue to fail closed. The response gives identity only; callers still need normal operation authority. - The request carries the original token. The reserved route uses a closed log projection on both accepted and refused requests. Privacy regressions remain a material risk. - Both repositories must agree on the action and digest contract. Older code rejects the unsupported action; no fallback creates authority. - This is a prerequisite private boundary. The remaining hosted discovery, wake, capability, operation, event, retry/reactivation and revocation lifecycle work, plus Muse, remains open. No profile or capability is enabled here. - No migration is needed. Rollback removes the private boundary and paired caller; normal OAuth and Personal MCP remain with their existing owners. - The engineering documentation describes this private machine boundary and its access restriction. No operator-facing feature is enabled. Documentation for full hosted enablement remains with that later work. ## Model Used OpenAI `gpt-6.1-sol` (Sol6.1), through Codex, produced the source and tests with tool use and local compiler/test execution. A GPT-6-based Codex coordinator reviewed the work. Its exact model ID is unavailable. Context window sizes are unavailable. ## 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 |
||
|
DottaandClaude Opus 5.5
|
b2af45f0f1 |
fix(runner): let OpenAI API-key Codex agents generate images with their existing key (#19654)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work.
> - New users often connect a Codex agent with an OpenAI API key during
onboarding.
> - A user asked such an agent for a logo. The agent said that the
built-in image generator was not available and that its fallback needed
a separate API key. It then drew an SVG by hand.
> - #19516 removed `--disable image_generation`. That change is correct,
but it does not reach this path. The pinned Codex 0.160.0 shows its
`image_gen` tool only when Codex signs in with ChatGPT. The check is
`image_generation_available` in `codex-rs/core/src/tools/spec_plan.rs`.
An API key never gets the tool.
> - The runner keeps the provider key out of the agent's shell on
purpose, so the skill's `scripts/image_gen.py` fallback cannot run.
> - This pull request adds a Paperclip `generate_image` tool. The server
makes the image with the agent's existing OpenAI key. The key never goes
to the agent process or the shell.
> - The benefit is that API-key Codex agents can make images with no
second key. The spend goes to the agent's budget and the budget can stop
it.
## Linked Issues or Issue Description
Refs #19516
**What happened?**
An agent connected through onboarding with an OpenAI API key (Codex
harness, `gpt-5.6-sol`) could not generate images. The agent said: "The
image skill's built-in generator isn't available in this run, and its
fallback would require a separate API key." It then drew an SVG instead.
**Steps to reproduce**
1. Create a company with a Codex agent that uses an OpenAI API key.
2. Ask the agent for a raster image, for example a logo PNG.
3. The agent reports that image generation is not available. It does not
make the image.
**Expected behavior**
The agent makes the image with the OpenAI key that already powers it.
**Root cause**
The agent ran on the native runner's Codex app-server path, with
`auth.json = {"OPENAI_API_KEY": ...}`. Codex 0.160.0 removes
`image_gen.imagegen` from the model's tools unless
`current_auth_uses_codex_backend()` is true. That function is true only
for a ChatGPT sign-in.
I proved this with the real pinned binary. I gave it the runner's
isolated launch arguments and the API-key auth file, and pointed it at a
local Responses endpoint. The first model request offers only `exec`,
`wait`, `request_user_input` and the collaboration tools. #19516's test
checked only `codex features list`, so it could not see this gate.
## What Changed
- **New `server/src/services/native-runtime/generate-image-tool.ts`:**
- The tool schema and definition.
- `imageGenerationBindingForRun`. It applies only to native Codex runs
on a direct OpenAI `api_key` connection with no router selection.
- The OpenAI Images API call (`gpt-image-2`, `b64_json`, PNG check, 180
s timeout). The reply is read as a stream with a byte cap before it is
parsed. A decoded image larger than 20 MiB is refused.
- A reviewed rate card for `gpt-image-2`: text input $5, image input $8
and image output $30 per 1M tokens, from
developers.openai.com/api/docs/pricing, retrieved 2026-11-06.
- A create-only, symlink-confined local write.
- **`paperclip-runner-tool-authority.ts`:**
- Adds the `imageGeneration` binding.
- Advertises the tool only for standard mode with a workspace. A remote
target must also have an installer. Dot, Muse, Grok and native review do
not get the tool.
- Adds the `#generateImage` handler. Before any spend, it checks again:
the run and task, budget authority, `issue:mutate`, case handoff, and
the connection.
- Stores a claim-before-spend per-call receipt in
`resultJson.imageToolReceipts`. A retry or recovery replays the receipt
and does not pay again.
- Saves a `charged` receipt (usage, price, request ID and charge time)
as soon as OpenAI returns. A failed save, failed accounting write or
crash can then still settle the spend. A retry or recovery records the
cost once and never buys again.
- Records one `cost_events` row through `createCostEventInTransaction`.
The row is `metered_api` with `costStatus: "estimated"`, and the budget
is evaluated. The row is recorded even if saving fails.
- Checks the task's project budget, as well as the company and agent
budgets, before spending.
- Writes content-free `runner.image_generate` activity entries: started,
completed, failed and unconfirmed.
- **`ai-connections.ts`:** adds `selectImageGeneration`. It checks again
that the connection is an active, healthy OpenAI `api_key` connection
with no custom routing. It also checks the responsible user's
membership, the grant audience and the agent or company installation.
These are the same rules that runtime selection uses.
- **`native-session-executor.ts`:** gets the binding from the run's
admitted `contextSnapshot.aiConnection`. The value comes only from the
server, never from tool input.
- **`native-session-resume.ts`:** the native tool contract changes from
`v18` to `v19` and now lists `generate_image`. A persisted provider
thread cannot add tools when it resumes, so older threads must rotate.
- **`doc/runner-api-tools.md`:** a new section about image generation
for OpenAI API-key Codex agents.
- **#19516: kept, not reverted.** Its changes have no effect on the
API-key path. They are still correct for Codex agents that sign in with
ChatGPT, because `image_gen` works there and saves to
`CODEX_HOME/generated_images`. Those agents need the read grant that
#19516 added. On the API-key path, the new tool replaces the built-in
tool. The tool's description tells the agent to use it instead of
`image_gen` or `scripts/image_gen.py`.
## Verification
**Red on `master` (
|
||
|
DottaandClaude Opus 5.5
|
5eac099fbb |
fix(agents): batch independent follow-up questions into one card (#19634)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work.
> - Agents ask users for input with the `request_human_input` tool or
the `ask_user_questions` interaction. Paperclip shows each call as one
question card.
> - In an onboarding walk, a Codex execution agent asked five
independent questions (name, menu and prices, contact, style, output).
It posted five cards, one after the other. Each card came 20-60 s after
the previous answer.
> - The tool description said: "For one question at a time, ask only the
next unanswered question and wait for its real answer." Agents read this
as a rule. The paperclip skill showed only a one-question example and
did not tell agents to batch.
> - No server cap exists. A question set accepts up to 64 questions.
> - This pull request changes the guidance. Agents now ask independent
inputs together in one card. They use a follow-up card only when a
question depends on an earlier answer. One-at-a-time stays available
when the user asks for it.
> - The benefit is fewer waits for the user: one card and one agent turn
instead of five.
## Linked Issues or Issue Description
**What happened?**
An execution agent asked a user five independent follow-up questions in
five separate question cards. Each card appeared only after the user
answered the previous card and the agent finished another turn (20-60 s
each).
**Expected behavior**
When an agent needs several independent inputs, it asks them together in
one multi-question card. The onboarding interview already does this ("1
of 4" pager).
**Steps to reproduce**
1. Complete onboarding on the Codex API path and hand off a task such as
"Create printable cake menu and logo".
2. Watch the execution agent's first turns.
3. See five sequential single-question cards.
**Paperclip version or commit**
master at
|
||
|
DottaandPaperclip
|
8812c1941d |
perf(storage): overlap bounded S3 multipart payloads (#19817)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The storage service uploads large workspace snapshots through S3 multipart uploads. > - The current uploader permits four requests but fills each streamed body before it starts the next one. > - Slow payload delivery therefore remains serial even when request acknowledgements overlap. > - This pull request fills bounded part buffers and sends those payloads concurrently through the existing SDK streams. > - This lets the four-request window carry payload data while it retains backpressure and failure cleanup. ## Linked Issues or Issue Description Related implementation: #18659. This fixes payload serialization within that existing multipart owner. A search found no duplicate open storage multipart fix. **What happened?** A controlled 24 MiB upload started part 2 only after part 1 had delivered 99.22% of its bytes. The same sequence occurred for part 3. The four-request cap overlapped acknowledgement waits but did not provide useful overlap during payload delivery. **Expected behavior** Up to four part payloads can travel at the same time. Memory stays bounded. Exact lengths, bytes, ordered completion, cancellation and abort cleanup still apply. **Steps to reproduce** Run the new slow-consumer test against the previous uploader. Hold all part consumers until four requests have started. The previous implementation starts one request and the assertion fails. The revised implementation starts four requests and then verifies every payload hash and byte count. **Paperclip version or commit** Reproduced on the source from `585c885168`. The final branch is rebased onto `ae1f781cac`. **Deployment mode** The S3 storage provider. Local tests use fake credentials and an owned loopback HTTP server. No hosted writes were made. ## What Changed - Share a 32 MiB part-buffer allowance across uploads. Stream up to four buffered payloads concurrently through the existing SDK requests. When the allowance is full, use the existing streaming path without an upload queue. - Keep the original streaming path for dynamic parts larger than 8 MiB. This retains the provider part-count limit without larger buffers. - Extend the existing tests for concurrent slow consumers, the shared memory window and cancellation in both paths. Verify allowance release after source and request failures. Check part identity independently of acknowledgement order. ## Verification - Baseline RED: the new controlled test failed with one request started instead of four. Its cleanup released and awaited the upload. - Final review-repair head: 26/26 tests passed across `storage-s3-multipart.test.ts`, `storage-s3-failure-diagnostics.test.ts` and `storage/config.test.ts`. - These tests include the real S3 HTTP client, exact payload hashes and ordered parts, excess and truncated input, early acknowledgements, sibling failure, abort cleanup, stalled input and the bounded four-part window. - Strict server TypeScript, module boundary checks and `git diff --check` passed on the final review-repair head. CI then caught an optional SDK `Key` type in the new test fixture. A focused test typecheck reproduced both TS2345 errors. The fixture now binds its required controlled key; that typecheck and the named shared-window regression pass. The uploader source is unchanged by this repair. - Controlled 24 MiB comparison: 640.58 ms before and 393.88 ms after. The new requests started within 7.37 ms. Both runs used 1 ms per consumed chunk and 250 ms acknowledgement delays. This measures the controlled fixture only. It does not predict hosted S3 throughput or total migration time. - Review RED: two blocked uploads consumed 64 MiB of source chunks under the original per-upload buffer scheme. The new shared allowance kept the source reads within 32 MiB plus stream lookahead. Three new controls passed for concurrent uploads and allowance release after source/request failures. - The first treatment run passed 12/13 cases. Its remaining test assumed completion order matched part order. The test now binds lengths to part numbers and still checks the ordered completion request. An initial typecheck setup attempt lacked the built plugin SDK; the canonical dependency build and final typecheck passed. - Final head `fbadf4f1cf427b896bbc255ffe2e7ad480ce06d3`: [full CI](https://github.com/paperclipai/paperclip/actions/runs/42373571408) passed, including build, test shards, Typecheck + Release Registry and `ci / verify`. All 61 reported contexts are terminal: 56 success, four skipped and one neutral review. Greptile and Paperclip both gave a fresh 5/5 for this head. No review threads remain open. ## Risks - Multipart uploads share at most 32 MiB of live part buffers per process, plus existing stream and source-chunk overhead. The previous path used smaller streaming buffers. With no buffer allowance left, an upload uses the original streaming path. It does not wait for a shared queue. - Objects whose dynamic part size exceeds 8 MiB retain the current serial payload behavior. No new object-size ceiling is introduced. - Request bodies remain non-replayable streams. The SDK retry policy, multipart protocol, cleanup policy, object format and public storage API remain unchanged. - No disk spool or cache is added. All current-head CI gates and both reviews passed. ## Model Used OpenAI `gpt-6.1-sol`, with reasoning, tool use and code execution. The context window size is 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> |
||
|
DottaandPaperclip
|
c92bc9e5ec |
perf: reuse authenticated workspace upload readback (#19811)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Workspace import saves local work to an encrypted immutable snapshot before it changes storage authority. > - The snapshot uploader reads the whole object back to verify its ciphertext. > - The importer then downloads that same object again for authenticated decryption and restore. > - Both reads are required by the current call sequence, but they need not transfer the object twice. > - This PR lets that caller retain the authenticated upload readback in private scratch. > - The importer still restores the full archive, recaptures the source, checks ownership, and publishes through the existing CAS. ## Linked Issues or Issue Description Refs #19642, #19805, and #19810 for the existing workspace import and preservation owners. **What happened?** A fresh workspace import performs a PUT and two full GETs of the same encrypted snapshot. The first GET verifies and discards the object. The next GET decrypts it for restore. Already-uploaded and lost-reply recovery paths also repeat the GET. **Expected behavior** Use one authenticated readback for the exact final winning reference. Preserve the default upload behavior and returned reference. Keep every restore, source, recovery, ownership, and publication check. **Steps to reproduce** 1. Capture a local source through the real workspace archive codec. 2. Upload it through the real snapshot ledger and filesystem object backend. 3. Load and restore its archive. Count the object GETs. 4. The three baseline cases each make two GETs. The opt-in path now makes one and restores the same bytes after the original source is removed. **Paperclip version or commit** Baseline `585c885168`. The first PR head was `5b18121609`; the follow-up repairs the review findings without changing the storage optimization. ## What Changed - Add optional `verifiedArchivePath` to the existing upload owner. The default path and `WorkspaceSnapshotRef` return contract stay unchanged. - Authenticate each readback with the existing ciphertext, GCM, key-binding, and plaintext checks. - Bind retained files to the full winning reference. Recheck private file ownership and plaintext SHA/size before atomic publication to a new caller-owned path. - Clean all invocation scratch on success or failure. Do not overwrite caller files or reuse state across calls. - Use this path in POSIX workspace import. Keep full extraction, the second source capture, closed alias/source/F/writer/task checks, and CAS publication. Windows keeps the existing download path and does not bypass private-file checks. - Cover real cold restore, fresh/replay/lost-reply GET counts, late winners, private-path rejection, local tampering, and GCM failure. ## Verification - Genuine baseline RED: three real PostgreSQL/crypto cases each report two GETs where the new contract requires one. - GREEN: all 30 snapshot storage cases, one actual online migration case, and seven focused migration controls pass; 38 total. The other 232 cases in those files were unselected locally. - Review RED: four obsolete second-GET fault triggers and one uploaded-journal retry case failed before repair. The triggers now fire on the retained readback and assert that they fired. The uploaded-journal case interrupts after its real ledger commit and proves recovery and cold Git restore through the same journal. - A focused platform-selection control proves Windows retains the original two-GET path. This is not native Windows acceptance. - Strict server TypeScript, module boundaries, and `git diff --check` pass. - Local command from `server/`: `node ../cli/node_modules/tsx/dist/cli.mjs ../packages/db/src/test-postgres-command.ts -- node ../node_modules/vitest/vitest.mjs run src/services/remote-workspace-snapshot-storage.test.ts src/services/remote-workspace-online-source.test.ts src/services/remote-workspace-migration.test.ts --testNamePattern 'remote workspace snapshot storage|imports an alias with unrelated|removes plaintext verification files|rechecks a service that becomes|rechecks a writer that appears|preserves a local edit|keeps the existing download path|missing-base-uploaded-retry' --hookTimeout 90000 --maxWorkers 1 --no-file-parallelism --fsModuleCache`. - The codec timing control used an exact earlier owner and a synthetic 72.9 MB archive. Discarding final-capture output saved only 3%; this PR keeps that capture unchanged. - This removes one full object GET. It does not claim a measured hosted wall-clock improvement. - Fresh Greptile review is 5/5 on `0933a0a29a`, with no open findings. Exact-head CI is complete and green: 56 successful checks, four intended skips, and one neutral result. The second review returned Ship (4/5); its optional test-maintainability suggestion is deferred. No source change or CI restart was made for that suggestion. ## Risks - A late upload can win with a different reference. The retained file must match all fields of that final reference; otherwise the owner authenticates the winner. - A private file can change before relay. Ownership, no-follow reads, stable file identity, plaintext SHA/size, and no-overwrite publication protect that boundary. - Authentication may overlap the encrypted upload scratch. Peak local disk use can grow by up to one ciphertext file. Data remains streaming; no whole-object memory buffer is added. - Opt-in callers must supply a canonical private directory owned by the process. Existing callers that omit the option retain their behavior. - No archive format, database schema, provider lifecycle, writer protocol, admission, or background polling changes. ## Model Used OpenAI `gpt-6.1-sol`, with reasoning, tool use, and local code execution. The context window size is not exposed. ## 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> |
||
|
DottaandPaperclip
|
ae1f781cac |
fix(native): preserve web files without settling remote recovery (#19814)
## Thinking Path > - Paperclip manages work that AI agents perform for users. > - Native sessions retain files on the web server and state in a remote provider. > - Replacing the web server must preserve its files without changing remote recovery authority. > - The existing maintenance owner requires destroyed-provider receipts even when all web writers have settled. > - This can keep the old web volume required only because a remote recovery obligation remains unresolved. > - This pull request adds a closed web-byte evidence mode to the existing preservation owner. > - The benefit is durable web-file coverage with unchanged remote references, failures and continuation policy. ## Linked Issues or Issue Description Refs #19748, #19806. A search for native harness filesystem evidence and open web/harness changes found no duplicate implementation. **What happened?** A settled successful native source can retain a raw controller, a separate warm supervisor and provider-home files while its historical external lease has no termination receipt. The existing filesystem-independence owner refuses before it can preserve these web files. One retained skill file can also remain physically present after its version row was deleted. **Expected behavior** Preserve every nonexcluded web file as authenticated evidence. Keep missing files absent and deleted skill versions deleted. Leave VM references, recovery actions, failures and execution authority unchanged. Validate the published web-byte receipt on an empty replacement using database bindings only. **Steps to reproduce** Use the included successful-source fixture with a settled web owner, an unresolved external lease, real controller/supervisor files and a retained skill whose version row is deleted. The baseline rejects at its destroyed-provider guard. The new explicit maintenance mode preserves the files and validates on a replacement. A separate fixture uses a prior successful source, a failed retained last run, a genuinely absent controller, a warm supervisor and 267 durable checkpoint parts. **Paperclip version/commit** Baseline `384fa2b7be`; final branch rebased onto `585c885168`. Current head: `990210d7e9`. **Deployment mode** Original-volume maintenance followed by an ephemeral replacement. No hosted operation or provider request is part of this PR. ## What Changed - Add explicit `web_evidence` maintenance mode and a separate `paperclip.native-harness-web-evidence.v1` receipt. Keep the existing terminated-provider mode and old manifest shapes. - Archive the closed canonical session-root inventory, the separate warm supervisor and exact admitted skill manifests through the existing encrypted `original_volume_source_evidence` owner. Preserve bytes and modes. Keep canonical credential exclusions. - Preserve local skill edits and deleted-version files as forensic evidence. Create no skill version, executable checkpoint, provider backup or recovery authority. - Require original-volume capability, settled web writers, physical/finalizer locks, exact task/source/cohort/input bindings and final byte/mode/membership comparisons. Keep genuinely absent raw controllers absent. - Bind allocation and stamp identity while allowing later external status and cleanup observations. Validate replacements from durable database and archive bindings without the original anchor, filesystem or storage reads. ## Verification - Node 24.22.2, PostgreSQL 18.1, macOS arm64. The unchanged baseline fails at the real destroyed-provider guard. The final revision passes all 85 owner, original-volume capability and unchanged continuation-policy cases in 21.14 seconds. - Tests perform real encryption, persistence through the S3 storage interface backed by the owned local fixture, authenticated restore and exact byte/mode comparisons. The 281-byte file remains present while its version row remains absent. The evidence archive stays separate from the provider-session backup scope. - The supervisor-only case preserves 267 database checkpoint parts, keeps the raw controller absent and validates without original-volume identity. Existing strict-mode and old receipt controls pass. - Negative controls cover active web writers, expired but owned finalizers, physical owners, foreign controller/supervisor identities, missing manifests, symlinks, changed bytes/modes/membership/source during upload, changed durable bindings, incomplete receipt coverage and invalid late publication. Concurrent attempts retain one valid winner and one audit event. - Server TypeScript, module boundaries, diff checks and private-data scans pass. The verification planner's first invocation used a wrong script path; its canonical invocation refused a plan because fixture inputs changed during selection. Neither attempt counts as a pass. Required CI will provide broader application coverage. - Earlier fixture and treatment failures remain recorded. They exposed a missing helper, JSONB property-order instability, missing separate-supervisor mode coverage and fixture timing/source/count errors. The final same owner suite passes without weakened assertions. - A canonical cleanup audit found two additional observation fields: the cleanup controller and manual-attempt ID. Their real publication updates reproduced false receipt invalidation on the first PR head. The narrow correction keeps WEB coverage valid through those observations while changed allocation identity still fails. The exact same 85-case suite passes afterward. The final `990210d7e9` head has fresh Greptile 5/5 with no comments or unresolved threads. Required CI run [42370607882](https://github.com/paperclipai/paperclip/actions/runs/42370607882) passes: 55 successful checks and four intended skips, plus successful Snyk. The old head's CI was superseded by this correction and is not counted as a pass. From `server/`: ```sh node ../cli/node_modules/tsx/dist/cli.mjs ../packages/db/src/test-postgres-command.ts -- node ../node_modules/vitest/vitest.mjs run src/services/native-runtime/native-harness-filesystem-independence.integration.test.ts src/services/native-runtime/native-harness-original-volume.test.ts src/services/native-runtime/native-harness-filesystem-independence-continuation.integration.test.ts --hookTimeout 90000 --maxWorkers 1 --fsModuleCache ``` ## Risks This changes a maintenance trust boundary. It proves web-byte preservation, not provider recovery or termination. External recovery obligations and existing resume failures remain. Unknown layouts, missing manifests, active local owners and changed authority fail closed. Invalid receipts are not automatically replaced. The inventory has independent limits of 128 entries, 16 MiB per file and 64 MiB total. Larger valid data stays on the original volume and blocks this mode. Canonical credential exclusions remain excluded. Evidence uses the existing encrypted ledger and retention roots; it does not become an executable backup. Old receipts remain byte-compatible. Startup adds bounded per-retained-scope database reads. Measure the affected cohort's actual serving startup cost before enrollment. Validation adds no filesystem or object read, worker or recurring poll. Live source qualification and publication remain separate after review, merge and normal delivery. ## Model Used OpenAI Codex, GPT-6 family, with reasoning, repository tools and PostgreSQL/filesystem test execution. The runtime does not expose a more specific serving model ID or context-window size. ## 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> |
||
|
DottaandPaperclip
|
585c885168 |
fix: preserve prepared sources held by provider artifact failures (#19810)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Online workspace migration archives local files before it changes their storage authority. > - A failed provider session can leave an active board-owned recovery hold over a prepared local source. > - The existing migration owner accepts finalized sources and workspace-export failures. > - It rejects a settled provider-artifact failure even when the prepared source and recovery allocation have complete bindings. > - This PR permits that exact preservation case under the existing source and finalizer locks. > - The remote recovery hold remains active. The archive preserves local bytes and the existing recovery recipe. ## Linked Issues or Issue Description Refs #19642 and #19805 for the existing online migration and source-closure owners. **What happened?** Online workspace migration rejects a prepared source held by a terminal `runner_remote_provider_artifact_incompatible` recovery. This also blocks another local workspace that shares the Git source. The provider session had executed work. A pre-execution exemption would be unsafe. **Expected behavior** Preserve the local source through the existing immutable archive owner when the board recovery, task version, terminal run, finalizer, and declared allocation all match. Keep the remote recovery action and authority unchanged. **Steps to reproduce** 1. Create a prepared native source and a failed run with provider session and tool events. 2. Set its finalizer to an unowned terminal artifact failure. Add the matching active board recovery and task status version. 3. Remove the local descriptor and inspect or migrate a shared local Git alias. 4. Before this change, the source proof is rejected. The regression fixture reproduces this failure. **Paperclip version or commit** Reproduced on master `1674902b4f`. The final branch is based on `5a92d53f7d`. ## What Changed - Accept only the matching active board-owned artifact failure with a current task block, no live execution authority, an exact producer fingerprint, and a declared prepared-source allocation. - Include the task version and complete existing source/recovery evidence in publication revalidation. - Lock the proven recovery tasks in sorted company scope before publication checks. A held descendant task produces the existing busy error. - Add a real PostgreSQL regression with 124 tool executions, a missing descriptor, shared Git aliases, immutable download, and cold restore of the local archive. - Add stale, foreign, owned, controlled, malformed, and publication-race rejection cases. - Document that this archive preserves local bytes and the recovery recipe. It does not prove remote export or release the remote hold. ## Verification - Genuine baseline RED: the positive migration regression rejects the source before the production fix. - Focused treatment: 12/12 PostgreSQL cases pass. - Review-race RED: both a selected source and a Git companion published while a descendant recovery task was locked. The corrected tests reject both cases. - Two affected owner suites after the lock repair: 91/91 pass in 23.18 seconds. - Strict server TypeScript check passes after the lock repair. - Module boundaries and `git diff --check` pass. - [Current-head CI](https://github.com/paperclipai/paperclip/actions/runs/42367152081) passes: 56 successful checks, four conditional skips, and one neutral review check. No checks remain pending or failed. - Greptile and Paperclip reviews are both 5/5 on `0a6e6c3ece`, with no open findings. - The first descendant fixture attempt hit the immutable task/privacy guard before it reached the race. The corrected fixture creates that task binding at admission and produces the two genuine REDs described above. ## Risks - A broad classification could let a migration move local authority without a complete recovery recipe. The new branch requires the exact failure, producer, task version, source allocation, and released execution owners. - Evidence can change during upload. Existing locks and full publication revalidation reject those races. - The remote allocation may contain changes absent from the local source. This PR preserves only local bytes and the durable recovery route. It does not mark remote work exported, finalized, or safe to discard. - No schema, provider lifecycle, admission, or polling behavior changes. ## Model Used OpenAI `gpt-6.1-sol`, with reasoning, tool use, and local code execution. The context window size is not exposed. ## 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> |
||
|
DottaandPaperclip
|
384fa2b7be |
Allow context compaction while Agent Chat waits (#19801)
## Thinking Path > - Paperclip manages agents and their work. > - Agent Chat can wait for a user between useful turns. > - The task Properties panel already lets that user request context compaction. > - The queue treated that maintenance request as an ordinary wake and refused it while the chat waited. > - Compaction must maintain that session without starting a new conversation turn. > - This change admits the authenticated request, preserves the waiting state during execution, and prevents competing requests from saving extra maintenance work. > - Ordinary and unauthorized wakes remain blocked. ## Linked Issues or Issue Description Refs #19378, which introduced the existing Compact now action. Refs #19377, whose review identified this independent queue admission gap. Searches for `context_compaction` with `waiting` and `Compact now` with `waiting` found no existing fix for this admission bug. **What happened?** An owner clicks Compact now for a waiting Agent Chat conversation. The real compaction service reaches the heartbeat queue, receives no run, and reports that compaction did not start. **Expected behavior** The authenticated owner can request one separate compaction run while the chat waits. The request must keep its existing actor, task, agent, budget, and execution checks. Ordinary waiting-chat wakes must stay blocked. **Steps to reproduce** 1. Use an open Agent Chat conversation whose agent supports compaction. 2. Wait until the conversation is waiting for its owner. 3. Request Compact now from the task Properties panel. 4. Observe the refusal before this fix. The new database regression calls the real compaction service and queue to reproduce it. ## What Changed - Admit only the existing complete manual compaction envelope. Require a user actor, no coalescing, the compaction intent, and matching nonempty request IDs. - Preserve the waiting conversation state, session generation, reply boundary and unanswered questions in the existing conversation preparation owner. - Recheck active and deferred maintenance under the existing issue admission lock. Competing clicks cannot save a second maintenance request. - Add execution coverage with and without a pending question. Add concurrent admission and deferred-request controls, including admission after the pending receipt is cancelled. Keep the existing authorization and ordinary conversation controls. ## Verification - The original service-to-queue request failed before the admission fix. - The execution regression then caught the review finding: compaction succeeded, but both waiting-state controls failed. The same complete controls passed after the conversation preparation fix. - A real concurrent request test recorded two durable maintenance receipts before the atomic admission fix. It counts deferred authority as well as admitted runs. - Current integrated candidate: normal current Runner core and surface TypeScript exports passed. All seven normal App typecheck programs passed. The whole queue/execution suites passed 86/86, the whole context-compaction suite passed 5/5, and 11 selected ordinary conversation controls passed. There were 102 actual passes; the 29 unselected conversation cases are not counted as passes. - Database controls use fresh dedicated PostgreSQL 17 databases, Node 24, one test worker and pools capped at four. Private fixture adapters use the existing dedicated database and normal migrations. They keep test bodies, assertions and time limits intact. - Earlier fixture and setup failures remain recorded separately from product failures. These include an omitted external URL, an invalid closed-task fixture, a missing crypto import, an incorrect result-envelope matcher, and an unavailable test-library helper. Each was corrected at its owning boundary. A test-output text matcher also missed Vitest's abbreviated failure; the raw assertion stack and failure were retained. - Local session tests exercise the actual admission, native Runner lifecycle, accepted result and persisted conversation state through the supported test backend. They do not prove live provider compaction, UI rendering or deployed suspension. - Exact-head remote CI passed: 59 successful checks, three intentional skips and one neutral check. Greptile and Ponytail both reviewed this head at 5/5 with no actionable findings. Ponytail explicitly marked it Ship. - Master advanced during review. A local merge evaluation preserves every unrelated master change and all four PR files. The current App programs and all three execution controls passed again on that projected squash tree to check the workspace-finalization seam. ## Risks The change affects admission and preparation for one maintenance request. A broad exception could start ordinary work or reopen a waiting chat. The request therefore keeps the complete existing manual envelope and authorization checks. Conversation preparation still checks the session generation. The existing issue lock owns duplicate admission. There is no new timer, database migration or provider protocol. ## Model Used OpenAI Codex based on GPT-6 managed the change and reviewed the shared queue fix. OpenAI `gpt-6.1-sol` authored the focused regression coverage. Both used reasoning, repository tools and local code execution. The runtime did not expose the context-window size. ## 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 (the existing Compact now documentation remains accurate; no new configuration is added) - [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> |
||
|
DottaandPaperclip
|
d2efe3392b |
fix(native): preserve retired authority evidence (#19806)
## Thinking Path > - Paperclip manages work that AI agents perform for users. > - Native sessions retain control-plane files when execution authority moves to a later run. > - Volume migration must preserve these historical bytes before a server can stop depending on its old volume. > - The existing preservation owner archives the current controller and supervisor, but refuses a nonempty authority-epoch directory. > - A settled four-run session can therefore remain blocked after its current controller archive is complete. > - This pull request adds a closed historical inventory to the existing encrypted evidence archive. > - The benefit is preserved source evidence and database-only replacement validation without granting execution or recovery authority. ## Linked Issues or Issue Description Refs #19748. A search of pull requests for filesystem independence and issues for authority-epoch history found no duplicate change. This fixes its uncovered historical-file case. **What happened?** A settled native session retained three prior controller files in authority-epoch directories. Its three historical database checkpoint headers were absent. Maintenance preserved the current controller, then refused filesystem independence because those historical files remained uncovered. **Expected behavior** Preserve the exact historical bytes and modes through the existing encrypted evidence owner. Keep original files, prior archive rows, run outcomes, warnings, stamps, session pointers, and recovery policy unchanged. Validate the resulting receipt on a replacement without the source volume. **Steps to reproduce** Use the included four-run fixture with a current-controller-only archive, three prior authority epochs, destroyed provider receipts, released ownership, and absent historical checkpoint headers. Call the canonical filesystem-independence owner. The base rejects the nonempty directory; the treatment preserves the complete evidence and validates it after removing source-volume identity from the test process. **Paperclip version/commit** Baseline `ef83d566cacf1d5f740d701560597dad2bfa4619`; final branch rebased onto `27170dca49`. **Deployment mode** Original-volume maintenance followed by an ephemeral replacement. This PR performs no hosted migration or provider operation. ## What Changed - Add optional closed authority-epoch entries to the existing `original_volume_source_evidence` archive. Preserve nested file bytes and modes through the existing encryption, readback, ledger, audit, and retention owners. - Bind each epoch to its retained run, canonical checkpoint identity when present, or explicit header absence plus the exact admitted authority binding. Keep current-source checkpoint and artifact checks strict. Historical evidence never becomes an executable checkpoint. - Recheck the complete historical inventory at archive and receipt publication fences. Keep physical locks, all-run ownership and termination checks, source capability checks, and final CAS. - Bind the history inventory and its database facts into the authenticated archive manifest. Replacement validation uses database bindings only. Later header presence, removal, retirement, or changed identity invalidates the receipt. - Leave history-free manifest bytes and receipt shapes unchanged. Keep an existing current-controller-only archive when a later complete archive is added. Unknown children and other nonempty history directories remain blockers. ## Verification - Node 24.22.2, PostgreSQL 18.1, macOS arm64. The final four-run fixture fails on the baseline owner at its nonempty-history guard. The same final fixture and treatment pass all 51 focused PostgreSQL/filesystem cases. - The positive case performs actual encryption, object-store persistence, authenticated restoration, and exact byte/mode comparisons. It proves replacement database-only validation with absent source paths and storage reads refused. - Controls cover foreign identities, unknown children, owned prior runs, changed bytes or new files after upload, missing or newly present historical headers, malformed and retired headers, an unknown authority override, receipt-identity tampering, and a late concurrent winner. Existing current-controller evidence and history-free manifest bytes remain intact. - The baseline/treatment driver restored only its two owned source files in `finally` and verified their exact hashes. Earlier setup and treatment failures remain recorded: missing bundled PostgreSQL library links, an overly narrow raw-schema check, and an incorrect synthetic transient-workspace binding. The final baseline and treatment use the corrected same fixture; those earlier attempts are not counted as passes. - Review follow-up `21183b75f4` closes the missing-history-field gap. Deleting history from an actual published receipt reproduced an invalid true result before the fix. Unconditional manifest reconstruction now rejects it; all 46 owner cases pass. Genuine history-free manifests retain their exact bytes. - Review follow-up `f3d305dd36` adds suspended historical Runner-state bytes and mode checks through authenticated restore, active/foreign state refusals, exact 16 MiB and 64 MiB boundaries, and retention of an oversized file without publication. All 51 cases and server TypeScript pass; production behavior is unchanged. - CI run `42362784772` on the previous head had one unrelated 30-second timeout in `manual-private-cell-report-assets.test.ts`, with 6,864 support tests passing. The failure remains recorded. The prior head `21183b75f4b223cd3ec28124cdc194f24b4f79ba` passes [required CI run 42363408227](https://github.com/paperclipai/paperclip/actions/runs/42363408227), including Runner support: 55 successful checks and four intended skips, plus successful Snyk. The final test/documentation head `f3d305dd3690a7db38b57c4a77389c17206cb76b` passes [required CI run 42364958386](https://github.com/paperclipai/paperclip/actions/runs/42364958386): 55 successful checks, four intended skips, one neutral review check, and successful Snyk. Fresh Greptile is 5/5; all four review threads are resolved. No timeout or assertion was weakened. - The count warning is resolved as independent bounds: the actual descriptor accepts 32 control-plane-only epochs (97 entries) and 32 epochs with 31 Runner states (128 entries), and rejects 129 entries. The 32-epoch and 128-entry maxima both apply; the capture budget is unchanged. - Server TypeScript and module-boundary checks pass. The local verification planner selects ten broad application lanes; required CI supplies that wider coverage instead of duplicating it locally. - Reproduce the focused owner suite from `server`: ```sh node ../cli/node_modules/tsx/dist/cli.mjs ../packages/db/src/test-postgres-command.ts -- node ../node_modules/vitest/vitest.mjs run src/services/native-runtime/native-harness-filesystem-independence.integration.test.ts --hookTimeout 90000 --maxWorkers 1 --fsModuleCache ``` ## Risks This changes a maintenance trust boundary. A source capture is limited to 128 entries, 16 MiB per file, and 64 MiB total. These limits are intentionally below the Runner's 256 MiB state maximum. Larger valid epochs remain on the original volume and continue blocking filesystem independence. It uses bounded directory reads and file descriptors, rejects aliases and unknown paths, and rechecks bytes before publication. Storage failures retain original files and ledger evidence; they do not settle recovery or authorize a retry. Startup adds bounded database reads for retained historical header bindings or their absence. It adds no source filesystem or storage reads. The selected cohort's serving startup cost still needs measurement before enrollment. Other historical layouts require their own canonical source qualification. No missing provider history is declared recovered, and no automatic corruption repair is added. ## Model Used OpenAI Codex, GPT-6 family, with reasoning, source editing, shell tools, and PostgreSQL/filesystem test execution. The runtime does not expose a more specific serving model ID or context-window size. ## 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> |
||
|
DottaandClaude Opus 5.5
|
1674902b4f |
fix(native): settle a crashed controller's running workspace finalize once a later attempt finishes (#19807)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A native run saves its workspace in a `workspace_finalize` operation after the agent gives a result > - A crash loop can kill the server after it records this operation as `running` and before it finishes > - The next server resumes finalization and records a new attempt, which succeeds > - Nothing settles the first attempt, so it stays `running` forever and looks like live work > - This pull request settles the old attempt when a later attempt for the same run finishes, under the proof that the old controller is gone > - The benefit is that no workspace operation stays `running` after its owner is dead ## Linked Issues or Issue Description Refs #19666, #19759, #19721, #19717 **What happened?** A control-plane restart chaos test sent SIGKILL to the server while a native run finalized its workspace. The server crash-looped. One boot recorded a `workspace_finalize` operation as `running` and then died. A later boot resumed finalization. Its new operation (with `resumedFromOperationId` set to the old one) succeeded, and the run succeeded. The old operation stayed `running` at rest. **Expected behavior** When a later finalization attempt for the same run finishes, and the owner of the earlier attempt is proven gone, the earlier attempt gets a terminal status with a reason. A live owner keeps its attempt. **Steps to reproduce** 1. Start a native run that finalizes a workspace. 2. Kill the server with SIGKILL after it inserts the `workspace_finalize` row, and before it updates the row. 3. Restart the server and let it finalize the run again. 4. See that the first `workspace_finalize` row stays `running`. **Paperclip version or commit** `ef83d566ca` (also on `27170dca49`). ## What Changed - Root cause: `attemptNativeWorkspaceFinalization` in `server/src/services/native-runtime/native-workspace-finalizer.ts` reads the previous attempt only to set `resumedFromOperationId`. It records the new attempt but never settles the previous `running` attempt. The early "already succeeded" return path also left it alone. Only the recorder that started a row can settle it (`workspace-operations.ts` `recordOperation`), and a killed process never does. - Add `settleSupersededNativeWorkspaceFinalizations` in `native-workspace-finalization-supersession.ts`. It sets earlier `running` native finalize attempts for the run to `failed`. It writes `metadata.supersededFinalization` (reason `superseded_workspace_finalization`, superseding operation id and status), an stderr note, a terminal-workspace wake (registered before the write), and a `workspace_operation.reconciled` activity entry. - The proof is the run's workspace-finalization ownership, not age. The caller holds the advisory lock and the durable owner receipt. That claim requires the previous controller to have released it or to be proven stopped (same-host process proof, platform proof, or a completed copyback barrier). The function also skips an attempt that this process is still running, and attempts with independent obligations (`uncertain`, `saveWork`). It only touches rows that `native_workspace_finalizer` recorded and that are older than the superseding attempt. - The finalizer calls it after a new attempt reaches a terminal status, and on the "already succeeded" path. A settlement error is logged and does not change the finalization result. ## Verification - `cd server && npx vitest run src/services/native-runtime/native-workspace-finalization-supersession.test.ts src/services/native-runtime/native-workspace-finalizer.test.ts` on embedded Postgres: 72 passed. - New tests in `native-workspace-finalization-supersession.test.ts` (a separate file, so `native-workspace-finalizer.test.ts` stays under 1,000 lines): - A `running` finalize from a dead controller plus a resumed attempt that succeeds: the old row becomes `failed` with the supersession reason, no `running` rows remain, and one activity entry exists. - The resumed attempt fails: the old row is still settled, and `supersedingStatus` is `failed`. - A `running` finalize older than an already succeeded attempt: the old row is settled on the next sweep. - While another controller holds ownership, the `running` row stays `running`. - A `running` finalize recorded by another service stays `running`. - Attempts with `saveWork` or `uncertain: true` stay `running`. - Red/green: with the settle call disabled, the three settle tests fail (row stays `running`). With the fix, all pass. - `npx tsc --noEmit -p server` passes. ## Risks - Low. The change only writes rows that are already proven abandoned, and only under the finalization ownership that already fences copyback. - A settled attempt now reads as `failed`. Readers pick the latest attempt by creation time, and the superseding attempt is always later, so the run's finalization outcome does not change. - Existing leaked rows on runs that never finalize again stay `running`. This PR does not add a backfill. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude Code with tool use (shell, file edit) and code execution. ## 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> |
||
|
DottaandPaperclip
|
e801be7aa9 |
fix: fence nested Git repositories during workspace migration (#19805)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Online workspace migration must preserve local files before it assigns remote authority. > - A task directory can contain a Git repository below its root. > - The source inspector treated that directory as ordinary, but the archive owner correctly required a shared Git source fence. > - This change includes each nested repository in the existing source ownership and locking checks. > - The archive still captures the full task directory, including sibling files and staged and unstaged Git work. ## Linked Issues or Issue Description Related: #19688 and #19409. Those changes cover physical source fences and nested Git archive reconstruction. This change repairs source classification for an ordinary wrapper that contains a repository. No duplicate fix was found. **What happened?** Online migration admitted a managed task directory whose root had no `.git` marker. A repository below `.review/repository` then reached the archive owner without a shared Git source fence. Capture failed with `remote_workspace_archive_online_source_git_requires_shared_fence`. Its operation and pending snapshot remained available for canonical resume. **Expected behavior** Migration must include the nested Git stores and all their source owners in the existing physical fence. It must preserve the wrapper directory and sibling files. It must refuse foreign aliases, unowned stores, busy shared sources, and source changes. **Steps to reproduce** 1. Create a managed ordinary task directory with a sibling file. 2. Create a real Git repository below `.review/repository` and leave staged and unstaged edits. 3. Run canonical online workspace migration. 4. Observe the shared-fence refusal, instead of a durable archive that can restore all the work. ## What Changed - Add a bounded metadata walk for nested Git roots. Reuse the existing strict Git pointer, common-directory, and alternate-store reader. - Add nested source identities to the existing alias closure, company checks, source locks, and metadata fingerprints. Keep the wrapper as the capture root. - Validate bounded Git-maintained reverse references in selected and borrowed stores. Require stable two-way pointers and containment by included registered workspaces. Bind the listings and pointer receipts into publication revalidation. - Add seventeen integration cases for fresh migration, pending-journal resume, shared linked and alternate stores, unrelated progress, foreign and unowned stores, and late metadata changes. ## Verification - Genuine baseline RED: both fresh and pending-journal cases failed at the existing archive shared-fence guard before the source fix. - The first treatment passed six of seven cases. The late-change case correctly failed at the earlier recapture guard; its expected-error assertion was corrected. The final test also verifies that the canonical Git fingerprint changed. - Head `8bb9f5a3ce`: real PostgreSQL, all **187 tests passed** in 59.22 seconds across `remote-workspace-migration.test.ts` and `workspace-git-source-roots.test.ts`. The canonical wrapper creates disposable databases and completed cleanup. Run from `server/`: ```sh node ../cli/node_modules/tsx/dist/cli.mjs ../packages/db/src/test-postgres-command.ts -- node ../node_modules/vitest/vitest.mjs run src/services/remote-workspace-migration.test.ts src/services/workspace-git-source-roots.test.ts --hookTimeout 90000 --maxWorkers 1 --no-file-parallelism --fsModuleCache ``` - Required alias RED: both hidden ordinary-wrapper cases failed before the reverse-reference fix. The shared case bypassed its held writer lock; the foreign case was incorrectly eligible. Both now pass. A further RED proved the same omission through a borrowed store; its hidden writer is now included too. - Review RED: a repository below `.review/.paperclip-runtime` reached the archive guard. Discovery now excludes the runtime name only at the capture root, matching the archive. - The positive cases download the authenticated immutable archive, remove the disposable source, and cold restore the sibling file, HEAD, index content, and working content. Resume retains the exact operation and pending snapshot IDs. - Resume fidelity: migration, snapshot, and layout owners are byte-identical to source revision `2d4f80504e`. Source kind comes from the workspace row, not online classification. The unchanged serializer writes the same empty source, source fingerprint, marker, fixed empty child scope, and pending generation-1/epoch-1 snapshot at the tested failure boundary. The test asserts that shape and retains the exact IDs on resume. The fixture injects the failure at that unchanged boundary; it does not execute the old binary. - Shared-store cases hold a real PostgreSQL source lock. The related import is refused before a journal exists, while an unrelated ordinary import completes. - Invalid-link controls reject foreign and unregistered roots, malformed pointers, and symlinked metadata before a journal exists. A new detached worktree after upload changes the listing fingerprint and blocks authority publication. - Final head `e525b6b1a5`: all 17 focused real PostgreSQL cases pass in 16.23 seconds after the enclosing-host-repository review fix. Its true RED first showed the valid managed wrapper as ineligible. The cold restore now excludes the ancestor Git directory and host-only file and leaves the host HEAD unchanged. Server TypeScript passes again. - Server TypeScript and module boundary checks pass. Canonical agent preflight passes. Environment: macOS arm64, Node 24.22.2, real PostgreSQL and Git. - A first test launch lacked the built plugin SDK, and a later sandbox launch could not open TSX IPC. Both were setup failures, not regression results. The existing dependency build and permitted isolated test runner resolved them. - Final exact-head CI passes: [run 42363940320](https://github.com/paperclipai/paperclip/actions/runs/42363940320). The final gate has 56 successful checks, four skipped checks, one neutral check, and no pending or failed checks. Greptile reviewed this head at 5/5, with zero open review threads. No hosted workspace was changed or imported by this PR work. ## Risks - Nested discovery and reverse-reference checks add bounded directory metadata I/O. Reverse listings allow at most 256 worktrees per store, and all links must resolve to registered physical workspace roots. Stale or unsupported links fail closed. - Nested discovery refuses more than 100,000 entries, 64 directory levels, or 64 repositories. A large or unsupported tree remains ineligible. This includes dependency or build trees that exceed those limits. No broad directory skip or metadata cache bypasses final source checks. - The archive guard, source lock owner, runtime writer checks, publication checks, and recovery predicates remain required. This change grants no permission to retire source work or ignore a held journal. - No schema, API, runtime setting, provider lifecycle, or background worker changes. - This PR documents the internal source-classification contract above. It adds no operator command or configuration to document separately. ## Model Used OpenAI `gpt-6.1-sol` through Codex, with reasoning, repository tools, code execution, and regression tests. The context window size is not exposed. ## 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> |