mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-10 01:50:13 +01:00
84908d176b2800f4c792c4249c610ec0baebdd28
2491
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 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> |
||
|
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 |
||
|
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
|
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
|
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> |
||
|
|
795ad16c1f |
fix(server): bound the shutdown steps before the run drain (#19165)
Bound initial shutdown waits and finish workspace repairs before teardown. Share a 20-second budget between waits before the heartbeat run drain. Retain the original workspace recovery stop promise and join it after the run drain and finalizers, before application and database teardown. Keep that join for hot restart when the run drain is skipped. Log step and drain durations and bound telemetry flush separately after the drain. Verification: all 154 tests across the three relevant shutdown/startup suites and every server source/test typecheck layer passed. Both actual server lifecycle variants reproduce premature teardown without the fix. All applicable CI passed and exact-head Greptile Apex scored 5/5. Co-Authored-By: Devin Foley <devinfoley@users.noreply.github.com> Co-Authored-By: Paperclip <noreply@paperclip.ing> |
||
|
Devin FoleyandPaperclip
|
bf12b2d8ad |
fix: retry reviewed chat setup after temporary contention (#19787)
Retry reviewed chat preparation with the original accepted input. Bound retries for authorization proof waits that expire before provider work. Preserve the original answer receipt and queue later wakes separately. Keep plan confirmation semantics by assigning the interaction marker only to question answer delivery and legacy question payloads. Verification: 433 tests across four backend suites, all 17 project-chat browser tests, and all server source/test typecheck layers passed. The plan approval regression was reproduced on the previous head before the fix. All applicable CI passed and exact-head Greptile Apex scored 5/5. Co-Authored-By: Paperclip <noreply@paperclip.ing> |
||
|
|
af0fe8fe81 |
Keep personal MCP connections functional while idle tenants sleep (#19427)
## Thinking Path > - Paperclip manages work for AI agents. > - Personal MCP connections should remain enabled while an idle tenant sleeps. > - Unfinished OAuth, transfer and delivery work must retain its owner and deadline. > - Cloud can receive bounded metadata and incoming work while the tenant remains the company-scoped authorizer. > - Current identity, revision and delivery acknowledgments establish when ownership has transferred. > - This change preserves useful wake, retries and revocation while making settled ownership inspectable without writes. ## Linked Issues or Issue Description Refs: #19377 and #19750. Hosted companion: [paperclip-cloud#1217](https://github.com/paperclipai/paperclip-cloud/pull/1217). **What happened?** Enabled personal MCP grants and subscriptions could keep an otherwise idle tenant awake. Ignoring those records would lose discovery, authorization, delivery and revocation obligations. **Expected behavior** Unconsumed authorization codes, device requests, file handoffs and delivery work retain their original deadlines. Settled connections remain enabled and become quiet after the current external owner acknowledges responsibility. Incoming useful work resumes normal authorization and delivery; revocation still takes effect for the current company. **Steps to reproduce** Complete normal PKCE/device consent and company selection, synchronize client/device and revocation state, retain the connection while idle, then exercise useful tools/events and revocation. Sleep eligibility requires current incoming/deadline ownership throughout. ## What Changed - Add bounded signed client/device metadata and revocation synchronization with exact revision, identity and ACK checks, startup recovery and finite legacy-device cutover. - Preserve company selection, authorization and operation identity. Register grant work before its INSERT in the same transaction; retain rollback, expiry and retry behavior. - Require current managed identity/resource before hosted publication. Receipt-cleared re-enable must obtain a fresh source ACK before device creation. - Reuse read-only settings and inspection budgets; connect MCP owners to normal startup, drain and shutdown. - Bound metadata pages and the actual signed request body while preserving the default limits of other operations. - Generate migration 0440 after the merged Legacy migration. Preserve all 438 existing executable migrations and the journal prefix; retain the normal five newest snapshots. - Preserve the merged transaction-owner forwarding and regression coverage from #19750 through normal ancestry integration. ## Verification - Current own Shared/SDK, database, eval-kernel and Runner builds passed, together with all seven normal application TypeScript programs. Missing generated package exports/workspace links were restored through normal setup; the original setup failures were retained. No source, compiler option, test assertion or timeout was weakened. - Normal migration generation changed only the five intended MCP tables; its SQL is byte-identical to the reviewed MCP delta. Fresh reference databases and both retained master/Muse histories upgraded through 0440, preserving historical hashes, row fields, schema and idempotence. Direct assertions cover the Legacy null default and MCP null/zero defaults. Two selected history cases passed; the other 21 database cases were unselected. - All six current MCP lifecycle cases passed with zero skips: normal PKCE approval intent, ACK recovery after outage, mutation during publication, disable/re-enable with original expiry, company-deletion rollback/commit and key rotation. - All 81 checks in five current whole files passed with zero skips: OpenAPI 34, read-only settings 2, diagnostic catalog 25, PostgreSQL diagnostics 7 and dependency scheduling 13. PostgreSQL fixtures ran normal migrations before test imports. - Earlier canonical proof at App `cd1c8c6` / Cloud `9f67531` includes real 60-second rate and three 15-second quiet windows, useful work and three revokes. Later paired proof at App `43af514` / Cloud `914eb0f` plus its reviewed candidate includes normal DCR, PostgreSQL metadata, a 144,017-byte signed publication, actual ACK, cap/default-413 controls and a real 15-second quiet window. The relevant producer/signing/wire inputs remain byte-identical in this integration; shared changes are qualified above. These remain source-scoped observations, not a fresh full canonical run on the integrated tree. - All known test owners, process groups and OS sockets closed; an independent final PostgreSQL audit found no remaining test sessions. One dependency-test trace recorded 46 opens and 45 close events. Its cause is unknown; this is not claimed as proof of graceful closure. - Fresh exact-head remote checks passed: 59 successful and 4 skipped contexts, with a complete current 5/5 Greptile review and all seven review threads resolved. Targeted local checks do not constitute a new whole-repository test run. ## Risks App and Cloud must agree on identity, revision, namespace, deadlines and revocation. Partial rollout, unavailable authority or missing ACK must retain awake debt. Shared startup/shutdown and settings changed since the earlier canonical profile, so historical observations carry only within their stated source scope. No deployed Railway/Neon suspension, Cloud alarm, savings or complete hosted Dot/Muse acceptance is claimed. ## Model Used OpenAI `gpt-6.1-sol` produced code and verification, with a Codex coordinator reviewing and integrating the changes. The coordinator's exact runtime model ID and context-window sizes were 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> Co-authored-by: Codex <noreply@openai.com> |
||
|
DottaandClaude Opus 5.5
|
7475577cdb |
fix(daytona): keep a runner sandbox alive through the controller reconnect grace (#19754)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A remote agent run executes in a Daytona sandbox and keeps working while the Paperclip server restarts > - The server reattaches that runner within a 7-minute reconnect grace > - Daytona stops an idle sandbox after its `autoStopInterval`, and it counts only SDK and lifecycle calls as activity > - With the server down, nothing touches the sandbox, so a 5-minute auto-stop kills the runner before the server returns > - This pull request makes the host tell the provider its reconnect grace, so Daytona never auto-stops a runner sandbox inside that grace > - The benefit is that a controller outage of up to 7 minutes no longer turns a healthy remote run into a blocked task ## Linked Issues or Issue Description Refs #19199 (the same floor for the warm runner idle window). Refs #19741 (follow-up delivery through restarts; no overlap). **What happened?** A restart-chaos wave on Daytona killed the server for 4m51s while a per-turn remote run was active. The environment used `autoStopInterval: 5`. The last sandbox activity was at 06:09:16. The server came back at 06:14:19 and then crashed again. At 06:15:21 recovery called the sandbox. The plugin found it not started and started it again. The runner identity marker was still on disk, but its process was gone: `native.remote_runner_recovery_failed {stage: process_verification, reason: process_not_running}`, then `runner_remote_process_identity_unavailable`. The task went from done to blocked. An earlier wave showed the same failure after a 4m45s outage. **Expected behavior** A sandbox that holds a live remote runner survives any controller outage up to the reconnect grace. The server reattaches the runner and the run continues. **Steps to reproduce** 1. Create a Daytona environment with `autoStopInterval: 5` and a `paperclip_runner` agent. 2. Start a remote run and kill the server for more than 5 minutes (less than 7). 3. Restart the server. Recovery reports `process_not_running` and the task becomes blocked. ## What Changed - `server`: add `resolveRunnerReconnectGraceMsForRun`. It returns `REMOTE_RUNNER_RECONNECT_GRACE_MS` for `paperclip_runner` on a sandbox environment. - `server`: forward `runnerReconnectGraceMs` to the provider worker on lease acquisition and on the recovery reattach resume. It uses the same provider-only path as `warmRunnerIdleTimeoutMs`. It is not lease identity, lease metadata, or saved environment config. - Daytona plugin: the effective auto-stop is at least the largest host floor (warm idle window or reconnect grace) plus 5 minutes. With the current grace this is 12 minutes. The plugin applies it at creation and raises an older sandbox's shorter interval on resume (bounded, best effort). - Daytona plugin: keep the forwarded grace out of the normalized config. Update the `autoStopInterval` manifest description. - `doc/execution-semantics.md`: document the reconnect-grace floor next to the warm floor. ## Verification - Red/green Daytona plugin tests: `cd packages/plugins/sandbox-providers/daytona && npx vitest run --config vitest.config.ts`. The new cases failed before the fix (for example `expected 120000 to be >= 720000`, and no `setAutostopInterval` call on resume). All 605 tests pass after the fix. - Server tests: `cd server && npx vitest run src/__tests__/environment-run-orchestrator.test.ts src/__tests__/environment-runtime.test.ts`. 247 pass. They prove acquisition forwards the grace only for `paperclip_runner` on a sandbox, that the reattach resume forwards it, and that it does not reach lease metadata or saved config. - `pnpm typecheck` for the plugin and `tsc --noEmit` for the server are clean. - `pnpm verify` asks for broad CI coverage because a doc changed. Required CI supplies it. - Evidence from the kept wave database: lease metadata shows `autoStopInterval: 5`, `reuseLease: false`, `runnerLifecycleMode: per_turn`. The run had no events between 06:09:16 and 06:14:19. ## Risks - Cost: a runner sandbox that leaks (no cleanup) now auto-stops after 12 minutes instead of a shorter configured value. Cleanup still stops or destroys released sandboxes. Explicit `0` and longer values are unchanged. Auto-archive and auto-delete are unchanged. - I did not inspect the sandbox state in Daytona directly. Cleanup destroyed it before this investigation. The proof is the timing, the lease config, and the `process_not_running` result after a restart that kept the disk. - No live Daytona replay ran. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, with tool use (shell, file edit, 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
|
e2c7d81067 |
fix(server): keep reply recovery off web startup critical path (#19769)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The server restores retained native replies during startup. > - Fair reply recovery can admit more due work while an earlier group remains active. > - Waiting for that whole pass can hold web readiness on unrelated retained replies. > - This pull request tracks reply recovery with the existing background startup owner. > - The server can become ready while idle and shutdown still account for that work. ## Linked Issues or Issue Description Refs: #19726. **What happened?** The startup path awaited the native response worker's first pass. That pass can remain active while fair dispatch admits later replies. A controlled `startServer` regression stays in recovery until the held reply pass finishes. A deployed backlog supported investigating this boundary, but no captured runtime stack proves it caused the observed startup delay. **Expected behavior** Retained reply recovery must keep its authority checks, retry deadlines, and fair dispatch. Its completion must not be a condition for unrelated web readiness. Active recovery must still block idle and be joined at shutdown. **Steps to reproduce** Run the startup regression with a held native response worker promise. Before this change, `startServer` does not complete. After this change, startup reports ready, idle work remains active, and shutdown waits until the promise settles. ## What Changed - Track the initial native response pass with the existing heartbeat scheduler work owner. - Extend the actual startup wiring test to cover readiness, idle accounting, worker stop, and shutdown join. - Document the response recovery boundary for idle and readiness. ## Verification - Mandatory agent preflight passed on macOS arm64 with Node 26.4.0. - True RED: the new startup case failed because `startServer` had not completed while reply work was held. The same case passed after the change. - Final head `279e97587b` passed the full startup test file: 123/123 tests. - The unchanged real PostgreSQL later-page case passed. It holds 100 old publication rows, publishes the independent 101st reply once while the first pass remains pending, then verifies completion and no recurring idle SQL. - Final-head CI passed: 60 successful checks and 3 skipped checks, including `ci / verify`. Fresh Greptile is 5/5 with no open threads. - Server TypeScript check passed: `node node_modules/typescript/bin/tsc -p server/tsconfig.json --noEmit`. - Initial test-launch and missing generated SDK setup failures were resolved before the regression ran. They are not RED or pass evidence. - No deployment or live recovery action was performed for this PR. Focused commands, from `server/`: ```sh node ../node_modules/vitest/vitest.mjs run src/__tests__/server-startup-feedback-export.test.ts --maxWorkers=1 --no-file-parallelism 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-response-publication-retry.test.ts -t 'publishes a later-page reply while older publication groups remain contended' --maxWorkers=1 --no-file-parallelism --hookTimeout 90000 ``` ## Risks Web readiness can complete before retained reply recovery finishes. The existing scheduler still owns errors, retries, scope checks, and delivery authority. The existing idle tracker and shutdown join retain in-flight work. Other startup recovery gates stay in place. ## Model Used OpenAI Codex, `gpt-6.1-sol`, with reasoning, tool use, and 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 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
|
b03ea621c6 |
fix(environments): retry a teardown interrupted by server death at once (#19749)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - After a remote run, Paperclip destroys its provider sandbox. The teardown first claims the lease with a 15-minute ownership fence, so no other cleanup runs at the same time > - #19696 made restart recovery end a fence when the claiming server process is proven to have exited. It only does that for claims that record the claimer > - The run-release teardown and the stop-only teardown did not record the claimer > - When the server died during a run-release teardown, the sandbox therefore stayed up until the 15-minute fence expired > - This pull request records the claimer on those claims, and stops a teardown attempt lost to a dead server from counting as a failed attempt > - The benefit is that a restart during sandbox teardown destroys the sandbox at the next startup sweep ## Linked Issues or Issue Description **What happened?** In a Daytona restart chaos wave, a run finished at 05:34:48. Its release claimed the lease and called the provider, and a SIGKILL stopped the server 2 seconds later. After restart the lease stayed `pending_cleanup` with an in-flight claim. The sandbox was destroyed at 05:49:49, when the 15-minute claim fence expired. There was no provider error. **Expected behavior** A teardown interrupted by the server's death is retried by the next startup sweep. It is not counted as a failed attempt. A real provider error keeps the normal backoff. **Steps to reproduce** 1. Release a run's sandbox lease with destroy, and hold the provider call. 2. Stop the server during the provider call. 3. Restart. On master, the sweep skips the lease until the fence expires 15 minutes after the claim. Refs #19696. ## What Changed - `environment-runtime.ts`: the run-release teardown claim (`destroyClaimedSandboxLease`) and the stop-only teardown claim save `pendingCleanupController`, bound to the claim's attempt id, the same way sweep claims do since #19696. - `heartbeat/recovery.ts`: sweep claims mark that they counted an attempt. When the sweep ends the fence of a lost claim, it gives that attempt back, so a teardown lost to a dead server does not move toward the cap or the long cooldown. Claims from other paths did not count an attempt, so nothing changes for them. - Workspace-repair and export-retry fences are unchanged. They protect workspace work, not teardown. ## Verification - Red, then green: - `environment-runtime.test.ts` → "retries a run-release teardown at the next sweep after the releasing server dies". The provider call is held during `releaseRunLeases(..., "destroy")`, the claim is marked as from an exited server, and a restarted sweep runs. On master the claim has no controller record and the sweep skips the lease. With the fix, the sweep destroys it at once and records attempt 1. - `heartbeat-pending-cleanup-sweep.test.ts` → "does not count a sweep attempt lost to a dead server as a failed attempt". On master the retry is recorded as attempt 4; with the fix it is attempt 3 again. - `environment-runtime`, `heartbeat-pending-cleanup-sweep`, `sandbox-creation-operations` and `environment-routes` pass (367 tests). `tsc --noEmit` for `server` passes. ## Risks - The proof that a claimer exited is the same-PID-space proof from #19696. A live, foreign or unidentified claimer keeps its fence. - Giving back a counted attempt applies only after that proof. A teardown that fails with a provider error still counts and backs off. - No schema change. The new fields live in existing JSON metadata. > 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, via Claude Code with tool use (shell, file editing, test execution). Extended reasoning enabled. ## 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> |
||
|
|
96bc021edb |
fix(server): certify settled legacy processes for future retries (#19377)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Bounded retries must preserve current authority, budgets and deadlines. > - The tenant can wait for supported future work after the source process and its cleanup stop. > - Retained process identity alone cannot prove that cleanup finished. > - This change records host-owned settlement and checks its exact identity before a retry can cover its wait. > - Uncertain process state, changed authority, live claims and cleanup debt keep the tenant awake. ## Linked Issues or Issue Description Refs #19001. Related dependencies: #19019 and #18988. Related #19340 covers explicit Native task Retry; this change covers future Legacy process retries. Current integrated source: `1645970a9322d1213ba25fc7b52366c285682977`, normally merged with actual master `131b566ca4fa866c4348739c75cfe48c805b3398`. The prerequisites above are merged. This PR targets `master`. **What happened?** A future issue, company or review retry can retain its failed source process identity. The deadline reader could not prove that host cleanup finished. Supported waits stayed blocked. Ordinary issue retries also needed an exact source-settlement check, and represented coalesced requests needed exact input coverage. **Expected behavior** A supported future retry covers only its existing wait, under current company and actor authority, after exact host settlement. Missing, malformed, stale, cancelled, Native or unfinished source state stays blocked. No retry gains new execution authority. **Steps to reproduce** Fail an ordinary process or fail environment preparation before adapter entry. Let normal cleanup schedule the existing retry. Inspect its settlement and deadline, stop and reconstruct the owner, and admit one useful successor at the saved deadline. Hold cleanup or change authority to verify that the reader refuses coverage. ## What Changed - Add nullable internal `legacyProcessSettlement` state through normally generated migration `0439_cultured_impossible_man`. No backfill or adapter result creates the record. - Write exact host settlement after stopped-process and cleanup checks. Compare the current run and reject live claims or cleanup debt. Optional persistence failure preserves normal cleanup and keeps sleep coverage blocked. - Add a separate preparation receipt for a claimed Legacy run that failed before any adapter or Native dispatch. Require the same boot, run times, preparing stage, released claim and completed cleanup. A null PID alone proves nothing. - Clear settlement on a new claim, process metadata or Native selection. Omit the internal record from public serialized data. - Check exact source lineage and current authority for ordinary issue, company and taskless retries. Cover completed coalesced requests only when the admitted retry represents their exact inputs. - Add the closed `current_review_retry` wait with current reviewer, stage and existing issue authority checks. Require a nonempty current stage ID. - Add actual producer, cold reconstruction, SQL negative, redaction and optional-persistence controls. - Repair CI fixture expectations for the additive column, startup mock, budget owner and popup return URL. Bind the controlled Native comment server explicitly to loopback. - Replace temporary Slack clock logging with an exact persisted-deadline assertion. Wait until that real deadline before the existing single default poll. Keep both forced polls, business assertions and timing limits. ## Verification - On final integrated source `1645970a`, normal Shared/SDK prerequisites, Runner TypeScript and the entire normal App layered typecheck pass. Tracked source and lockfile remain unchanged. - All 26 whole-file settlement, recovery-reader and bootstrap controls pass. Seven separate real PostgreSQL producer/negative cases pass at `cf16c6a`; each has one selected passing case and 18 filtered registrations. Filtered registrations are not acceptance. - Master then changed the preparation source-lock scope. Its three intersecting cases were rerun on final `1645970a`: an actual failed company process, an actual failed ordinary issue process, and held preparation cleanup. Each preserves its source identity and original deadline and admits one useful successor. Issue and preparation cases exercise normal automatic retry; the company case uses the normal explicit bounded-retry producer. - The unchanged 26 controls and four other PostgreSQL cases retain their `cf16c6a` proof after a scoped source comparison. Optional settlement/preparation persistence failures keep certification blocked while normal cleanup completes. An entered adapter without process identity stays uncertified. This is scoped carry, not a claim that every earlier case ran again on the final head. - All recorded test owners and controlled groups closed. Final read-only SQL audits returned no test sessions. One earlier postcase resource barrier observed a transient unattributed database backend; that database and original evidence remain preserved. Its later disappearance is recovery, not an explained cause. No assertion, deadline or test body was relaxed. - Migration0439 was generated normally after master history. All executable SQL and journal history are preserved; the five retained snapshots are0435–0439. Every Legacy-owned patch and unrelated actual-master blob/mode is preserved. - Published exact head `1645970a9322d1213ba25fc7b52366c285682977` passes all current required checks: 68 successful contexts and eight expected skips after exhaustive pagination and superseded-run accounting. The fresh complete Greptile review scores this exact head 5/5. All four inline review threads are resolved. The latest Ponytail turn stopped without a verdict; one normal retry was requested, with no active turn or new finding observed. No old-head score is used. The original old-base policy failure, its dependent aggregate failure, and the corrected description-gate failure remain recorded. No full local repository suite is claimed; remote typecheck, test and build gates pass. To repeat the real database coverage, set both `PAPERCLIP_RECOVERY_DEADLINES_TEST_DATABASE_URL` and `PAPERCLIP_RECOVERY_DEADLINES_TEST_ADMIN_DATABASE_URL` to the same fresh dedicated test database and run the selected `recovery-deadline-acceptance.test.ts` cases with one worker. An unset URL, skipped case or filtered registration is not acceptance. The fixtures use controlled local processes and real PostgreSQL. Reconstruction creates new service owners inside the same test process. They do not prove deployed Cloud alarms, physical host/database suspension, new-host recovery or Native capacity settlement. The original30-second retry remains below the Cloud270-second minimum wake lead. The merge projection also includes actual master `c3180398faca815fed7a62674bd4b0c4e88884ae`, whose common completion path now sends deferred/recovery notifications. Its unpublished normal merge tree is `16b028d57150f8caa576e913f38343a365b542dd`. Normal Shared/SDK, Runner TypeScript and the full App graph pass on that combined source. One unchanged ordinary-issue acceptance case was rerun there: an actual failed process retains the original automatic retry deadline, admits one useful successor, and passes the settlement, authority and no-duplicate assertions. It passed in 45.055 s; 18 filtered registrations are not acceptance. All known owners and groups closed, and the independent final database audit found zero remaining test sessions. The successful database was removed normally. Recorded worker socket peak is unavailable for this run; parent-process zero values are not used as worker measurements. All 29 Legacy-owned paths and every unrelated master blob and mode are preserved. Two outside-diff review observations concern existing waiting-chat compaction and cross-host Native result-lease recovery. Their files are byte-identical to current master. This focused Legacy change does not fix or qualify those separate paths. ## Risks Settlement proves exact local closure. It does not cover live provider work, Native ownership, cancellation, costs, claims, cleanup debt or changed authority. Independent checks remain. Ship normal migration0439 with the reader and writer. Preserve executable history and the newest five snapshots0435–0439. Generate the MCP child's next migration normally after this parent. The prerequisites are merged; the final remote checks must use this integrated exact head. This proof uses controlled local processes and external PostgreSQL. It does not establish deployed Cloud alarm acceptance, physical host or database suspension, cold host recovery, or R5 Native capacity settlement. The30-second retry is below the Cloud minimum wake lead. This focused repair follows the roadmap's bounded recovery and Cloud deployment work. It adds no new provider policy. ## Model Used OpenAI `gpt-6.1-sol` assisted with readers, fixtures, source review and verification through Codex. A coordinating GPT-6-family agent implemented and reviewed shared lifecycle wiring and integration. Tools included Git, native TypeScript checks and controlled tests. Exact served revisions and context limits were 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 and its available capability details - [x] I have checked ROADMAP.md and related work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have linked existing public work and described the issue - [x] I have not included internal issue IDs or private links - [x] My branch name describes the change - [x] Required focused checks pass at their explicitly recorded source/carry scopes above - [x] I have added or updated meaningful tests - [x] I have recorded the internal contract and migration requirements - [x] I have documented risks and evidence limits - [x] All required published exact-head Paperclip CI gates are green - [x] Published exact-head Greptile is 5/5 with no open findings in this focused diff; separate unchanged master observations are identified above - [x] I will address reviewer findings before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> Co-authored-by: Codex <noreply@openai.com> |
||
|
DottaandClaude Opus 5.5
|
e620ce3678 |
fix(heartbeat): deliver each accepted follow-up exactly once through restarts and admission holds (#19741)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A user can comment, click Retry, or rely on automatic recovery while an agent works on a task > - Restart-chaos waves showed accepted follow-ups being dropped, or delivered twice, around restarts and temporary company admission holds (accounting still settling after a restart) > - A dropped comment means the user's request is never acted on. A duplicate reopens work that is already done > - This pull request makes accepted work wait through a temporary hold and resume when it clears. It also stops a comment from being promoted again after a completed run already received it > - The benefit is that each accepted follow-up is delivered exactly once, eventually, unless the task is cancelled or a reached hard stop applies ## Linked Issues or Issue Description Refs #19702 (never-started tasks during a temporary hold), #19700 (bounded accounting hold), #14910 (stale reopen from a deferred wake; no file overlap). **What happened?** Restart-chaos waves found these shapes: 1. **Comment wake cancelled.** A comment was saved behind a run and promoted when the run ended. A temporary company hold refused the unstarted queued run, and claim cancelled it terminally. The recovery wake that followed was cancelled the same way, and the task rested `blocked` without the comment. 2. **Dead Retry.** A Retry click during the hold stayed `deferred_issue_execution` after the hold cleared. Nothing woke it, and the issue held no execution lock. 3. **Refused assignment.** A new task's first wake was refused during the hold. Nothing woke stranded-work recovery when the hold cleared, so the task stayed `todo` with no run. 4. **Silent comment after recovery.** A comment was saved behind a run that restart recovery finalized. A company-scoped hint waited out the 30 s deferred-receipt rescan, so the task looked done with the message unread. 5. **Comment delivered twice.** A run whose provider process was lost had a saved comment. Its retry started after the comment, received it in its prompt (task history), did the work, and finished the task. The saved wake was then promoted anyway, which reopened the done task. **Expected behavior** An accepted follow-up (comment, Retry, recovery) is delivered exactly once, eventually, unless the task is cancelled. A temporary refusal never ends in a terminal cancel, skip, or block. Reaching a hard stop remains the board's decision. ## What Changed - `heartbeat/queue.ts`: on a budget block at claim, an unstarted queued run stays queued when the block is temporary. A reached hard stop or blocked unpriced usage still cancels it. - `recovery/service.ts`: stranded-work recovery waits out a temporary hold for any task. It does not escalate to `blocked`. This extends #19702 beyond never-started tasks. - `budget-block-ownership.ts`: a shared `isBoardOwnedBudgetBlock`, moved out of the recovery service. `budgets.ts` is unchanged. - `run-cost-accounting.ts`: settling a run's accounting wakes queued-run dispatch, the deferred-receipt worker, and stranded-work recovery. Held work waits passively; this is what resumes it. - `deferred-execution-notifications.ts`: new `notifyDeferredWakesForIssue`. `native-run-projection.ts` (a run reaching a terminal status) and `environments.ts` (lease release) now hint each saved wake of the issue by id, so delivery does not wait out the company rescan interval. - `wake-queue/adapters/postgres.ts`: release treats a saved comment as delivered only when a succeeded run of the wake's agent on that issue started after the comment's latest revision, and either received the comment as its own wake input (`wakeCommentIds`) or was a retry of the interrupted turn whose task history included it. In that case it cancels the saved wake with "Queued messages were already delivered to a completed run" and leaves the task as it is. Task history alone never consumes a message saved for its own turn. One query checks the whole batch. - `run-cost-accounting.ts`: settling late receipt debt also sends the hold-release notifications. - `doc/execution-semantics.md`: documents the rule. ## Verification Red on `master` and green with this change, on embedded Postgres: - `server/src/__tests__/heartbeat-held-comment-wake.test.ts`: - A promoted comment refused by the hold stays pending, with no cancelled run and no `blocked` task. It is delivered once after the hold clears through real `accountRunCost`. - A Retry clicked during the hold starts exactly once after accounting settles, which wakes its owner. - A new task whose assignment was refused during the hold starts once accounting settles, which wakes stranded-work recovery. - A comment that a later succeeded retry already received as history, or that a later run received as wake input, is consumed without reopening the task. - Guard tests (green on both): - A comment is still delivered in each of these cases: the later run never received it, it was edited after that run started, it was posted while the run was already running, or a non-retry run only saw it as history. - A task whose continuation was refused waits out the hold. - A reached hard stop is still refused. - `server/src/__tests__/native-run-projection-work.test.ts`: projecting a recovered run terminal hints the saved comment's own wake (red on `master`). - Related suites pass locally (14 files, 482 tests): wake-queue, queued comments routes, refused assignment wake, native run projection. - Live replays (local, unmasked, kept instances): - `--replay w-20261107T031005-s7501/t04` (Claude C1, K1): PASS. The comment's run was created at 03:56:24 during the hold, stayed queued, started at 04:06:24 when the hold cleared, and succeeded. It is the only run carrying the comment. - `--replay w-20261107T002911-s7401/t01` (before the per-wake hint): it reproduced shape 4. Run 592d772f was killed 250 ms after `run.result.accepted`, released and finalized at restart, and the task rested `done` with the comment still saved. Calling the existing sweep on the kept database promoted it, which confirmed that the missing piece was the trigger. ## Risks - A queued run can now sit queued for the length of a temporary hold, which #19700 bounds. It does not hold the issue's execution lock, so later comments still coalesce into it. - The delivered-comment rule depends on the run's recorded prompt context. A succeeded run that received the comment in its history counts as delivery, even if the agent did not act on it. This matches exactly-once delivery semantics. - Settling accounting now emits three extra notifications per run. They are coalesced per company. ## 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, live restart-chaos replays). ## 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
|
9f0ebf6f70 |
fix(accounting): close a run whose turn was lost on restore, and stop succeeded runs holding all work (#19724)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Budgets hold new work while the cost of completed runs is unknown > - In a restart chaos wave, a native ACPX Claude run crossed a server kill, resumed, and succeeded. Its usage receipt never became complete. > - For the 10-minute closure bound, admission refused all work in the company. This included tasks that never crossed a fault. > - This pull request fixes the receipt for a turn that was lost on restore. It also stops a succeeded run from holding all new work. > - The benefit: one late usage report no longer freezes a company, and hard stops still apply. ## Linked Issues or Issue Description Refs #19700 (the accounting closure bound that this follows). **What happened?** A native ACPX Claude run crossed a server SIGKILL in the middle of a turn. The Runner restored the session, and the run succeeded 54 seconds after it started. All 17 usage receipts stayed incomplete. Accounting settled the run only at the 10-minute closure bound. During those 10 minutes, admission refused all company work with "Company cannot start work while completed runs await accounting". **Root cause** ACPX reports provisional usage (`runDeltaComplete: false`) until a turn ends. The kill lost the turn that was active, so that turn never got a complete report. The restored Runner failed that turn with `provider_turn_lost_on_restore`. Native turn accounting (`createNativeTurnAccounting`) required every terminal turn to have usage, so this turn kept every later receipt incomplete. This was true even after the continuation turn completed with full usage. The admission gate then counted the succeeded run as a pending terminal run for the whole company. **Expected behavior** The receipt closes when the run's turns end. A turn that was lost on restore makes the usage a lower bound. A succeeded run with only usage outstanding does not hold other work. ## What Changed - `native-usage-accounting.ts`: a turn that failed with `provider_turn_lost_on_restore` and has no complete usage no longer holds the receipt open. The snapshot marks `accountingUsageLost`. - `native-session-executor.ts` and `native-usage-delivery.ts`: such a receipt carries the provenance note "a provider turn was lost on restore and never reported its usage; observed usage is a lower bound". - `anthropic-pricing.ts` and `codex-pricing.ts`: the rate-card pricers keep a producer's provenance note. - `native-usage-delivery.ts` and `usage-receipts.ts`: remote producer delivery keeps a `lostOnRestore` marker on the receipt's producer event, so replay also excuses the lost turn. Receipts saved without the marker still replay. - `budget-exposure.ts` and `budgets.ts`: admission waits only for pending runs that ended without succeeding (`waitingRunCount`). A succeeded run whose provider reported observed usage is bounded like a running run: its observed cost counts toward spend and reservation capacity, and its settled charge can still reach the hard stop. - `budget-exposure.ts`: a closure that timed out because of a control-plane restart keeps the unpriced-usage exemptions, whatever its token count. Restart evidence is a replaced receipt source, a native recovery transition that is not an ordinary same-run retry, or a turn lost on restore. A second chaos run showed the need. It recovered and succeeded, but its final usage report was lost with the restart, so it settled as unpriced 0 cents. The blocking policy then opened a hard incident and paused the company. A timed-out closure with no restart evidence still counts. - `run-cost-accounting.ts`: a lost receipt that carries a positive provider-reported cost settles at that cost as an estimate marked as a lower bound, not as unpriced. - `paperclip-runner-tool-authority.ts` and `budgets.ts`: a running run's own tool mutations are evaluated as admitted work. Hard stops and pauses still refuse them. Accounting holds, recovering runs and budget inputs that await evaluation do not. In a third chaos run, a live run's write to its task document was refused during a temporary hold, so the run proposed a blocked result. - `doc/SPEC-implementation.md`: documents these rules. Policy choice: a succeeded run's provider work is over, so it has less unknown spend than a running run. Running runs never held admission. Failed, cancelled, timed-out and interrupted runs still hold new work, because their provider may not have stopped. A per-agent gate would still freeze unrelated work of the same agent. A per-reservation gate does not help when `reservationCents` is 0, which was the case in this wave. ## Verification - Live: a restart chaos wave ran master plus this PR (c2a48fb1e0) with no masking. It passed 6/6, and every task crossed a restart. There were 0 accounting holds, 0 refused agent writes, 0 company pauses, and every cost settled as estimated. - Red then green: `native-usage-accounting.test.ts`, "closes a run whose earlier turn was lost on restore, as a lower bound". It uses the event sequence of the chaos run. - Red then green: `closed-native-receipt-accounting.test.ts`, "does not hold other work for a succeeded run with only usage outstanding". It also checks that the hard stop still applies. - I changed "neither pauses nor holds admission while a succeeded run's charge commits" to the new policy. A failed run in the same state still holds new work. - Red then green: `closed-native-receipt-accounting.test.ts`, "never hard-stops a blocking policy for a restart-derived timed-out receipt". It uses the zero-token row shape of the chaos run and a case with unpriced tokens. Without the fix, the company was paused. - Red then green: `runner-api.integration.test.ts`, "lets a running run write during a temporary accounting hold, but not after a hard stop". - Red then green: `closed-native-receipt-accounting.test.ts`, "does not hold other work for the s7504 succeeded row shape". - Red then green: `closed-native-receipt-accounting.test.ts`, "settles a timed-out restart receipt at its provider-reported cost, without holding or pausing". It uses the OpenCode/OpenRouter row shape from a fourth chaos run. - `anthropic-pricing.test.ts`: the rate-card provenance keeps the lower-bound note. - Local: `closed-native-receipt-accounting`, the budget suites, `native-usage-accounting`, `native-usage-delivery.integration`, `native-runner-restart-recovery.integration` and `anthropic-pricing` all pass (187 tests). Server `tsc --noEmit` is clean. `pnpm verify` asks for CI coverage because the spec changed. ## Risks - A succeeded run whose final charge is higher than its observed cost can let one more task start before the charge commits. The hard stop then pauses the scope as it does for a running run that overruns. - A run that includes a lost turn settles with a lower-bound usage figure. The provenance says so, and a later receipt still reconciles through the late-receipt path. ## 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> |
||
|
|
131b566ca4 |
fix(workspaces): report safe Git restore diagnostics (#19736)
Return the restore stage and failure category with the existing HTTP409 rejection. Preserve the unsaved comment and editable draft. Keep nested saved-commit and clone-timeout diagnostics through preparation wrappers, and distinguish local Git file errors from authentication failures. Retain only safe process flags privately; no raw command cause is exposed.
All seven affected local suites pass (288 tests) and the layered server typecheck passes on supported Node25.6.1. Reviewed head
|
||
|
|
839d41aabb |
fix(server): retire owned plugin listeners before idle sleep (#19358)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Plugins receive durable agent lifecycle events through a registered host. > - The host uses a PostgreSQL LISTEN connection to wake delivery. > - An idle report could return `none` while that host-owned connection stayed open. > - This change gives that listener an exclusive owner and closes it during the current idle hold. > - Release or expiry restores listening before journal replay. > - Failed close and pending work continue to block sleep. ## Linked Issues or Issue Description Refs #18988, which is now merged. This focused ten-file change now targets master. Current integrated candidate: `0f83005122ef1bdb1f1e08f8f58424c9a840ebb1`, with actual master `644ee66f5d4ae8365fe8b9eca57df2be7405b5ab` preserved through normal merges. No history was rewritten. **What happened?** A normal local App with an installed lifecycle plugin could return an idle `none` report while its lifecycle LISTEN backend and socket stayed open. **Expected behavior** The host must close its own listener before it reports idle safety. It must retain other subscriptions, durable events, company scope, retry deadlines and useful reactivation. **Steps to reproduce** Start the normal server with a lifecycle plugin. Create an agent and wait for its durable acknowledgment. Stop the server, create another agent while it is stopped, and restart with the same files. Request an owned idle hold after ordinary replay. Check the exact listener backend and matched Node socket before shutdown. Let the valid hold expire naturally, then make an ordinary authorized agent update and check its durable plugin acknowledgment. Live executing or persistent preview services remain active. The separate hosted preview dependency [paperclip-cloud#478](https://github.com/paperclipai/paperclip-cloud/pull/478) retains its own App and operator prerequisites. This PR does not claim deployed preview or suspension acceptance. ## What Changed - Add a private, exclusive database listener owner that inherits the normal client policy. - Prepare plugin lifecycle retirement after native retirement under the same current idle hold. - Use one delivery-eligibility helper for dispatch and idle journal inspection. It preserves the same manifest, capability, company-disable query, and skip rules. - Close only the owned listener. Keep failed close ownership and inspection failures visible. - Resume listening on normal release or expiry, before journal replay. - Preserve disabled-feature retry history and the original deadline on re-enable. - Await lifecycle disposal during normal shutdown and consumer teardown. - Add owner regressions and document the lifecycle contract. ## Verification **Current integrated source:** `0f83005122ef1bdb1f1e08f8f58424c9a840ebb1`. Fresh normal Runner TypeScript passed in 0.785 seconds. The complete normal App layered semantic check passed in 28.284 seconds. All tracked source, the lockfile and the 18 separate private witness inputs stayed unchanged. Known compiler owners closed and the isolated temporary home was removed. **Current product qualification:** at measured integration `896cc9b62993e6ed10418d7a7417e1ddbf8dc89c`, all three whole files passed with no failures or skips: lifecycle delivery 40, runtime hooks 20, and runtime assignments 24. The actual physical LISTEN and natural-expiry controls passed. All owned clients and captured processes closed. Independent final session checks were zero before normal deletion of each successful test database. **Normal full-App physical retirement:** the original case passed in 47.574 seconds. It exercised real startup, readiness, SDK callbacks, durable ACK, foreign-company exclusion, physical stop, offline input and cold replay. The initial unknown hold and its ordinary release remain in the evidence. After the original 30.5-second cadence, a new valid hold returned quiescent. The exact current cold-App LISTEN backend and its matched Node socket closed before server stop. **Normal full-App natural expiry:** the separate original case passed in 77.391 seconds under its original 90-second body cap. The final valid 30-second hold expired naturally. A new listener backend and matched current App socket appeared before fresh input. An ordinary authorized agent-name PATCH produced journal event 20, the actual SDK callback and durable ACK. Foreign exclusion and readiness assertions passed. The final measured hold had no manual release, replacement hold or shortened TTL. The original preparatory unknown hold, ordinary release and 30.5-second cadence remain explicit. Both App cases used the normal current server helpers and actual SDK bundling/worker entry. A private compatibility change supplied the same owned HOME at the exact existing child-fork boundary. It did not change the product helpers, business assertions or timing limits. These separate acceptance fixtures remain outside the focused product diff. **Source carry to the current candidate:** all nine other owned files, listener mechanics, actual producer paths, SDK paths and measured hold behavior are unchanged. The sole owned intersection from newer master is an independent environment-delete cleanup callback in `app.ts`. The plugin witnesses do not call that route. Normal merges preserve every unrelated master blob and file mode. The fresh current semantic checks above qualify the integrated source. New master Native crash-recovery and saved-copyback Retry capabilities are not claimed by these plugin cases. **Actual master merge target:** newer master contains retained plugin Runner release/resume and other Native recovery fixes. The projected combined target passed fresh normal Runner TypeScript in 0.799 seconds and the full App semantic check in 29.715 seconds. All ten plugin-owned blobs and modes remain exact to the published head; every unrelated master change is preserved. The measured plugin cases create no Native provider turn or warm Native session. The unchanged native-first/plugin-second ordering still checks the native result before listener retirement. Later sparse-checkpoint and Native response-publication changes do not enter those paths. This is a scoped source carry, not new hosted Runner acceptance. **Failures retained:** an earlier reactivation fixture's real timer returned one millisecond before its original wall-clock expiry. Its private fixture now waits any positive remainder and retains the same expiry assertion and limits. The earlier partial-composition `unknown` result remains distinct from normal-startup proof. Current qualification also exposed missing normal workspace dependencies and a private guard that assumed loose Git refs. Normal owned dependency setup and an exact loose-or-packed reader fixed those setup failures. No business assertion, timeout, permission boundary or product sleep gate was relaxed. Repeat focused product coverage with a fresh external fixture database and the normal server Vitest command, one worker: ```sh pnpm --filter @paperclipai/server exec vitest run src/__tests__/plugin-lifecycle-delivery.test.ts --maxWorkers=1 --no-file-parallelism pnpm --filter @paperclipai/server exec vitest run src/__tests__/agent-runtime-hooks.test.ts --maxWorkers=1 --no-file-parallelism pnpm --filter @paperclipai/server exec vitest run src/__tests__/agent-runtime-assignments.test.ts --maxWorkers=1 --no-file-parallelism ``` Exact current head `0f83005122ef1bdb1f1e08f8f58424c9a840ebb1` has 60 successful checks, three skips and one neutral result, with no pending or failing contexts. Greptile completed its full review at 5/5 with no actionable issue. All root and nested review pages were read; the earlier eligibility finding is resolved. Ponytail's latest turn stopped before a verdict. A normal retry was requested; no current Ponytail verdict or score is claimed. Previous `3ca86ec` CI and scores are historical. A full local repository typecheck/test/build or browser run is not claimed. Controlled local App/SDK/PostgreSQL proof does not establish live provider, deployed Railway/Neon suspension, signed Cloud alarms or global exactly-once delivery. ## Risks - A listener close failure or uncertain inspection must continue to block sleep. - Release, expiry, configuration changes, and journal arrival can race with retirement. Epoch and current-hold checks retain conservative results. - Delivery remains at least once across uncertain ACK boundaries. This change does not add global exactly-once delivery. - The database helper closes only its exclusive listener client. Other owners and subscriptions keep their resources. ## Model Used OpenAI `gpt-6.1-sol`, reasoning and code/tool use. The managing Codex agent implemented shared database/composition seams and reviewed the change; its exact model ID was not exposed. The runtime does not expose a verified 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> Co-authored-by: Codex <noreply@openai.com> |
||
|
DottaandPaperclip
|
1c91bacf97 |
fix(native): admit independent replies during contended publication (#19726)
## Thinking Path > - Paperclip manages work done by AI agents. > - A completed native run must deliver its saved final response. > - The response worker also recovers retained replies that can wait on publication locks. > - A pass waited for its old publication groups even when other check slots were free. > - This could delay an independent final reply beyond a full page of retained candidates. > - This PR enables the existing shared coordinator option that admits independent due work during an active pass. > - Existing authority checks, resource keys and the four-check limit still control publication. ## Linked Issues or Issue Description **What happened?** A native run completed after replacement recovery. Its final response and accounting were saved, but its final issue reply was missing. The response queue contained more than 100 retained chat replies and repeated endpoint-lock failures. The existing worker did not admit independent due entries until its current groups finished. **Expected behavior** A contended publication owner must not hold free worker slots. An independent accepted final reply must publish once, including when its candidate is on a later SQL page. **Steps to reproduce** Create 100 accepted, committed replies that share one publication owner. Create an independent reply after them in candidate order. Hold the old coordinator rows with a database transaction and start the real response worker. Before this change, the independent reply stays unpublished until the old group is released. Related: #18977 changes response selection in standing conversations. This PR changes only worker admission and preserves response selection. ## What Changed - Enable `admitDueWhileActive` in the native response worker's existing shared coordinator configuration. - Add a real PostgreSQL regression with the full 100-row SQL page, the default dispatch batch, real row-lock contention and a later-page independent reply. - Verify one comment per reply after settlement and no further SQL or scheduled deadline across 60 seconds of idle time. - Update one existing principal/rescope integration assertion to observe the reconciled invalidation revision. Keep its actual per-run replies, authorization, publication and idle checks. ## Verification - Regression on the unchanged worker: the independent reply remained unpublished while the first 100 replies were locked. The real PostgreSQL test failed as expected. - Final head `748fb9609104c016c33ff7caf282ba9e15e88495`: all six publication regressions and both principal/rescope controls passed (8/8, 47.96 seconds). Checks cover later-page admission, exact per-run comment counts, retry and rollback behavior, authorization, and 60 seconds with no idle SQL or scheduled deadline. - The tests used the canonical PostgreSQL wrapper and one owned disposable PostgreSQL 18.1 fixture. All clients and test databases were closed before fixture cleanup. - The current-head [full CI run](https://github.com/paperclipai/paperclip/actions/runs/42338953241) supplies typecheck, build and broad test coverage. All current-head checks passed; the merge gate is green. - Local full typecheck was blocked by unbuilt dependencies; it is not counted as a pass. The prior timer-state CI failure and the final successful rescope-generation regression remain recorded. - Greptile reviewed the final head at 5/5 with no unresolved threads. `git diff --check` passed. - Deployed acceptance remains separate. No serving instance was changed by the PR tests. ## Risks The worker can now use free slots during a slow pass. The shared coordinator still limits checks to four and preserves task, endpoint and principal exclusions. Authorization, result identity, retry limits, response selection and transaction ownership are unchanged. The regression checks idle quiet after all concrete obligations settle. ## Model Used OpenAI GPT-6.1 (`gpt-6.1-sol`), with reasoning, code execution and tool use. The runtime does 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 - [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
|
aed053633e |
fix(environments): run pending sandbox cleanup when an environment is deleted (#19716)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Operators delete a sandbox environment, optionally with consent to destroy its sandboxes (`DELETE /api/environments/:id?destroyReusableSandboxLeases=true`) > - A sandbox whose teardown failed or was interrupted is kept as a `pending_cleanup` lease, and it blocks the delete > - The cleanup sweep retries such a lease only after a cooldown of up to 5 minutes > - A consented delete therefore returned 409 `pending_sandbox_cleanup` for minutes. A caller with a shorter deadline gave up, and the environment and its sandbox stayed behind > - This pull request makes the consented delete run one teardown attempt for the environment's pending cleanups now, then re-check > - The benefit is that a consented environment delete completes as soon as the provider teardown succeeds ## Linked Issues or Issue Description **What happened?** After a restart chaos wave, the harness deleted the wave's Daytona environment with `destroyReusableSandboxLeases=true`. The server returned 409 `pending_sandbox_cleanup` every 3 seconds for the whole 120-second client deadline. A run lease had entered `pending_cleanup` after its run failed, and the sweep's retry cooldown had not passed. The environment was not deleted. **Expected behavior** With explicit consent, the delete tears down the environment's own pending sandbox cleanups and completes when that teardown succeeds. A teardown that fails, or one already in flight elsewhere, still blocks the delete. **Steps to reproduce** 1. Leave a sandbox lease of an environment in `pending_cleanup` with a future retry time. 2. `DELETE /api/environments/:id?destroyReusableSandboxLeases=true`. 3. On master, the response is 409 `pending_sandbox_cleanup` until the cooldown ends. Refs #19696 (restart recovery for creation and teardown claims; this builds on it). ## What Changed - `heartbeat/recovery.ts`: `sweepPendingCleanupLeases` accepts `environmentRetry: { environmentId, actorId }`. Like the existing run-scoped explicit retry, it bypasses the cooldown for one attempt, never takes over an in-flight attempt, and logs `environment_lease.cleanup_retried` with reason `environment_deleted`. It touches only that environment's leases. - `routes/environments.ts`: when the delete has consent and the only blockers are pending cleanup and reusable leases, it runs that retry first, then re-reads the blast radius. The existing reusable-lease destroy and the final guard are unchanged. The new dependency is a narrow, optional `retryPendingEnvironmentCleanup` option; without it, behavior is unchanged. - `app.ts`: wires the option to the existing heartbeat service instance. ## Verification - Red, then green: - `environment-routes.test.ts`: "runs the environment's pending sandbox cleanup now and deletes with explicit consent" and "keeps rejecting when the consented pending cleanup attempt does not finish" fail before and pass after. "does not run pending sandbox cleanup without consent or past another blocker" passes in both. - `heartbeat-pending-cleanup-sweep.test.ts`: "runs one attempt for a deleted environment's cleanup during its cooldown, scoped to that environment" fails before and passes after. It checks that another environment's lease and an in-flight claim are untouched, and that the activity log is written. - `sandbox-creation-operations.test.ts`: "reclaims an issue-bound creation after its run failed and the issue finished on a later run" covers another chaos finding. A creation intent bound to an issue was interrupted by SIGKILL 0.5 s after it committed, its run ended `process_lost`, and a later run finished the issue. The test fails on the code before #19696 and passes on master, so #19696 already covers this case. The test is kept as a regression. - After rebasing on master, `environment-routes`, `heartbeat-pending-cleanup-sweep`, `agent-runner-qualification-routes` and `sandbox-creation-operations` pass (192 tests). `tsc --noEmit` reports no errors in the changed files. ## Risks - A consented delete now makes provider teardown calls for pending cleanups. Those sandboxes were already queued for destruction, and the operator consented to destroy the environment's sandboxes. - One explicit attempt counts toward the lease's attempt cap, the same as a user Retry. - The delete request now waits for those teardown attempts. Each attempt uses the existing plugin timeouts. > 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, via Claude Code with tool use (shell, file editing, test execution). Extended reasoning enabled. ## 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
|
51eb4ff705 |
fix(recovery): finalize an accepted native result after a hard kill (#19713)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A native run hands its accepted result to a controller lease, and the controller finalizes the task from that result > - A graceful shutdown releases that lease for the next server (#19591), but a hard kill (SIGKILL, OOM, host crash) cannot > - After a hard kill between result acceptance and finalization, the next server waited for the dead controller's 20-minute lease, so the run stayed `running` and the user's follow-up comment waited with it > - This pull request releases that lease at startup when the controller is proved dead by its boot ID and process identity, never by age > - The benefit is that the saved result finalizes within seconds after a restart, without another provider turn, and queued follow-up work continues ## Linked Issues or Issue Description Refs #19591 (the graceful-shutdown counterpart) and #19578 (boot-identity proof for legacy runs). **What happened?** A restart-chaos wave sent K1 (SIGKILL) 20 ms after a Claude run recorded `run.result.accepted` and `run.terminal`. The run then stayed `running` for more than 10 minutes, through four more restarts. The finalization row was `workspace_finalizing` with the stored result. Its lease belonged to the killed server: a different `controller_boot_id`, a `controller_pid` with no process, and `lease_expires_at` 20 minutes ahead. Startup recovery skips runs that already have a result. The result-commit worker waits until the lease expires. The task's follow-up comment could not run until then. **Steps to reproduce** 1. Let a native local run accept its result. 2. SIGKILL the server before finalization finishes. 3. Restart the server. The run stays `running` until the old lease expires. **Expected behavior** After a restart, a result lease held by a stopped controller of an earlier boot is treated as released at once. The stored result finalizes once, with no new provider turn, and the follow-up wake runs after it. ## What Changed - New `server/src/services/native-runtime/native-dead-controller-result-lease.ts`: `releaseDeadControllerResultLeases` finds `running` native runs with an accepted result and an unexpired lease. It requires the lease to belong to a different controller boot. It proves the controller stopped with `evaluateNativeControllerTakeover` (`requireStoppedController`): the process is dead, or its PID now names another process. It then expires that lease with a compare-and-set on the exact owner token, boot ID, and PID, and notifies the result-commit and finalization workers. - `server/src/services/heartbeat/recovery.ts`: `recoverNativeRunsAfterRestart` calls this first and records a `native.recovery.transition` event (`awaiting_result_finalization`) for each released run. - Only executor leases are released, because only the executor claim writes the controller identity together with the lease. Finalizer (`native-finalizer:`) and cleanup (`native-cleanup:`) claims keep an earlier executor's identity, so they are never released this way and expire on their own short leases. A finalization-resume token (`:resume:`) on a result row always belongs to the executor that adopted it, because a pending resume claim is taken only on a row without a result. - Also not released: a live owner, an owner without a recorded process, a lease of the current boot, a run this process is executing, and remote workspaces (`nativeWorkspaceSync` or `remoteWorkspace`), which keep their fenced lease. - The compare-and-set does not compare the stored expiry. Postgres stores microseconds that a JavaScript `Date` drops, and an exact-expiry comparison silently skipped the real lease in the live reproduction. - `doc/execution-semantics.md`: documents the rule. ## Verification - Red then green on embedded Postgres (`server/src/__tests__/heartbeat-native-restart-suspension.test.ts`): - A hard kill with a dead controller's executor lease (first claim, restart claim, or resumed claim) now finalizes on the next startup: run `succeeded`, finalization `committed`, task `done`, one result, one run, no provider dispatch. These tests failed before the fix with `status: running`. - A lease expiry stored with microsecond precision is released. This test failed against the first version of the fix and reproduces the live miss. - A live controller of another boot keeps its lease, and so does a lease of the current boot. - A live claimant's finalizer or cleanup lease is kept when the recorded earlier executor is dead. - `npx vitest run src/__tests__/heartbeat-native-restart-suspension.test.ts`: 20 passed. - Related suites pass: `native-session-resumption`, `native-runner-restart-recovery.integration`, `native-run-projection-work`, `live-recovery-work`. One `live-recovery-work` legacy case timed out once under load and passed when run again by itself. - Live, local: `tests/restart-chaos/main.ts --workloads C1 --triggers result --faults K1 --rc06-masked`. The failing case above came from such a wave. I started its kept database and confirmed the stored lease state and the compare-and-set miss. Against that same database, the fixed release expires the stuck executor lease. Two later waves (12 tasks, `--triggers result`) passed every task. Run events were not kept for passing tasks, so I cannot confirm that a kill landed in the narrow pre-finalization window in those waves. - Live proof on head `8971874654`: local ACPX Claude, workload A1, seed 7411, `--triggers result` (a local variant that fires on A1 2.5–3.5 s after the run's result proposal), `--keep`. The run passed 4 of 4 tasks. In two tasks, K1 landed 1.5 s and 2.0 s after `run.result.accepted`. On the next startup, each run recorded "Server restart released the accepted native result of a stopped controller for finalization" (`controller_process_dead`). Each finished `succeeded` about 1 s later, with its task `done` and exactly one run per task (no extra provider turn). Before this fix, the same shape stayed `running` until the 10-minute budget ran out (seeds 7411, 7422, and 7423 from the RC-33b investigation in #19691). ## Risks - Startup could take a lease from a finalizer that is still running. Mitigations: the lease must belong to another boot, the controller process must be proved stopped on this host (the same proof the existing runner takeover uses), and the update matches the exact owner. Remote workspaces are excluded. Multi-host deployments keep the existing PID-liveness assumption that `claimNativeRestartRecoveries` already makes. - Low behavioral risk otherwise: finalization uses the existing reconciler path, which #19591 already exercises after a graceful drain. ## 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, live restart-chaos waves). ## 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
|
08f7ac03dc |
fix(recovery): keep a never-started task waiting through a temporary accounting hold (#19702)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Admission checks every wake. A short company accounting hold can
refuse new work after a control-plane restart.
> - When the hold refused a new task's assignment wake, stranded-work
recovery found the task with no run while the hold was still active. It
moved the task to `blocked` with a board recovery comment.
> - A `blocked` task leaves the stranded-assignment sweep, so nothing
woke it after the hold cleared. The task stayed `blocked` with no run,
no interruption and no Retry.
> - The hold was temporary. Tasks created a few seconds later (after the
sweep) started normally once accounting settled.
> - This pull request keeps a never-started task `todo` while a
temporary budget hold lasts. The existing assigned-todo liveness
dispatch then admits it once the hold clears. The task shows the hold it
waits for.
> - The benefit is that a restart no longer silently drops new tasks
created during the accounting settle window.
## Linked Issues or Issue Description
**What happened?**
In a restart-chaos storm on master `06bbcea12f`, three new tasks were
created while the company accounting hold was active. Each assignment
wake got 409 "Company cannot start work while completed runs await
accounting." (or "...native run recovers its unfinished accounting.").
One task rested `blocked` with 0 runs, interruption `null` and
`canRetry=false`. The other two ran later.
**Root cause**
`reconcileStrandedAssignedIssues`
(`server/src/services/recovery/service.ts`, the
`readInvocationBudgetBlock` branch near line 4826) escalates any
assigned task to `blocked` when a budget block exists. It does this
before the assigned-todo no-run branch, which already skips a
budget-blocked task and dispatches it later. So a task that never
started was treated as failed recovery work. The refused wake itself is
stored as a `skipped` `budget.blocked` diagnostic, which is correct; the
escalation removed the only path that re-admits it.
**Expected behavior**
A wake refused by a temporary admission hold waits and is admitted once
the hold clears, with one run. A real hard stop is still a board
decision. Removed assignees and cancelled tasks still do not wake.
**Steps to reproduce**
Create a company with a hard-stop budget and a completed run that has
`cost_accounting_pending = true`. Assign a new `todo` task. The wake
gets 409. Run the stranded-assignment sweep. Before this change the task
becomes `blocked` and never runs.
Related: #19700 bounds the accounting hold itself. This PR is
independent: it handles what happens to a task whose first wake the hold
refused.
## What Changed
- Stranded-work recovery skips a `todo` task with no run while a
temporary budget hold blocks admission. It does not escalate it to
`blocked`. The existing assigned-todo liveness dispatch admits the task
once the hold clears.
- A hard stop (spend at the limit, or blocked unpriced usage) keeps the
existing board escalation. Recovery checks this with the read-only
`policyBlocks` for the policies of the blocking scope. It does not
change budget hold semantics.
- The task interruption projection now covers a `todo` task with no run
whose latest wake was refused with `budget.blocked`. It shows "Waiting
for the budget to allow new work. The task starts automatically when it
does." with no Retry. A budget pause shows the existing "budget" user
action.
- The no-run projection applies the same person-owned gates as the
run-backed projection: a budget pause shows the "budget" action, and an
ordinary agent, project, company or task pause shows "resume". Only an
accounting hold shows the automatic-start text.
- `ExecutionBlockerNotice` shows that wait text for a `working`
presentation that carries a wait reason.
- `doc/execution-semantics.md` records the rule.
## Verification
- New embedded-Postgres regression
`server/src/__tests__/heartbeat-refused-assignment-wake.test.ts`:
- hold active, new task assigned, wake refused (409, `skipped`
`budget.blocked`), two recovery sweeps: the task stays `todo` with no
run and shows the wait reason. Before the fix this failed with `expected
'blocked' to be 'todo'` (the CHA-9 shape).
- hold cleared, two sweeps: exactly one provider turn starts, from
`issue.assigned_todo_liveness_dispatch`, and the task finishes. A later
sweep does not start another turn.
- counter-tests: a terminated assignee and a cancelled task get no run
after the hold clears.
- Existing `creates a board recovery action for budget-blocked assigned
work` (real hard stop) still passes unchanged.
- `pnpm exec vitest run
src/__tests__/heartbeat-refused-assignment-wake.test.ts
src/__tests__/heartbeat-process-recovery.test.ts
src/services/task-interruption-presentation.test.ts
src/__tests__/heartbeat-issue-liveness-escalation.test.ts` (server) and
`src/components/ExecutionBlockerNotice.test.tsx` (ui).
- Server typecheck and `pnpm check:token-gates`.
- Live restart-chaos storm (local, codex/claude/opencode, 6 in flight,
K2 every 3-4 minutes for 20 minutes, no RC-06 mask), revision
|
||
|
DottaandClaude Opus 5.5
|
8a34c1b62c |
fix(budgets): bound the accounting hold so one missing receipt cannot wedge a company (#19700)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Pending run accounting holds new company work until it settles (#19497, #19606, #19673) > - A run settles only with closure evidence: a verified local stop, or a terminated remote sandbox. Without that evidence it held the whole company forever. In restart-chaos storm 4, a single run held 35 tasks for hours > - Two more shapes kept the hold alive. A run whose result had already been accepted never counted as closed. A recovery that already owned its held reservation was refused by other tasks' recovering accounting, so concurrent recoveries waited on each other > - This pull request bounds the hold, closes accepted-result runs on a verified stop, and lets an owned recovery finish > - The benefit is that no single missing receipt can wedge a company, while genuinely unknown spend still surfaces under the blocking policy ## Linked Issues or Issue Description Follow-up to #19497, #19606 and #19673. **What happened?** - Storm 4: run `f879895f` timed out in `session.open`. Its local stop was never verified (its process identity is still recorded), recovery ended in `terminal_failure`, and its receipt never became ready. From then on, every new-work wake got 409 "New work must wait while a native run recovers its unfinished accounting". That was 93 refusals, 35 blocked tasks, and no Retry. - Storm 4: run `6d3c76bf` was resuming as the recovery owner with its own held reservation. `reserveRunBudget` refused it because other tasks' runs were recovering. - Wave s7401: run `0a3a849d` failed after its result was accepted, with a verified local stop. It had 10 receipts across 2 sources, all incomplete with 0 tokens. It stayed "completed runs await accounting" because closure only covered coordinators in terminal failure with no result. **What did you expect?** A run without closure evidence holds new work only for a bounded time, then settles visibly. An accepted-result run with a verified stop closes like a terminal failure. A recovery that already owns admitted work can finish. **Steps to reproduce** 1. Restart the control plane during several native turns so that one run's local stop is never verified. 2. Observe the run stay "recovering" and every new wake refused indefinitely. ## What Changed - `run-cost-accounting.ts`: - A coordinator counts as settled when it ended in terminal failure without a result, or when it has an accepted result. The verified-stop and remote-termination closures use that rule, which covers the s7401 shape. - A native run whose provider never started settles at once, like a pre-provider failure, with no charge and its reservation released. Its recovery has settled, no usage receipt was ever journaled (a provider turn journals one as soon as it starts), and its local process never launched or was verified stopped (wave s7403, run 3c1d7b70, `session.open` timeout). - New bound `ACCOUNTING_CLOSURE_TIMEOUT_MS`: the remote runner reconnect grace (7 minutes) plus 3 minutes. A terminal native run with a settled coordinator and no closure evidence settles at the bound: - its known usage is recorded as unpriced (or estimated from the rate card), with provenance evidence "closure evidence never arrived…"; - it is marked `accountingClosureTimedOut`; - a `cost.accounting_closure_timed_out` activity entry names the run. - The reconciler selects these runs, and `readCostAccountingWork` schedules a wake at the bound. - `budget-exposure.ts`: a timed-out closure's final charge is exempt from none of the unpriced-block exemptions. Under the `block` policy it raises the normal budget notice ("recorded usage has no reliable price") instead of a silent hold. - `budgets.ts`, `budget-reservations.ts`, `run-dispatch/adapters/postgres.ts`: when a validated recovery owner continues admitted work, admission and both dispatch rechecks apply only hard stops and pauses. Pending and recovering accounting holds still gate new work. - `doc/SPEC-implementation.md`: documents the bound, the accepted-result closure and the provenance. ## Verification - `closed-native-receipt-accounting.test.ts` (embedded Postgres). All of these fail on master and pass here: - s7401 shape (accepted result, verified stop, all receipts incomplete with 0 tokens): settles and clears admission. - A run without closure evidence: held before the bound with a scheduled wake, then settled with the "closure evidence never arrived" provenance, the `accountingClosureTimedOut` flag and an activity entry. Admission clears under `allow`. - Storm 4 `f879895f` shape (terminal failure, recorded process identity, no stop event): held, then released after the bound. - The blocking policy still blocks on a timed-out closure. - s7403 shape (no receipts, model unknown): a run whose process never launched, or was verified stopped, settles at once with no charge and its reservation released. With an unverified process it waits for the bound. - Storm 4 `6d3c76bf` shape: a recovery owner with its held reservation reserves past another task's recovering accounting, while new work for another task still waits. - Existing suites pass locally: budget-enforcement-scoping, budget-enforcement-worker, native-stop-accounting, cost-data-quality, heartbeat-cost-accounting, run-cost-accounting, budgets-service. Server typecheck passes. - The other storm 4 terminal-failure runs (`2e477ae9`, `e4d2e14d`, `6d3c76bf`, `7d21a44c`, `4bc49df0`, `1d85fbd9`) were already settled by #19673 in that database (`cost_accounting_pending=false`) and were not holding admission. `f879895f` alone held it. ## Risks - After the bound, a run whose provider might still be running settles as unpriced. Under the `block` policy that raises a hard-stop budget notice, by design. Under `allow`, work continues and a late receipt still reconciles. - A recovery owner skips accounting holds, so concurrent recoveries can spend while other tasks' accounting is unknown. Hard stops still apply, consistent with best-effort budget enforcement. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file editing, embedded Postgres tests). ## 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> |
||
|
|
18be7ac696 |
fix(server): retire idle native sessions before tenant sleep (#18988)
## Thinking Path > - Paperclip lets people manage AI agents and their work. > - Native agents can retain a warm session between useful turns. > - That owner must close and save its files before the tenant can sleep. > - A failed close must keep the owner visible until retirement succeeds. > - This PR prepares idle resources under the exact current admission hold, then takes a fresh safety snapshot. > - Busy turns, uncertain closure and changed holds still prevent sleep. > - CI also exposed a chat authorization race. The fix repeats full authorization after the exact delivery commits. ## Linked Issues or Issue Description Refs #18639 for local work and incoming wake boundaries. Related #18829 covers suspension receipts after admission. **What happened?** Idle preflight did not retire warm Native owners. Three failed-close paths could forget an owner without proving that its provider stopped. CI also found a false chat rejection when a delivery changed from processing to processed between two validation reads. **Expected behavior** Retire only idle owners under the exact bounded hold. Wait for closure and file collection. Retain failed owners. Scan safety after preparation. Keep diagnostic reads passive. A newly committed chat delivery must pass the full current authorization checks before attestation succeeds. **Steps to reproduce** Finish a warm Native turn. Start an idle drain and request safety with its owner ID. Delay or reject provider closure, or replace the hold during preparation or scanning. The report must keep the instance awake. For the chat race, commit the exact delivery between the first binding read and its fallback read. **Paperclip version or commit** Current head `7f6f02b611dcf5a9a2b05173a44e9631eaf78058` normally integrates actual master `1aa0c3639714f4976cfa56f95aae6e2b7d1235ac`. The merge preserves every unrelated master path. Fresh exact-head CI and complete review remain required. ## What Changed - Retain uncertain Native owners after failed restart, idle timeout and failed-turn closure. - Prepare Native retirement through the existing heartbeat facade. Reuse provider closure and stopped-file collection. - Track preparation under the exact idle owner and expiry. Reject new activity, changed holds and expired holds before and after awaited work. - Take the final durable and local safety snapshot after preparation. Keep diagnostic inspection passive. - Cover ephemeral workspace retirement and useful next use, supported plugin drain and SDK persistence, busy owners, failed closure and hold replacement. - Keep dedicated recovery storage explicit. Use the same validated options for support and storage. Probe-free helpers retain the original implementation bodies and preserve default callers. - Complete the Native fixture through real retirement and budget evaluation. Keep its rejection, file, credential, accounting and strict NONE assertions. - Repeat the complete chat authorization once if the fallback sees the exact batch now processed. Preserve owner locks, principal and binding checks, denial handling and the original retry deadline. ## Verification - The support-mode regression reproduces one failure with four passes under the old policy, then passes all five controls. It retains prior allocation and cleanup cases. The explicit mode does not probe embedded PostgreSQL or use an admin alias. - The deterministic chat race baseline reproduces two authorized failures with all four current-authority denial controls passing. Both Slack and Discord use real PostgreSQL and the real authorization function. The negative setup commits before the attestation transaction; it does not claim to revoke access through a row lock. - The product fix passes normal own App declaration emission with no public declaration changes, plus both current normal owning compiler programs (399 and 611 roots). The same six controls pass after the fix. The full original file plus these controls passes all 30 tests with no skips. - The original controlled Native continuation passes: one test, no failures or skips, 1.718 seconds in its body and 23.138 seconds in its launcher. The original 40-second case and 180-second outer bounds are unchanged. It verifies held closure, strict NONE, an ordinary authorized comment, saved session/files, useful output, encrypted credentials, accepted results and settled accounting. All ten observed processes and groups close; worker connections and listeners reach zero. A separate post-owner audit finds zero sessions; the successful owned database is normally removed. - Earlier preparation, HTTP, lifecycle, workspace and plugin controls remain source-specific evidence. They are not a claim that every final-tree test ran locally. - The original CI assertion log is retained. The controlled race proves a possible cause; the original CI SQL schedule was not captured. Two other jobs lost their self-hosted runners and provided no test verdict. Earlier asset timeouts, readiness failures and unexplained later recovery remain separately recorded. No timeout, failure predicate or authority check was weakened. Focused public commands: ```sh pnpm exec vitest run server/src/__tests__/idle-sleep-preparation.test.ts server/src/__tests__/instance-settings-routes.test.ts --maxWorkers=1 --no-file-parallelism pnpm exec vitest run server/src/__tests__/heartbeat-reviewed-chat-binding.integration.test.ts --maxWorkers=1 --no-file-parallelism ``` Local database qualification uses fresh owned test databases on existing PostgreSQL, bounded pools and one worker. The local allocation adapter changes only database allocation for this test module; it runs normal migrations and client cleanup. It is not part of the published source. No new cluster is created. Local acceptance measured the `0a69efa6` integration with the same committed authorization bytes. The later normal master merge adds startup-timeout classification and recovery changes. Those paths are outside the controlled healthy-backend witness; retirement and authorization hunks are preserved. Exact-head remote CI at `7f6f02b611dcf5a9a2b05173a44e9631eaf78058` has completed with 56 successful contexts and four skips. The complete current Greptile summary is5/5 with no new actionable findings; all review threads are resolved. Ponytail’s current turn is explicitly paused without a current-head verdict; earlier actual findings are fixed and accounted for. No whole final-tree local repository build/test run, live-provider restore or deployed suspension is claimed. Actual master subsequently advanced to `d5c4f7dd02d408a261cedd9a1926b9900a4aedc4`. Its local normal-merge projection with this reviewed head preserves all20 owned paths and every unrelated master blob and mode. Current Runner TypeScript, App declarations and their full owning compiler programs pass. Focused Native cleanup and warm-continuation composition checks are being completed before merge; the new upstream suspension-retry behavior is not inferred from the older controlled-backend witness. The remote PR head remains unchanged. ## Risks - Privileged idle preparation can close idle warm Native sessions. It requires the exact current hold. - Failed closure retains its owner and can keep the tenant awake until normal retry succeeds. - A changed hold cannot authorize sleep. Started cleanup remains tracked until it settles. - The chat repair can perform one extra full authorization read. It keeps the existing deadline and does not grant authority from the fallback query. - Unsupported resources and active work retain their existing blockers. This PR does not certify every transport, plugin or deployed host. ## Model Used OpenAI `gpt-6.1-sol` produced the delegated changes and verification. OpenAI Codex, based on GPT-6, performed managing review and integration. Both used repository tools and code execution. The coordinator's exact backend snapshot and context-window limits were 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 at exact head `7f6f02b611dcf5a9a2b05173a44e9631eaf78058` - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups at the exact current head - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> Co-authored-by: Codex <noreply@openai.com> |
||
|
DottaandClaude Opus 5.5
|
a5fe5deebe |
fix(environments): reclaim sandboxes left by a restart during acquire or teardown (#19696)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Remote agent runs execute in provider sandboxes (for example Daytona) that Paperclip creates and must clean up > - Before each provider create, Paperclip saves a durable creation intent (`sandbox_creation_operations`). Recovery uses it to find and destroy a sandbox whose lease was never committed > - Recovery waited 30 minutes after creation before it checked an intent, because a live server could still be waiting for the provider > - After a restart, the creating server is gone, but the intent still waited the full 30 minutes. A short-lived or redeployed instance never reclaimed those sandboxes > - The teardown that follows has the same problem: a teardown claim fences other sweeps for 15 minutes, and a restart during the claim kept the sandbox alive until the fence expired > - This pull request saves the server process identity with each intent and each teardown claim, and ends the wait when same-host proof shows that process exited > - The benefit is that a restart during sandbox acquire or teardown no longer leaves paid provider sandboxes behind ## Linked Issues or Issue Description **What happened?** A control-plane restart during Daytona sandbox acquire left the created sandbox without an `environment_leases` row. In one restart chaos wave, 6 creation intents stayed `pending` with `next_attempt_at` = creation + 30 minutes. The wave ended (and its environment was deleted) before that time, so 4 sandboxes stayed `stopped` in Daytona. Only Daytona's own auto-delete would remove them. Two of the runs did not exist in `heartbeat_runs` because they were non-run acquires; their intents still had the run id. **Expected behavior** After a restart, each sandbox that this instance and company created, with no committed lease and no live creator, is found by its exact creation name and labels and destroyed through the plugin, after the existing ownership checks. Sandboxes from another instance, another host, or a live server stay untouched. Nothing acts org-wide. **Steps to reproduce** 1. Start a Daytona-backed run. 2. Kill the server (SIGKILL) after the creation intent commits and before the lease commits. 3. Restart the server. On master, the intent stays `pending` for 30 minutes and the sandbox stays in Daytona. 4. The regression tests in `server/src/__tests__/sandbox-creation-operations.test.ts` reproduce this with the controlled provider fixture. Refs #18357 (added the durable creation intent this change completes). **Paperclip version or commit** master |
||
|
DottaandPaperclip
|
d5c4f7dd02 |
fix: preserve shared Git sources during online workspace migration (#19688)
## Thinking Path > - Paperclip preserves the work produced by its agents. > - Remote workspace migration must preserve legacy Git history and local edits. > - Linked checkouts can share administration and objects with other tasks. > - A task lock alone cannot exclude writers of a shared or borrowed store. > - The online source owner must derive and retain the full physical ownership closure. > - This change reuses task, project, lifecycle and finalizer locks for that closure. > - Idle legacy Git work can then migrate while unrelated remote turns continue. ## Linked Issues or Issue Description Refs: #19642. **What happened?** Online migration keeps legacy Git sources blocked because several task checkouts can share a project base or a private Git cache. A read-only source observation found 160 pending Git records, 120 distinct paths and nine common stores. Its largest project cohort contained 126 records and 110 task owners. An ordinary-directory fence cannot preserve these sources by itself. **Expected behavior** Derive the physical source owners from stable Git metadata. Hold their existing source and lifecycle keys through publication. Retain all history and recovery obligations. Reject foreign aliases, unknown stores and changed sources. Let unrelated object-backed work continue. **Steps to reproduce** 1. Create a managed project checkout and linked task worktrees. 2. Keep a settled prepared recovery source on a sibling task. 3. Keep unrelated remote work running. 4. Request an online import of an idle leaf checkout. 5. The existing owner keeps the shared source blocked. **Paperclip version or commit** This branch builds on merged #19642 (`7ac99718a4`) and fresh master `fb3cf1a072`. It adds the Git closure. It does not add a writers-stopped shortcut. **Deployment mode** Hosted native Runner with remote workspace storage. Local qualification uses owned synthetic Git repositories, disposable PostgreSQL databases and the actual archive and storage owners. ## What Changed - Read bounded stable Git pointers, common-directory metadata and local alternates. Derive the complete physical source ownership cohort. Preserve all object stores. - Retain common and borrowed store owners in preparation, lifecycle, runtime, expiration and actual reopen paths. Keep company boundaries and the existing 128-key bound. - Hold exact sibling native recovery proofs under their existing physical finalizer keys. Recheck descriptors, allocations, recovery actions and source identities before capture and publication. Preserve original states and warnings. - Require leaf-first nested imports. Reuse the existing immutable child download, restore and source comparison before parent exclusion. - Bind the self-contained archive owner to the held Git namespaces. Keep its three walks, history normalization, fsck, bytes and digest format. Reject bare, detached, network, unknown or changed Git storage. - Check both foreign path fields. Keep foreign metadata private. Document the proof, bounds, source observation and remaining eligibility limits. ## Verification - Final revision `2934e41c923b93e16508b978c849b747c86be645`: all checks are terminal, with 56 successful and four skipped, zero failures and zero pending. The [required CI aggregate](https://github.com/paperclipai/paperclip/actions/runs/42326464408/job/128266279210) passed. Build, types, all server/Runner/serialized suites, end-to-end aggregate and Snyk passed. [Fresh exact-head Greptile review](https://github.com/paperclipai/paperclip/pull/19688#issuecomment-6699760104) is 5/5 with no open threads. GitHub reports MERGEABLE and CLEAN. - Review then found two correctness gaps: an ordinary managed directory inside an unrelated host repository was blocked, and a Git parent could miss a completed nested operation after its child snapshot became missing or deleted. Three actual-owner regressions reproduced both gaps in 9.70 seconds. The corrected owner passes 12 focused cases in 17.08 seconds. It imports and restores only the ordinary root bytes, preserves the outer repository, rejects both missing/deleted child receipts before a parent journal, and retains valid/changed child coverage and private-cache contention. Its wrapper exited 0 and cleanup completed. The first bootstrap attempt failed before tests ran because the host exhausted its 32 shared-memory segment limit; no shared services were stopped and no pass is claimed for that attempt. The final qualified source/test type graph, module boundaries and diff checks pass. - Receipt scans keep canonical path resolution. A lexical SQL prefix can omit children reached through symlinked ancestors. The observed full metadata closure completed in 1.17 seconds; no measured bottleneck justifies weakening that alias proof. Writer admission still rejects uncertain Git ownership before mutation. Expiration returns its typed skip, and reopen returns its structured failure. Runtime and lifecycle do not bypass an unknown source. No broad error-contract change was added. - Full CI at `720104ea975e` exposed five existing boundary cases across three server shards. Four exact cases reproduced locally as failures in 25.98 seconds. Reopen now preserves its structured failure when source inspection rejects the path, without reconstructing it. Runtime recognizes only the exact regular ignore-only `.git/info/exclude` fragment; migration still rejects that fragment. The original arbitrary-path and file-URL import/delete-host/resume fixtures keep both supported input forms and use the genuine offline setup path after their sole writer finishes. The production mode gate is unchanged. A new real-owner control proves the offline flag still rejects a running legacy database writer. - The final focused run passed all ten cases across four suites in 22.75 seconds, including all five CI failures, invalid-pointer admission, fragment rejection, offline writer exclusion and actual nested reopen lock ordering. Its wrapper exited 0 and cleanup completed. The known invalid-pointer case remains fail-closed; the separate unexpected-I/O diagnostic retains safe error metadata without command text or credentials. Existing Git stderr redaction coverage remains intact. The final metadata reader suite passed 20 cases in 2.68 seconds. The qualified server/test source type graph, module boundaries and diff checks pass. No production assertions or timeout defaults were weakened. Fresh CI and review are required for the repair revision. - A later review found that the ordinary early return still used only one foreign path field. The actual ordinary providerRef-only regression failed as incorrectly eligible. One source-path owner now enumerates both local locators for company/foreign ordinary and Git scans, layout checks, early admission, lifecycle/reopen/runtime and expiration/move/preparation retention. Capture and display keep their original preferred path. The actual owner canary passed 49 cases across two suites in 109.15 seconds. It covers cwd-only/ref-only/different/symlink locators, same-company mismatches, foreign owners, lifecycle ordering, Git/recovery imports and expiration/reopen. Three selected-locator fallback cases also failed before their guard; the final eight-case control passed in 31.05 seconds, including the configured alias and valid import/private-child controls. All wrappers exited 0 and completed cleanup. The metadata/locator suite passed 19 cases. The final source/test type graph, module boundaries and diff checks pass. - Review found two P1 gaps. Both configured-directory symlink cases failed before canonicalization. The actual parent import lost an already-imported child's unique private cache and failed its archive fence instead of retaining that owner. The corrected real-owner canary passed four cases in 20.78 seconds: configured symlink runtime provisioning, private-cache contention before the parent journal, successful parent publication after release, and unchanged/changed child coverage. The metadata suite passed 15 cases in 4.14 seconds. Both wrappers exited 0 and cleanup completed. - A fresh source read-only observation found 160 pending Git records and nine common stores. An additional exact receipt query found 1,243 completed migration receipts and 557 nested source paths under pending Git roots. All 126 records in the largest project were present in the captured Git metadata set. For all 110 present candidate workspace records, the derived closure fits the unchanged 128-key cap. The worst case needs 110 issue keys and one project key, with 17 keys of headroom. Its 173 imported children introduce no physical issue owner outside those 110 logical task owners. This checks the observed key bound only. It does not claim migration eligibility or a coherent snapshot. - The initial stacked CI did not run four Runner test lanes: trusted master passed new --package-lane/--native-server options to the old base lane script. The branch is now rebased onto actual merged master and retargeted before synchronization. It includes the canonical current lane parser without an unrelated Runner patch. Fresh new-head CI is required. - The actual importer/source owner run passed 36 cases across three suites in 187.75 seconds. It covers sibling prepared recovery, unrelated running remote work, common-cache contention, changed coordinator/allocation/path state during upload, exact nested coverage, dirty unpushed work, and existing expiration/reopen owners. Wrapper exit 0 and cleanup completed. - A separate previously hanging lifecycle-worker restore passed in 17.33 seconds with the unchanged 15-second test-body limit. An earlier run timed out in three expiration/reopen cases and blocked cleanup; its wrapper later exited 1. These failures remain recorded. They are not counted as passes. - Two boundary controls first failed: a foreign providerRef-only alias was incorrectly eligible, and actual reopen did not admit its distinct cache owner at the initial source boundary. The narrow guards now inspect both foreign locators and derive physical Git keys before the raw lifecycle transaction. The foreign alias and existing real nested reopen contention passed. The private-cache control first retained contradictory recorded source provenance and stayed fail-closed after release. The corrected present legacy-worktree fixture passed in 12.61 seconds. These failed attempts remain recorded; they are not passes. - The metadata reader and serialized/local archive controls use actual Git repositories. They compare archive bytes and digests, remove every original source, restore staged and unstaged work, and run fsck. The metadata reader passed 13 cases in 2.85 seconds. The local/serialized archive controls passed nine cases in 9.27 seconds. All processes exited 0. An additional actual importer control changed a relative commondir pointer to the absolute name of the same store during upload. Publication was rejected, the uploaded journal remained available, and original files stayed intact (one case, 5.86 seconds; wrapper exit 0 and cleanup complete). - The final rebased full local server/test type graph passes with db/shared/Runner source aliases because reused dependency declarations predate new master exports. The first rebased attempt reported those stale declaration fields/tables, and is not counted as a pass. Module boundaries and diff checks pass. Standard package builds, types and broad regression checks remain required in CI. - No hosted import, admission hold, cancellation, provider mutation, source filesystem write or shared-cache Git command was performed for this change. The source observation is read-only and is not a locked eligibility proof. ## Risks - A missing writer owner could make physical source exclusion incomplete. Existing mutation owners retain the same company/task/project keys, and uncertain metadata fails closed. - A project can contain a large cohort. Limits remain finite. A bound failure keeps the source blocked. - Bare, detached, unowned or network Git stores remain blocked online. Missing archived sources retain their existing authenticated reconstruction path. - Exact recovery proofs preserve authority and obligations. They do not infer provider termination or mark a prepared descriptor finalized. - This change does not claim all observed Git records are immediately eligible. Existing preparation, provider, recovery, runtime and source-change blockers remain. ## Model Used OpenAI GPT-6 through Codex, with reasoning, tool use and code execution. The host does not expose a more specific backend 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
|
d26372dc8a |
test(restart-chaos): add a result trigger and recovered-run comment-wake guards (#19689)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The restart chaos harness kills the control plane at chosen moments and grades whether each task still finishes correctly > - One wave lost a user's follow-up comment after a kill that landed after the run accepted its result but before the run settled > - No existing trigger aims a fault at that moment, so the case was hard to reproduce on purpose > - The recovery path that settles such a run had no regression test that covers the saved comment > - This pull request adds a `result` trigger to the harness and a regression test for the recovery path > - The benefit is that the harness can aim faults at the result-accepted window, and a change that drops, repeats, or re-wakes a saved comment fails a test ## Linked Issues or Issue Description **What happened?** In a restart-chaos storm wave, a user posted a comment while a native run was active. A hard kill (K1) landed after the run accepted its result and before the run settled. The run settled `succeeded` and the task stayed `done`, but no follow-up run read the comment. **Steps to reproduce** The original failure did not reproduce. Three targeted waves ran 12 tasks with K1 at comment time or in the new result window. Every follow-up comment was consumed exactly once. The new regression test also passes on current `master`. The test and the trigger guard the path and do not claim a fix. **Expected behavior** A comment posted during a run is consumed exactly once after the run ends, including when restart recovery settles the run. ## What Changed - `tests/restart-chaos`: add `--triggers result` (C1 only). K1 fires 0–3 s after the run that received the follow-up proposes or accepts its result (`run.result.proposed` or `run.result.accepted`). The fault fires only while that run is still `running`: the observer checks the latest sample and rereads the runs right before dispatch. A run that settled during the delay, or a follow-up posted at rest, records a `result` miss. Add poll-driven unit tests for these cases and a README row. - `server/src/__tests__/native-run-projection-work.test.ts`: add embedded Postgres tests in which background recovery, not the executor, settles the run (projection, environment settlement, and deferred-receipt workers), with a deferred user comment and an active local lease left by the killed executor. The tests check: - The comment is promoted into exactly one follow-up run, and a second restart adds none. - A comment the run already received as a same-turn steer is not woken again. The other saved comment still is. - A queue whose only comment was steered stays cancelled, and no follow-up starts. - A Retry click does not consume the comment a second time. A click that observed the source is stale. A click that observed the follow-up joins it. ## Verification - `cd tests/restart-chaos && npx vitest run --config vitest.config.ts` (96 tests pass) - `cd server && npx vitest run src/__tests__/native-run-projection-work.test.ts` (54 tests pass) - Live, local, ACPX Claude, Codex and OpenCode: `node cli/node_modules/tsx/dist/cli.mjs tests/restart-chaos/main.ts --env local --workloads C1 --triggers comment --faults K1 --adapters codex,claude,opencode --tasks 4 --concurrency 2 --rc06-masked` passed 4 of 4. Two waves with `--triggers result` consumed each follow-up exactly once. One task in those waves found a separate defect: a kill 1.4 s after the result commit left the run `running` behind the dead controller's 20-minute finalization lease. A separate pull request fixes that defect. ## Risks - Low risk. These are test and harness changes only. No production code changes. - The `result` trigger depends on the observer's poll interval (about 3 s), so the exact kill point within the window varies. ## 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). ## 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
|
5e4d24b6c8 |
fix(recovery): preserve admitted workspace on native restart (#19693)
## Thinking Path > - Paperclip manages AI agents and their work. > - A Retry can admit a new workspace before native provider work starts. > - The native execution input records that admitted workspace. > - A server restart must recover the same run under that recorded contract. > - Retry readiness previously evaluated fresh admission again after the run had started. > - This could replace the workspace and make the retained lease fail its correct identity checks. > - This change keeps the admitted workspace and leaves reattachment authority with the existing lease and controller checks. ## Linked Issues or Issue Description **What happened?** A native run started by Retry could do useful work, then fail after a server restart. Preparation evaluated the original fresh-workspace receipt again. It reserved a replacement workspace under the Retry intent ID. The existing runner still owned the original workspace, so setup failed with `native_remote_recovery_lease_mismatch`. **Expected behavior** Restart recovery must retain the workspace recorded in the same run's native execution input. A later configuration or admission decision must not replace it. **Steps to reproduce** 1. Admit a remote workspace for a Retry run. 2. Persist its native execution input and keep its original controller and environment lease records. 3. Recover the same run while its canonical Retry receipt selects fresh admission. 4. Preparation must reach the original lease with the original workspace. Related: #19340 preserves verified Retry workspaces. This change handles an already admitted native run instead of selecting a workspace for a new turn. Public duplicate searches for admitted/fresh workspace restart found no matching repair. ## What Changed - Stop fresh admission when a parsed native input names a real workspace in the run's company. - Validate the company, run, agent, and task binding before using that input. - Keep synthetic run-ID bootstraps on the existing admission path. Hold missing real bindings. - Test both recovery-action and ordinary user-Retry receipts against real workspace, controller, and lease rows. Check malformed, foreign, missing and opaque bindings, synthetic bootstrap behavior, and no-files API controllers. - Align the resume-owner fixture with the real transient run-ID binding. Give each invocation a unique issue label so a CI test retry cannot mask the original failure. ## Verification - RED on unchanged production: the new reservation regression failed with `native_remote_recovery_lease_mismatch` instead of reaching the admitted lease. - Review RED: an opaque task-workspace ID reached the UUID query. The new guard returns the controlled missing-admission error before the query. - Review RED: a valid no-files Cursor controller was treated as a missing task workspace. The existing task-workspace projection now keeps that controller on its prior path. - Clean final production-source regression: both restart cases passed in a fresh embedded PostgreSQL cluster. These cover both receipt paths and all binding, bootstrap and API-controller controls. - The initial focused checks passed 11 cases. Seven existing ordinary Retry and environment selection cases also passed. A later combined final-base run had 17 assertions pass but failed workspace teardown at 90 seconds and task-Retry setup at 30 seconds. That attempt is retained as a failure; the two new restart cases then passed alone with exit 0. Earlier PostgreSQL support-probe failures are also retained as setup failures. - The worktree's own Runner, shared and SDK dependencies were built. All seven server typecheck layers passed for the final production source. The final additional commit changes only the existing resume-owner fixture. - Required CI found an invalid fixture binding in [server shard 11](https://github.com/paperclipai/paperclip/actions/runs/42322379917/job/128250151095). The first attempt failed before lease acquisition because the fixture named a random workspace with no row. The automatic test retry then failed on its fixed issue label. The fixture now uses the transient run-ID contract and a unique label. Its exact original acquisition and owner-release assertions pass locally with exit 0. - Whitespace and added-code-comment checks passed. - Final commit `f0e41e8c6b949f43863ec60fc7b96b97025eafc6` passed required [CI](https://github.com/paperclipai/paperclip/actions/runs/42323826555): 56 successful checks, four intended skips, one neutral result, and no pending or failed checks. Greptile and Ponytail both gave this exact head 5/5 with no actionable findings. No review threads remain open. Hosted deployment and original-task recovery remain separate from local proof. ## Risks Malformed or missing native admission records now hold rather than selecting fresh work. Valid initial admissions and synthetic run-ID bootstraps retain their prior path. The change does not grant reattachment authority or settle provider effects. Existing controller, workspace writer, lease, Stop, and trust checks still apply. No API, schema, or configuration contract changes. ## Model Used OpenAI GPT-6 family through Codex. The runtime does not expose the exact serving model ID or context window. No model override was used. The assistant used reasoning, code editing, shell tools, and local tests. ## 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
|
b94963f91e |
fix: retry temporary repository catalog failures before dispatch (#19690)
## Thinking Path > - Paperclip manages AI agents and their work. > - A task must check repository access before it starts agent work. > - Repository discovery can fail while the user's existing grant remains valid. > - Discovery previously replaced a known temporary provider refusal with an empty catalog. > - The task then reported lost access and did not schedule recovery. > - This change keeps temporary discovery failures distinct and uses the existing bounded retry owner. > - Each retry must pass a fresh access check before it can use the saved repository. ## Linked Issues or Issue Description **What happened?** A task with a saved catalog repository stops with an access denial when GitHub returns an exhausted-quota response or HTTP 503. The task does not start provider work, but the discovery layer discards the temporary failure and returns an empty catalog. **Expected behavior** Known temporary failures before provider dispatch use the existing bounded infrastructure retry. The successor checks the same responsible user's current grants and reads the saved files only after fresh discovery succeeds. A completed denial remains terminal. **Steps to reproduce** 1. Bind a task to an authorized catalog repository with saved local changes. 2. Make its authorized GitHub discovery return HTTP 403 with zero remaining quota, or HTTP 503. 3. Let discovery recover before the existing retry deadline. 4. Observe one successful successor using the original repository bytes. **Paperclip version or commit** Reproduced on master at `b8d94b3ad1`. A search found no existing PR for this temporary catalog failure. ## What Changed - Keep a safe typed failure for known HTTP availability and quota responses. Honor the later provider deadline and bound it to one day. - Propagate that failure only when fresh discovery cannot find a required repository. Preserve successful partial catalogs and the existing connection setup error contract. - Classify the failure only before provider dispatch. Reuse existing bounded retry, interaction ownership, and bootstrap settlement rules. - Keep current grant checks, saved repository selection, normal Stop, and provider-started failure policy. - Keep provider deadlines when accepted interactions use their own retry owner. Recognize secondary HTTP 403 rate limits with valid retry advice while keeping ordinary permission denials terminal. This matches [GitHub rate-limit guidance](https://docs.github.com/en/rest/using-the-rest-api/rate-limits-for-the-rest-api). - Keep direct repository request responses outside the automatic bootstrap retry contract. Test successful partial discovery and missing-required-repository recovery with two independent eligible connections. - Document task repository discovery recovery. ## Verification - Behavioral RED: real heartbeat and PostgreSQL fixtures reached actual HTTP 403 and503 discovery responses, started no adapter, and found zero successors instead of one. Both cases failed before the production change. - Behavioral GREEN: both cases waited for the real retry deadline, used the normal queue worker, ran the controlled adapter once after fresh discovery, and read the original saved bytes. Two cases passed in 74.73 seconds. No external model or provider request was made. - Deadline RED/GREEN: a combined Retry-After and quota-reset response initially selected the earlier deadline. The regression now selects the later deadline. - Initial repair at `d8d4714a18` (same production source as `018e8c52d345d79972d7907468080dc9908c299f`): seven affected files passed, 69 cases in 185.16 seconds. That test-only cleanup then passed all five real-database acceptance cases in 134.76 seconds. - Commands: `pnpm exec vitest run` with `heartbeat-repository-catalog-recovery.test.ts`, `github-catalog-timing.test.ts`, `github-grant-metadata.test.ts`, `project-repositories-persistence.test.ts`, `task-workspace-repositories-persistence.test.ts`, `github-repository-catalog-failure.test.ts`, and `services/heartbeat/retries.test.ts` from the server source tree. The acceptance file also ran fresh at `018e8c52d3`. A fresh five-case run after the P1 repairs passed at `f490ca6b78` in 122.06 seconds. The final eight-case file passed fresh at `e5b2efab11919467850f55fd6345317132b9b672` in 124.62 seconds, using real owned embedded PostgreSQL. It covers automatic recovery with saved bytes, genuine denial, provider-started classification, normal Stop, the direct HTTP route, and both partial-catalog outcomes. - Review repairs at `f490ca6b787cf77df96d3d6edfb2ab9115eb19a6`: both reported P1s reproduced RED. The real database interaction scheduler chose a retry after 24 seconds before the provider deadline of one hour. A secondary HTTP 403 with valid Retry-After lost its retry classification. The narrow fixes then passed 44 cases across scheduler, header, and metadata-contract files in 37.66 seconds. - A real direct request through the task workspace router and normal error middleware reproduced a 500 response instead of the existing 403. Required-ID retry mode now applies only to admission and its rechecks. The route and both two-connection partial-catalog cases then passed in 29.09 seconds. Both partial cases use actual owned grants and credential bindings. - All seven server type layers passed again at final head `e5b2efab11919467850f55fd6345317132b9b672`. The affected test type layer passed again after the test-only cleanup. Environment: macOS arm64, Node 26.4.0, pinned pnpm 9.15.4, Vitest 5.0.3, owned disposable PostgreSQL 18 on loopback. The current checkout's shared packages, plugin SDK, and Runner were built first. - One local acceptance run under concurrent owned test/typecheck load hit the unchanged 15-second Stop-case timeout. Its four other cases passed. The unchanged complete file then passed serially. The timeout receipt remains recorded; no assertion or timeout was weakened. - A provider-started failure retains the existing generic adapter retry policy. It does not receive the catalog no-provider certificate or a provider retry deadline. Fixture cleanup cancels its scheduled retry through the normal owner. - Final head `e5b2efab11919467850f55fd6345317132b9b672` has 59 successful checks, 3 allowed skips, no pending or failed check, and Greptile 5/5 with no open findings. [Completed CI workflow](https://github.com/paperclipai/paperclip/actions/runs/42322202977). - Additional local diagnostic for the late recovery-owner advisory: on unchanged `e5b2efab`, the dedicated scheduler returned `retry_exhausted` with attempt 3/max 2. The normal stranded sweeper then escalated once, created no extra run, and started no adapter, even after catalog access recovered. One case passed in 17.32 seconds. The existing failed-predecessor path delegates to that bounded scheduler and returns; it does not enter a new generic retry budget. No production registry change was made. This diagnostic was not a CI case. Its local harness is preserved separately at `74e3854eea3f7c4121fdcd9c8a8ad522cd8c7477` on `codex/catalog-exhaustion-acceptance`; its production source is identical to `e5b2efab`. - Module boundary and whitespace checks passed. ## Risks - A temporary failure from another eligible connection can delay a truly missing repository for the existing limit of two attempts. It cannot grant access. - Unknown network errors and genuine access denials retain their existing behavior. This change handles known HTTP failures only. - Historical provider quota causality is unproven. This repair has a separate local producer regression. It does not rewrite past failures or queued user intents. - Acceptance uses real local database and heartbeat owners with controlled HTTP and adapter fixtures. It does not claim a live native-provider recovery. ## Model Used OpenAI GPT-6.1 Codex (`gpt-6.1-sol`), with high reasoning, repository tools, code execution, and controlled local PostgreSQL tests. The host did not expose a 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
|
06bbcea12f |
fix(budgets): price lost receipts and stop restart-derived accounting from hard-stopping companies (#19673)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Budget admission and run accounting decide whether any task in a company may start > - #19497 made pending accounting an admission hold and settles a stopped run's lost receipt as an unpriced charge > - Restart-chaos waves still stopped whole companies. A lost Codex receipt with complete token counts settled as unpriced because the rate card only prices complete receipts. Under the default blocking policy that hard-stopped the company at 0.38 of 3000 cents. In another storm, a resumed run's continuation was refused by its own task's recovering-accounting hold. Separately, unpriced charges with zero tokens or a known partial cost hard-stopped a company at 120 of 2000 cents > - This pull request prices a lost receipt's observed usage from the rate card, lets a task's continuation pass its own recovering accounting, and stops restart-derived or evidence-free unpriced charges from tripping the blocking policy > - The benefit is that a restart no longer pauses a company by itself, while genuinely unpriced usage still blocks ## Linked Issues or Issue Description Follow-up to #19497. Refs #19606. **What happened?** - Wave A (K6 crash loop): native Codex run `5037b8d1` was killed mid-turn. Its receipt was lost, but `usage_json` held gpt-6.1-sol with 28690 input and 220 output tokens. The charge was written `cost_status=unpriced`, `pricing_provenance.source=unknown`, because `priceCodexReceipt` returns early when `!receipt.complete`. 0.2 s later the default `unpricedUsagePolicy=block` opened a hard incident at 0.38 of 3000 cents. The company was paused, and 6/6 tasks were blocked with `canRetry=false`. - Wave B (K1 storm): Claude run `2a7a8abf` resumed after the restart. `reserveRunBudget` refused its interaction continuation with "New work must wait while a native run recovers its unfinished accounting", so the task could not settle its own recovery. A 0-token unpriced charge and an unpriced charge with a known 0.81-cent cost then hard-stopped the company at 120 of 2000 cents. **What did you expect?** A restart-lost receipt with known numbers is priced as an estimate, and no restart-derived accounting state pauses a company by itself. A task's own continuation is never held by its own accounting. Real usage above the limit, and genuinely unpriced usage, still stop work. **Steps to reproduce** 1. Give a company a hard-stop budget with the default unpriced-usage policy. 2. Kill a native Codex turn after it has reported tokens, so the final receipt is lost and native recovery ends in terminal failure with a verified stop. 3. Observe an unpriced charge for known tokens, a hard incident, and a paused company. ## What Changed - `run-cost-accounting.ts`: when a lost receipt settles and its usage has known tokens and no known cost, price it with `priceCodexReceipt`/`priceAnthropicReceipt` as a complete per-run checkpoint. The charge is `estimated`, its provenance keeps the rate-card source, and its evidence says "final receipt lost; observed usage is a lower bound". The estimate is also written to the run's usage so a later receipt reconciles against it. Zero observed tokens, and per-model usage, stay unpriced (unknown is never shown as zero, and aggregate totals never take one model's rate). - `budget-exposure.ts`: - The pending/recovering accounting hold excludes the run being admitted and earlier runs of the same task (same `native_issue_id`). A task's continuation and finalization can therefore settle its own accounting. Other tasks stay held, and those runs still count as spend. - The blocking unpriced-usage policy no longer counts these unpriced charges: the settled final charge of a lost receipt (restart-derived, any usage; later growth still counts), a charge with a known partial cost, and metered API or subscription billing with no observed tokens. Unknown billing with no tokens (for example Cursor Cloud) and complete-receipt usage that has no price still block. - `budgets.ts` / `budget-reservations.ts`: admission accepts the admitted `runId` and passes it to the exposure read. `reserveRunBudget`, the final dispatch recheck and the read-only admitted recheck all supply it, so a continuation that reserves can also dispatch. - Daytona wave s12 (run f1d98917, a blocked remote recovery with a 0-cent held reservation and every lease destroyed) is closed by #19606. A Retry for that same task passes the hold through the same-task exclusion here. ## Verification - `closed-native-receipt-accounting.test.ts` (embedded Postgres): - Lost Codex (gpt-6.1-sol) and Claude (claude-sonnet-5) receipts with observed tokens settle as `estimated` rate-card charges with the lower-bound evidence. The company stays active and admission is clear under the blocking policy. - A lost receipt for a model with no rate card stays unpriced but opens no incident and no pause. - A task's continuation reserves budget past its own recovering accounting, while another task's run is still held. - Unpriced charges with no tokens on metered billing, or with a known partial cost, do not hard-stop. - Complete-receipt usage with no price still blocks. - The six new cases fail on master and pass with this change. - Existing budget and accounting suites pass locally: budget-enforcement-scoping, budgets-service, heartbeat-cost-accounting, run-cost-accounting, native-stop-accounting, cost-data-quality, budget-live-attempts, budget-live-usage. Server typecheck passes. ## Risks - A lost receipt's estimate is a lower bound. The provenance says so, and a later complete receipt still reconciles. - Restart-derived unpriced charges no longer trip the blocking policy. Usage lost with a killed provider is not counted against that policy, which is consistent with budget enforcement being best effort. Unknown-billing and complete-receipt unpriced usage still block. - The same-task exemption relies on `native_issue_id`. Legacy runs without it keep the old behavior. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file editing, embedded Postgres tests). ## 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
|
29e05f7e06 |
fix(heartbeat): sweep orphaned run scratch directories (#19671)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Each heartbeat run gets a scratch directory in the system temp folder > - Each process attempt makes a new directory with a marker file > - Cleanup removes only the directory of the current attempt > - A restart kills the old attempt, so its directory stays forever > - One restart storm left 83 directories across 78 runs > - This pull request sweeps those directories when the run ends and at startup recovery > - The benefit is that temp space no longer fills with leftover directories ## Linked Issues or Issue Description **What happened?** Run scratch directories made before a restart are never removed. A restart storm left 83 `paperclip-run-*` directories across 78 runs. Each holds only `.paperclip-run-scratch.json`. **Steps to reproduce** 1. Start a run that makes a scratch directory. 2. Kill the server process and start it again. 3. Let the run finish. 4. List `$TMPDIR/paperclip-run-*`. The old directory is still there. **Expected behavior** The old directory is removed after the run ends, or at startup. ## What Changed - Add `sweepOrphanedRunScratch` in `server/src/services/run-scratch.ts`. - The sweep removes a directory only if it has a valid regular-file marker and its run is terminal in this instance database. It keeps a directory whose run it cannot see, because another instance can share the temp root. - The sweep keeps active runs, unmarked directories, symlinks, and directories outside `paperclip-run-*`. - The sweep checks each directory again before removal and bounds the scan (500, with a rotating start so kept entries do not starve later ones) and removals (100). A per-run sweep filters by directory name first. Marker run ids that are not UUIDs never reach the database query. - Call the sweep after the current attempt's cleanup in `heartbeat.ts`, for that run only. - Call the sweep in `reapOrphanedRuns` (startup recovery) with run status from the database. ## Verification - `pnpm --filter @paperclipai/server exec vitest run src/services/run-scratch.test.ts` - New tests cover: leftover directories of a terminal run are removed, an active run is kept, an unmarked directory is kept, a symlink is not followed, and removals are bounded. ## Risks - Low risk. The sweep acts only on marked directories of terminal runs. - A directory left by a previous attempt could hold a live grandchild process. The sweep does not check old process groups. The run is already terminal. ## Model Used - Claude, claude-opus-4-5 class model 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 - [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> |
||
|
Nicky LeachandClaude Opus 5.5
|
0b73c805e8 |
feat(environments): fill an unset instance default with a managed plugin environment (#19551)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A managed instance gets its instance-level environments from the `PAPERCLIP_MANAGED_CONFIG` document, which can declare plugin environments next to the managed sandbox > - A platform that runs managed instances wants a new instance to start with one of those plugin environments as its default, without changing the default of any instance that already has one > - #19632 made boot fill the instance default only when it is unset, local or deleted, in one locked transaction, and removed the revert, so every existing managed instance keeps a set default > - This pull request adds an `instanceDefault` flag to a plugin entry. That entry fills an unset default through the same locked fill, before the managed sandbox does > - The fill follows the same readiness rule as an admin's default write, so it never moves ready agents onto a plugin that has not prepared them > - The platform also needs a way to know whether a release understands plugin entries, so the release now ships a small capabilities file > - The benefit: new instances get the plugin environment as their default, and existing instances and admin choices are never changed ## Linked Issues or Issue Description Refs #19632 (the locked fill-if-unset rule this builds on), #19508 (plugin entries in the managed config) **Subsystem affected** Server environments: the managed config parser and the managed environment bootstrap. **Problem or motivation** The managed config can declare plugin environments, but it can't make one the instance default on a new instance. An external platform also has no way to tell whether a given release supports plugin entries before it sends one. **Proposed solution** - Add `instanceDefault: true` to at most one plugin entry. Right after that entry's boot ensure succeeds, it fills the instance default through `fillUnsetInstanceDefault`. The managed sandbox fill runs after it. - On a new instance the plugin environment wins. On an existing instance the default is already set, so neither fill changes it. - The fill declines while an active agent that follows the default hasn't been prepared by that plugin. This is the rule the instance-settings route enforces. When the plugin fill declines, the managed sandbox fills as usual. - Ship `server/src/services/managed-config-capabilities.json`, which lists `pluginEnvironments`. **Alternatives considered** - A recorded one-time decision with a retry worker. It needed pending states and extra checks for many writer and trigger combinations. Filling only an unset default, under the existing lock, needs none of that. - Rejecting the fill whenever agents exist. That is stricter than the admin route and blocks harmless cases, such as agents that are paused or already prepared. **Roadmap alignment** This extends existing managed-environment behaviour. It adds no UI. ## What Changed - `server/src/services/managed-config.ts`: - `ManagedPluginEnvironmentSpec.instanceDefault` (boolean, default `false`). - The parser refuses `instanceDefault` on more than one plugin entry, on the sandbox entry (even `false`), or as a non-boolean. - `MANAGED_CONFIG_CAPABILITIES` and `MANAGED_CONFIG_CAPABILITIES_SCHEMA_VERSION` constants. A plugin entry is refused unless `pluginEnvironments` is listed. - `server/src/services/managed-config-capabilities.json`: `{ "schemaVersion": 1, "capabilities": ["pluginEnvironments"] }`. A test pins it to the constants and to this literal contract. - `server/src/services/managed-environments.ts`: - New `fillPluginInstanceDefault`. It runs right after a successful boot ensure of the flagged entry and logs a fill or a decline. Recovery after boot never fills. - The managed sandbox fill now runs once, after the plugin entries. - `server/src/services/managed-instance-default.ts`: `fillUnsetInstanceDefault` runs `agentsNeedSetupForEnvironmentChange` inside its locked transaction and returns `declined: "agents_need_setup"`. A sandbox target never needs agent setup, so the managed sandbox fill behaves as before. - `server/src/services/plugin-environment-driver.ts`: the readiness query is factored into `agentsNeedSetupForEnvironmentChange`. `assertAgentsReadyForEnvironmentChange` wraps it and still throws the same conflict. - `doc/DEVELOPING.md`: documents `instanceDefault`, the fill order, the readiness decline, and the capabilities file. ## Verification - `cd server && npx vitest run src/__tests__/managed-plugin-environments.test.ts` (embedded Postgres), under `instanceDefault (fill only an unset default)`: - A new instance gets the plugin row as its default. - An existing sandbox default is kept, and so is an admin's choice. - If the plugin was down at the first boot and the sandbox filled, a later recovery and boot keep the sandbox default. - An admin-cleared or local default is declined while a ready or verifying agent follows the default without the plugin's setup. The managed sandbox then fills it. - It fills when the agents that follow the default are paused or already prepared, or are pinned to another environment. - Without the flag, nothing happens. - A new agent that follows the default gets the plugin in `requiredPluginIds`. - `npx vitest run src/__tests__/managed-instance-default.test.ts` (embedded Postgres): the earlier fill, lock and overlap cases pass. A sandbox fill also succeeds while active agents follow the default. - `npx vitest run src/services/managed-environments-plugin-entries.test.ts`: - The fill order is plugin then sandbox. - When the plugin is down there is no fill, and recovery never fills. - A failed ensure gives only the sandbox fill, and a failed fill keeps the ensure counted. - When the plugin fill declines, the sandbox still fills. - `npx vitest run src/__tests__/managed-config.test.ts src/services/managed-config-capabilities.test.ts`: parser refusals and the capabilities contract. - `npx vitest run src/services/managed-environments.test.ts src/__tests__/instance-settings-routes.test.ts src/__tests__/environment-routes.test.ts`: pass. The route still throws the same readiness conflict. - `pnpm --filter @paperclipai/server typecheck` passes. - I did not run the full `pnpm test:run`, `pnpm -r typecheck` or `pnpm build`. CI covers them. ## Risks - An instance whose default is unset at the first boot after this release gets the plugin environment, if its agents are ready for it. That covers a new instance, and an instance whose admin cleared the default or chose the local environment. This is the intended behaviour. - The readiness check doesn't lock agent rows. If an agent becomes ready during the boot fill, it can miss the check, which is the same race the admin route has. Agents created after the fill are prepared for the plugin, because setup reads the current default. - If the plugin is down at a new instance's first boot, the instance keeps the managed sandbox default from then on. - No schema change. > 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, through Claude Code with tool use (file edits, shell, test runs). ## 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> |
||
|
DottaandPaperclip
|
7ac99718a4 |
fix(workspaces): fence legacy sources for online migration (#19642)
## Thinking Path > - Paperclip coordinates AI agents and preserves their saved work. > - Remote workspace storage replaces legacy host directories with durable snapshots. > - Several legacy workspace records can name one physical project directory. > - A task lock alone cannot exclude a new task that selects that directory. > - Terminal recovery records can retain valid sources after physical writers settle. > - This change fences the source through its real preparation and lifecycle owners. > - Idle ordinary sources can then migrate while unrelated remote work continues. ## Linked Issues or Issue Description **What happened?** Online migration rejects ordinary legacy directories when several workspace records name the same path. A source inspection found 45 aliases across three project roots with no root Git marker or previously imported nested child. The instance also had unrelated running remote work. Requiring the whole instance to drain prevented useful migration of these idle sources. Terminal native sources and superseded finalization attempts also remain blocked after their exact physical and durable owners settle. Their files, provider allocations and recovery records must remain available through the change of file authority. **Expected behavior** Migrate an idle ordinary source under a derived physical source fence. Let unrelated object-backed turns continue. Preserve provider and recovery obligations. Keep unknown writers, shared Git stores and uncertain source state blocked. **Steps to reproduce** 1. Create several local workspace records for one managed project directory. 2. Give the aliases different source tasks. Leave their local writers idle. 3. Keep a valid remote workspace turn running on another source. 4. Inspect or apply migration to an alias. The old owner returns `overlapping_workspace_requires_maintenance`. 5. Retain a terminal native prepared source with a null coordinator owner, or an old finalize attempt with an exact later committed receipt. The old online path keeps these sources blocked. **Paperclip version or commit** Current tested head: `7478c817f09f7eed396922c0f7831af07ae6ff14`, based on master `8621bcf53ee2e4bbefb7d193add42dd544ca06e8`. Related recovery work: #19131. **Deployment mode** Hosted native Runner with remote workspace storage. Qualification uses disposable local PostgreSQL databases, actual archive/storage owners and synthetic source trees. The source observations were read-only. ## What Changed - Retain the existing company/task source keys and add one physical managed-project key. Heartbeat source selection, materialization, reopen and lifecycle owners share it. Configured aliases of canonical task directories retain their original task key. - Fence runtime provision and each configured service CWD through both startup owners. Check all unsettled service CWDs for physical source overlap, even when workspace IDs differ. Runs with no services keep their filesystem-free path. - Derive a bounded ordinary alias closure. Lock every alias task and lifecycle key. Reject foreign companies, changed membership, unproven descendants, included Git metadata and other unresolved writer obligations. - Commit remote authority on the connection that holds source exclusion. Refresh a stale selected local workspace after admission if migration published remote authority. Publish activity after commit. - Admit exact terminal native recovery sources under the existing physical finalizer keys. Bind descriptors, allocations and owner checkpoints to journaled fingerprints. Recheck them before capture and publication. - Add a separate company-scoped online entry for exact superseded finalization receipts. Keep the offline entry's deployment prerequisites and keep inventory read-only. - Document the proof and the remaining Git/common-store extension. The original files stay in place. No new scheduler, deployment action or database migration is added. ## Verification - Review found that the real reopen path could hold lifecycle exclusion while waiting for a project source. The new actual nested-reopen regression failed before the repair: lifecycle lock count was 1 while another connection held the project fence. The repair acquires declared and physical project sources before reconstruction and lifecycle. The contention case, ordinary-fetch restore and runtime nested restore passed. The lifecycle-worker restore first completed in 16.64 seconds but exceeded its unchanged 15-second body limit; a bounded warm rerun passed at that same limit. Both wrappers exited and completed cleanup. The source type graph, module-boundary check and diff check pass. Final 7478 CI passed: 58 successful jobs, three skipped, one neutral and a successful Snyk context. Fresh Greptile is 5/5 for exact7478, with no unresolved threads. [Final CI](https://github.com/paperclipai/paperclip/actions/runs/42318150314/job/128240631573). - Full CI on `e4283b3452` passed the other server, build and type checks. One exposure owner case failed: two execution workspaces received one preview pair after source admission serialized their starts. The first local seven-case run reproduced that failure; adding the owner argument alone did not change the hash payload. The completed repair binds run-scoped reuse to its execution-workspace owner. All seven existing real-process/fake-broker cases then passed in 13.81 seconds, with wrapper exit 0 and cleanup complete. Public scope IDs and explicit project/agent sharing are unchanged. The final7478 head passed all required CI and fresh review gates. - The service/source/restore owner canary passed 25 cases across four suites before the final no-service fast-path guard (41.33 seconds). This includes actual project service processes with a different project ID and no execution-workspace ID. Their startup waits for source ownership, migration stays blocked while they run, and the source becomes eligible after their verified stop. The final three-case runtime canary passed at the final source inputs (18.70 seconds). It covers that fast path and both service-CWD variants. Both wrappers exited 0 and completed cleanup. - The boundary-repair canary passed 9 cases across four suites at `7c18f1c170`. It covers the real rehome/conversion gateway, project command provisioning, future task selection, stale imported authority, foreign-company binding, and existing standalone filesystem callers. The shared PostgreSQL wrapper exited 0 and completed cleanup. Duration: 94.15 seconds. - The P1 diagnostic passed 46 cases across five suites at the pre-rebase P1 revision. It covers the actual importer with an export warning created by `recordNativeFinalizationFailure`, strict prepared-source/action bindings, finalizer locks, source/lifecycle races, unrelated remote execution, and bare Git with external alternates. It uses the repository shared PostgreSQL wrapper, `--fsModuleCache`, and a local-only 90-second hook allowance. Duration: 245.44 seconds. No product timeout changed. - The canonical retained-directory compatibility check passed 22 cases across two suites. Missing archived task directories still import from authenticated restore points; they do not need a present-directory source proof. - Both initial P1 regressions were observed before the fix. The real finalizer warning made the importer ineligible. The old archive owner accepted a nested bare repository borrowing external objects. The new cases pass and preserve the original warning, prepared descriptor, allocations and files. - Prior attempts remain non-passes: the first 23-case canary passed on an earlier head; its exact-head rerun hit 30-second database setup/cleanup limits. A read of the owned PostgreSQL server found a template clone waiting on `DataFileExtend` for 58 seconds. A broader owner run was intentionally interrupted during slow existing heartbeat setup. One diagnostic invocation used an invalid CLI option and executed no tests. A later review canary passed 21 cases and failed the new real restore contention case: cwd-only admission missed the declared project of an external worktree. Its fixture also omitted the required file-repository authorization callback and produced an unhandled rejection after the failed assertion. The repaired run passed 22 cases, exited 0 and completed cleanup. These earlier failures are not reported as passes. - The recovery owner passed 37 PostgreSQL cases, including 14 new and 23 existing finalized-recovery cases. The operation reconciliation owner passed 40 cases. The initial archive run passed 103 cases and found a missing closed diagnostic entry; the corrected diagnostic and expanded Git rejection cases pass in the P1 run. - The full server type graph passes with the Runner source exports mapped locally because this checkout has no built Runner declarations. The latest module-boundary check and `git diff --check` pass. Standard package build/typecheck and broad final-head regression coverage remain required in CI. - The initial CI exposed missing admission scopes for standalone filesystem APIs and a changed foreign-company error. The latest repair keeps those APIs independent of a database and holds physical fences at their actual database-backed mutation owners. The branch also includes the existing master fix for unchanged Runner default-model assertions. New review findings and new-head CI are being checked. - The local canary command and bounded source selection are documented in `doc/plans/2026-11-06-online-ordinary-source-fence.md`. Source observations only read data. A fresh bounded metadata probe found 254 pending legacy records and zero unsettled local-process services. No live import, hold, cancellation, provider action or source filesystem write was performed for this change. ## Risks - Run-scoped reuse now includes execution-workspace ownership in its private key. Explicit project/agent sharing keeps its previous identity. - A missing writer owner could make a source proof unsafe. Admission derives physical aliases, includes future project selection and retains all existing source/lifecycle/finalizer locks. Unknown and changed state fail closed. - Cross-company physical alias checks use bounded minimal-path pages. Lexical-prefix filtering cannot safely replace them because a symlink can hide a foreign alias. Unknown paths or more than 10,240 foreign records leave the source explicitly unproven. More than 1,000 same-company local records also fail closed. - The project key can serialize local preparation within one project. A first project waits; additional project acquisition fails on contention to avoid a lock cycle. Remote authority keeps its remote preparation path. - Recovery preservation does not prove provider termination or successful file recovery. Existing provider allocations, checkpoints, task intent and recovery actions remain authoritative. - This slice does not finish shared Git migration. Git administrative/common-object/alternates closure and exact nested imported-child coverage remain the next owned extension. No global writers-stopped shortcut is used online. ## Model Used OpenAI GPT-6 through Codex, with reasoning, tool use and code execution. The host does not expose a more specific backend 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> |
||
|
Devin FoleyandPaperclip
|
2a4a001d37 |
Preserve GitHub access repair guidance for MCP callers (#19655)
Return GitHub permission denials as failed MCP tool results with trusted, connection-specific recovery guidance. Explain the personal-account and organization App installation boundary without replaying writes or bypassing provider-content checks. Align the repair steps with the Permissions UI. Cover enabled model checks and malicious provider failures across direct calls, connection tests and stored approvals. Repair the CI-exposed synthetic onboarding environment fixture without weakening cancellation assertions. Verified 311 targeted tests, six onboarding Playwright tests, server typecheck, module boundaries, full CI and Apex 5/5 with all threads resolved. Co-Authored-By: Paperclip <noreply@paperclip.ing> |
||
|
DottaandPaperclip
|
a3eecf1fc9 |
Preserve admitted runtime skill bundles during server replacement (#19675)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Native runs keep the exact skill files admitted for execution and recovery. > - Older inputs can name immutable local bundles without a skill version ID. > - Installed skill heads alone do not preserve those admitted bundles. > - Server replacement needs a durable source for each retained bundle. > - This pull request copies authenticated bundles through the existing package owner and restores their exact projections. > - The benefit is preservation during online migration without stopping active work or settling recovery warnings. ## Linked Issues or Issue Description Refs #19656. Related: #19660. **What happened?** Older native execution inputs can contain skill bundles with a null version ID. Recovery skips those bundles. Startup installation checks can pass even when those retained inputs still require the old filesystem. **Expected behavior** Preserve each retained admitted bundle in object storage. Restore its original bytes, modes and digests on an empty replacement host. Keep its null pin and all run, controller and recovery state. Block startup and enrollment while a concrete local asset obligation remains. **Steps to reproduce** Retain a native run with an admitted local skill bundle and a null version ID. Convert its installed skill head to durable storage. Replace the server home and attempt recovery. The old bundle path remains in the recovered context. ## What Changed - Add a bounded `runtime-assets` phase to the existing skill package maintenance CLI. It copies exact admitted files under the runtime asset publication lock, verifies the durable package, and appends a run-bound receipt after source revalidation. - Keep original files, null pins, task intent, warnings and controller leases. Support an active run through copy-only preservation. Do not start a turn or claim provider ownership. - Verify and reuse a healthy canonical local bundle before storage reads. Restore missing or damaged bundles through the existing package and native asset materializers. Use the authenticated original manifest to preserve exact read and execute modes. Accept a later contract revision only when the run binding and exact skill selection remain unchanged. - Check retained recovery obligations and current task sessions during both startup and migration eligibility. Ignore ordinary admission that has no completed execution input. - Protect receipt-backed packages from orphan collection. Serialize receipt publication and deletion on the package row, then check references in a fresh statement after the lock. - Add real PostgreSQL and filesystem regressions. Replace a delete-method proxy with an actual upload-renewal row-lock race. Publish mutation activity only after commit. ## Verification - Baseline production startup gate failed the new concrete-asset regression as expected. The fixed gate passed. The actual before-fix preservation owner also failed recovery from a healthy local bundle during a simulated storage outage. A verified local-first path fixes that regression. - Eight preservation tests passed with real PostgreSQL, streaming storage, archive extraction and native materialization. They cover cold recovery, exact modes and binary bytes, active ownership, changed sources and bindings, contract revisions, missing or corrupt receipts, and publication/collection races. - Six affected package collection/deletion controls and three existing startup controls passed. The existing versioned recovery consumer passed a sequential retry. - One fixture attempt failed on its cleanup foreign-key order; the fixture was corrected. One existing suite hit its unchanged 30-second database startup hook before test execution. The unchanged single consumer passed on retry. These failures remain recorded. - Source, changed-test and CLI TypeScript checks passed. Rebase preserved both the local instruction-copy and runtime-asset guards. Current-head changed-test checks passed. Final PostgreSQL rerun passed all eleven preservation/startup cases. Current CLI check passed. Required CI remains pending at the trusted-runner gate. - Current canonical bundled creation uses package preparation plus atomic content/head publication. Native preparation requests immutable version snapshots before delivery. Converted installations and new canonical bundled creation therefore do not recreate the historical null-version asset dependency. - Fresh Greptile review on exact head `54c7dd048c9016c1af9fc68213d5ae053ac16499` passed at 5/5 with zero unresolved threads. - Module boundaries, whitespace, and credential/internal-reference scans passed. No live source writes, provider actions or migration apply were performed. ## Risks Missing, changed or unsafe local files remain blockers. This command does not repair a previously published package whose objects later disappear or become corrupt. It also refuses malformed preservation receipts. It retains the original local files and prior evidence and keeps removal blocked. Automatic repair requires a separate canonical package-owner recovery operation; direct database edits are not an accepted recovery path. Failed publication keeps the original files and any unreferenced package for normal collection. The readiness check is explicit startup or enrollment work, not a background scan. Its cost grows with retained admitted asset obligations. Older readers must be upgraded before the old volume is removed. This change preserves asset bytes; it does not settle business recovery or qualify unrelated volume dependencies. ## Model Used OpenAI GPT-6 (Codex). The runtime did not expose a more specific backend model ID or context-window size. Used reasoning, code editing, and tool 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 - [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
|
6d1ecfc420 |
fix(accounting): close a remote run's lost receipt once its sandboxes are terminated (#19606)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Budget admission waits while completed runs await accounting; #19497 made that a hold on new work, never a hard stop > - #19497 also closes a lost usage receipt once a local provider is verified stopped, so the hold ends > - A remote (Daytona) run has no local stop event and keeps its recorded remote process identity, so its lost receipt never closed. The company then held all new work until a receipt that can no longer arrive > - This pull request closes a remote run's lost receipt once every sandbox it used carries a provider termination receipt > - The benefit is that a restart that kills a Daytona turn no longer stops a company's new work ## Linked Issues or Issue Description Follow-up to #19497. Refs #19544 (Daytona chaos harness). **What happened?** In a Daytona restart wave, run `a261c30b` failed after native recovery was blocked. Its usage receipt stayed incomplete (`accountingReceiptReady: false`). Both of its sandbox leases were released with a `destroyed` termination receipt, but accounting stayed pending. Admission then refused new company work with "New work must wait while a native run recovers its unfinished accounting". **What did you expect?** Once every sandbox of the run is stopped or destroyed, no runner can report usage for it. The known part of its usage should settle as an unpriced charge, as it does for a verified local stop, and new work should be admitted. **Steps to reproduce** 1. Run a native task on a Daytona environment with a hard-stop budget. 2. Kill the control plane during the provider turn so the turn's receipt is lost and native recovery ends in terminal failure. 3. After the sandbox lease is released with a termination receipt, observe that the run stays `cost_accounting_pending` and new work is held. ## What Changed - `run-cost-accounting.ts`: the closed-receipt predicate gains a remote branch. A native run in terminal failure without a result (failed, cancelled, timed out or interrupted) is closed when it has at least one environment lease and every lease carries a `paperclip.remote-termination.v1` receipt bound to that lease, run, company, provider and provider lease ID, with state `stopped` or `destroyed`, after a successful release. This mirrors `hasRemoteTerminationReceipt`. One SQL predicate still serves the reconciler scan and settlement. - The local branch is unchanged: a failed run with a verified local stop event and a cleared process identity. - A sandbox that may still run (no receipt, lease still active), a receipt bound to another sandbox, or a run with no lease keeps its debt pending. - `budget-enforcement-worker.ts`: the accounting worker also wakes when environment-cleanup or retained-session-cleanup work commits. Those are the signals the lease writers already send when they release or stop a sandbox. - `doc/SPEC-implementation.md`: documents the remote closure rule. ## Verification - `closed-native-receipt-accounting.test.ts` (embedded Postgres): - A failed and a cancelled remote run, with one destroyed and one stopped sandbox, settle as one unpriced charge, settle the reservation, and clear admission. These are red on master. - Debt stays pending for an active sandbox with no receipt, for one of two sandboxes without a receipt, for a receipt bound to another sandbox, and for a run with no lease. - `budget-enforcement-worker.test.ts`: the accounting worker wakes on environment-cleanup and retained-session-cleanup commits, and stops waking after `stop()`. These are red on master. - Existing suites pass locally: native-stop-accounting, budget-enforcement-scoping, heartbeat-cost-accounting, run-cost-accounting, cost-data-quality, budgets-service. Server typecheck passes. - Live Daytona restart wave: results in the comments. ## Risks - A cancelled remote run now closes once its sandboxes are terminated. Stop recovery could otherwise reattach a recorder, but a stopped or destroyed sandbox has no runner to reattach to. Local cancelled runs keep their drain. - Workspace export can restart a stopped sandbox and removes its termination receipt first. A receipt that arrives after settlement still posts through late-receipt reconciliation. - Reusable-lease chains (`resolveRemoteTerminationLease`) and recovered-cleanup stop receipts are not mirrored, so those runs stay pending. That is the conservative side. - Low query cost: the predicate runs only for pending terminal runs. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file editing, embedded Postgres tests, live restart-chaos harness). ## 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
|
b8d94b3ad1 |
test(server): verify status and object deadlines across cold sleep (#19347)
## Thinking Path > - Paperclip manages agents and their useful work. > - Status cards and external objects can have future refresh deadlines. > - A stopped tenant needs a durable Cloud alarm to return for those deadlines. > - A local quiet report alone does not prove that handoff or later useful work. > - Acceptance must exercise the normal producer, authorization, claims and alarm owner. > - This PR adds opt-in signed cold-owner acceptance and bounded launcher controls. ## Linked Issues or Issue Description Refs #19001 and #19191, which have merged. This PR targets master and adds acceptance for the existing status and object owners. **What happened?** The existing local coverage did not join useful status generation or built-in GitHub refresh with signed Cloud alarm persistence, physical child retirement and cold reactivation at the original deadline. **Expected behavior** Keep the feature enabled. Complete useful work through its normal owner. Persist the exact authorized alarm before stopping the child. Wake and process the same original deadline without losing identity, claims, retries or useful results. **Steps to reproduce** Use the checked-in Node24 launcher with separate owned loopback App/Cloud test databases, a compiled Cloud checkout and the pinned Node22 executable. Select one status or object bootstrap, cycle or identity case. The command validates fresh evidence and database ownership before starting its guarded worker. **Paperclip version or commit** Current candidate `ea911278b22c855d6722e43afc026bfc5c031405` normally merges fetched master `07a548073ab48f13cb93b2fdc31553a5da88a070`. All eight owned acceptance files are byte-identical to `c08e64fa857d39bdb79352e94d156db7a4dbebf3`; all unrelated paths exactly match master. The guard-argument repair and source-specific historical cycle proof are preserved. ## What Changed - Add one named opt-in case per owner/stage/diagnostic combination. Keep the shared business body and its original deadlines, assertions and cleanup. - Drive actual managed App startup, authenticated producers, persisted claims and the signed Cloud SQL alarm before physical child stop. - Retain the generated five-secret fixture profile across cold boots. Store it in private directories and retain only names, hashes and modes in evidence. - Add optional query timestamps to the existing proxy and harness. Existing callers keep their default behavior. - Add launcher refusal controls and assert its complete child argument list. Both explicit guard preloads and the exact child `NODE_OPTIONS` must match. ## Verification - The current preflight repair has causal proof: the old controls pass all nine cases with an invalid worker preload flag. The new assertions reject that same command in both success controls (two failures/seven passes). Restoring the correct command passes all nine cases. The original refusal, freshness, symlink and byte-preservation assertions remain unchanged. All 33 recorded Node PIDs and three groups close; temporary copies and private directories are removed. These controls stop at the declared spawn stub. They verify the full command contract and do not claim to execute Vitest. - The prior real named status bootstrap passed in 72.942 seconds: one selected case/six explicit skips. It exercised the actual launcher and Vitest fork, useful normal generation, current authorization, settled claims, original 900000ms interval and signed SQL alarm generation/operation-lock acknowledgement before physical child stop. Recorded processes and final SQL sessions closed; both successful databases were normally dropped. - Earlier normal status and GitHub-object cycles qualified their real 900-second and 300-second original deadlines, cold reactivation and useful second results. Their complete original failure and recovery records remain source-specific; no shortened business clock or manufactured completion is used. - On current integrated head `ea911278`, own frozen offline setup and normal shared/SDK, Runner and App declaration prerequisites passed. The status test and its imported closure pass the affected normal test-tree compiler program with original options/references. The actual named normal status bootstrap then passed: one selected case/six skips, 82.202 seconds in Vitest and 85.616 seconds in the launcher. It used the real checked-in launcher, normal App/SDK/process agent and exact compiled Cloud `389878364b42dd129835d263f91882c5774d5123`. The original 900000ms interval and 120000ms alarm lead remain unchanged; useful v1 and signed SQL alarm generation/operation-lock acknowledgement precede physical App stop. All six recorded process IDs and the runtime group were absent; independent final test-database sessions were zero, with no observer during execution. Both fresh success databases were normally dropped. This refreshes the changed consumer seams through handoff and stop; it is not another cold-wake/useful-v2/full-cycle result. Historical cycles remain tied to their measured source. - Fresh exact-head CI and complete edited bot reviews are required before merge. A whole final-tree local build/test or deployed suspension is not claimed. Local provider, process and S3 boundaries are controlled fixtures; no live GitHub provider or Railway/Neon suspension claim follows. ## Risks - These tests use real SQL and wall clocks and are opt-in. Their fixtures must stay isolated and retain failed evidence. - Stable fixture secrets must remain private and must not appear in logs. Cleanup keeps them when owner closure is uncertain. - The acceptance body stays shared to preserve measured evidence. The named registration and command controls make selection failures visible. - This PR does not change production, schema or feature enablement. ## Model Used OpenAI `gpt-6.1-sol` produced the delegated acceptance work. OpenAI Codex, based on GPT-6, performed managing review, source integration and the preflight argument repair. Both used repository tools and code execution. The coordinator's exact backend snapshot and context-window size were 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 - [x] I have checked ROADMAP.md and confirmed this does not duplicate planned core work - [x] I have searched related PRs and linked the existing foundations - [x] I have described the issue with the bug-template fields - [x] I have not referenced internal or instance-local issues or links - [x] My branch name describes the change - [x] I have run the stated focused local tests and preserved their limits - [x] I have added or updated tests - [x] I have documented the opt-in command and evidence requirements - [x] I have documented risks - [ ] All current-head CI gates are green - [ ] Current-head Greptile is 5/5 with no open findings - [x] I will address reviewer findings before merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
DottaandClaude Opus 5.5
|
7c673e695f |
fix(chat): name tasks by identifier, not UUID, in tool rows and agent text (#19635)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - New users meet Paperclip through the onboarding chat with their
first agent, and through task briefs that agents write
> - A browser walk of onboarding found internal handles and
control-plane jargon in those user-facing surfaces
> - A tool row showed `Read taskId: e3ecf9bc-…`, an agent-written brief
cited `task … (id 2b330993-…)`, and a Codex agent told a new user about
"receipts", "execution policy", and a "rejected policy patch"
> - Raw UUIDs and mechanism narration make the product look broken and
give the user nothing to act on
> - This pull request makes tool rows name a task by its identifier, and
tells agents to reference identifiers and to describe outcomes, not
mechanics
> - The benefit is that a new user sees "Read <task identifier>" and
plain progress updates
## Linked Issues or Issue Description
**What happened?**
In the task chat, a collapsed tool row rendered the raw call argument
`taskId: <uuid>`. An agent-written task description included a raw task
UUID next to the task identifier. A Codex chief of staff narrated
internal mechanics (receipts, execution policies, a rejected policy
patch) to a new user in chat.
**Expected behavior**
Tool rows name the task by its identifier. Agent-written text links
tasks and documents by identifier. Agents tell the user what happened,
what it means, and the next step.
**Steps to reproduce**
1. Run onboarding with the Codex API runner and accept a single-task
proposal.
2. Watch the handoff in the chief-of-staff chat.
3. See the task lookup tool row and the narration. With the Claude API
runner and the plan path, open a child task created from the plan and
read its description.
## What Changed
- `ui/src/components/task-chat/transcript-adapter.ts`: a tool row's
target never contains a UUID argument. A `taskId`/`issueId` UUID is kept
as `taskRefId` on the tool item.
- New `ui/src/components/task-chat/task-reference.ts`:
`useTaskReferenceLabel` reads the task from the shared issue detail
query cache and returns its identifier. It returns "task" until the task
resolves or when it cannot.
- `TaskChatRunnerActivityGroup` and `TaskChatToolCard` show that
identifier as the row subject. They load the task only for rows that
have a task reference.
- `skills/paperclip/SKILL.md` (Comment Style): two new required rules.
"No raw IDs in prose" and "Describe outcomes, not mechanics".
- First-task onboarding skill: a "Talk in outcomes" rule. Non-inline
handoff inputs use an identifier link, not a raw UUID.
- Chief-of-staff persona (`AGENTS.md` for the first agent): the same
plain-language rule, so it applies in every chat.
- `skills/paperclip-converting-plans-to-tasks/SKILL.md`: task
descriptions reference other tasks and documents by identifier link,
never by raw UUID.
- No change to `skills-releases/` snapshots. They are frozen releases.
## Verification
Red on master (fix files reverted, new tests kept):
```
ui tool-row-task-reference.test.tsx
FAIL ... > falls back to a plain word while the task cannot be resolved
AssertionError: expected 'ReadtaskId: e3ecf9bc-099a-4b10-8169-9…' not to contain 'e3ecf9bc-099a-4b10-8169-9ce490a91e3d'
Tests 3 failed (3)
server user-facing-wording-guidance.test.ts
Tests 6 failed (6)
```
Green with the fix:
```
ui tool-row-task-reference.test.tsx Tests 3 passed (3)
server user-facing-wording-guidance.test.ts Tests 6 passed (6)
```
Neighbouring suites:
- `cd ui && npx vitest run src/components/task-chat
src/components/TaskChatThread src/components/transcript`: 70 files, 1448
passed, 1 skipped.
- From the repo root: `onboarding-first-task-assets`,
`paperclip-skill-utils`, `onboarding-first-task-admission`,
`hiring-operational-examples`, `cli-invocation-safety`, and the new
guidance test all pass.
- `pnpm check:token-gates`: all gates clean. The change adds no styles.
- `ui` typecheck: no errors in the changed files.
- Not run locally: `agent-skills-routes.test.ts` hit a 30 s `beforeAll`
import timeout on a cold cache. CI covers it.
## Risks
- Low risk. A row that names a task loads that task once through the
shared issue detail cache. The thread usually has it cached already. A
failed load shows "task" and does not retry.
- Rows now show other non-UUID arguments after the identifier (for
example `<identifier> · status: done`). Rows without a task reference do
not change.
- The skill text changes the guidance that agents read. They do not
change permissions, tools, or API behavior.
> 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, run in Claude Code
with tool use (shell, file edits, 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
- [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
|
6c7cc2c5d6 |
fix(native): do not spend a recovery attempt resuming a cleanly drained run (#19625)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Native runs have an execution recovery budget of three provider attempts, so a run that keeps failing stops retrying > - A graceful shutdown (deploy or SIGTERM) drains native runs: it suspends them for reattachment by the next server > - When the runner did not survive, the next boot resumed the turn and charged one attempt > - Restarts that the run did not cause used up its budget, so recovery stopped and the task needed a manual Retry > - This pull request does not charge an attempt for resuming a run that a graceful drain suspended > - The benefit is that deploys and graceful restarts no longer exhaust a healthy run's recovery budget ## Linked Issues or Issue Description **What happened?** In restart-chaos testing, native runs reached `execution_recovery_budget_exhausted` (3/3) only because of server restarts. In one wave, two restarts crossed each of two runs. In a second schedule (K1, then K2 when ready, then K2 again), each drain correctly suspended the local ACPX Claude run, yet the fourth boot blocked it with `native_restart_recovery_blocked`. Every resume after a drain was classified `resume_dead_runner` and charged an attempt (`nextNativeProviderAttempt` in `native-restart-recovery.ts`, applied by the executor's execution claim). **Expected behavior** A restart that the run did not cause should not consume its recovery budget. A graceful drain records `restart_suspended` and `awaiting_runner_reattach`. The resume that follows should keep the attempt count. **Paperclip version or commit** `master` at `4ef3fa4271`. **Steps to reproduce** Start a native local run. Send the server SIGTERM three times while the run is active, with a full restart each time. The third resume is blocked with `execution_recovery_budget_exhausted`. ## What Changed - `suspendedByCleanDrain()` in `native-restart-recovery.ts` recognizes a run whose most recent recovery claim is older than the drain's `restart_suspended` entry. Ownership waits and deferred blocks between the drain and the next claim do not erase it. The next claim consumes it. - A scan that waits for the old controller to exit (`awaiting_evidence`) records one history entry per reason, so repeated scans cannot push the drain record out of the bounded history. - Restart claims carry `chargesProviderAttempt`. It is `false` for a claim that follows a clean drain. - `nextNativeProviderAttempt()` takes that flag. The executor passes it when it takes the execution lease, so a drained resume keeps the current attempt. - The first execution is always counted: a run drained at attempt 0 (no execution claimed it yet) still charges its first provider start. - A run drained during its third attempt may continue that attempt. The local resume and remote bootstrap gates skip the `>= 3` exhausted check only for a non-charging resume, and the executor still refuses a fourth attempt. - A crash restart (no drain record) still charges one attempt. Reattachment to a live runner still charges none. - `doc/execution-semantics.md` states the rule next to the three-attempt budget. ## Verification - New test in `server/src/__tests__/heartbeat-native-restart-suspension.test.ts` (embedded Postgres, real heartbeat drain and restart recovery): - Red before the fix: the claim after the drain had no `chargesProviderAttempt: false`. - Green after the fix, for drained attempts 0, 1 and 3: the crash claim charges (`true`). After the drain, a controlled backend lets the executor take its execution lease and records the saved `native_run_finalizations.attempt` at provider dispatch: 1, 1 and 3. Attempt 3 is claimed instead of blocked. - Red at the executor: when the executor ignores the flag, the drained attempt-1 resume saves 2. - Ownership-wait case: the drained controller (the test process) is still alive, so two scans wait. The history keeps `restart_suspended` followed by one `awaiting_evidence` entry. After the controller is gone, the claim does not charge, and attempt 3 continues. - Classifier cases (local resume and remote bootstrap): attempt 3 after a crash is still `execution_recovery_budget_exhausted`. Both new cases were red on the previous revision. - `heartbeat-native-restart-suspension.test.ts` 12/12, `native-runner-restart-recovery.integration.test.ts` and `native-runner-ownership.test.ts` 94 passed. - `tsc --noEmit` for `server` passes. ## Risks - A run that itself makes the server shut down gracefully, again and again, would no longer be stopped by this budget. Graceful shutdowns are operator or deploy actions, so this is unlikely. A crash loop (SIGKILL, OOM) still charges an attempt on every restart, and a resumed turn that fails still goes through the existing failure and replacement budget. - This changes only the attempt count. Ownership checks, checkpoint verification and the exhausted gate are unchanged. > 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, via Claude Code with tool use (shell, file edits, test execution). Extended reasoning enabled. ## 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> |
||
|
Nicky LeachandClaude Opus 5.5
|
55244f1e9c |
test(server): remove two timing flakes in cloud adapter and native cleanup tests (#19668)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Contributors wait for PR CI before merge, and a flaky test forces a
re-run of the whole CI run
> - Four server tests failed on 2026-11-06 on commits that passed on a
later attempt
> - Two of them failed because of real slowness inside a fixed time
limit on contended 4-vCPU runners
> - This pull request removes that slowness from the two tests. It does
not raise their time limits
> - Of the other two tests, one is already fixed on master, and one is
not reproduced (see below)
> - The benefit is fewer whole-run re-runs
## Linked Issues or Issue Description
**What happened?**
These tests failed in PR CI and passed on a re-run of the same commit:
- `server/src/adapters/cloud-external-adapters.test.ts` › "Cloud image
adapter boundary › cold boots built-ins…" failed with `Test timed out in
15000ms` (3 times).
- `server/src/__tests__/retained-native-session-cleanup-work.test.ts` ›
"drains a backlog across discovery pages…" failed with `Test timed out
in 15000ms` (2 times).
**Expected behavior**
Both tests pass on a loaded CI runner.
**Steps to reproduce**
1. Run the test file in a 4-CPU container (`docker run --cpus=4
--memory=16g`) with 4 copies at the same time.
2. Before this change, the adapter case fails in 4 of 4 runs at 15,008
to 15,041 ms.
## What Changed
- `cloud-external-adapters.test.ts`: add a `beforeAll` warm-up with a
90-second limit.
- The warm-up imports `registry.js`, `plugin-loader.js` and
`adapter-plugin-store.js` with an empty temporary home and the managed
signal, so it reads and runs no persisted adapter package.
- Before this change, the first case paid the one-time transform of the
whole built-in adapter graph inside its own 15-second budget. In CI the
first case timed out at 15 seconds, and the next case took 1.1 seconds.
- Each case still gets fresh module state from the existing
`vi.resetModules()`. No assertions change.
- `retained-native-session-cleanup-work.test.ts`:
- `flush()` takes an optional poll `interval`, which defaults to 50 ms,
so other tests do not change.
- The backlog test uses 5 ms. The worker admits one cleanup pass per
1-second fake-time step, so 103 entries need 103 polls in a row.
- The next wake is scheduled before a pass releases its idle-work hold,
so a faster poll cannot see a false quiet state. No assertions change.
## Verification
| Test | Before | After |
| --- | --- | --- |
| Adapter cold boot, container, 4 copies | failed 4/4 (15,008–15,041 ms)
| passed 4/4 (537–756 ms per case; the warm-up takes about 23 s) |
| Adapter cold boot, host | failed 2/2 | passed 3/3 (0.6–1.6 s) |
| Backlog drain, container, 4 copies with load | 6.3–6.6 s | 2.0–2.1 s |
| Backlog drain, host, unloaded | 5.1 s | 1.1–1.3 s (5 runs, 29/29 pass)
|
- Final host run of both files with `--maxWorkers=2`: 33/33 pass.
- The backlog test did not reach the 15-second timeout in local runs.
The evidence for that fix is the measured poll-bound cost.
**Not changed in this PR:**
- `connection-management.integration.test.ts` (expected 3 to be 2):
- Cause: a GitHub webhook recovery worker from an earlier test fetched
through the suite-wide fetch spy 60 seconds later.
- Master already fixes this with commit
|
||
|
DottaandPaperclip
|
6be71028ad |
Prepare online ephemeral enrollment through signed Cloud control (#19660)
## Thinking Path
> - Paperclip manages agent work through a control-plane web server.
> - A replacement web server needs shared durable data and the original
signing identity.
> - Existing enrollment exports that identity only after a global drain
and offline migration.
> - Online enrollment needs a separate preparation operation while agent
work continues.
> - This change adds a purpose-bound Cloud control operation with
existing data and key owners.
> - The result is an audited preparation receipt, with no permission to
stop agents or detach storage.
## Linked Issues or Issue Description
Refs #19038. Refs #19024.
**What are you trying to improve?**
Allow Cloud to prepare an already migrated tenant for volume removal
without the offline migrator or a global-idle gate.
**What problem does this solve?**
Cloud must preserve the original decision key before starting a
replacement with static ephemeral mode. A tenant session must never gain
key-export authority.
**What would the improvement look like?**
A fresh exact-action Cloud assertion obtains a private, deployment-bound
preparation receipt. Unfinished durable-data migration and invalid
replacement configuration block export.
## What Changed
- Add the signed `ephemeral-enrollment:prepare` control action and a
strict UUID-only POST route.
- Check existing durable-data eligibility and ephemeral configuration
before reading the original key. Never create a missing key.
- Audit only public preparation metadata. Keep HTTP diagnostics
secret-sensitive and the response non-cacheable.
- Bind the receipt to the attempt, stack, build commit, Railway
deployment, time, static mode and key digest.
- Always return `enrollmentReady: false`. Cloud still needs to arm a
replacement, verify old writers have stopped, repeat eligibility and
qualify native handoff before detaching the retained volume.
## Verification
- PASS: 80 focused tests across the preparation owner, signed route,
existing Cloud control, HTTP privacy and OpenAPI route coverage.
- PASS: 2 real PostgreSQL cases. Concurrent copies of one signed request
export once; the durable receipt still rejects reuse after clearing the
process fence. Normal run admission before skill pins is allowed;
missing durable skill installation remains blocked.
- Genuine prepatch RED: the same two PostgreSQL cases exported twice and
rejected ordinary admission. The fixed owner was restored automatically
after the comparison.
- PASS: server TypeScript check with workspace dependencies built.
- Fixed the first CI route-coverage failure by declaring this private
control endpoint in the existing explicit OpenAPI exclusion registry.
- Initial local HTTP execution lacked sandbox socket permission; the
reviewed run passed. Initial PostgreSQL fixtures enabled durable files
too late and reused an issue prefix; those fixture defects were
corrected before the red/green comparison.
- All required CI gates pass on
|
||
|
|
c82211251b |
test(chat): cover Slack forwarding deadlines through registered owners (#19348)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - People answer agent questions through Slack commands, buttons, and forms. > - Slack requires a response within three seconds. A modal trigger also expires after three seconds and can be used once. > - Durable event delivery does not prove these synchronous response paths. > - This pull request exercises the registered app owners and pinned SDK with original signed requests and controlled forwarding delays. > - The tests record deadline failures without clearing invalid forms, extending triggers, or changing production behavior. ## Linked Issues or Issue Description Refs #19019, which has merged. This PR now targets `master`. Related work: #8300 handles raw-body capture and plugin event fan-out. #19328 handles Slack connection onboarding. This PR changes neither path. I searched related open and closed work and read those PR bodies and changed paths. **What happened?** A forwarding delay can leave an authenticated Slack callback too late to acknowledge a command, exchange a modal trigger, or return field errors. The existing direct modal test did not cover the normal registered ingress or these provider deadlines. **Expected behavior** Tests must distinguish useful app persistence from a response that Slack can still accept. They must preserve current authorization, field validation, original signatures, and exactly-once effects. **Steps to reproduce** Use an owned PostgreSQL test database and the repository launcher below. Run warm first. The default run adds one real 3500ms delay independently before command, form-open action, and invalid submission forwarding. **Paperclip version or commit** Production remains byte-identical to `54fa701d41b4ced62f4ff338d39b946015670ae3`. This test-only candidate is `a3cce4e9868fd3e6e25d9df7d0090899bdf6a204`. It adds a recovery-fixture setup guard after `dac0010649c435f673172c496b3383e99780e8bf`. **Deployment mode** Source-checkout integration test with real PostgreSQL, registered app owners, and the pinned Slack SDK. Provider API responses and the heartbeat receipt helper are controlled fixtures. No Slack account, model process, deployment, or physical tenant suspension was tested. ## What Changed - Route original signed command, action, and modal requests through the normal chat service and real SDK runtime. - Check forged-signature refusal, callback evidence, current-role refusal, invalid-field errors, one corrected answer, and duplicate fencing. - Intercept the pinned SDK HTTP transport. Model trigger expiry and one-use exchange without changing request timestamps or signatures. - Observe the real platform exception through the actual bound `views.open` callable. Delegate once and rethrow the same error. - Restore the network guard after every setup failure. Attempt all owned cleanup and preserve the original error before any cleanup errors. - Guard auxiliary database cleanup when recovery test setup fails before it creates a client. Keep temporary-cluster cleanup reachable and preserve the original setup error. - Add a repository launcher with the current prewire guard, one worker, explicit test-database validation, and closed warm-only and action-delayed-only selectors. The default still selects all four cases. ## Verification - Current candidate `c6ae7957988e220ea096ab678f070d3bd1c5f9c0` normally merges fetched master `8621bcf53ee2e4bbefb7d193add42dd544ca06e8`. All three owned protocol/recovery acceptance files are byte-identical to the preceding head. All other paths exactly match master. The shared Runner default test conflict uses master's independently landed constant-based repair. That supersedes this branch's temporary two-literal repair; no fourth file remains in the PR diff. - Original CI server4/12 had two stale `gpt-5.6-sol` expectations after the normal default changed to `gpt-6.1-sol`. The complete failure log is retained. Before the master repair landed, the focused two-literal correction passed the whole six-case file with explicit-model, non-Codex, lifecycle, permission and skill checks intact. That local result is source-specific; it is not a fresh full merged-tree pass. - The original guard-leak counterfactual reproduced unrestored global network functions after setup rejection. The repaired callback passed four controlled cleanup diagnostics. Cleanup attempts each initialized owner and restoration even after a failure. It preserves the primary error first and retains separate cleanup errors. - A fresh registered SQL/SDK warm case passed on the qualified protocol source with one persisted answer, 17.15 seconds total and normal original assertions. Final independent SQL and recorded PID/group audits were empty. A diagnostic sampler initially hit EPERM, and the worker exit guard line was absent; those limits remain explicit. The cleanup diagnostics are local qualification tools, not four newly checked-in CI tests. - Delayed modes remain explicit diagnostic commands. Ordinary CI runs warm. Earlier mode results do not establish a final-source four-case single-command pass. The provider transport is controlled; no live Slack display, deployed sleep or wake result is claimed. - The original server6/12 cleanup-backlog case hit its 15-second whole-case timeout. Its phase and cause remain unknown. The original E2E shard received runner shutdown without an assertion verdict. Complete original logs are retained; neither failure has been labeled a flake or repaired by extending a deadline. - Own offline dependency setup passed with an unchanged lockfile. An omitted private guard dependency initially prevented test entry; that setup failure is retained separately from product results. The immutable helper closure corrected it. No local full-repository typecheck/build/test pass is claimed. Fresh exact-head CI and complete edited bot reviews are required before merge. ## Risks This is a test-only change. It changes no production code, credential flow, schema, API, or sleep eligibility. The existing large integration fixture remains large; this PR does not cross a file from below 1000 lines to above it and adds no code comments. The provider transport is synthetic. The tests demonstrate deadline consequences and app persistence, not that Slack displayed a late response. They do not make commands or open forms safe while the tenant sleeps. Other Slack lanes and persistent transports remain open work. CI may expose additional harness assumptions. The roadmap's Apps and Connections work is unchanged. This focused regression complements the existing integration; it adds no separate feature or provider setup path. ## Model Used OpenAI, configured model `gpt-6.1-sol` in Codex. The exact served backend revision and context window are not exposed. Assistance used reasoning, source inspection, Git, native TypeScript checks, and controlled integration tests. A coordinating GPT-6-family agent reviewed boundaries and evidence. ## 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> Co-authored-by: Codex <noreply@openai.com> |
||
|
DottaandPaperclip
|
bb621880e6 |
fix(storage): preserve agent and skill files during online conversion (#19656)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Agent files and installed skills need durable storage before a legacy volume can be removed. > - Their maintenance commands currently require every writer to stop. > - Normal file edits already use agent row locks and skill publication locks. > - Shared agent heads also remain durable while their agents run. > - This pull request uses those owners to copy settled files online and keeps unresolved recovery states intact. > - The benefit is useful migration progress while unrelated work continues. ## Linked Issues or Issue Description Related: #19642. **What happened?** Agent-file adoption and skill installation conversion reject apply requests without a global stopped-writer acknowledgement. Agent-file maintenance also reports an active agent as blocked when its shared head is already durable. Live enrollment omits the existing unresolved-local-instruction-copy guard. **Expected behavior** Copy settled agent files and skill installations under their existing writer locks. Keep old skill directories, versions, and inline bytes. Keep unresolved local copies and unknown historical pin obligations visible to their owners. **Steps to reproduce** Run agent-file maintenance with `--apply` on the original host with durable storage enabled. A managed agent without active execution is rejected without `--writers-stopped`. The same gate rejects skill installation conversion. An agent that already has a shared head is also reported as blocked while it runs. ## What Changed - Adopt settled agent files under the existing agent row lock. Recognize an existing shared head before checking active execution, without rewriting its projection. - Reuse the local working-copy preservation predicate for first adoption and live enrollment. Keep its original public export. - Prepare skill uploads outside the shared run-pinning lock. Verify the exact local tree and source identity under the cache, slug, and row publication locks. Publish catalog origin history in that same transaction. - Retain old skill directories, versions, and inline bytes. Keep pin retention and volume-removal guards unchanged. - Add PostgreSQL and filesystem coverage for active durable heads, unresolved copies, concurrent source edits, and linked files. Update the maintenance docs. ## Verification - Agent-file durability and live enrollment suites: 16 tests passed with real PostgreSQL and filesystem fixtures. - Managed skill suite: 22 tests passed. Seven final controls also passed: upload-time file edits, source relocation, unknown historical pins, hard-linked files, pinning during a blocked upload, and both rejected/successful catalog-origin publication. - Server source typecheck and focused changed-test typecheck passed. - Module boundaries, diff whitespace, and added-line credential/internal-reference scans passed. - Baseline production rejected both online paths at their stopped-writer guards. Review regressions also reproduced a busy pinning lock during upload and extra catalog history after rejection; their fixes passed real PostgreSQL controls. All expected red results are preserved. - One final-control attempt failed the existing 30-second fixture startup hook before executing tests. A sequential retry passed all four controls without changing guards or timeouts; the failed attempt remains recorded. - Exact head `001a5beed9d0e55c7c178906fbad1f9a47f87788`: [required CI passed](https://github.com/paperclipai/paperclip/actions/runs/42309446426), including typecheck, build, tests, Runner and end-to-end gates. Greptile completed successfully at 5/5 with zero unresolved review threads; GitHub reports mergeable/clean. ## Risks Changed or unsafe sources remain blocked. Storage failures keep the original files and recovery state. This conversion does not settle retained provider copies or historical execution. Unknown pin obligations still block retention and volume removal; this PR alone does not qualify removal of the original volume. ## Model Used OpenAI GPT-6 (Codex). The runtime did not expose a more specific backend model ID or context-window size. Used reasoning, code editing, and tool 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 - [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
|
a3f80ea043 |
fix(onboarding): close the onboarding task when its interview moves to chat (#19643)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - New users start in an onboarding task. The first agent can run the onboarding interview there or in its chat. > - #19512 moves the interview card to the chat when the agent asks it there. The task's card expires. > - The onboarding task then stays `in_review` with no card, run, wake or reviewer. Review attention calls it "stalled". > - Needs you then shows "Choose how this task continues" with Send back to work / Request changes / Mark task done. A new user cannot know what to pick. The card stays after the interview, plan approval and handoff finish in chat. > - This pull request closes the onboarding task when its interview moves to chat, and links the chat in a system comment. > - The benefit is that the user never decides what happens to an internal task. ## Linked Issues or Issue Description Refs #19512 **What happened?** In a browser walk (Codex API path), the user chose "Interview me" in the onboarding task. Then the user asked in Chat: "Can we do my onboarding interview here in chat instead of in the task?". The chat card worked. The onboarding task stayed `in_review`, and Needs you showed "Choose how this task continues" for it until the end of onboarding. **What did you expect?** The onboarding task follows the conversation. It closes with a link to the chat. No decision about an internal task goes to the user. **Steps to reproduce** 1. Start onboarding. Choose "Interview me" on the onboarding task. 2. Open Chat with the agent. Ask to do the interview in chat. 3. Open Inbox. See "Choose how this task continues" for the onboarding task. ## What Changed - `server/src/services/issue-thread-interactions.ts`: after the chat question supersedes the onboarding task's interview cards, the onboarding task is closed (`in_review` → `done`) when no other card is pending on it. A system comment links to the chat. The change writes an `issue.updated` activity entry (`source: "onboarding_moved_to_chat"`). Activity publications and issue post-commit actions run after commit. - The task stays open if it is not `in_review` or if another card is still pending (for example a card for a teammate). - `first-task` skill: tells the agent that the task closes when onboarding moves to chat. The chat then replaces the task for the rest of the workflow (plan, proposal cards, child tasks, results). The agent skips the steps that keep the task `in_review` or make it wait for a child. - `onboarding-assets/first-task/README.md`: documents the behavior. - Tests in `server/src/__tests__/coordinator-handoff.test.ts`: the new red/green case, plus a case that keeps the task open when another card is pending. ## Verification Red on master (fix reverted, same test): ``` FAIL src/__tests__/coordinator-handoff.test.ts > Coordinator canonical handoffs > resolves the onboarding task when its interview moves to the agent chat AssertionError: expected [ 'stalled' ] to deeply equal [] - [] + [ + "stalled", + ] ❯ src/__tests__/coordinator-handoff.test.ts:257:40 256| // No "Choose how this task continues" stalled-review card for the… 257| expect(await stalledReviewCards()).toEqual([]); Tests 1 failed | 3 passed | 24 skipped (28) ``` The test reads the real Needs you feed (`attentionService.list`). On master it holds a `review` item with `reviewAttentionState: "stalled"` for the onboarding task. That is the "Choose how this task continues" card. Green with the fix: ``` cd server && npx vitest run src/__tests__/coordinator-handoff.test.ts Tests 28 passed (28) ``` Neighbour suites: ``` npx vitest run src/__tests__/onboarding-first-task-admission.test.ts src/__tests__/issue-onboarding-first-task-routes.test.ts \ src/__tests__/issue-thread-interaction-routes.test.ts src/__tests__/issue-thread-interactions-service.test.ts \ src/__tests__/issue-thread-interactions-telemetry.test.ts src/__tests__/coordinator-handoff.test.ts Test Files 6 passed (6) Tests 373 passed (373) ``` - `server` typecheck (`tsc --noEmit`): pass. - `pnpm verify` exits 2 (the README is a broad input). Required CI supplies broad coverage. - Not run: a live browser walk. The Vitest case drives the same service path the chat uses. ## Risks - Low risk. The change runs only when an agent asks a question in its own chat and that question supersedes a pending onboarding-task question (the #19512 path). - The onboarding task becomes `done` without a recorded first-task decision. Later onboarding happens in chat, so nothing waits on this task. A done onboarding parent still accepts child-completion report wakes. - The onboarding task rows are locked before their question rows, the same order question creation on the task uses (Greptile review follow-up). The chat row is locked first; no path locks an onboarding task and then a chat row. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edits, test runs). ## 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
|
0c904e2fd5 |
fix(tasks): treat a queued-message Interrupt as a hand-off, not a failed attempt (#19633)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - A user can send a message while an agent works. The message waits in
a queue strip above the composer. The strip has an **Interrupt** button
that stops the current turn so the message starts now.
> - In a browser walk of onboarding, Interrupt showed a red "Couldn't
finish this attempt. Your message is saved." banner with Retry. The
thread also showed two rows for the same stopped run ("Stopped for 22s"
and "Stopped after 21s"). After about 20 s it recovered by itself.
> - A stop that the user asked for is not a failure. The red banner and
Retry make a normal action look broken.
> - #19369 stopped resource waits from showing as failed attempts. This
is the same banner, reached through the Interrupt path.
> - This pull request shows the Interrupt hand-off as working, and
renders the stopped turn once.
> - The benefit is that steering a running agent looks calm and correct:
one stopped row, no error, and the queued message starts next.
## Linked Issues or Issue Description
Refs #19369 (same banner class, resource-wait path)
**What happened?**
While an agent ran, the user queued a message and clicked Interrupt on
the queued strip. A red banner "Couldn't finish this attempt. Your
message is saved." with Retry appeared at the top of the task. Two rows
showed the same stopped run: "Stopped for 22s" and "Stopped after 21s".
About 20 s later the banner went away and the message was delivered.
**Expected behavior**
A user Interrupt is not a failure. No red banner or Retry. One "Stopped"
row, and the queued message starts next.
**Steps to reproduce**
1. Start a task with a Paperclip Runner agent.
2. While it works, send a message so it waits in the queue strip.
3. Wait more than 10 s, then click Interrupt on the queued strip.
4. See the red banner, Retry, and two "Stopped" rows.
**Paperclip version or commit**
master
|
||
|
DottaandPaperclip
|
c9391720a1 |
fix(heartbeat): send saved follow-ups after task completion (#19600)
## Thinking Path > - Paperclip manages agents and their work. > - Task messages remain in a durable queue while an earlier run saves its result. > - That result can mark the task done before the user sends the saved messages. > - Send records current user intent, but its wake guard allowed only active task states. > - The old queue therefore remained pending with an obsolete finalization message. > - This change admits that exact Send receipt and reopens the task through the issue service in the successor transaction. > - The agent can receive the saved input once while the completed result remains unchanged. ## Linked Issues or Issue Description **What happened?** Send on a saved message can leave the message queued after its native predecessor succeeds and commits. The old wait message remains visible. Allowing admission alone also fails: continuation construction rejects a task that remains done. **Expected behavior** Explicit Send must deliver the saved input once through normal admission. It must retain the previous run and its result. **Steps to reproduce** 1. Save a follow-up while a native run finishes its result. 2. Let that run succeed, commit its accepted result, and mark the task done. 3. Send the saved follow-up. Before this change, the receipt stays deferred. Related public work: Refs #19542. That change selects recovered workspace files. This change handles the later saved-message admission. Refs #19602 for the independent workflow fixture repair required by CI. ## What Changed - Allow the exact explicit Send wake guard to accept a done task. Keep automatic recovery and cancelled task guards unchanged. - Reopen the task through the company-scoped issue service after admission gates pass. Commit reopening, queue consumption and the successor together. Publish the audit after commit. - Add a native succeeded/committed predecessor regression with unchanged saved work and one successor after two resume calls. - Verify real heartbeat startup under the clicking operator. Keep automatic input, cancelled tasks and paused agents held. - Check workspace preflight for the prospective reopened task. Use the normal company-scoped transition if preflight blocks a done task, so its completion metadata is cleared. - Check a successful native predecessor for its committed accepted result, current ownership and pending cleanup. Keep approvals and questions effective even without a recovery action. - Document the saved Send contract and update the queue test dependency. ## Verification - Final merge qualification for `675914f0fa9b5eb1d2c9c8fcd448e0669d0c4836`: all 56 applicable status checks pass, with no pending or failed checks. [Final CI](https://github.com/paperclipai/paperclip/actions/runs/42307623559) includes typecheck, build, all applicable test and E2E shards, and aggregate verification. Current-head Greptile is 5/5; all five review threads are resolved. Any paused or queued application review is not claimed as a new completed review. - Final base qualification at `675914f0fa9b5eb1d2c9c8fcd448e0669d0c4836`: official preflight passes (1.5s); all six integrated model-normalization cases pass (19.07s); both affected test typecheck layers pass. The integration changes no production server/shared/Runner/adapter inputs, so the preceding 12-case Send proof, two ordinary status guards, and unchanged production typecheck layers remain applicable. Fresh final-head CI supplies broad coverage. - New review regression: both failed and cancelled native predecessors on a task manually marked Done fail before the repair with `continuation_task_ownership_changed` through real `resumeQueuedCommentInterrupt` admission. At `bfb0eff7941090ac3ac43ce4366ba2e3d46612e9`, all 12 affected Send/source/cleanup/workspace/decision cases pass (246.80s), ordinary Done/cancelled rejection cases pass (2; 100.31s), and all seven server typecheck layers pass. History validation uses the prospective reopened status only for an already-verified explicit native Send. No status is changed during a dry run. - Prior `e64e4bb1` CI passed the affected serialized server suites, including the complete saved-message route suite, typecheck and build. General server shard 4 still failed two unrelated stale default-model expectations (`gpt-5.6-sol` versus the new `gpt-6.1-sol` default). The equivalent repair has already merged in #19610 as `8621bcf53ee2e4bbefb7d193add42dd544ca06e8`; the separately prepared #19645 is closed as superseded. This source fix does not weaken or edit those assertions. - Original RED: the committed-source and completed-task startup cases left the saved receipt deferred. - Review RED: Done plus an unrunnable isolated worktree created one successor instead of zero. A successful committed predecessor with pending lease cleanup also created one instead of zero. The new Done-to-blocked path retained stale completion time. Pending approvals and questions each allowed a successor. All failures were captured before their respective repairs. - Integrated production revision `99018264dd1ef002ff3500378184bcd167d9206b`: 417 affected cases pass across the complete native continuation, responsible-user startup, and workspace preflight suites (386 + 18 + 13). These include the original successful startup case and six cleanup/finalization faults that hold the exact saved receipt until settlement, then deliver it once. - `pnpm --filter @paperclipai/server typecheck` passes all seven layers on that integrated head, including the local Runner/SDK dependency build. Module-boundary, whitespace, and added-code-comment checks pass. - Normal master integrations preserve branch history. No force push or assertion weakening was used. Fresh CI and automated review are running on `675914f0fa9b5eb1d2c9c8fcd448e0669d0c4836`, which normally integrates the already-merged #19610 fixture repair after the reproduced inherited CI failure. - The earlier CI attempt failed in five unchanged workflow fixture cases because IMAGE_ONLY was unset. The merged #19602 repairs that fixture. A later inherited duplicate default in that independent fixture has a separate repair; this PR does not edit it. - Follow-up route-fixture RED: three existing native queue delivery cases failed because their fixture marked a run successful without an accepted result or committed finalization. The same three cases pass after the fixture records the complete native success contract (42.08s); delivery assertions and production gates are unchanged. The affected test-tree TypeScript layer also passes. The full route run passed all 119 assertions but failed its 30-second database teardown hook, so it is not recorded as a suite pass. That result is retained. One unchanged bounded rerun was stopped after more than 15 minutes without a final verdict; it is inconclusive, not a pass. - Final integrated head `e64e4bb1`: all 13 focused route and completed-task admission cases pass across two real-PostgreSQL suites (188.95s), including the three repaired route fixtures and every new cleanup/finalization/decision hold. Fresh preflight passes; all seven server typecheck layers pass. Required final CI supplies the broader route and server coverage. - These tests use real PostgreSQL and normal admission, cleanup, queue consumption, and startup services. The startup suite controls the provider boundary. Hosted provider acceptance remains pending normal deployment. ## Risks - Explicit Send can reopen a done task. This is limited to its validated saved receipt and current assignee. A cancelled task remains held. - Reopening uses the existing company-scoped issue transition and shares the successor transaction. A failed admission does not reopen the task. Provider Stop, workspace, approval, pause and budget checks retain their existing owners. - Cleanup and pending decisions retain the done task and saved receipt. An unrunnable workspace uses the existing blocked-task diagnostic. No schema or runtime package changes. ## Model Used - OpenAI Codex, GPT-6 family; exact serving snapshot/context window not exposed; repository analysis, code editing, tool 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 - [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
|
2e340d6235 |
fix(ui): show unread message counts in agent chat pickers (#19627)
## Thinking Path > - Paperclip helps people manage AI agents and their work. > - People switch between agent chats to read replies. > - The agent picker does not show how many messages are unread. > - People need that count before they select a chat. > - The chat list already has the viewer's read state. > - This pull request adds numeric badges from that shared list. > - People can find unread replies without extra requests for each agent. ## Linked Issues or Issue Description **What existing behavior does this improve?** The chat header agent picker and the secondary sidebar's Add chat picker. **Subsystem affected** Agent Chat UI, shared task types, and the chat read-state service. **Current behavior** The pickers do not show the unread message count for each agent. **Proposed behavior** Show the unread count beside each agent. For example, show 3 for three unread agent messages. Hide the badge when the count is zero. Keep switching available while chat data loads. **Reason and benefit** People can see which chats need attention before they switch. The count uses the existing shared request and cache. **Breaking changes** The chat response gains an optional `unreadMessageCount` field. Existing fields and read receipts keep their behavior. There is no database migration. **Additional context** Related: #19425 changes chat structure. This change adds counts to the current agent pickers. The duplicate search found no PR for these count badges. ## What Changed - Count unread agent comments in the existing grouped read-state query. Keep company and viewer scope. - Exclude human messages, deleted comments, system notices, and fast-response receipts. - Pass memoized count maps to both agent pickers. Use a direct lookup for each row. - Show numeric badges and accessible singular or plural labels. - Test count changes, account isolation, cached opens, search, and large agent lists. - Update the chat stories, fixture data, and development guide. - Make the Slack owner retry-deadline test deterministic after reproducing its CI failure. ## Verification - On current head `05f5fa6201`, the [main CI workflow](https://github.com/paperclipai/paperclip/actions/runs/42305845930) passed, including build, typecheck, all applicable test shards, browser shards, and final verification. The prior server shard 2, 6, and 7 failures do not recur. - [Greptile's current-head review](https://github.com/paperclipai/paperclip/pull/19627#issuecomment-6696161631) is 5/5, its check succeeded, and both previous review threads are resolved. Current head check inventory is complete: 55 successful checks, three intentional skips, and no pending or failed checks. The Paperclip application review summary reports paused; no new completed Paperclip review is claimed. Its existing findings are resolved, and the independent current-head Greptile review is 5/5. - Current head `05f5fa6201` merges master `8621bcf53e` to resolve an import conflict. Master now includes the same model-default test repair. This branch keeps its upstream import, so that file no longer differs from master. - After this merge: all 21 server tests in the Slack owner and model-default suites pass, all 22 tests in the three picker/sidebar suites pass, and token gates pass. The standard preflight still cannot build Rust because `cargo` is absent. Fresh CI must supply the broad checks. - Head `7d125fb8c7` replaces the Slack owner test's real timer wait with the worker's existing clock input. It checks that the intent stays pending one millisecond before the retry deadline and clears at the deadline. All 15 Slack owner integration tests pass locally. Runtime retry behavior is unchanged. - On `4cfb1e9d55`, both review checks gave 5/5. CI [server shard 2](https://github.com/paperclipai/paperclip/actions/runs/42302458830/job/128183671349) failed the Slack credential-lease retry test. The same assertion failed in an unchanged local run. A diagnostic run with added logging passed; the exact timer deviation was not captured. The new test removes the real-time wait assumption and checks the clock boundary directly. The earlier model-default and cleanup failures did not recur in that CI run. - On `a3378dfc40`, both Greptile and Paperclip Review gave 5/5 with no open findings. Typecheck, build, all eight browser shards, and most test shards passed. - That CI attempt failed two server shards. The model-default test still expected `gpt-5.6-sol`, while the current default is `gpt-6.1-sol`. I reproduced both failures locally. Commit `4cfb1e9d55` uses the exported default-model constant; all six tests in that suite now pass. Production model selection does not change. - The retained-cleanup backlog test exceeded its 15-second limit in [server shard 6](https://github.com/paperclipai/paperclip/actions/runs/42299858777/job/128175193320). The unchanged test passes locally with the same limit. Its CI timeout cause is not established. No assertion or timeout was changed. The fresh CI run must recheck it. - Review repair: commit `0db631b215` keeps the existing-chat and unread-agent arrays stable when chat data and the active agent do not change. A new 500-agent test fails before the repair and passes after it. The test also checks count refreshes. - The first CI typecheck failed on duplicate `IMAGE_ONLY` and `TESTOPS_REQUESTED` keys in `tests/runner-e2e/workflow-target.test.ts`. This was present in upstream code. Master commit `e482f0f632` removes the duplicate keys. This branch now includes that commit. All nine workflow-target tests pass locally. - After the latest master merge: all 22 tests in the three affected picker/sidebar suites pass. Before that merge, all 38 focused UI tests passed. - The current head is `05f5fa620134ad199a3d85b66fb2eb93d3bb6077`. All current status checks are complete, the aggregate verify gate succeeds, mergeability is CLEAN, and no unresolved review threads remain. The maintainer has instructed normal merge after checks pass. - After merging current master: 37 focused UI tests pass across AgentChatPicker, CoordinatorAgentSwitcher, AgentConversationsSidebar, useAgentChatRead, and SidebarAgentChats. - After merging current master: all 38 proactive-chat integration tests pass with a real disposable PostgreSQL database. They check counts of 2, 1, and 0 at exact read cutoffs. - The prior revision passed UI typecheck and build, shared build, isolated low-memory server typecheck, token gates, and 30 map/catalog tests. - The 500-agent test bounds index work and count lookups. Search does not rebuild the existing-chat index. - Browser checks used production components with simulated API data. Dark, light, mobile, and Add chat cases passed. Counts of 3 and 12 stayed visible through search. Read agents had no badge. Cached opens made no chat API requests. - Twenty local cached opens took 34.5–57.6 ms, with a 38.4 ms median. This is an unthrottled fixture measurement. - Earlier full local checks could not complete because Rust cargo is absent and the workspace has limited memory. The standard worktree preflight also stops at missing cargo. Required CI must supply broad checks for this PR head. - The first server typecheck ran out of memory while Storybook was active. Its isolated low-memory retry passed. No visual baseline comparison or live provider delivery is claimed. ## Risks - Low risk. The count replaces a boolean aggregate in the same grouped query. Very long chat histories still need that existing scan. - A count can be stale until the existing chat-list cache refreshes. Read receipts still control the unread cutoff. - The change adds no polling, per-agent history request, database migration, or permission change. ## Model Used OpenAI GPT-6 through Codex, with code editing, terminal tools, and browser verification. The runtime did not expose a more specific served 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> |
||
|
Nicky LeachandClaude Opus 5.5
|
913102870d |
fix(environments): fill the managed instance default once, under a lock (#19632)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - On a managed instance under `enableManagedSandboxOnly`, boot points the instance default at the managed sandbox when nothing deliberate is set > - That write read the default and wrote it later without a lock. Two overlapping boots, or a boot and an admin write, could overwrite a value the other had just committed > - A revert path also reset that stamped default whenever the mode was turned off, so a platform-filled default could change again later > - A planned plugin-environment default should fill only an unset default and never change a set one, which needs every instance's default to stay set > - This pull request makes the fill one locked transaction and removes the revert, so a filled default is an ordinary set value > - The benefit is a single, race-free rule: boot fills an unset default once, and nothing changes a set default afterwards ## Linked Issues or Issue Description Refs #19551 (the plugin-environment default that builds on this rule) **Subsystem affected** Server environments: the managed environment bootstrap and the instance settings route. **Problem or motivation** `applyManagedEnvironments` read the instance default, decided, and wrote it in separate steps with no lock, so a concurrent boot or admin write could be overwritten. When `enableManagedSandboxOnly` was not declared, a revert reset a default the platform had filled. That needed a `managedDefaultStamped` marker, plus a marker clear on every admin default write. A later boot could therefore change a default that had already been set. **Proposed solution** Fill the default in one transaction that locks the instance default row and rechecks it. Fill only an unset, local or deleted default. Remove the revert and its marker, so a filled default behaves like any other set value. **Alternatives considered** Keeping the revert and adding locks to it. A revert on a mode change would still move an existing instance's default later, which a fill-only plugin default cannot tolerate. **Roadmap alignment** This narrows existing managed-environment behaviour. It adds no product surface. ## What Changed - New `server/src/services/managed-instance-default.ts`: `fillUnsetInstanceDefault(db, environmentId)`. In one transaction it creates the settings row if needed, locks the instance default row (`FOR UPDATE`), and rechecks the current value. It writes only when the default is unset, names the local environment, or names a deleted environment. Any other set default is kept. - `server/src/services/managed-environments.ts`: - Under `enableManagedSandboxOnly`, the managed sandbox fill now uses that helper, through a new `fillInstanceDefault` test seam. - The mode-off revert and the `managedDefaultStamped` marker are removed. - The unused `instanceSettings` seam option is removed, along with the `getById`, `update` and `findManagedSandboxEnvironment` methods of the environments seam. - `server/src/routes/instance-settings.ts`: an explicit default write is a plain `update` again. The marker clear and its transaction are gone. - `doc/DEVELOPING.md`: the "Managed environments" section states the fill rule. Existing rows may still carry `managedDefaultStamped` in their metadata. Nothing reads it any more, and it is not part of the managed stock hash. ## Verification - `cd server && npx vitest run src/__tests__/managed-instance-default.test.ts` (embedded Postgres): - The fill writes an unset default, including on a fresh instance with no settings row. It also writes a default that names the local environment, or one whose environment was deleted. - It never changes a default set to an ssh, plugin or other sandbox environment, and leaves a default that already names the target alone. - Controlled overlap: an uncommitted admin default write makes the fill wait, and the fill then keeps the admin's value. This test fails without the row lock. - Two overlapping fills: exactly one writes. - Real boot pass: the default is filled under managed-sandbox-only and stays the same after a boot with the mode off. A set default stays on every boot. - `npx vitest run src/services/managed-environments.test.ts src/services/managed-environments-plugin-entries.test.ts`: the fill runs only under managed-sandbox-only, with the managed sandbox row. It never runs for a plugin environment. Nothing is written when the mode is off. A failed fill does not fail the ensure. - `npx vitest run src/__tests__/instance-settings-routes.test.ts`: an explicit default write is a plain update that touches no environment row. - `npx vitest run src/__tests__/managed-plugin-environments.test.ts src/__tests__/environment-routes.test.ts src/__tests__/server-startup-feedback-export.test.ts src/__tests__/instance-settings-service.test.ts`: pass. - `pnpm --filter @paperclipai/server typecheck` passes. - I did not run the full `pnpm test:run`, `pnpm -r typecheck` or `pnpm build`. CI covers them. ## Risks - Behaviour change: when `enableManagedSandboxOnly` is turned off for an instance, a default the platform filled with the managed sandbox now stays. Local execution is still allowed then, but it is no longer the default unless an admin picks it. - No schema change. Stale `managedDefaultStamped` keys stay in some row metadata and are ignored. - The fill holds the instance default row lock for one short transaction at boot. > 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, through Claude Code with tool use (file edits, shell, test runs). ## 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> |
||
|
Devin FoleyandPaperclip
|
8621bcf53e |
Cancel subscription sign-in requests when leaving agent setup (#19610)
Back during pending subscription sign-in could accept a late response and
reopen agent setup. Abort pending reads, release abandoned sessions, and
ignore stale responses. Wait for cleanup before starting a replacement,
and preserve sessions belonging to another account or environment.
Cover deferred resume/start/prompt responses for both providers, local
login cancellation, quick reselection, and browser request abort. Stabilize
Runner fixtures and normalization assertions exposed by integration CI.
Verified: 483 focused UI tests, six onboarding browser cases, UI typecheck
and token gates, Runner fixture checks, and the complete CI run on
|