mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-14 00:26:28 +01:00
120017e39dfd301f8b5a52c8cc5379c9be1a3145
2679
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 DottaandClaude Opus 5.5
|
120017e39d |
fix(workspaces): read saved workspace files in place instead of extracting the whole archive (#20442)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - When a cloud task finishes, its workspace is saved as an encrypted
snapshot and its sandbox is released. The board's Files view then reads
that saved copy
> - Saved Files access downloaded, decrypted and extracted the whole
archive for every request, and refused archives over 512 MiB with
`too_large`
> - A finished task in a Paperclip-sized repository saves a snapshot of
about 730 MiB, so its files could not be listed or opened at all
> - The archive format already records each file's size and SHA-256, and
the snapshot records the manifest digest, so single files can be read
and verified without the rest of the archive
> - This pull request reads the saved archive in place, one verified
file at a time
> - The benefit is that users can open any file of a large finished
workspace quickly, with bounded memory and no whole-archive transfer
## Linked Issues or Issue Description
**What happened?**
In restart-chaos waves on Daytona with a real Paperclip clone, the
committed workspace snapshots were 763,944,535 bytes. After the sandbox
was released, every saved Files read returned 422 `too_large` (506 reads
in total), because `createSavedWorkspaceLoader` refuses archives over
`maxArchiveBytes` (512 MiB). The board Files view uses the same loader,
so it fails the same way.
**Expected behavior**
A user can list and open any single file of a saved workspace of
Paperclip-repository size without the server extracting or buffering the
whole archive. Memory and time stay bounded per request, and integrity
is still verified.
**Steps to reproduce**
1. Run a cloud task whose workspace is a large repository, so the final
snapshot exceeds 512 MiB.
2. Let the task finish and its sandbox be released.
3. Open the task's Files view, or call `GET
/api/issues/:id/file-resources/content?workspace=execution&path=…`: it
returns 422 `too_large`.
**Paperclip version or commit**
`master` at
|
||
|
DottaandClaude Opus 5.5
|
d064bd21e4 |
perf(plugins): pick each lifecycle resource's head in one pass (#20406)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Plugins receive resource lifecycle events (agent, project and agent-runtime changes) through a durable inbox > - The inbox lists an event only when it is the first unacknowledged event of its resource, so each resource delivers in order > - The list query checked that rule with a `NOT EXISTS` for every unacknowledged event. Each check walked the resource's whole acknowledged history > - A resource with a long acknowledged history and a backlog behind a deferred head costs history × backlog probes. On the hosted staging instance three copies of this statement ran for more than 23 minutes each, on a database that was at 6–8 of 8 vCPU all day > - This pull request picks the deliverable head of each resource in one pass > - The benefit is a list call that is linear in the company's events, and database CPU that other requests can use ## Linked Issues or Issue Description **What existing behavior does this improve?** `pluginLifecycleInbox(...).list()` in `server/src/services/plugin-lifecycle-inbox.ts`. The delivery worker calls it in a loop and when it checks for idle work. **Subsystem affected** Plugin lifecycle delivery (server service, PostgreSQL query). **Current behavior** Neon "Active queries" for the staging database on 2026-11-09 showed three copies of this statement at 1,381–1,401 s each. In the same view, trivial primary-key reads waited 100–280 ms. Locally, one resource with 2,000 acknowledged events and a 2,000-event backlog takes 3.5 s for one `list()` call. The cost grows with acknowledged history × backlog. **Proposed behavior** Only the head of a resource is deliverable: its unacknowledged creation if it has one, otherwise its lowest unacknowledged event. This is exactly the set that the old `firstForResource` predicate admits. The query now selects these heads with `DISTINCT ON (resource_type, resource_id)` ordered by `(action <> 'create'), id`, and then applies the page cursor, order and limit. **Reason and benefit** The same fixture lists in 8–24 ms instead of 3.5 s. Long-running copies of this statement no longer hold database CPU that every request needs. **Breaking changes** None. `acknowledge()` keeps its per-event eligibility check. ## What Changed - `pluginLifecycleInbox.list()` uses a `lifecycle_heads` CTE with `DISTINCT ON` to pick one head per resource, then filters by the `afterId` cursor and orders by id with the limit. - New test in `server/src/__tests__/resource-lifecycle-events.test.ts`: one resource with 2,000 acknowledged events and a 2,000-event backlog, plus a second resource. It asserts the exact heads in id order and a list time below 1 s (3.5 s before, 8–24 ms after). ## Verification - `cd server && npx vitest run src/__tests__/resource-lifecycle-events.test.ts -t "acknowledged backlog"` — red on `origin/master` (3,546 ms), green with the change (24 ms, 8 ms). - `cd server && npx vitest run src/__tests__/resource-lifecycle-events.test.ts src/__tests__/plugin-lifecycle-delivery.test.ts src/__tests__/agent-runtime-assignments.test.ts src/__tests__/agent-runtime-hooks.test.ts src/__tests__/project-instance-secrets.test.ts` — the existing ordering, paging, per-plugin isolation, backfilled-creation and late-commit tests pass. (One `agent-runtime-hooks` case is timing-sensitive on a loaded machine and also fails on `origin/master` there. It passed 22/22 on rerun.) - `pnpm verify` selection and CI. - After deploy: Neon "Active queries" no longer shows long-running `resource_lifecycle_events … NOT EXISTS` statements. ## Risks - Low risk. The result set is the same: per resource, the unacknowledged creation, otherwise the lowest unacknowledged id. The cursor still applies after the head is chosen, as before, so paging past failed heads does not skip later events. - Each call still reads the company's events of the requested types once. A company with millions of acknowledged events still pays one linear pass per call. A per-plugin acknowledgment watermark would remove that and is out of scope. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, browser, code execution). Extended reasoning on. ## 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
|
0991d9bda1 |
fix(workspaces): protect migration sources and add guarded recovery service (#20443)
## Thinking Path > - Paperclip manages agents and their task workspaces. > - Online migration saves files before it publishes remote workspace authority. > - A saved migration journal can survive the importing process. > - Git cache expiration did not count that journal as a consumer. > - Expiration could remove a source and change its fingerprint while its import was incomplete. > - This PR protects unfinished imports and exposes a guarded internal service transition for an already affected cohort. > - New imports can preserve the current sources while all old recovery data stays retained. ## Linked Issues or Issue Description **What happened?** Git cache expiration could archive and remove a workspace after an import process ended but before its saved cohort finished. The saved source fingerprint then differed from the current archived workspace. Exact cohort resume correctly refused that changed source. Reopening the workspace cannot recreate the old metadata or fingerprint. **Expected behavior** An unfinished migration remains a durable consumer after process loss. If an older server already expired some sources, one canonical transition can fail the exact unpublished cohort without losing its snapshots, objects, source history, or recovery obligations. New attempts use the current verified sources. **Steps to reproduce** 1. Start a cohort import of eight Git worktrees with local changes. 2. Interrupt it after four snapshot uploads. 3. Let an older server expire two idle source worktrees. 4. Resume the saved cohort. It refuses the changed archived sources. Related: #20429 preserves exact cohort identity when independent companion imports change only the scope proof. #20424 preserves prepared recovery state for archived sources. This PR handles completed source expiration, not a scope-only change. Public searches found related expiration work and no matching compensation owner. ## What Changed - Count saved migration markers and unfinished migration journals as Git expiration consumers. Keep failed migration retention holds as consumers too. - Expose `supersedeExpiredRemoteWorkspaceMigrationCohort` as an internal service API for a privileged operator with a real database handle and reviewed company/cohort inputs. It is called directly by the PostgreSQL/Git test and reviewed one-time administrative qualification. It adds no CLI mode or HTTP route. The existing CLI still refuses a damaged cohort; this PR does not claim automatic CLI recovery. - Require an exact unpublished cohort of up to eight members. It requires completed expiration and an unexpired restore recipe for each missing member. The other members must retain their saved source fingerprints. - Recheck current source and recovery state under source, lifecycle, finalizer, and row locks. One transaction fails all old journals, writes audit records, and removes only their exact matched active markers. - Keep old cohort proofs, operations, snapshots, and object references. Genuine failure receipts use the existing failed-save retention hold. This transition does not report migration success. - Reuse the existing bounded source-lock queue when `sourceLockWaitMs` is set. Keep fast refusal by default. Cancellation targets only this waiter, and current checks still run after admission. - Record predecessor IDs on new attempts and reserve a snapshot generation above the existing ledger maximum. New success does not retire the old hold. - Document the owning transition and retention rules. ## Verification - The two new journal-consumer cases failed under the old expiration predicate and passed under the new predicate. - The full new recovery case passed in 21.12 seconds with real PostgreSQL locks, Git repositories, encrypted local object storage, and archive restores. It covers four retained uploads, two real expirations, unchanged old resume refusal, authority change refusal, transaction rollback, a held native finalizer, idempotence, eight new imports, dirty/untracked file restores, and a retention pass 30 days later with no object deletion. - Server TypeScript and module-boundary checks passed. - On `7cec8c915acfad2d08874eaf26abaa852f560219`, the queued-admission regression failed before forwarding the option (24.07 seconds), then passed (22.25 seconds). It verifies a real held project lock, cancellation that preserves the holder and all journals, release into full compensation, retention, and eight restored fresh imports. Current server TypeScript and module/diff checks passed. The only production follow-up changes are the optional queue input and its existing-owner call. - The earlier `737a0df3a71b10fe48d7bbfe7ffd8ae296b4852c` CI run had one Runner support failure: the unchanged credential-free Python/Pi calibration exceeded its outer 25-second subprocess deadline. That job passed 6,926 other tests and skipped four. The timeout is retained as a nonpass. The changed logical head requires fresh CI and review; no test deadline was raised. - The affected cohort, Git expiration, and eligibility suites passed all 148 cases in 228.41 seconds on `aa589f3f447e36d0277c32f8b44cbe5e84a673be`. Merged current master `a9d0946e64`; the only conflict retained both independent added test blocks. The complete merged cohort suite passed all 30 cases in 70.00 seconds on `737a0df3a71b10fe48d7bbfe7ffd8ae296b4852c`, including upstream larger-cohort controls. Final-head server TypeScript, incremental preflight, module-boundary, and diff checks passed. Unchanged expiration/eligibility evidence remains valid. Fresh `809ab704e619812d8f1bf38cf5de9c65893861cc` CI and review are pending. - On `809ab704e619812d8f1bf38cf5de9c65893861cc`, the complete recovery fixture passed in 21.38 seconds after adding one valid archived recipe with no restore point. It refuses with `migration_supersession_expiration_unverified` and keeps all workspaces, journals, and snapshots unchanged. The same case then verifies queue cancellation/release, full compensation, retained history, fresh imports, and restored bytes. Server TypeScript passed. This follow-up changes only the test and service-scope documentation; product bytes remain identical to qualified `7cec8c915acfad2d08874eaf26abaa852f560219`. - Earlier fixture attempts did not pass. They exposed the selection of a held source and an incorrect expected refusal string. The fixture now selects eligible sources and asserts the canonical authority refusal. Production guards were not relaxed. - Live qualification of the exact owning product bytes completed in 230.419 seconds. All eight old operations became failed with auditable supersession lineage. All eight old snapshot rows remained byte-for-byte identical, and their object references remain retained. The existing source-lock queue waited 1.151 seconds for a normal finalizer handoff. The three complete source/recovery passes took about 218 seconds including their transaction, which explains why the earlier 180-second total budget was insufficient for that measured workload. No source proof or publication guard was skipped. Compensation made no provider or object-storage calls. - Earlier live qualifications remain nonpasses: the 180-second attempt did not finish; its instrumented successor completed source checks and refused a busy source lock before mutation. The successful bounded-queue attempt retained those failure records. Fresh archived/online imports are a separate subsequent step and are not claimed complete here. - The local checks do not claim browser acceptance, app deployment, fleet enrollment, or a completed zero-downtime cutover. ## Risks - This owner mutates migration bookkeeping. It preserves all-or-nothing row comparisons and current source/recovery locks. A changed journal, authority, writer, finalizer, expired recipe, or partial transaction refuses the transition. - Historical snapshots and objects remain pinned after new success. This intentionally retains storage. Release requires a separate reviewed action. - An older importer cannot use the successor generation contract. Use the containing canonical owner for recovery. Normal app delivery must also carry the expiration guard; a private maintenance graph does not patch a running server's cleanup worker. - The migration owner crosses 1,000 lines. The bounded compensation transition remains beside its private source inspection, fingerprint, fence, reservation, and publication contracts. Moving it now would require exporting or duplicating those safety owners. No unrelated refactor or new framework is included. ## Model Used OpenAI GPT-6 through Codex, with reasoning, source editing, Git, and local execution tools. The exact serving model ID and context-window size are not exposed in this session. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `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 --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
DottaandPaperclip
|
5bad074baa |
feat: add individual agent interaction permissions (#20411)
Add eleven sparse incoming and outgoing restrictions, human CLI/API administration, shared mutation enforcement, and pending-delivery checks. Preserve existing defaults, agent settings, and existing authorization boundaries. Co-Authored-By: Paperclip <noreply@paperclip.ing> |
||
|
DottaandClaude Opus 5.5
|
99a6daf5bc |
perf(auth): resolve a Cloud tenant actor in one database round trip (#20390)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - On Paperclip Cloud, each request resolves its board actor from trusted Cloud headers in `actorMiddleware` > - This runs for every request after the liveness probe, also for static files such as `favicon.ico` and `index.html` > - The resolution ran three database statements one after another: the stale `instance_admin` purge, the membership read and the owner-elevation read > - On a loaded database, each statement waits. Three sequential waits make every request slow, also a request that needs no data > - This pull request keeps the purge and the tenant sync in order and runs the two reads at the same time as them > - The benefit is one database wait per request instead of three ## Linked Issues or Issue Description **What existing behavior does this improve?** Cloud tenant actor resolution in `server/src/middleware/auth.ts` (`resolveCloudTenantActorOnce`). **Subsystem affected** Server authentication middleware (Cloud trusted-header path). **Current behavior** Measured on the hosted staging instance on 2026-11-09 from a signed-in browser, 45 paired samples in 40 s: - `/api/liveness` (mounted before actor resolution): client median 92 ms, p90 255 ms, server time ≤ 1 ms. - `/favicon.ico` with a session (after actor resolution): client median 413 ms, p90 1,751 ms, max 4.6 s. - `/api/cli-auth/me`: server median 346 ms, p90 1,528 ms. The same Node process answers liveness quickly, so the event loop is not blocked. The database is busy (6–8 of 8 vCPU for the whole day), and trivial primary-key reads waited 100–280 ms each. Three sequential reads explain the ~320 ms median gap between liveness and favicon. **Proposed behavior** The purge and the debounced tenant sync stay ordered. The membership read and the owner-elevation read run at the same time as them. The sync writes only the primary company's membership, and the membership read excludes that company, so the result does not change. **Reason and benefit** A synced request costs one database wait instead of three. With 150 ms of injected database latency, a synced resolution goes from 914 ms to 310 ms (3 runs each, same machine). **Breaking changes** None. ## What Changed - `resolveCloudTenantActorOnce` starts the purge-and-sync, the additional-membership read and `resolveOwnerInstanceAdmin` together and awaits them with `Promise.all`. A membership read failure still degrades to the primary company. The owner-elevation read still fails closed. - New regression test `server/src/__tests__/cloud-tenant-actor-round-trips.test.ts`. It connects through a TCP proxy that delays every client write by 150 ms and asserts that a synced resolution finishes in less than 4 × the delay. Before the change it takes about 6 × (914 ms); after the change about 2 × (310 ms). - `auth-session-route.test.ts`: the fake database binds each `returning()` to its own insert, and the test matches inserts by content instead of position, because the instance-settings insert can now happen in parallel with the tenant sync. ## Verification - `cd server && npx vitest run src/__tests__/cloud-tenant-actor-round-trips.test.ts` — red on `origin/master` (914 ms, 913 ms, 916 ms), green with the change (310 ms). - `cd server && npx vitest run src/__tests__/cloud-tenant-company-provisioning.test.ts src/__tests__/auth-session-route.test.ts src/__tests__/liveness.test.ts src/__tests__/cloud-warm-standby.test.ts src/middleware/cloud-tenant-actor.test.ts` - `pnpm verify` (selected tests), and CI. - After deploy: compare `/favicon.ico` and `/api/cli-auth/me` client time with `/api/liveness` on staging. Expect the median gap to drop from about 3 database waits to about 1. ## Risks - Low risk. Authorization does not change: the same statements run and the request waits for all of them before it continues. - Each request now uses up to three pooled connections at the same time instead of one. The staging pool limit is 500, so this is not a concern there. Smaller pools reach their limit sooner under a burst. - The diff re-indents the purge-and-sync block. The statements in it do not change. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, browser, code execution). Extended reasoning on. ## 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
|
1ca3e8ea2a |
fix(native): let a resume claimed after a restart write the run's existing log (#20427)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - When a native run is interrupted, Paperclip can persist a same-run resume: a later attempt continues the same run from its exact checkpoint > - If the server restarts before that resume is dispatched, the new server process claims it. The resume then reopens the run's existing log, and only the current controller may write it > - The resume claim took the lease owner but left the dead process's controller identity on the lease. The log writer check compares the boot id, so it refused, and the exact checkpoint resume failed as `run_log_writer_authority_lost` > - This pull request stamps the claiming process's identity on the resume claim, as restart recovery's own claim already does > - The benefit is that a resume persisted before a restart continues after it, instead of failing and needing a manual Retry ## Linked Issues or Issue Description Refs #19831. Storage chaos wave `w-20261110T003423-s9903` (local ST2, seed 9903), task t01 (codex C1, K1). The wave went 7/8; t01 was the one task that needed a Retry. - Run 279772b3 was SIGKILLed during its first turn. The restarted server resumed it, and the Codex snapshot failed ("cannot safely recover an ambiguous Codex turn start"). A same-run resume was persisted. - A second SIGKILL hit (00:35:37), and the next process dispatched the scheduled resume at 00:35:56. - The resume's run-log claim failed in `createRunLogWriterAuthority`: the coordinator lease still carried the killed process's `controllerBootId`. The exact checkpoint resume was refused (`run_log_writer_authority_lost`), and the run ended `adapter_failed`, needing a Retry. The wave had S3 latency injected, but the failure does not depend on it. The new test reproduces it on master with no storage faults. ## What Changed - `server/src/services/native-runtime/native-finalization-reconciler.ts`: `dispatchNativeSessionResumptions` writes the claiming process's controller identity (boot id, pid, host hash, process start) along with its lease owner and expiry. This matches the restart-recovery claim, so ownership proofs and the run-log writer check see the process that actually owns the resume. - `server/src/__tests__/native-session-resumption.test.ts`: a regression test for this case. ## Verification - `native-session-resumption.test.ts`: - A coordinator persisted for resume by a previous process (stale boot id), on a run with an existing object-store log, is dispatched. - The run-log writer check accepts the new claim, and the lease carries this process's identity. - The test fails on master with `run_log_writer_authority_lost`. - Regression: the native resumption, binding, recovery-deadline, restart-recovery, scheduled-resume, owner-release, runner-ownership and recovery suites pass locally. `tsc --noEmit` passes for `server`. ## Risks - The resume claim now also refreshes the controller pid, host hash and process start on the coordinator row. These describe the process that owns the lease, exactly as after a restart-recovery claim. A claim by the same process writes the same values as before. - No schema migration. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edits). ## 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 LeachandPaperclip
|
3a5572c9e5 |
Extend login PTY contracts for existing computers (#20410)
## Thinking Path
> - Paperclip manages agent execution across environments.
> - Environment providers own execution resources.
> - Codex and Claude already share the login PTY transport.
> - A login on an existing computer needs its own allocation and
deadline.
> - Input retries must retain their identity.
> - Codex also needs a fixed credential-file read; Claude already
returns its token through PTY output.
> - This change extends the shared contract and keeps both existing
provider parsers.
## Linked Issues or Issue Description
**Problem**
The existing login contract assumes a temporary sandbox. It cannot
describe a login allocation on an existing computer or export its fixed
credential result without generic execution.
**Proposed solution**
Add shared allocation purpose, command identity, PTY deadline, and input
identity fields for both Codex and Claude. Add one optional
credential-result hook for Codex's auth.json file.
**Alternatives considered**
A separate Claude login implementation would duplicate the current
setup-token parser and owner-bound credential writer. Claude continues
to use that existing path. The new artifact hook is Codex-specific
because Claude's setup-token result comes through the existing shared
PTY output stream.
## What Changed
- Export the login contract types from the main SDK entry point.
- Forward login purpose and command through plugin allocation and retain
them in lease metadata.
- Forward the PTY deadline and give each input write a stable identity.
- End a login route when the worker cannot confirm input delivery.
- Preserve the opening company scope for input, stop, and close
callbacks.
- Advertise and dispatch the optional fixed Codex credential-result
hook.
The allocation, deadline, input, output, and cleanup contracts serve
both providers. This PR does not enable either provider on an existing
computer. The dependent consumer PR connects admission, services,
credential completion, and UI for both providers.
## Verification
- Close-ordering regression: all 91 host worker-manager tests passed,
including concurrent close callers waiting behind a held plugin
acknowledgement after input failure. Server TypeScript check passed. All
current CI gates, ordinary review, and the selected follow-up review
pass on
|
||
|
DottaandClaude Opus 5.5
|
347d85fef3 |
perf(chat): index GitHub review lookups and share review-wait reads (#20400)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - GitHub chat endpoints let agents review pull requests. The server reads review state when it lists issues and when it updates GitHub checks. > - On a hosted instance, two GitHub review queries used the most total database time. One query ran about 660,000 times at about 33 ms. The other query ran about 10,000 times at about 1 s. > - The first query finds a source comment's chat link. No index covers `chat_message_links.comment_id`, so each call reads the full table. Issue lists also read the same review waits two times per request. > - The second query joins deliveries, links and runs on a JSON key. The planner walks the agent's run history in creation order and reads each run's context. > - This pull request adds four concurrent indexes, splits the run lookup into two small reads, and shares review-wait reads in one issue-list request. > - The benefit is less background database load, so request reads wait less. The results stay the same. ## Linked Issues or Issue Description **What existing behavior does this improve?** GitHub review state reads. These are the inherited review-wait lookup by source comment, and the review-check lookup of the run that a delivery's comment woke. **Subsystem affected** Server chat channels (GitHub review waits and checks), issue list attention maps, and the database schema. **Current behavior** - `readInheritedGitHubReviewWait` filters `chat_message_links` by `company_id`, `comment_id` and `direction`. No index has `comment_id`, so PostgreSQL reads the full table on each call. The run-origin check does the same lookup first. - An issue list calls the external review-wait read from the review attention map and again from the blocked-inbox attention map. The two maps read the same in-review issues. - `githubReviewCheckState` joins deliveries, links and runs with `context_snapshot->>'wakeCommentId' = comment_id::text`, sorts by `created_at desc` and takes one row. The planner reads the agent's runs in creation order and reads each run context. If no run matches yet, it reads all of them. The delivery filter on `normalized_event` and the link join on `delivery_id` also have no index. **Proposed behavior** The same rows come back from small index reads. Each issue's review wait is read one time per issue-list request. **Reason and benefit** These two statement families used about 33,000 s of database time in one cumulative statistics window. The database had about 50 concurrent active queries, and request reads waited behind this background work. **Breaking changes** None. The migration adds indexes only. ## What Changed - Migration `0456_chat_review_lookup_indexes` builds four indexes with `CREATE INDEX CONCURRENTLY IF NOT EXISTS`. It runs through the existing top-level statement path, which repairs an invalid index on retry: - `chat_message_links_company_comment_idx` on `(company_id, comment_id)` where `comment_id is not null`. - `chat_message_links_delivery_idx` on `(delivery_id)` where `delivery_id is not null`. - `chat_deliveries_github_automatic_delivery_idx` on `(endpoint_id, normalized_event->'githubAutomatic'->'context'->>'deliveryId')` where that value is not null. - `heartbeat_runs_company_wake_comment_idx` on `(company_id, context_snapshot->>'wakeCommentId')`. This index is not partial. The planner does not use expression statistics from a partial index. Without these statistics, it estimates many matches and keeps the creation-order walk. - `githubReviewCheckState` reads the delivery's comment IDs first. If there is a comment, it reads the newest same-agent run with one of these `wakeCommentId` values. If there is no comment, it skips the run read. The comparison stays text equality on `comment_id::text`. - `createExternalReviewWaitReader` shares one read per issue between the review and blocked-inbox attention maps of one issue-list request. The reader exists only for that request. - `doc/DATABASE.md` describes the indexes and the new read order. - `scripts/benchmark-review-lookup-indexes.ts` compares the pre-fix and new code with and without the indexes. `scripts/benchmark-review-lookup-index-migration.ts` times the migration on a large synthetic database. ## Verification Local SQL measurements on a synthetic fixture. They are not production timings. Environment: embedded PostgreSQL 18.1, baseline revision `02bfe71a6b`. Fixture: 50,000 links, 5,000 deliveries with about 2 KiB events, and 4,000 runs with about 12 KiB incompressible contexts. The wake run is in the middle of the run history. Each value is the median server execution time from `EXPLAIN ANALYZE` (2 warmups, 9 samples). The order was: no indexes, indexes, then no indexes again. | Caller and statement | Before (no indexes, two passes) | After | Shared buffers before → after | | --- | --- | --- | --- | | Inherited review wait, link by source comment (same SQL) | 2.72 / 2.36 ms | 0.076 ms | 929 → 12 | | Review check, wake run found | 31.9 / 21.9 ms | 0.29 ms (two statements) | 25,776 → 62 | | Review check, no wake run yet | 71.0 / 47.5 ms | 0.06 ms (two statements) | 34,861 → 12 | - Indexes alone do not fix the review check. The old SQL with the new indexes took 8.6 ms (run found) and 29.7 ms (no run), because it still walked the run history. The new SQL without the new indexes was neutral (23.5 and 35.1 ms). - Service wall times on this shared machine were noisy, so the table reports server execution time. - Correctness: the benchmark fails unless the inherited statement returns the same positional rows in each index state. The check service result and the final wake-run rows of the new code must also match the pre-fix module for both cases. - Migration timing, normal migration runner, synthetic data: 200,000 runs (1.8 GB with 8 KiB incompressible contexts), 500,000 links and 100,000 deliveries. First run 4.2 s; rerun 0.38 s. Separate builds: links by comment 1.5 s, run wake comment 1.1 s, deliveries 0.7 s, links by delivery 0.3 s. All indexes were valid. The run index was 8.3 MB. - New tests: - `server/src/__tests__/github-review-check-state.test.ts` checks that the newest same-agent run is selected. It also checks that other agents, other companies, uppercase IDs, non-string values, list-only `wakeCommentIds`, comment-less links and unknown deliveries do not match. It checks that the generated statements can use the new indexes. - `server/src/services/native-runtime/external-review-wait.test.ts` checks that the reader reads each issue one time, keeps concurrent callers correct, and rejects all waiting callers when a read fails. - Red/green: with the pre-fix `review-check-state.ts`, the behavior test passes and the index test fails. With this change, both pass. - Focused runs pass: `issue-blocker-attention`, `heartbeat-issue-liveness-escalation`, review-wait, review-check-state, external-review-wait, review-repair (106 tests), and `external-chat-wait.integration` (285 tests). `pnpm --filter @paperclipai/server typecheck` and `pnpm --filter @paperclipai/db check:migrations` pass. Commands: ```sh pnpm --filter @paperclipai/server exec tsx ../scripts/benchmark-review-lookup-indexes.ts --baseline-ref=02bfe71a6b pnpm --filter @paperclipai/server exec tsx ../scripts/benchmark-review-lookup-index-migration.ts ``` ## Risks - The migration builds four indexes concurrently. It does not block writes. On a large database, the run index reads every run context one time. The local run took 1.1 s for 1.8 GB from a warm cache. Cold storage on smaller compute can be slower. - The run index adds one entry for each run insert and for each run update that is not a heap-only update. Many runs have no `wakeCommentId`, so these entries hold a null key. - The review check now sends two statements instead of one when a delivery has a comment. Each statement reads only a few index entries. - When two matching runs have the same `created_at`, either can be selected. The previous query had the same tie behavior. - This change does not change the number of in-review issues that one issue list reads. The blocked-inbox map still reads all in-review issues of the company. A smaller scope needs a separate change to the liveness classifier. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`) by Anthropic, in Claude Code with tool use (shell, file edits, local PostgreSQL benchmarks). ## 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
|
326df52673 |
perf(api): rank Mine inbox candidates from the participation set (#20413)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work.
> - The Mine inbox reads `GET /api/companies/:companyId/inbox/mine`. Its
main query is `issueService.list` with "touched by me", "not archived by
me", six statuses including `done`, and `LIMIT 100` by canonical last
activity.
> - That query evaluates `touchedByUserCondition` (a five-way OR of
EXISTS) on every status-matching issue in the company. Because `done` is
included, the scan grows with company history, not with the user's work.
> - The unread badge already solved the same problem with a UNION of
indexed participation sources (#17973). The list path did not.
> - This pull request makes the Mine list rank page ids from that same
participation UNION, then load full rows (with the expensive select-list
subqueries) only for the 100 selected ids.
> - The benefit is that the main Mine query scales with the user's
participation instead of the company's issue count. Results stay
identical, and a new test proves it against the original scan.
## Linked Issues or Issue Description
**What existing behavior does this improve?**
The latency of the Mine inbox snapshot, `GET
/api/companies/:companyId/inbox/mine`.
**Subsystem affected**
Server: issue list service (`server/src/services/issues.ts`) and the
Mine route.
**Current behavior**
Measured on 2026-11-09: 280–570 ms server time on a dev instance and
1.9–3.2 s on staging. The list query scans every status-matching issue
(28k of 30k in the fixture below) to test participation, then sorts the
touched ones.
**Proposed behavior**
The list query reads only the user's participation set and computes the
row projection only for the page.
**Reason and benefit**
Inbox opens are frequent. The old cost grows with every completed task
in the company.
Refs #17973 (same UNION for the unread badge).
## What Changed
- Extracted the participation UNION from `unreadInboxIssueIds` into
`mineParticipationIssueIds()`. The unread badge uses it unchanged.
- New `IssueFilters.touchedCandidates: "participation"`. When it is set
with `touchedByUserId` (and there is no search and no
`oneChildPerParent`), `list`:
1. replaces `touchedByUserCondition` with `issues.id IN (<participation
UNION>)`. Each UNION member is one OR branch of the old condition, so
the set is the same;
2. runs a narrow `candidates` query (`SELECT id … WHERE <all other
filters> ORDER BY <same order> LIMIT/OFFSET`);
3. loads the full projection for only those ids (re-checking company,
visibility and the read condition), and restores the phase 1 order.
- Only the Mine route sets the flag. Every other `svc.list` caller keeps
the old SQL.
- A new Server-Timing phase, `inbox_issues_candidates`.
- `server/src/__tests__/inbox-mine-candidates.test.ts`: a
result-equivalence test.
- `scripts/benchmark-inbox-mine.ts`: a local benchmark that seeds a
large fixture, compares full payloads, and records `EXPLAIN (ANALYZE,
BUFFERS)` per query.
### Not changed, with evidence
- **Recomputed comment/activity maxima** (`lastActivityStatsForIssues`):
0.9 + 0.2 ms per call in the plans. Not worth a second code path.
- **Touched condition inside `userCommentStatsForIssues`**: 1.4–2.1 ms.
Not changed.
- **`liveDescendantCountMapForIssues`**: 12 ms at 30k issues and 40 ms
at 90k. It now costs about as much as the list query. It scans all live
runs and hashes all company issues. It is company-wide work, outside the
candidate selection, so I left it for a separate change.
## Verification
### Correctness
`inbox-mine-candidates.test.ts` runs the old scan and the new path on
the same data. It asserts equal ordered ids and equal full JSON
payloads. Fixture: 170+ touched issues (more than one page, with tied
timestamps) plus these edge cases:
- included: created by me, assigned to me only, user activity, my direct
comment, `done`, pending request addressed to me, archived then
re-activated by a newer outside comment, read state with a later agent
comment, a live child (live descendant count);
- excluded: archived with no newer activity, mention-only, read audit
only (`issue.read_marked`), uppercase entity id, channel-sourced
comment, agent-created with my user id, `cancelled`, hidden, harness,
plugin operation, private to another user (`issueReadSqlCondition`),
answered request, untouched issues, another company.
It also compares other shapes: no archive filter, `status=done` with the
full projection, `offset`/`limit`, ascending order, unrestricted read
condition, a second user, and a user with no participation. Mutation
check: removing the comment source from the UNION makes the test fail.
The benchmark also asserts identical full payloads for every sample of
both arms.
Focused suites (local): `inbox-mine-candidates`, `inbox-mine-routes`,
`authorization-privacy-query`, `issues-unread-inbox`,
`inbox-archive-routes`, `issues-assignee-unread`,
`issues-creation-visibility` — all pass.
### Performance (local SQL measurements)
Command: `pnpm --filter @paperclipai/server exec tsx
../scripts/benchmark-inbox-mine.ts --baseline-ref=18c3f4ef7b
--issues=<N> [--touch-density=<d>]`. Baseline =
|
||
|
DottaandClaude Opus 5.5
|
4a2eb920cf |
fix(storage): wait out a store outage on a run's critical path instead of failing it (#20299)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A cloud instance keeps run logs and evidence in object storage, so a run touches the store as soon as it starts > - In storage chaos wave `w-20261109T145628-s9902` (ST1: 75–176 s windows where every S3 call returns 503), only 1 of 8 tasks finished. A run starting or resuming during an outage failed within a second as `adapter_failed`, generic recovery burned its attempts seconds apart, the task was escalated to the board, and the user saw the raw S3 error > - This pull request treats a transient storage error as an infrastructure wait: the run is retried on a minute-scale schedule, rapid recovery is suppressed, the board shows it as working, and the run error says storage was unavailable > - The benefit is that a store outage of several minutes delays work instead of failing and stranding it ## Linked Issues or Issue Description Refs #19831. Storage chaos wave `w-20261109T145628-s9902` (master `610b54f551`, `--storage s3`, ST1). **Where the run fails.** At run start or resume, `run-output` `begin` opens the run log: `getRunLogStore().begin` → object run log store → object byte journal `claim` → S3 conditional PUT. The journal retries only 3 times within about 300 ms, so a 503 escapes and fails the run as `adapter_failed`. The async stack ends in the SDK middleware with no app frame. `log_store` stays null on the failed run, which pins the claim. **What followed.** - `failRun` released the issue into generic immediate recovery. Recovery retried 1–2 s apart inside the outage (t01: 14:58:33, :35, :59, 14:59:01), then stopped and opened a board-owned `stranded_assigned_issue` action. These are the open recovery actions on CHA-1, CHA-4, CHA-7 and CHA-8; a later Retry hit the outage again and escalated the same way. - The run error was the raw S3 text ("Injected storage outage"). **Slow store (ST2, `w-20261109T151946-s9903`).** With about 9.7 s of added latency per call, the same claim hit the journal's 15 s per-operation deadline (`AbortSignal.timeout(15_000)`). The run failed after about 36 s with `AbortError: Request aborted` and burned 3 runs the same way; 4 of 8 tasks needed a Retry. t02/t03 (Retry parked with `process_identity_missing`) are a separate native-recovery class and are handled in a follow-up. **Daytona storage chaos ST1 (`w-20261109T161202-s9912`, 1/8):** with S3 returning 503 for 72 to 154 s at a time: - Workspace restore failed at once (`remote_workspace_restore_failed: step=snapshot_download; storage=provider_error`), three times in 40 s, then `setup_failed`. It left an active recovery action behind. - Runs reattached after a restart died within 2 s on the run-log claim, then became reconciliation-required. - Setup failed on other storage reads. - 6 of 7 failures needed a UI Retry. So the rule here is one class rule: a transient store error on a run's critical path waits, it never fails the run. ## What Changed - New `server/src/storage/storage-unavailable.ts`: - `isStorageUnavailableError` reads the recorded storage failure diagnostics first, then the SDK error shape. Unavailable means 429/500/502/503/504, the S3 throttling and timeout names, network codes (`ECONNRESET`, `ECONNREFUSED`, `ETIMEDOUT`, `EPIPE`, `EAI_AGAIN`, `ENOTFOUND`), or an `AbortError` whose cause is a `TimeoutError` (a per-operation deadline). 403, 404 and 412 are not outages, and neither is an abort caused by the run's own Stop. - It defines the `storage_unavailable` code, the `storage_unavailable_wait` retry reason, the retry policy (60 s × 10 attempts, about 10 minutes) and the user-facing message. - `server/src/services/heartbeat/run-completion.ts`: - `failRun` classifies such a failure as `storage_unavailable` with the plain message. - Unless native recovery owns the run, it schedules the storage wait with `scheduleBoundedRetryForRun`, skips the interaction-continuation retry, and releases the issue with `suppressImmediateRecovery`. - `failRunSetup` uses the same policy for this code. - `server/src/services/heartbeat/retries.ts`: `storage_unavailable` joins the transient infrastructure codes. It gets its own retry budget and policy, like `runner_revision_mismatch`. - `server/src/services/task-interruption-presentation.ts`: `storage_unavailable_wait` is a resource wait, so the board shows the task as working with no Retry while it waits. - **One shared outage wait for the run's critical path** (`server/src/storage/storage-unavailable.ts`): - `retryThroughStorageOutage` retries a transient store failure with backoff (2 s doubling to 30 s) until that storage operation's outage has lasted 5 minutes, then gives the error back. The outermost wait of an operation records its first failure, and nested and parallel waits inside it share that start; no other request's success extends it. Shared probes and shared reads run under their own scope, so one run's Stop never cancels work another run waits on. A stop signal ends the wait, and non-outage errors are never retried. - Inside `waitOutStorageOutages`, every replayable S3 request waits instead of failing: get, head, delete, buffered and file puts, and multipart create, file parts and complete. A consumed upload stream is never resent, and requests outside the scope keep failing fast. - The scope covers run execution (`heartbeat.ts` `executeRun`: setup, workspace restore, run-log claim and the run itself), plus `loadWorkspaceSnapshotArchive` and `uploadWorkspaceSnapshot`, which copy-back and recovery workers use too. - The run-log claim retries as a whole, because the journal bounds each operation with a 15 s deadline. - Past the budget, the run takes the bounded `storage_unavailable` wait below. - The classifier follows the error's `cause` chain, so a restore failure wrapping an outage counts. `failRunSetup` checks storage before restore failures, so that case takes the storage wait instead of two quick retries and an escalation (the R6 leak). - `object-byte-journal.ts`: when its deadline expires, the journal rethrows the recorded storage error instead of a bare `TimeoutError`. ## Verification Red/green (each new test fails against master and passes with this PR): - `run-completion.test.ts` "waits a minute and suppresses rapid recovery when a run fails because storage is unavailable (storage chaos ST1)". An S3-shaped 503 records `storage_unavailable` with the plain message (no raw S3 text), schedules `STORAGE_UNAVAILABLE_RETRY`, releases with `suppressImmediateRecovery: true`, and skips the continuation retry. "keeps an ordinary adapter failure on the normal recovery path" guards the default. - `task-interruption-query.test.ts` "does not present an overdue storage_unavailable_wait wait as a failed attempt": working, `canRetry: false`. - `storage-unavailable.test.ts`: which errors count as an outage and which do not, including recorded diagnostics, a deadline-aborted call (ST2), and a Stop-aborted call that must not count. Static checks: - `tsc --noEmit` passes for `server`. Live verification: Phase 3 will rerun ST1 (seed 9902) with the MinIO fault proxy after merge. - `server/src/__tests__/storage-outage-wait.test.ts` runs against a local S3 that answers 503 for 3 s: - Inside the scope, a snapshot `getObject` and a run-log conditional PUT succeed once the store is back. - Outside it, a request fails fast. - A stop ends the wait. - Unit cases cover the budget, non-outage errors, and a `RemoteWorkspaceRestoreError` wrapping an outage. - `run-completion.test.ts`: a setup failure from a restore that wrapped an outage takes the storage wait (red before). - `object-byte-journal.test.ts`: a real `claim` whose head PUT hits its deadline is classified as a storage outage (red before). ## Risks - A storage outage now holds a task for up to about 10 minutes of automatic waits instead of escalating within seconds. A longer outage still ends in the existing board recovery. - A run whose failure is misclassified as storage-unavailable waits instead of using fast recovery. The classifier accepts only server-side or network failures from the storage SDK, never 4xx answers. - No schema change. - A run can now wait until a storage outage has lasted 5 minutes before it fails over to the bounded wait. It shows as working throughout, and Stop ends the wait. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edits). ## 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
|
ab80fea62f |
perf(recovery): probe run bindings per comment in the lost-wake check (#20412)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The ownership reconciler finds user comments whose wake a restart lost, and it saves a wake for them (plan invariant 4) > - Its lost-wake check matched a task's runs with one OR across the legacy context ID and the native issue column > - With that OR, PostgreSQL can join runs only by company. It then tests each recent comment against every run of its company and reads each run's context > - On a hosted staging database, this statement used about 23 s per call on average (384 calls, about 8,900 s in total). One call ran for 35 s. Other requests waited behind it > - This pull request reads the check in three stages and probes the two run bindings separately with existing indexes > - The benefit is the same result with bounded, index-driven work for each recent comment ## Linked Issues or Issue Description **What happened?** The reconciler's user-comment lost-wake check was one of the most expensive statements on a hosted staging database. Query statistics showed 384 calls with a mean of about 23 s. One active call was seen at 35 s. The database compute was busy, and short authentication reads waited behind long statements. The statement matched runs with `(context_snapshot->>'issueId' = issue OR native_issue_id = issue)` in three subqueries. The OR gives PostgreSQL no join key except `company_id`. A local plan on synthetic data showed the cost: a hash join of 273 comments against all 38,002 runs, which took 1.0 s of a 1.3 s statement. The work grows with recent comments multiplied by the company's run count, and each test reads a run context. **Expected behavior** Each recent comment uses a small number of indexed probes. The check returns the same comments in the same order, with the same limit and grace logic. **Steps to reproduce** 1. Seed a company with a deep run history and a few hundred recent user comments. 2. Let some recent comments have no wake (for example, after a restart lost the wake of a steered comment). 3. Run the check. PostgreSQL can pick a hash join on `company_id` with the OR as a join filter. With nested loops off, the old statement takes seconds. The local benchmark below shows this. **Paperclip version or commit** Baseline: `02bfe71a6b`. **Deployment mode** Server with PostgreSQL. Local proof uses disposable PostgreSQL 18.1. Production and staging use hosted PostgreSQL. Related: the earlier change that split native and legacy bindings for other ownership reads (see the "Execution recovery query performance" section in `doc/DATABASE.md`). A search found no open pull request for this check. ## What Changed - `checkUserCommentsWithoutWake` reads in three materialized stages: 1. Recent user comments on assigned, non-conversation tasks that no wake request carries. This stage keeps the same issue, status and agent-closed rules. 2. Of those, comments with no live or later run and with at least one earlier run. 3. Of those, comments that no live run acknowledged in an item event (the steer check). Only this stage reads event payloads. - Each run lookup is two probes: one on the legacy context ID and one on the native issue column. Both use existing indexes. `exists (A or B)` becomes `exists (A) or exists (B)`, which is logically the same. - Each probe ends with `offset 0`. This keeps it a per-row index lookup. Without it, PostgreSQL can build a hashed subplan over all runs, or a hash join over all item events of the company. The repository already uses this fence in `execution-blocker.ts`. - The final `order by c.created_at limit N`, the projection, the grace window and the reported reason do not change. - Add `scripts/benchmark-comment-wake-check.ts`. It loads the check from an explicit baseline commit and from the working tree, and it compares the complete service result and the complete positional SQL rows before it records time. - Add a regression test for each run binding on its own: native only, legacy only, live run, later run, steered run, an acknowledgement before the comment, a log-only mention, a run that ended before the comment, and a run in another company. - Document the check and the benchmark command in `doc/DATABASE.md`. - No schema change, index, migration, or new startup work. ## Verification - `pnpm exec vitest run server/src/__tests__/ownership-reconciliation-report.test.ts --maxWorkers=1`: 34 passed. This includes the existing lost-wake, restart, steered and agent-closed tests and the new binding test. - `pnpm --filter @paperclipai/server typecheck`: passed. - `pnpm --filter @paperclipai/server exec tsx ../scripts/benchmark-comment-wake-check.ts --baseline-ref=02bfe71a6b --samples=7` Benchmark setup: PostgreSQL 18.1 (embedded, local), `shared_buffers` 128 MB, `work_mem` 4 MB. All data is synthetic, with md5-hex filler so contexts and payloads do not compress well. There are two companies per fixture. Two warmup rounds, then 7 timed samples for each arm, in alternating order. Each sample runs the real check function. Before any timing, each fixture asserts that the complete check result and every positional SQL row are equal for baseline and treatment, and that the fixture reports at least one owed comment. Every timed sample also asserts the same result. | Fixture | Runs / recent comments / owed | Baseline median | Treatment median | Speedup | Hash-join plan, baseline → treatment | | --- | --- | --- | --- | --- | --- | | small | 2,541 / 50 / 6 | 8.5 ms | 2.6 ms | 3.2x | 19 ms → 3 ms | | representative | 75,502 / 750 / 75 | 26.4 ms | 23.3 ms | 1.1x | 2,262 ms → 17 ms | | lost steer wakes | 38,002 / 750 / 75 | 454.7 ms | 66.9 ms | 6.8x | 2,717 ms → 68 ms | | deep history | 150,750 / 1,125 / 114 | 26.9 ms | 18.8 ms | 1.4x | 4,006 ms → 34 ms | | busy hour | 128,750 / 5,625 / 200 (limit) | 79.2 ms | 47.4 ms | 1.7x | 20,047 ms → 75 ms | | long steered runs (stress) | 19,100 / 600 / 60 | 626.4 ms | 457.4 ms | 1.4x | 5,871 ms → 502 ms | - The table above used baseline `02bfe71a6b`, the branch base at that time. After the rebase, a repeat run with baseline `8004ad069e` (the new merge base, with the helper guard fixed to run from the repository root) gave the same picture on a busier machine: small 15.4 → 9.1 ms, lost steer wakes 1,733 → 139 ms, busy hour 103 → 65 ms, all with equal results. - The "lost steer wakes" fixture shows the slow plan with default planner settings. PostgreSQL hashed all runs by company and tested 273 comments against 38,002 runs. - The last column runs each statement once with `enable_nestloop = off`. This makes the company-only hash join that the old statement allows visible, and it shows that the new statement has no such plan. The result is checked as equal in this mode too. Staging's 23 s mean matches this growth: recent comments multiplied by the company's run count. - Shared buffers fell in every fixture except "small" with default settings (for example, busy hour 176,416 → 89,803). - Neutral path: in the "long steered runs" stress fixture, every steered comment lost its wake. These comments correctly reach the steer check. That check must read the payload of each item event after the comment in the run that was live, so it is the remaining cost. In normal operation a comment sent to a live run has a deferred wake that carries it, so stage one removes it. ## Risks - Low risk. The predicate is the same conjunction of conditions. Only the evaluation order and the probe shape change, and the benchmark asserts complete-row equality on six fixtures and in both planner modes. - Rows with the same `created_at` have no defined order, as before. The benchmark uses distinct times so positional comparison is meaningful. - `offset 0` stops PostgreSQL from turning these probes into joins. If a task someday has a very large number of runs, each probe still uses the company and issue indexes. - Upgrade safety: this change reads saved run, wake and event state only. It changes no persisted shape, run identity, or saved session, so state saved by the previous release carries forward unchanged. A cutover test is not applicable. - Not verified on the hosted database. The local proof uses synthetic data, and I did not capture a hosted plan. Confirm with a fresh query statistics window after deployment. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, with tool use (shell, file editing, local PostgreSQL) in Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
50df9a19b9 |
perf(inbox): read only the execution workspaces Mine rows reference (#20388)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The Mine inbox is the first page a board user opens to see the tasks that need them > - The page loaded the company's full execution-workspace summary list and kept task rows hidden until that list arrived > - On a company with thousands of workspaces, that read checks each workspace row against the privacy predicate. It was the slowest request on the page > - The inbox needs workspace names only for the workspaces its rows reference, and only when the view shows workspace data > - This pull request adds an `ids` filter to the summary read, makes the inbox request only referenced workspaces, and stops rows from waiting for workspace data when the view does not show it > - The benefit is that Mine rows appear when the task data arrives, and the workspace read becomes a small bounded query ## Linked Issues or Issue Description **What existing behavior does this improve?** The Mine inbox (`/:companyPrefix/issues?view=mine`) shows its task rows late on companies with many execution workspaces. **Subsystem affected** Board UI inbox and the execution-workspace summary API. **Current behavior** The page calls `GET /api/companies/:id/execution-workspaces?summary=true` with no filter. On a large company this returned 8,758 rows (2.5 MB) and took 12.3 s on the server. The reveal gate included this read, so rows appeared at 13.4 s although `inbox/mine` arrived at 1.6 s. **Proposed behavior** The inbox requests summaries only for the workspaces its rows reference. Rows appear when the Mine task data arrives, unless the view shows workspace data. **Reason and benefit** The workspace read becomes a small bounded query, and it leaves the critical path for the default Mine view. **Breaking changes** None. The `ids` parameter is optional and additive. ## What Changed - `packages/shared`: add `executionWorkspaceSummaryIdsQuerySchema` (comma-separated UUIDs, 1–100 distinct values) and `EXECUTION_WORKSPACE_SUMMARY_IDS_MAX`. - `server`: `GET /companies/:companyId/execution-workspaces` accepts `ids`. Bad input returns 422. The service adds `inArray(execution_workspaces.id, ids)`. The company scope and the actor read condition still apply, so privacy does not change. - `ui/src/api/execution-workspaces.ts`: `listSummaries` accepts `ids`. New `listSummariesByIds` reads ids in chunks of 100 and concatenates the results. - `ui/src/pages/Inbox.tsx`: the workspace query is keyed on the sorted distinct `executionWorkspaceId`s of the rows the inbox holds (Mine, touched, context and remote search issues). It uses `keepPreviousData` and runs only when isolated workspaces are on and at least one id exists. - Reveal gate: rows wait for the workspace read only when workspace data changes what the rows show. This is workspace grouping, a visible Workspace column, or an active search (workspace names are searchable). Otherwise rows reveal without it. - Tests: route ids pass-through and 422 cases (non-uuid, empty, separators only, more than 100); service test for the id filter with company scope and the read condition; UI api chunking test; Inbox tests for "rows reveal while the workspace read is pending when no workspace column or grouping is active", rows waiting when the Workspace column is visible, and "shows the resolved isolated workspace name" with the id-filtered call. Follow-ups (not changed here, they still read the unfiltered list): - `ui/src/pages/LegacyInbox.tsx:772` - `ui/src/components/IssuesList.tsx:985` - `ui/src/components/LegacyIssuesList.tsx:888` - `ui/src/components/NewIssueDialog.tsx:570` (filtered by project and reuse, but not by ids) ## Verification Focused checks on this revision: - `cd server && npx vitest run src/__tests__/execution-workspaces-routes.test.ts src/__tests__/execution-workspaces-service.test.ts -t "summar|ids"`: 14 passed (embedded Postgres ran the service test). - `cd ui && npx vitest run src/api/execution-workspaces.test.ts src/pages/Inbox.test.tsx`: all passed. - `pnpm --filter @paperclipai/server --filter @paperclipai/ui --filter @paperclipai/shared typecheck`: passed. - `pnpm check:token-gates`: clean. - `pnpm verify` selected 2,481 Vitest files (the shared validator change reaches most suites). I stopped it at a 25-minute limit on a heavily loaded shared host. Before the limit, three server tests outside this change failed: `agent-commentary-skills` (skill mount), `agent-config-read-access` (24.8 s timeout) and `agent-directory-working-copies`. Required CI supplies the full coverage. - Review fix `fc3f82dae0`: when workspace data is visible, the reveal gate also waits while `keepPreviousData` supplies the previous id set's summaries. A new test switches Mine → Recent to a new workspace and checks the Recent row stays hidden until its workspace loads. It fails without the change. - Follow-up commits `bf34f84823` and `8fc743b5ca` add a 100-row Mine test with a counted workspace id scan. It fails with the old `= []` defaults and passes with the stable `NO_ISSUES` default. Performance (local, synthetic fixture): - Instance: isolated worktree instance, `authenticated` mode, signed in as a non-admin company member so the real workspace privacy predicate runs. Production UI bundle (`PAPERCLIP_UI_DEV_MIDDLEWARE=false`). Headless Chromium 1440×900, a new browser context per run (cold HTTP cache and cold query cache). macOS, Apple Silicon, with other local sessions running. - Fixture: 8,750 execution workspaces (each with a source task and a privacy-source key), 38,750 tasks, 150 Mine tasks; the Mine snapshot returns 100 rows that reference 100 workspaces. - Finish event: the first `[data-slot="task-row"]` is in the DOM. | Measurement | Before (`02bfe71a6b`) | After | | --- | --- | --- | | Cold time to rows, median of 7 | 1,209 ms (1,046–1,312) | 702 ms (567–1,209) | | Workspace summary read, server wait (time to first byte), median of 7 | 496 ms unfiltered, 2.0 MB | 18 ms with 100 ids, 22 KB | The difference of medians is 507 ms. This is close to the unfiltered read's wait on this fixture (about 550–590 ms in the browser runs). The runs were sequential batches, not alternating pairs. Limits: this fixture does not reproduce the 12.3 s server time seen on the development instance. Here the unfiltered read cost 0.5–1.7 s and grew with the company's task count. The browser gain on the original instance should be close to that instance's unfiltered read time, but I did not measure it there. ## Risks - Low risk. The `ids` filter only narrows the existing query. It does not change the read condition or the company scope. - A row whose workspace is not readable by the actor shows no workspace name, as before. - When the Workspace column or grouping is visible, rows still wait for the workspace read. That read is now small. - When the id set changes, the query refetches all chunks. Before a view first reveals, rows wait for the new summaries if workspace data is visible. After that, the previous names stay visible while a refresh loads. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edit, headless browser). Extended reasoning on. ## 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
|
1c868b86be |
fix(server): tell agents who owns a plugin-managed routine and where it is changed (#20409)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - A plugin can create and own routines ("plugin-managed routines").
The routine row shows the plugin in `managedByPlugin`.
> - An agent reads routines through `GET` and `PATCH
/api/routines/{id}`. These API catalog entries had no description.
> - So an agent that read `managedByPlugin` did not know what it meant
or where a person changes the routine.
> - In chat evals, the agent searched for managed-routine routes and
probed plugin files for over four minutes. It did not reply.
> - This pull request adds one shared description to both catalog
entries. It says the plugin owns the definition, links the board's
plugin settings page, and tells the agent not to rewrite, detach or
duplicate the routine.
> - The benefit is that the agent can answer at once and send the person
to the correct page.
## Linked Issues or Issue Description
No public issue exists. This is a bug report.
**What happened**
A person asked the CEO agent in chat to rewrite a routine that a plugin
manages. The agent read the routine and saw `managedByPlugin`. No tool
result or catalog entry told it what that field means. The agent
searched the API catalog ("plugin managed routine", "managedByPlugin",
"detach plugin managed routine"), tried to read plugin UI files, and
listed agents. After more than four minutes it had not replied, and the
chat timed out. This happened in two separate runs.
**Expected behavior**
The agent replies promptly. It says that the plugin owns the routine,
does not rewrite or copy it, and links the plugin settings page where a
board user changes what the plugin exposes.
**Steps to reproduce**
1. Install a plugin that declares a managed routine (`routines.managed`
capability, top-level `routines[]`).
2. Open a chat with the CEO and select the routine.
3. Ask: "Rewrite this plugin's daily maintenance routine."
4. Look at the agent's `search_api` results for `GET /api/routines/{id}`
and `PATCH /api/routines/{id}`. They have no description.
## What Changed
- `server/src/routes/openapi.ts`: add a shared
`PLUGIN_MANAGED_ROUTINE_GUIDANCE` description to `GET
/api/routines/{id}` and `PATCH /api/routines/{id}`. It names
`managedByPlugin`, links `[plugin
settings](/<prefix>/company/settings/instance/plugins/<pluginId>)`, and
tells agents to reply with that link instead of rewriting, detaching or
duplicating the routine. The `PATCH` entry also states the existing
agent rule (own routines only, no reassignment).
- `server/src/__tests__/plugin-managed-routine-guidance.test.ts`: new
regression test. The queries the agent actually used now find these
entries with the guidance, and the linked settings route is mounted in
`ui/src/App.tsx`.
## Verification
- Red then green: `cd server && npx vitest run
src/__tests__/plugin-managed-routine-guidance.test.ts` failed 6/6 before
the change and passes 6/6 after.
- `cd server && npx vitest run
src/services/native-runtime/runner-api.test.ts
src/__tests__/openapi-routes.test.ts
src/__tests__/person-only-page-guidance.test.ts
src/__tests__/case-history-api-discovery.test.ts
src/__tests__/skill-import-discovery.test.ts` passes.
- `cd server && npx tsc --noEmit -p .` passes.
- Live chat eval `routines.plugin-managed-routine` on a branch with this
commit: see the comment on this PR for the run.
## Risks
- Low risk. The change adds documentation text to two API catalog
entries. Routes and permissions do not change.
- The catalog search scores description words, so these two entries can
rank a little higher for queries about plugins or managed resources.
## Model Used
- Anthropic Claude Opus 5.5 (`claude-opus-5-5`), tool use and code
execution in Claude Code.
## Checklist
- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [ ] All Paperclip CI gates are green
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
|
||
|
DottaandClaude Opus 5.5
|
e1010e7678 |
perf(api): per-phase Server-Timing for the Mine inbox snapshot (#20408)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work.
> - The Mine inbox page reads one snapshot: `GET
/api/companies/:companyId/inbox/mine`.
> - This read is slow. It takes 280–570 ms on a dev instance and 1.9–3.2
s on staging.
> - The response has only the total `paperclip_api` Server-Timing value.
We cannot see which phase is slow.
> - Issue detail already reports per-phase Server-Timing
(`issue-read-timing.ts`). The Mine snapshot does not.
> - This pull request adds per-phase Server-Timing to the Mine snapshot,
including each lookup inside the issue list.
> - The benefit is that DevTools on any instance shows the slow phase
directly. A follow-up PR uses it to prove a query rewrite.
## Linked Issues or Issue Description
**What existing behavior does this improve?**
The Server-Timing header of `GET /api/companies/:companyId/inbox/mine`.
**Subsystem affected**
Server API: issues routes and the issue list service.
**Current behavior**
The header has only `paperclip_api;dur=…`. Slow access checks, issue
lists, parent context, failed-run reads and list enrichment lookups all
look the same.
**Proposed behavior**
The header also has `paperclip_inbox` (total) and one `inbox_<phase>`
entry for each phase that ran.
**Reason and benefit**
We need phase evidence from staging and production to rank the slow
parts. No telemetry is sent; this is a response header only.
## What Changed
- Added `server/src/services/inbox-mine-timing.ts`. It times closed,
content-free phases and formats a Server-Timing value. It emits no spans
and no telemetry events.
- `issueService.list` accepts an optional third argument, `{ timePhase
}`. It wraps the list's own reads: `query`, `labels`, `active_runs`,
`user_stats`, `read_stats`, `last_activity`, `archives`, `blocked_by`,
`live_descendants`, `attention`. Callers that do not pass it are
unchanged.
- The Mine route reports `inbox_access`, `inbox_issues`,
`inbox_parents`, `inbox_failures`, `inbox_failure_context` and
`inbox_issues_<list phase>`. The existing middleware merges these with
`paperclip_api`.
- The index-projection list no longer awaits a `Promise.all` of
already-empty attention maps. The result is the same.
- `inbox-mine-routes.test.ts` asserts that all phases are present with a
valid duration.
## Verification
- `pnpm --filter @paperclipai/server exec vitest run
src/__tests__/inbox-mine-routes.test.ts` — 6/6 pass, including the new
header assertion.
- `tsc --noEmit -p server` — no errors in the changed files.
- `pnpm verify --explain` selects 2,417 of 3,204 test files for this
change, because `services/issues.ts` is imported almost everywhere. I
did not repeat that full suite locally (AGENTS.md §7); required CI
supplies it.
- Manual: open the Mine inbox, then DevTools → Network → `inbox/mine` →
Timing. The phases show under Server Timing.
## Risks
- Low risk. The header adds about 13 short entries. Phase names are
fixed strings and contain no task data.
- `timePhase` is optional, so other `svc.list` callers do not change.
## Model Used
- Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with
tool use (shell, file edits). Standard context window; extended thinking
not used.
## 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
|
7a93cc6215 |
fix(server): redact invitation tokens from agent run logs (#20360)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - People join a company through an invitation link. The link carries a bearer token in its URL path. > - An agent can create an invitation in chat and give the person the join link. > - The server writes each agent run to a run log file (`.ndjson`). This file is a plain diagnostic copy. Local agents run as the same OS user, so they can read it. > - #20343 removed invitation tokens from HTTP logs. The run log still kept the full join link and the raw token. > - This pull request removes invitation tokens from run-log chunks when the server writes them. > - The benefit is that a live join link does not stay in a file that other local agents can read. The person still gets the working link in the chat reply. ## Linked Issues or Issue Description Refs #20343 (same token policy for HTTP logs). **What happened?** An agent created an invitation in chat and replied with a `/invite/<token>` join link. The agent run log file under `data/run-logs/` kept the full token. The token was in the agent message and in the tool result. **Expected behavior** The run log keeps the route and drops the token: `/invite/***REDACTED***`. A raw `pcp_invite_...` token also becomes `***REDACTED***`. The chat reply that the person reads keeps the working join link. **Steps to reproduce** 1. Check out master. 2. Run `server/src/services/heartbeat/run-log.test.ts` from this PR. 3. The "run-log invitation token privacy" tests fail. The token stays in both the legacy text chunk and the canonical native envelope. **Paperclip version or commit** Master at the merge base of this branch. **Deployment mode** Source server, local run logs (`local_file`). The same functions also format object-store run logs. ## What Changed - Add `redactInvitationTokens()` in `server/src/middleware/invitation-token-redaction.ts`, next to the HTTP log redactor from #20343. It removes the token from `/invite/<token>` segments and from raw `pcp_invite_` tokens inside free text and JSON text. The character classes stop at JSON, Markdown and URL delimiters, so the record structure stays valid. `redact-sensitive.ts` stays byte-identical, because the chat eval pins its exact bytes before it accepts a real invitation link. - Apply it in `compactRunLogChunk()` and `redactCanonicalNativeRunLogChunk()`. These are the two functions that format every agent run-log chunk before the store writes it. The run excerpt columns use the same chunk, so they are also redacted. - `canonicalNativeRunLogRedactionForms()` also yields the earlier, unredacted projection. A run log written by an earlier release still verifies after an upgrade. - `restoreCompactedEvent()` accepts a truncated record that the earlier compactor wrote with the token. Cold recovery of such an old log still works. - Add a test that proves the chat reply comes from database run events, not from the run log. The reply keeps the full join link, and the presentation path never reads the run log. ## Verification - From `server`: `node ../node_modules/vitest/vitest.mjs run --config vitest.config.ts src/services/heartbeat/run-log.test.ts src/__tests__/invitation-token-redaction.test.ts src/__tests__/redact-sensitive.test.ts src/__tests__/heartbeat-run-log.test.ts src/__tests__/logger-invitation-redaction.test.ts src/services/native-runtime/native-run-log-watermark.test.ts src/services/native-runtime/native-run-log-recovery.test.ts src/__tests__/heartbeat-run-output.test.ts src/services/native-runtime/agent-chat-response-recovery.test.ts` passes. - Before the `run-log.ts` change, the two new redaction tests in `run-log.test.ts` failed (red), then passed (green). - `native-run-log-watermark.test.ts`: a legacy-compacted record with a token in its retained head cold-recovers. It failed with `native_log_source_coverage_unproven` before the restore fix. - `agent-chat-response-recovery.test.ts` (embedded Postgres): a final reply with a join link is saved as the exact comment with the token. No run-log read happens. - `tsc --noEmit -p server` shows no errors in the changed files. - A live chat eval (`access.invite-teammate`) run with this change confirms the run-log file has no token. The run is linked in a comment. ## Risks - Low risk. Run-log readers (run transcript view, feedback export, support inspection) now show `***REDACTED***` in place of invitation tokens. This removes a live credential from a diagnostic copy. It does not change the chat reply or the stored run events. - A path segment after `/invite/` in free text is always redacted, even if it is not a token. This loses only diagnostic detail. - A token that a provider streams across two separate chunks is not joined first. Each chunk is redacted on its own. A complete token in one chunk, such as a final message or a tool result, is removed. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), with tool use and code execution. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandPaperclip
|
b09f17fbfd |
fix(agents): repair queued setup through task connection cards (#20361)
## Thinking Path > - Paperclip manages AI agents doing work for people. > - Agent verification must finish before an accepted task can start. > - A missing AI account default can prevent verification before a provider starts. > - Lifecycle policy treated that owned configuration refusal as a temporary failure and retried each minute. > - The accepted turn remained Queued without explaining the prerequisite. > - Park account configuration failures and reuse the task's existing inline connection card. > - Successful repair rechecks verification and releases the original queued turn under its original authority. ## Linked Issues or Issue Description **What happened?** An OpenCode agent selected the responsible user's OpenRouter default, but that default did not exist. Verification retried unchanged configuration indefinitely and its accepted task stayed Queued. **Expected behavior:** Preserve accepted work, explain the setup prerequisite, and let the appropriate person repair it using the existing task connection UI. Completion must verify the agent before starting the same queued turn. Genuine pause, approval and access gates remain effective. **Reproduction:** Create an OpenCode Runner agent with a responsible-user OpenRouter binding and no personal default; assign a task while verification is pending. The base retries verification and shows an unexplained queue. Related: #18702 handles pending task connection recovery after account repair. This PR handles the earlier agent-verification boundary. #20330 addresses a separate Codex governed-wait timeout and is already merged. ## What Changed - Share owned account-selection failure evidence through a pure library contract. Lifecycle policy parks those configuration failures with fixed, redacted guidance; temporary infrastructure errors retain their retries. - Batch setup-state reads for unstarted queued turns. Distinguish manual repair from automatic setup retries and preserve Stop, started/history state and lifecycle holds. - Reuse the existing `connection_intent` card, account selection and completion APIs. Its controller-only `agentSetup` marker accepts only never-started legacy turns; runtime tokens still require a running run. - Address the card to the lifecycle verifier's account owner, preserving the task's execution principal and private-task visibility. Attach missing cards on admission, verification, startup and full recovery, including retained retries and continuations. Repeated delivery and publication remain bounded. A new queued turn supersedes the setup card of a stopped turn; generic connection cards retain cross-run reuse. - Recheck lifecycle verification after usable-account completion, then release the existing turn through the normal readiness path. Preserve decline, Stop, reassignment and changed setup-owner gates. Fence an in-flight verification failure from overwriting a repair; add no provider wake or recurring worker. - Park owned account refusals during resume as well as initial verification. Bind setup cards to the lifecycle operation, settle obsolete deliveries, and keep explicit Retry guidance accurate. - Reuse the router’s existing lifecycle pin for setup repair without rerouting the task, advancing the pool cursor, or replacing its saved binding. Legacy direct-account cards remain readable. - Hide the incorrect Resume banner during setup, retaining genuine manual/budget pause controls. Document the execution and retry contract. ## Verification - Red: eight original behavior assertions failed on the base, including the real selector's retry decision and the real batched queue projection. Six owned selection cases failed classification. Temporary-error presentation, different task/verifier users, retained retry/continuation recovery and the setup Resume banner also have failing-before/passing-after regressions. - Green: 430 affected backend, lifecycle, shared-contract and UI tests passed before the final review corrections. Subsequent focused runs cover one inline card, real account selection/completion, duplicate delivery/publication, verification before readiness, unchanged run authority, private tasks, no extra provider wake, decline/Stop/reassignment, company isolation, invalid runtime tokens, transient retries and redacted diagnostics. - Final setup/card and generic connection-service suites: 49 tests passed. The stopped-turn case failed before the correction, then passed with real account completion, the stopped source remaining cancelled, and the replacement source remaining queued. - After rebase, all 129 focused assertions passed. The local queue suite’s 88 execution assertions passed repeatedly, but its database-fixture teardown exceeded both 30-second and 120-second hooks. An isolated queue case completed, and the setup/connection suites fully passed. The original queue test and cleanup remain unchanged; broad CI supplies the full-suite result. - Server/UI typechecks, module boundaries, token gates and diff checks passed. `pnpm verify --explain` requested broad CI because the execution-contract document changed; no redundant full local suite was substituted. - Real source acceptance: macOS arm64, Node 26.4.0, isolated embedded PostgreSQL and local board identity. Created an OpenCode/OpenRouter agent and task through the API with no default. Admission retained one unstarted turn and produced the existing inline Connect OpenRouter card. The browser opened its credential form and closed it without entering credentials. The same card/run persisted after server restart and reload, with the setup label and no incorrect Resume. The browser also reproduced Stop followed by a new queued turn retaining the stale card on the prior head. On `12f609c98372fd1e50644028db75cfcc8deb210d`, startup superseded the stale card and retained the same replacement turn, with the original turn still cancelled and no provider execution. - The earlier CI case-search failure also reproduces on exact base `0e3fdebea8257bdff11573081c76b306f12db230` without this PR. Its repair is now in master via #20371; this branch is rebased onto it and contains no API-search changes. - Staging's actual Runtime test returned “Connect an account and choose your personal default” before provider startup. The original turn later started independently on deployed code, then hit a separate workspace-export failure. Automatic file recovery subsequently marked that run successful; its task remains Blocked and the trial’s useful output is unverified. This PR is not deployed; provider authentication through its card and downstream publication remain unverified. No account, credential, default, permission or binding was changed during diagnosis. - Previous revision review: Greptile 5/5 on `12f609c98372fd1e50644028db75cfcc8deb210d`, with all four inline findings addressed and resolved. [Full CI passed](https://github.com/paperclipai/paperclip/actions/runs/42713494508) on that revision: 59 successful checks, one neutral check and three intentional skips, with no pending or failed checks. - Latest revision `976bcae993805a8a2222395bbff9394f6791b424`: resume, router and Retry-copy regressions failed before repair. Green: 92 lifecycle/setup/shared tests, 83 setup/generic-connection/router tests, and 137 UI/shared tests. Server/UI typechecks, module boundaries and token gates passed. [Full CI passed](https://github.com/paperclipai/paperclip/actions/runs/42720731137) on this exact revision: 59 successful checks and three intentional skips, with no pending or failed checks. Greptile reviewed this head at 5/5; all six review findings are addressed and resolved. Real Stop/retry acceptance retained the same replacement turn and current-operation card through two actual server restarts, with no provider execution. ## Risks - Known account configuration failures now require repair or explicit Retry. Temporary failures keep automatic retries; accepted work and authority remain intact. - The projection adds one company-scoped batch read only for unstarted queued turns. Card attachment examines at most 100 active candidates and adds no timer, worker, schema or migration. - The optional stored setup-card marker requires an aware reader during rollback. No run/session/checkpoint identity format changes. Setup presentation grants no access and cannot bypass verification or other gates. ## Model Used - OpenAI GPT-6 (Codex), with reasoning, tool use, code editing and test execution. The exact serving model identifier and context-window size are not exposed in this session. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `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
|
7e93ccb4b3 |
perf: fetch recent task summaries in one request (#20389)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The board UI sidebar shows the person's recent tasks on every page, including the Mine inbox > - The sidebar refreshes those rows through `GET /companies/:id/issues/summaries`, which accepted at most five IDs > - `useRecentTasks` sent the chunks one after another, so a 25-task history cost five sequential requests > - On staging each request took 3–6 s, so the recent-task rows settled about 20 s after load and every request competed with the inbox rows > - This pull request raises the per-request cap to 50 and fetches any remaining chunks in parallel > - The benefit is one summaries request for a typical history instead of a serial chain ## Linked Issues or Issue Description **What existing behavior does this improve?** The refresh of the sidebar's recent tasks on page load. **Subsystem affected** Board UI (`ui/src/hooks/useRecentTasks.ts`) and the `/issues/summaries` route bound (`packages/shared`, `server/src/routes/issues.ts`). **Current behavior** A history of N recent tasks sends ceil(N/5) summaries requests in series. Measured on staging on 2026-11-09 for a cold Mine load: five requests of five IDs, started at 1.0 s, 4.3 s, 8.1 s, 12.3 s and 18.4 s, each taking 3.3–6.1 s. **Proposed behavior** Up to 50 recent tasks are one request. Longer histories send their chunks at the same time. **Reason and benefit** The staging server is saturated, so each extra serial round trip delays the sidebar and the inbox rows. One request removes four round trips from a typical load. ## What Changed - `ISSUE_SUMMARIES_MAX_IDS` is now 50 (was 5). The shared validator, the route's 400 message, the service guard and the OpenAPI description use the new bound. - `useRecentTasks` builds all chunks first and fetches them at most three at a time (`RECENT_TASK_SUMMARY_CONCURRENCY`). The recent list has no size cap, so this keeps a long history from becoming a burst. The cache key, omission handling and pruning are unchanged. A failed chunk still fails the batch, so nothing is pruned on a partial failure. - Tests: the route test accepts exactly `MAX` IDs and rejects `MAX + 1`; a hook test checks that at most three chunks are in flight; the API test uses the constant; `useRecentTasks` tests cover a 25-task history in one request and a 103-task history with three chunks in flight together, with and without a failed chunk. - New browser check `tests/perf/collections/recent-task-summaries.spec.ts`: a Mine load with a 12-task history sends summaries requests that each carry all 12 IDs, and the server answers 200 with all 12 tasks. ## Verification - `pnpm verify` (selected Vitest files) passes. - `server`: `npx vitest run src/__tests__/issue-summaries-routes.test.ts src/__tests__/issue-display-status.test.ts` passes (embedded PostgreSQL). - `ui`: `npx vitest run src/hooks/useRecentTasks.test.tsx src/api/issue-summaries.test.ts src/components/task-chat/TaskChatBlockerLinks.test.tsx src/components/CoordinatorSidebar.test.tsx src/components/SidebarRecentTasks.test.tsx` passes. - Browser check against a disposable local instance: `PAPERCLIP_COLLECTION_PERF_BASE_URL=http://127.0.0.1:<port> pnpm exec playwright test --config tests/perf/collections/playwright.config.ts recent-task-summaries.spec.ts`. It passes on this branch. With `master`'s hook and constant it fails: three requests of 5, 5 and 2 IDs. ## Risks - Low risk. The route still validates UUIDs, deduplicates, and applies the same read authorization per row. Blocker attention is computed with set-based queries, so 50 IDs is the same number of queries as 5. - During a rolling deploy, a new UI against an old server would get a 400 for a history above five tasks. The server serves the UI bundle, so both change together on a normal deploy. The sidebar keeps its stored snapshot when the refresh fails. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edits, browser). 1M-token context window. Extended thinking was off. ## 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
|
c985541a57 |
perf(db): index retry-exhaustion receipts for the attention scan (#20386)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The attention feed shows runs that failed after automatic retries stopped > - To find these runs, the feed reads every `heartbeat_run_events` row of the company and filters the rows by a message prefix > - Run output makes this table very large, so each feed request does a large scan > - On a hosted test database, this one statement used the most total database time: 923 calls at a mean of 5 s (4,709 s in total) > - Request authentication waits behind this background work, so all users feel the load > - This pull request adds a small partial index that holds only the retry-exhaustion receipts > - The benefit is that the same query reads a few index entries instead of all company events ## Linked Issues or Issue Description **What existing behavior does this improve?** The attention feed query that finds retry-exhausted runs (`listAttentionExhaustedRuns`). **Subsystem affected** Server attention feed and the `heartbeat_run_events` table schema. **Current behavior** The query reads all company rows in `heartbeat_run_events`, filters them by `event_type = 'lifecycle'` and the `Bounded retry exhausted` message prefix, and then joins the few matches to failed or timed-out runs. The cost grows with all run output. It does not grow with the number of exhausted runs. **Proposed behavior** A partial index holds only the matching receipts. The query plan reads the index and joins a few rows. The query text and the result do not change. **Reason and benefit** The scan was the most expensive statement by total time on a loaded hosted database. A local reproduction shows a 54x faster query and about 116x fewer buffer reads. **Breaking changes** None. The API, the feed rows, and their order do not change. ## What Changed - Add the partial index `heartbeat_run_events_retry_exhausted_idx` on `(company_id, run_id, id DESC)`. The index covers only rows where `event_type = 'lifecycle'` and `message like 'Bounded retry exhausted%'`. - Add migration `0454`. It uses `CREATE INDEX CONCURRENTLY IF NOT EXISTS`. The migration runner runs it as a top-level statement, the same as migrations 0432, 0436 and 0450. - Add a regression test. It pins the row order (newest run first, then the newest receipt on a tie) and the latest receipt per run. It also checks that the query plan can use the new index. If the query predicate changes so that the index no longer applies, the test fails. - Add `scripts/benchmark-exhausted-run-attention.ts`. It measures the same query with and without the index, and the time to build the index. - Document the index in `doc/DATABASE.md`. ## Verification Cause and thesis. The scan cost comes from reading all company events, not from the few exhausted runs. Thesis: an index that holds only the receipts makes the query cost depend on the number of receipts, and keeps the result identical. The benchmark below confirms the thesis. Rejected alternative without a migration. I first tried a LATERAL lookup that starts from the failed and timed-out runs. On the same fixture it was about 2x slower (median 379 ms against 194 ms). It reads the full event history of each failed run with random I/O, and that costs more than one sequential scan. Local SQL benchmark (the SQL that the service generates; same SQL with and without the index): ```sh pnpm --filter @paperclipai/server exec tsx ../scripts/benchmark-exhausted-run-attention.ts --runs=30000 --events-per-run=100 --samples=9 ``` Fixture (synthetic): one company with 30,000 runs (3,819 failed or timed out) and 3.0 M events, plus a decoy company with 0.5 M events. The table is 1.4 GB. About 5% of events are `lifecycle` events. Events of 20 concurrent runs are interleaved. 1 in 23 runs has a receipt, some runs have two receipts, and output decoys contain the prefix. Two agents are terminated. Embedded PostgreSQL 18.1 on an Apple laptop. The hosted database is Neon with different compute, so the absolute times are local SQL measurements only. The script runs two cycles (without index, then with index; then with index, then without index) to expose cache bias. Each measurement has 2 warmups and 9 samples. | Query | Without index | With index | Samples and spread | Output check | | --- | --- | --- | --- | --- | | `listAttentionExhaustedRuns` | median 157 ms (96–1,271 ms) | median 3.3 ms (2.2–5.7 ms) | 18 each | 133 rows, positional rows equal in all 4 measurements | | Buffers (EXPLAIN ANALYZE) | 127,743 blocks | 5,580 blocks | | plan uses the new index (asserted) | Speedup: 48x (median), on head `ae6037b2a0` (the same SQL and index as the final head). The machine was under heavy load from other work, so the spread without the index is wide. An earlier run on the first head gave 248 ms against 4.6 ms (54x). Index build (`CREATE INDEX CONCURRENTLY` from the migration file): 1.7 s and 2.5 s for the 1.4 GB table. Index size: 152 KiB. Review fix: the query now writes `event_type = 'lifecycle'` as a SQL literal, not a bound parameter. With prepared statements, PostgreSQL can reuse a generic plan. A generic plan cannot prove that a bound parameter matches the index predicate, so it would not use the index. Tests: - `cd server && npx vitest run src/__tests__/attention-service.test.ts`: 44 passed. The new test checks a fresh plan and a forced generic plan (`plan_cache_mode = force_generic_plan` with `PREPARE`). It failed with the bound `event_type` parameter and with a changed message prefix (confirmed red), and it passes with the fix. Locally, the file's `afterAll` database cleanup hook timed out on a heavily loaded machine (load average about 28). The timeout also occurs when the new test is excluded. - `pnpm --filter @paperclipai/db check:migrations`: passed. - `pnpm --filter @paperclipai/db typecheck` and `pnpm --filter @paperclipai/server typecheck`: passed. - `pnpm verify`: see the CI results for this head. ## Risks - Migration time. A concurrent index build reads the full `heartbeat_run_events` table and does not block writes. Locally, the build took 1.2–2.5 s for 1.4 GB. The time grows linearly with the table size. I did not time the build on a production-size clone. On a database with a 5 s full company scan, I estimate the build at tens of seconds, which is in the 1–2 minute budget. This estimate is not proven. - If the build stops, PostgreSQL can leave an invalid index. The migration runner repairs invalid concurrent indexes when it runs again, and `IF NOT EXISTS` makes a rerun safe. - The index predicate must stay textually equal to the query predicate. If someone changes the message prefix or binds a predicate value as a parameter, the planner cannot use the index. The new test catches both cases, for fresh and generic plans. - Each new receipt writes one more small index entry. Receipts are rare, so the write cost is very small. - The hosted improvement is not verified until deployment. After deployment, compare a new query-statistics window for this statement. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude Code with tool use (shell, file edits, local PostgreSQL). Standard context window and reasoning mode. ## 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
|
c57cd1bdb9 |
perf(server): read plugin lifecycle inbox heads in one pass (#20405)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Plugins get agent, project and agent runtime changes from the `resource_lifecycle_events` journal. Each plugin acknowledges events one resource at a time. > - The inbox page query checked each unacknowledged event against all older events of the same resource. Each check walked the acknowledged history of that resource. > - Agent runtime streams get a new event for each configuration change, so one resource can have thousands of acknowledged events. The cost grew as pending events times acknowledged history. > - On a hosted instance, three copies of this read ran for 601 seconds each, and one earlier copy ran for 19 minutes. The delivery worker runs it again after each acknowledgment. > - This pull request selects the head of each resource in one pass and keeps the same result. > - The benefit is that the page read is linear in the company journal size. In local tests it is 250x to 660x faster, and the result rows are equal. ## Linked Issues or Issue Description **What happened?** The plugin lifecycle inbox page query (`select "id", "company_id", "resource_type", "resource_id", "action", "created_at" from "resource_lifecycle_events" where ...`) ran for 601 seconds on a hosted database. Three copies were active at the same time. **Expected behavior** A lifecycle page read must stay fast when a resource has a long acknowledged history and a pending backlog. **Steps to reproduce** 1. Give a company 40 agents with 1,500 `agent_runtime` events each. 2. Acknowledge the first 1,200 events of each agent for one plugin. 3. Call `pluginLifecycleInbox(db, pluginId).list(companyId, 100, undefined, ["agent", "project", "agent_runtime"])`. 4. The read takes about 10 seconds for 60,000 events of that company. At 200,000 events it takes about 6 minutes. **Paperclip version or commit** `02bfe71a6b` (master). **Deployment mode** Hosted (PostgreSQL). Local repro uses embedded PostgreSQL 18. ## What Changed - `server/src/services/plugin-lifecycle-inbox.ts`: `list` now selects one head per resource with `DISTINCT ON (resource_type, resource_id)`. The head is the unacknowledged creation, else the oldest unacknowledged event. Then the query applies `afterId`, id order and the limit. This is the same set that the old `firstForResource` predicate admits. `acknowledge` keeps the old predicate for its single-event check. - `plugin-lifecycle-inbox.ts`: new `hasPending(companyId, types)`. It returns whether any unacknowledged event exists. Each resource with an unacknowledged event has a head, so this gives the same answer as `list(companyId, 1).length > 0`. - `server/src/services/plugin-lifecycle-delivery.ts`: the idle-sleep journal check uses `hasPending`. This keeps its early exit. A one-row `list` page would read the full journal. - `server/src/__tests__/resource-lifecycle-events.test.ts`: new database test for heads across acknowledged history, a creation recorded after transitions, acknowledged gaps, shared ids across resource types, per-plugin progress, another company, `afterId` pages, limits, `hasPending`, and the next head after an acknowledgment. The test also passes on the old query, so it locks the contract. - `scripts/benchmark-plugin-lifecycle-inbox.ts`: new benchmark. It loads the baseline service from git, runs both on the same fixture, alternates order, asserts equal rows, and prints plans. - `doc/DATABASE.md`: describes the read cost and the benchmark command. - No schema change, index or migration. ## Verification Thesis: the old read cost is (unacknowledged candidates) x (acknowledged older events per resource), because each candidate walks the acknowledged prefix of its resource. One pass that keeps the per-resource head removes that product and returns equal rows. Baseline plan at 220,080 events: 40,020 candidates each walked about 3,999 older rows, which gave 160,039,980 acknowledgment probes and 562,140,833 buffer hits. The treatment plan reads the journal once with a hash anti join and sorts the 40,020 unacknowledged rows (3,925 buffer hits). Local SQL timings (embedded PostgreSQL 18, Apple Silicon; event counts include a 20,000-event second company, synthetic data, warm cache, one warmup, order alternates): | Fixture (events / acknowledged) | Case | Before median | After median | Samples (after) | Speedup | | --- | --- | --- | --- | --- | --- | | 40,060 / 16,040 | page of 100, all types | 3,263 ms | 13.1 ms | 3 (12.0-14.1) | 250x | | 80,080 / 48,060 | page of 100, all types | 10,137 ms | 23.9 ms | 5 (22.7-32.6) | 424x | | 220,080 / 160,060 | page of 100, all types | 355,718 ms | 538 ms | 3 (72-785) | 662x | | 1,020,080 / 960,060 | page of 100, all types | not run (est. > 30 min) | 1,682 ms | 7 (1,203-2,384) | - | | 80,080 / 48,060 | page of 100, agent and project only | 1.5 ms | 1.6 ms | 5 | neutral | | 80,080 / 48,060 | idle-sleep check | 14.5 ms | 13.8 ms | 5 | neutral | | 1,020,080 / 960,060 | idle-sleep check | not run | 406 ms | 7 | - | - Result rows are equal (`assert.deepEqual` on all mapped rows, in order) for each case where both versions ran. - The agent runtime revision read in `plugin-agent-runtime-state.ts` and `agent-file-store.ts` (`select id ... order by id desc limit 1`) uses a backward index-only scan and reads 5 buffers at 1M events. It is not changed. Commands: ```sh pnpm --filter @paperclipai/server exec tsx ../scripts/benchmark-plugin-lifecycle-inbox.ts --baseline-ref=02bfe71a6b --agents=40 --per-agent=1500 --acked=1200 --samples=5 pnpm --filter @paperclipai/server exec tsx ../scripts/benchmark-plugin-lifecycle-inbox.ts --baseline-ref=02bfe71a6b --agents=40 --per-agent=25000 --acked=24000 --samples=7 --treatment-only cd server && npx vitest run src/__tests__/resource-lifecycle-events.test.ts src/__tests__/plugin-lifecycle-delivery.test.ts src/__tests__/agent-runtime-hooks.test.ts ``` - `resource-lifecycle-events.test.ts`: 28 passed. `plugin-lifecycle-delivery.test.ts`: 40 passed (includes idle-sleep "none" and "present"). `agent-runtime-hooks.test.ts`: 22 passed. - `tsc --noEmit -p server`: pass. ## Risks - The page read is still linear in the company journal size, because acknowledgments are per plugin and the journal has no retention yet. At 1M events a page takes about 1.7 s locally. Each acknowledgment wakes another full pass. A per-plugin cursor or retention can remove this later. - The planner can choose a different plan on the hosted database. The local plans use a hash anti join and one sort. The hosted improvement is not yet measured. - `acknowledge` still checks one event against older events of its resource. That check is linear in that resource's history per call, as before. - Low behavior risk: the new test passes on both the old and new queries. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude Code with tool use (shell, file edits, local PostgreSQL). Extended reasoning on. ## 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
|
17014cd7db |
perf(server): make run liveness backfill lookup issue-scoped (#20393)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Each finished agent run gets a liveness classification. The task
page shows it, and recovery reads it.
> - Run finalization writes the classification. A backfill repairs runs
that do not have it, for example older runs or runs whose finalization
failed.
> - The task page runs read (`GET /issues/:id/runs`) starts this
backfill on each read, for each task.
> - The candidate lookup reads every unclassified finished run in the
company. For each row, it reads the run context and probes
`activity_log`. Rows of other tasks never match, so each read repeats
the same work.
> - On a hosted test instance, this lookup used approximately 1 s per
call, 2,636 calls, and 3,500 s in total.
> - This pull request makes the lookup read only the rows of the task,
and skips a task after a pass finds nothing left to repair.
> - The benefit is less database work per task page read. The
classification still gets written.
## Linked Issues or Issue Description
**What existing behavior does this improve?**
The run liveness backfill that the task runs read starts
(`scheduleRunLivenessBackfill` in `server/src/services/activity.ts`).
**Subsystem affected**
Server: activity service, task runs read, run liveness backfill.
**Current behavior**
Each task runs read runs this lookup:
```sql
select ... from heartbeat_runs
where company_id = $1 and liveness_state is null and status not in ('queued', 'running')
and (context_snapshot ->> 'issueId' = $2
or exists (select 1 from activity_log where company_id = $3 and entity_type = 'issue'
and entity_id = $4 and run_id = heartbeat_runs.id))
limit 20
```
PostgreSQL reads all unclassified finished runs of the company through
`heartbeat_runs_company_liveness_idx`. It then reads the context of each
row, which can be large, and probes `activity_log` for each row.
Unclassified runs of other tasks, and runs without a task, stay
unclassified, because no read of their own task repairs them. Thus the
cost grows with the company, not with the task. The lookup also runs
again on each read when the task has nothing to repair.
**Proposed behavior**
- The lookup reads only the runs of this task. One branch uses
`heartbeat_runs_company_ctx_issue_created_idx` (context `issueId`). The
other branch uses `activity_log_company_entity_created_idx` (issue
activity rows with a run id). The result keeps the same filter: same
company, liveness missing, status not `queued` or `running`, at most 20
rows.
- When a pass finds fewer than 20 rows, it has repaired all rows of the
task. The service then skips the task for 10 minutes. A full batch of 20
does not start the skip window, so the next read repairs the next batch.
**Reason and benefit**
The lookup is now proportional to the history of one task. Most reads
find nothing, and those reads now do no database work for 10 minutes.
**Breaking changes**
None. No migration, no API change, no new background work.
**Additional context**
The chat bootstrap read already skips the backfill
(`skipLivenessBackfill`). That option still works, and a test covers it.
## What Changed
- `server/src/services/run-liveness-backfill-query.ts`: new
`missingRunLivenessCandidatesQuery`. It replaces the `OR` and the
correlated `exists` with `id in (context branch union all activity
branch)`. Both branches are filtered by company and task.
- `server/src/services/activity.ts`: the backfill uses the new query and
returns the number of rows it found. `scheduleRunLivenessBackfill` keeps
a bounded in-memory map (at most 10,000 tasks, oldest entry is removed
first) of tasks with nothing left to repair. It skips these tasks for 10
minutes.
- `server/src/__tests__/activity-service.test.ts`: four new
real-database tests.
- The backfill repairs runs found by context and runs found only by
issue activity. It does not change a running run, an unclassified run of
another task, or a run of another company that names the same task id.
- After a pass leaves nothing to repair, more reads do not repair a new
unclassified run. After the skip window ends, the next read repairs it.
- A task with 25 unclassified runs (more than one batch) gets all of
them repaired over later reads.
- A read with `skipLivenessBackfill` does not repair. The next normal
read does.
- `scripts/benchmark-run-liveness-backfill.ts`: a local SQL benchmark
that compares the pre-fix query with the new query.
## Verification
Local SQL benchmark (embedded PostgreSQL 18.1, macOS, synthetic data):
```sh
cd server
pnpm exec tsx ../scripts/benchmark-run-liveness-backfill.ts --baseline-ref=02bfe71a6bbd7c5676cb7e72778b361c5bc72765
```
- Baseline: `02bfe71a6b` (pre-fix master). Treatment: `80fcfcf7f1`,
clean tree.
- Fixture assumptions (staging row counts were not available): one
company with 30,000 classified runs, 6,000 unclassified finished runs of
other tasks or of no task, each with an approximately 20 KiB hex
context, 400 other tasks, and 300,000 `activity_log` rows (half with a
run id). The target task has 40 classified runs, 200 activity rows, and
the missing rows of each case. Decoys: one `running` run of the task,
and one run in another company that names the task id. A small control
uses 500 classified and 50 unclassified runs.
- Each arm: 2 warm-up runs and 11 measured runs, in alternating order.
| Case | Candidates | Baseline median (min to max) | New median (min to
max) | Speedup | Shared buffers (baseline to new) |
| --- | --- | --- | --- | --- | --- |
| Task with nothing to repair | 0 | 37.2 ms (31.1 to 44.1) | 1.21 ms
(0.42 to 1.79) | 30.8x | 34,247 to 138 |
| Task with 3 missing | 3 | 37.8 ms (27.2 to 39.6) | 1.16 ms (0.77 to
2.05) | 32.6x | 34,265 to 148 |
| Task with 25 missing | 25 | 37.5 ms (34.6 to 69.8) | 1.63 ms (1.06 to
3.47) | 23.0x | 34,361 to 154 |
| Small control | 3 | 0.79 ms (0.70 to 1.06) | 0.70 ms (0.48 to 1.70) |
1.1x (neutral) | 314 to 18 |
The `EXPLAIN ANALYZE` execution time for the large cases goes from
approximately 35 ms to approximately 0.15 ms. These are local SQL
measurements. The hosted latency improvement is not yet measured.
The benchmark also checks result equality. For each case, it runs both
queries without a limit and compares every column of every row (as
sorted positional rows). It also checks that the limited batch of 20
comes from the baseline candidate set. All cases match.
Beyond the query, the skip window removes the lookup for most reads of a
task with nothing to repair.
Tests:
```sh
cd server
pnpm exec vitest run src/__tests__/activity-service.test.ts # 24 passed
pnpm exec vitest run src/__tests__/activity-routes.test.ts src/__tests__/task-cold-open.test.ts src/__tests__/liveness.test.ts
pnpm exec tsc --noEmit -p .
```
- The skip-window test fails on the pre-fix `activity.ts` (red), and
passes with this change (green).
- `pnpm verify` asks for broader coverage, because the change adds a
script. Required CI supplies that coverage.
## Risks
- A run that misses its classification after a pass finds nothing for
its task gets repaired up to 10 minutes later. Normal runs get their
classification at finalization, so only the repair path is late.
- The skip map is in memory for each server process. A restart clears
it. That only causes one more lookup for each task.
- The new query keeps the same rows, filter, and limit. The query has no
order, as before. Thus the 20 rows that a pass picks can differ when a
task has more than 20 candidates. Later reads repair the rest in both
versions.
- Low risk. No migration and no API change.
## Model Used
- Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, with tool use and code
execution in Claude Code.
## Checklist
- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [ ] All Paperclip CI gates are green
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
|
||
|
Nicky LeachandClaude Opus 5.5
|
d95fdd8d79 |
feat(observability): span the unnamed gaps in run preparation (#20396)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Before a task reaches the agent, the server prepares the run: it reads the task, the workspace and the configuration, and then hands the run to the native controller > - With an OTLP endpoint, each preparation step emits a `heartbeat.prepare.<phase>` or `native.controller.<phase>` span > - On a hosted test deployment, 0.3 to 0.5 s of each `task.prepare` sits between those spans, and about 0.5 s more sits between `execution.dispatch_admission` and `native.coordinator.claim` > - Nobody can say which step is slow in that time, so latency work in this area is guesswork > - This pull request adds spans around the existing awaited work in those gaps, with the existing timing helpers and naming conventions > - The benefit is that a trace of a slow preparation now names the step that used the time ## Linked Issues or Issue Description **What existing behavior does this improve?** The OpenTelemetry trace of task preparation. Today it has unnamed gaps between the named preparation spans. **Subsystem affected** Server: heartbeat run preparation and the native controller (`server/src/services/heartbeat.ts`, `server/src/services/native-runtime/native-session-executor.ts`). **Current behavior** On a hosted test deployment (8 runs per group), `task.prepare` takes 2.6 to 3.4 s. Between `heartbeat.prepare.runtime_state` and `heartbeat.prepare.run_identity`, about 0.25 s has no span. Between `execution.dispatch_admission` and `native.coordinator.claim`, about 0.5 s has no span, and in one pair of runs it was 1.4 s. Nobody can tell which step used that time. **Proposed behavior** Spans name the existing awaited reads and writes in those gaps. The change adds no reads and records no content. The operator endpoint gate and the no-operation behaviour stay. **Reason and benefit** Latency work in this area needs a measurement. With these spans, a trace of a slow preparation names the step that used the time, so a fix can be confirmed or rejected after deployment. **Breaking changes** None. The change adds span names to the closed list. It changes no API, schema or behaviour. **Additional context** The server uses postgres.js, which the auto-instrumentation does not cover, so database spans do not fill these gaps. ## What Changed - `server/src/services/execution-preparation-timing.ts`: add the new closed phase names. - `server/src/services/heartbeat.ts`: add `heartbeat.prepare.*` spans for `chat_retry_admission`, `workspace_selection`, `dependency_readiness`, `issue_checkout`, `instance_settings`, `workspace_context`, `project_context`, `plan_wake_routing`, `routine_environment`, `responsible_user` and `execution_start`. The re-reads of the issue after a checkout use the existing `issue_context` name. - `server/src/services/native-runtime/native-session-executor.ts`: add `native.controller.*` spans for `attachment_staging`, `warm_restore`, `span_persistence` (persistence of the preparation spans to the run log) and `state_migration` before the coordinator claim. - `doc/observability.md`: list the new spans. - `server/src/__tests__/heartbeat-preparation-gap-spans.test.ts`: run a real heartbeat through native preparation with an in-memory span exporter. Check that each new span is emitted and that the new preparation spans are children of `task.execution_attempt`. ## Verification - `pnpm --filter @paperclipai/server typecheck` - `npx vitest run src/__tests__/heartbeat-preparation-gap-spans.test.ts` (fails without the source change, passes with it) - `npx vitest run src/services/heartbeat/preparation-timing-context.test.ts src/services/execution-preparation-timing.test.ts src/services/native-runtime/native-session-executor.test.ts src/__tests__/heartbeat-context-compaction-execution.test.ts` ## Risks - Low risk. The change only wraps existing awaited calls. Values and errors pass through the timing helper unchanged, and a checkout conflict still reaches the same `catch`. - Phases of 50 ms or more inside `heartbeat.prepare.*` are also written to the run log, as for the existing phases. This adds a few run-log rows on slow runs. - Without an OTLP endpoint, the helpers keep their current no-operation behaviour. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude Code with tool use (shell, file edits, test runs) and extended thinking. ## 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
|
09241442ea |
fix(server): let a native reviewer approve its review stage (#20373)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Execution policies can require an independent review stage before a task is done > - A native (Runner) agent that is the active reviewer had no way to record its decision > - `PATCH status: "done"` is refused for a native run that owns the task (#18411), and `paperclip_finish` accepts the report but leaves the stage pending > - The reviewer then searches the API until a deadline, and the task stays in review with nobody able to decide it > - This pull request records the final-stage approval on `PATCH` without releasing the run's lock, and lets the run's own finish report set the final status > - The benefit is that an agent reviewer can actually approve its review stage, in one short turn ## Linked Issues or Issue Description Refs #18411 (added the native status guard). Related: #20264 (explicit stage-decision operation, still open; this change is narrower and compatible with it). **What happened?** A native reviewer agent was woken as the active `review` participant. It read the deliverable and wrote its review in about 100 seconds. It then called `paperclip_finish` with `done`. The tool answered "Completion report accepted; the task's configured review stage is still pending". It then tried `PATCH /api/issues/{id}` with `status: "done"` and a comment. The route refused it with `native_run_status_reported_by_finish`. The agent searched the API for 2 more minutes and found no other path. The stage stayed pending. When the run ends with `done`, the status arbiter sees the pending stage and keeps the task in review (`governed_gate_pending`), so the stage can never resolve. **Expected behavior** The active review/approval participant can approve its stage from a native run. The task reaches `done` after the approval. The implementer still cannot approve. A run that is not the active participant still cannot change the task status. **Steps to reproduce** 1. Create a task with an execution policy that has one `review` stage with a native agent participant. 2. Move the task to `in_review` so the stage is pending and the reviewer agent wakes. 3. In the reviewer run, call `PATCH /api/issues/{id}` with `{ "status": "done", "comment": "Approved: ..." }`. 4. Observe `409 native_run_status_reported_by_finish`. Call `paperclip_finish` with `done` and observe that the stage stays pending. ## What Changed - `PATCH /api/issues/:id`: when the requester is the native run that holds the task lock and is the active participant, a `status: "done"` request on the final stage now records the approval decision (`executionState` completed, decision row, activity). The route removes `status` from the write, so the task stays `in_review` and the run keeps its execution lock. The run's `paperclip_finish` report then sets `done` through the normal status arbiter. - An intermediate-stage approval (which would hand the task to the next participant) and every other status change from such a run keep the existing `native_run_status_reported_by_finish` refusal. - `paperclip_finish` from the active participant (`done` or `needs_review`) while its stage is still pending is now rejected with the exact decision path, instead of being accepted with "stage still pending". - The native finalizer now treats the final-stage approval that the run itself recorded (read from the `issue_execution_decisions` row, bound to this run and agent) as the completion authority for its clean `done` report. Before this, a reviewer that cited free-text evidence had its report graded as incomplete, and the task was reopened as `in_progress` with a new wake for the reviewer. Failed verification, blocking remaining work, attention requests, other dispositions, and a still-pending stage keep the old outcome. - The reviewer/approver wake text names the decision path (`PATCH` with `status` and a required decision comment) and says a completion report does not record the decision. A Runner reviewer that needs changes is told to report them with `paperclip_block`, because a changes request from the run that owns the task is a reassignment that stops that run. - The locked review-policy recheck treats the deferred approval as a verdict, so a policy changed to `human_only` before the locked write still refuses it. ## Verification - `pnpm exec vitest run server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts server/src/__tests__/issue-execution-policy-routes.test.ts server/src/__tests__/issue-comment-reopen-routes.test.ts server/src/__tests__/issue-execution-policy.test.ts packages/adapter-utils/src/server-utils.test.ts`: 5 files, 595 tests pass. - `pnpm exec vitest run server/src/services/native-runtime/native-completion-feedback.test.ts`: 29 tests pass (embedded PostgreSQL), including the decision-row binding for `executionStageApprovedByRun`. - `pnpm exec vitest run server/src/services/native-runtime/status-arbiter.test.ts server/src/services/native-runtime/native-run-finalizer-telemetry.test.ts server/src/services/native-runtime/native-stale-owner-completion.test.ts`: 72 tests pass. - New route tests fail without the route change (409 instead of 200/422) and pass with it. The new feedback test fails without the feedback change ("Completion report accepted" instead of a rejection). - `npx tsc --noEmit -p server` and `-p packages/adapter-utils`: pass. - A route test changes the review policy to `human_only` between the first check and the locked write; the deferred approval is refused (403). - Live full-stack chat eval `task-coordination.independent-review` (Runner, OpenCode) with this change: passed in GitHub Actions run 42705964209 (eval branch with the first two commits cherry-picked). The reviewer run recorded its approval 24 s after its review brief was released, reported `done` 14 s later, and the finalizer set the task to `done` with reason `execution_stage_approved`; total reviewer run 73 s. The earlier run 42702224713 exposed the finalizer evidence issue that the second commit fixes. ## Risks - Low to medium. The change is limited to one request shape: a native run, holding the lock, that is the active participant of a pending final stage, sending `status: "done"`. All other requests keep the old guard. - Between the approval and the finish report, the task shows `in_review` with a completed `executionState`. If the run fails before it reports, the existing native recovery owns the task, as for any other run. - Previous-release story: no persisted shape changes. A reviewer run that started before the deploy could not have recorded a deferred approval (the old route refused it), so the new finalizer input is false for it and the old outcome applies. - Intermediate-stage approvals and changes requests from native runs are still refused by the route. A Runner reviewer reports required changes with `paperclip_block`, which keeps the stage pending for a person. A self-service changes request needs the deferred handoff semantics in #20264. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), tool use and code execution in Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandPaperclip
|
3cc1032f6a |
fix: deliver saved answers to dependency-blocked tasks (#20363)
## Thinking Path > - Paperclip lets people direct agent work through tasks and structured answers. > - Dependencies stop task execution until prerequisite work finishes. > - A person must still be able to answer or redirect the assignee of a blocked task. > - Ordinary comments already get a bounded answer/triage turn. A saved question response has no comment ID, so the same gates reject it. > - Later dispatch also requires an active task and tries to check it out. That conflicts with bounded interaction on a blocked task. > - This pull request verifies the stored human response at each gate and keeps the turn in bounded interaction mode. > - The assignee can receive the answer without starting dependency work or removing its blockers. ## Linked Issues or Issue Description Refs #19923. That change covers dependency waits and comment retries. This change covers stored interaction responses that have no comment ID. **What happened?** A parent task waits on a parked child. The assignee asked a setup question. After a person answers, the response stays queued. Sending that saved response is refused as `issue_dependencies_blocked`. Ordinary new messages can reach the same assignee. Passing admission alone is insufficient. The dispatch status check rejects a blocked task, and automatic checkout then tries to start work whose prerequisites are unfinished. **Expected behavior** Deliver the stored human answer or confirmation to its requesting assignee as a bounded answer/triage turn. Keep the task blocked and keep its dependency relationships. Preserve current-owner, pause, approval, budget, recovery, and execution gates. **Steps to reproduce** 1. Assign a parent task and make a backlog child block it. 2. Save a question on the parent and answer it as a person. 3. Dispatch its resolved-interaction wake, or send the saved answer from the task queue. 4. Observe that the answer does not reach the provider while an ordinary comment can run in answer/triage mode. **Paperclip version or commit** Reproduced on master at `653ab8b18f76`. This branch is rebased onto master at `0e3fdebea8`. ## What Changed - Verify the stored response by company, task, requesting agent, status, kind, human resolver, and resolution time. A context flag alone is insufficient. - Allow a verified response through dependency admission and claim in the existing bounded interaction mode. - Recheck that response during scheduled retry promotion and final provider dispatch. - Keep bounded responses from checking out the blocked task or setting it to `in_progress`, including when the last blocker finishes during preparation. - Preserve continuation classification through workspace and AI-account resource waits, so the final response and owner checks still run. - Correct the task-case API summary so the existing `case for task` discovery regression passes on the first search page. This fixes a pre-existing failure exposed by full CI, without changing search ranking or assertions. - Ignore obsolete Retry notices after newer conversation work, without acknowledging saved input or changing execution authority. - Document the response contract. Add real PostgreSQL and assembled-heartbeat regressions. ## Verification - Final commit `ba4b337cf1bb0236b7c0a742665b222ee894ff51`: [required CI passed, attempt 2](https://github.com/paperclipai/paperclip/actions/runs/42706363120/attempts/2), including typecheck, production build, server/chat tests, Runner checks, and all 8 browser shards. PR status: 59 passing checks and 3 normal skips. - Red: answered questions and accepted/rejected confirmations return no admitted run before the fix. All three new positive cases fail at the dependency gate. A resource-wait retry also reproduces as refused before its gate is fixed. - Green: at delivery revision `9375a603e4`, focused heartbeat integration and classification passed 14 tests; retry scheduling and dispatch safety passed 241 tests. These use real PostgreSQL and controlled adapter and resource-availability boundaries. The wider initial dependency, native-continuation, and queue suites also passed before the final refinements. - Negative cases keep pending, missing, wrong-task, and agent-resolved responses blocked. Changing the response proof immediately before dispatch cancels the run without calling the provider. Policy controls retain the current assignee, task status, and pending approval checks. - Positive cases execute once, leave the parent blocked, preserve the dependency edge, and leave the child in backlog with no child execution. The same saved response survives a resource wait; removing its human proof refuses promotion. - The resource-wait provider-boundary and blocker-completion regressions also failed before their fixes. All 14 focused integration and classifier cases passed at delivery revision `9375a603e4`, including a changed response during retry preparation. - Red: a retained Retry for an older run makes a newer successful conversation display as failed. Green: all 78 database-backed query and interruption-presentation tests pass, including a later real failure and independent queued input controls. - Server layered typecheck and module-boundary checks passed at `11992aad0c7fbaf13e7548d71ee633560bd8bb6f`, before the one-line API summary correction. That CI passed build, typecheck and every browser shard, but exposed the pre-existing case-search assertion and a report-assets test timeout. Final-head CI passed as linked above. - Fresh Greptile review is 5/5 on final commit `ba4b337cf1bb0236b7c0a742665b222ee894ff51`, with no unresolved threads. - The existing API discovery suite reproduced its first-page assertion before the summary correction; all 3 cases pass afterward. The timed-out report-assets privacy test passed locally in isolation and the full support job passed on the final-head rerun. Runner shutdowns interrupted the first attempt; affected jobs were rerun once without code changes. A separate onboarding timeout also passed on that rerun. - `pnpm verify --explain` requests broad CI because the execution contract is a global input. The final CI run supplied that coverage. - Separately recovered the reported staging conversation through an ordinary message and the existing steering control. The agent acknowledged the message and the saved-answer queue drained. The fresh Notebook action subsequently succeeded. This was on the previously deployed revision; it does not claim this code is deployed. ## Risks - This changes admission and dispatch. Each gate checks the persisted response; no caller-supplied marker grants the exception. - The bounded turn does not check out the task, resolve a dependency, or approve another action. Existing pause, ownership, budget, approval, and stopped-execution controls still apply. - Each additional lookup is one primary-key read and runs only for a response on a dependency-blocked path. There is no new polling worker. - No migration is required. Older queued contexts get the exception only when their stored human response still verifies. No prior run or action is replayed by this change. - This PR does not roll out the code to staging. ## Model Used OpenAI Codex, GPT-6 family, with repository tools, code execution, and browser inspection. The runtime does not expose the exact model ID, context-window limit, or reasoning-mode label; those values are not asserted. ## 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
|
0e3fdebea8 |
perf: remove repeated reads and checks from run preparation (#20358)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Before each run starts, Core prepares the run: it opens the run log, resolves skills, and prepares the AI runtime > - Every run pays this preparation time before the agent can begin, for a new task or a follow-up, on a cold or a warm computer > - A trace on a hosted test deployment showed some preparation steps that repeat a read or a check that a later step does again > - This pull request removes three of these repeated steps and keeps the check that each one duplicated > - The benefit is that each run starts sooner, with no new mechanism and no change to what a run can see or write ## Linked Issues or Issue Description **What existing behavior does this improve?** Run preparation (`task.prepare`). On a hosted test deployment it takes about 2.8 to 3.6 s. Skills preparation has a P50 of 275 ms (1,161 ms on a deployment with more skills and projects). `heartbeat.prepare.run_output` takes 386 to 724 ms. **Subsystem affected** Run logs in object storage (`object-byte-journal`, `object-run-log-store`) and runtime skill delivery (`company-skills`, `skill-execution-content`). **Current behavior** - An object journal claim is preceded by a head read. Its only use is to give the current owner to `claim()`. `claim()` then reads the head again, and it rejects the claim only when the two reads differ. The run log, provider trace capture, and run evidence migration all do this. - Run log `begin` reads the source-prefix proof object for every claim. For a new run log, no proof can exist, so the read always returns "missing". - Each delivered skill version calls `withVersion`, which authorizes the skill again for the run's actor. The skill row was read with the same actor's visibility condition in the same call. With an agent actor this repeats the company source and project permission reads once per version. **Proposed behavior** - `claim()` reads the head once. It still refuses a deleted journal. The conditional head write still rejects any change after that read, and a stale writer is still fenced. - `begin` does not read a proof for a first claim (generation 1). A resumed log still reads and validates its proof, so a changed or malformed proof still fails closed. - Runtime skill delivery authorizes each skill once, when it reads the selected rows. Other callers of `withVersion` keep their own check. **Reason and benefit** Each removed step protects nothing that a later step does not protect again: - Head read: the claim builds on its own head read, and the conditional write enforces it. - Proof read: a proof is written only by a writer that holds this log's head. The journal never removes a head (deletion leaves a tombstone that `claim()` refuses). So a first claim cannot find a proof. - Skill check: every caller of `materializeVersionSnapshot` passes a row that it read through the same actor's visibility condition (`readFullSkillRows`, `getById`, `listFull`). Expected saving per run: two object-storage GETs on `run_output` (about 100 ms each on the trace), and the per-version visibility reads in skills preparation (4 versions per run on the test deployment, about 388 ms per version on a heavier deployment). **Breaking changes** None. `claim()` no longer takes `expectedOwner`. It is an internal API with three callers in this repository, all updated. Refs #20284 (it made run-log tail reads parallel and cached; this change covers the begin path). ## What Changed - `object-byte-journal`: remove the `expectedOwner` input and comparison from `claim()`. Remove the extra head read in its three callers (run log `begin`, provider trace capture, run evidence migration). - `object-run-log-store`: `begin` reads the source-prefix proof only when the claimed head has `generation > 1`. - `skill-execution-content`: split `withVersion` into the access check and `withReadableVersion`. `withVersion` keeps the check for its other callers. - `company-skills`: `materializeVersionSnapshot` uses `withReadableVersion`, because each of its callers already read the skill row under the actor's visibility condition. - Tests: - A new run log `begin` reads `head.json` once and reads no proof. A resumed log reads `head.json` once and reads its proof once. - Runtime skill delivery for an agent actor does no per-version source permission reads. ## Verification - `cd server && npx vitest run src/services/object-byte-journal.test.ts src/services/object-run-log-store.test.ts src/services/object-run-log-store-tail-read.test.ts src/services/run-log-store.test.ts src/services/provider-trace-object-store.test.ts` - `cd server && npx vitest run src/__tests__/company-skills-runtime-selection.test.ts src/__tests__/company-skills-runtime-concurrency.test.ts src/__tests__/heartbeat-runtime-skills.test.ts src/__tests__/company-skills-service.test.ts src/__tests__/managed-skill-content.test.ts` (embedded Postgres) - `cd server && pnpm run typecheck` - Each new test fails on the base commit: the run log test sees 2 head reads and 1 proof read; the skill test sees 1 source permission read per delivered version. - Existing tests still pass for: stale-writer fencing after a takeover, a deleted journal that refuses a claim, a malformed proof that fails closed, and a denied project-scoped skill that is not delivered. - I did not run the full `pnpm test:run`. CI covers it. ## Risks - Low. Two concurrent claims of the same journal: before, the second claim failed when the first published between its two head reads. Now the later claim takes over, and the earlier writer's next append fails with a conflict. This is the same result as when the later claim starts after the first publish. Writer authority (`assertAuthority`) is unchanged. - A skill whose visibility is revoked in the moment between its row read and its materialization in the same call is still delivered for that run. This was already true for a revocation just after the old per-version check. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`) in Claude Code, with tool use (shell, file edits, test runs). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [ ] I have updated relevant documentation to reflect my changes - [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
|
380b3d49c0 |
fix(api): make case lookup and audit history discoverable to agents (#20354)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Cases are an experimental saved record with facts, audit history and handoffs, beside the documents on their tasks > - Agents find REST operations with `search_api`, which ranks catalog entries by words in the method, path and summary > - The case read routes had generated summaries such as "GET cases events" and one generic description > - So a chat agent that searched "list company cases" or "case facts history" did not get any case read route on the first page > - In a chat eval, the agent wanted to check the case for "who changed the diagnosis", could not find it, and answered from task document revisions only > - This pull request gives the case read routes real summaries and descriptions, and tells chat agents where a task's case and its fact-change history are > - The benefit is that agents can find and cite a case's audit history when a person asks about it ## Linked Issues or Issue Description Refs #20265 (case page link guidance for chat agents). **What happened** A person asked a chat agent "Who changed the diagnosis, and what did it say before?" about a case. The agent called `search_api` with "cases list facts history" and "list company cases". The case list, task-to-case lookup and case events routes were not on the first result page. The agent said "Not found", skipped the case audit and answered from the task document revisions only. It did not mention the case fact change or link the case. **Expected behavior** An agent that searches for a company's cases, a task's case or a case's fact-change history finds the matching route on the first page. The catalog says that a `fields_changed` event records who changed the facts and when, but not the previous values. **Steps to reproduce** 1. Enable Cases. 2. Call `searchRunnerApi({ query: "list company cases" })`. 3. See that `GET /api/companies/{companyId}/cases` is not in the five results. The same is true for `GET /api/cases/{id}/events` with "cases list facts history". ## What Changed - `server/src/routes/experimental-api-paths.ts`: add `experimentalApiDocs` with summaries and descriptions for the case list, task-to-case lookup, case read, case events and case document revision routes. - `server/src/routes/openapi.ts`: use those summaries and descriptions when present. Other experimental routes keep the generated summary. The experimental notice stays in every description. - `server/src/services/agent-conversations.ts`: the Cases chat directive now names `GET /api/issues/{issueId}/cases` to find a task's case and `GET /api/cases/{id}/events` for who changed its facts. - `server/src/__tests__/case-history-api-discovery.test.ts`: regression test with the queries from the failed run, and a check on the directive. ## Verification - `cd server && pnpm exec vitest run src/__tests__/case-history-api-discovery.test.ts` fails on master (2 failures) and passes with this change. - `cd server && pnpm exec vitest run src/services/heartbeat/task-markdown.test.ts src/services/native-runtime/runner-api.test.ts src/__tests__/openapi-routes.test.ts src/__tests__/skill-import-discovery.test.ts src/__tests__/primary-agent-guidance.test.ts`: 2148 tests pass. - `npx tsc --noEmit -p server` reports no errors in the changed files. - A live chat eval (`cases.case-history`) runs on a branch with this commit. ## Risks - Low risk. The change edits catalog text and chat guidance only. Route behavior and authorization do not change. - Search ranking changes for queries that use these words. Case routes now rank higher for case queries. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), with tool use and code execution in Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
1baac27888 |
feat(server): chat bootstrap reads for instant task opens (#20355)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - People open tasks from the task list to see the latest messages and to reply to an agent > - Before the task chat shows its first frame, it waits for about ten separate reads: the issue, recent comments, comment input facts, recent activity, recent runs, interactions, attachments, live runs and the active run > - On a busy instance each read waits for the database, and the slowest read decides when the chat appears. A cold open takes 1.5–4 s on staging > - This pull request adds one read that returns all of that data for up to ten tasks, in the same JSON shapes as the existing endpoints, plus a single-task form > - The benefit is that the task list can preload its top tasks, and a cold open needs one round trip instead of about ten. The UI change is in the stacked pull request ## Linked Issues or Issue Description **Problem or motivation** A task open waits for about ten reads. With the database under load, these reads queue, and a cold open takes seconds. The list cannot preload a task cheaply, because each task costs about ten requests. **Proposed solution** `GET /api/companies/:companyId/issues/chat-bootstrap?ids=` returns the first-frame data for up to 10 tasks. `GET /api/issues/:ref/chat-bootstrap` returns it for one task. Each field equals the matching single endpoint's JSON exactly. **Alternatives considered** - Prefetch the existing endpoints for each list row. For 30 rows this sends about 210 requests to a database that is already the bottleneck. - A new projection with different shapes. This would duplicate client merge logic, and freshness rules could drift. **Roadmap alignment** This is part of the task-loading performance plan in #20276. It follows #20277, #20284 and #20286. ## What Changed - `packages/shared`: - `ISSUE_CHAT_BOOTSTRAP_MAX_IDS` (10). - `issueChatBootstrapQuerySchema`: UUIDs or identifiers, normalized, deduplicated and strict. - The `IssueChatBootstrap` and `IssueChatBootstrapItem` types. - New `server/src/services/issue-chat-bootstrap.ts`: - Resolves the refs inside the company and applies the `issue:read` decision to each task. - Reads the tasks with a concurrency of 2, and returns them in request order. - `server/src/routes/issues.ts`: - The core issue read and the recent-comments read are now shared helpers, so the bootstrap and the single endpoints use the same code. The core view keeps its Server-Timing header. - Adds the batch route and the single-task route. - Excludes the single-task route from the issue ETag middleware, so a fetch without validators always gets a 200 with `Cache-Control: private, no-store`. - New `server/src/services/issue-run-presence.ts`: - The live-runs and active-run payload builders, moved from `agents.ts` without change. - The existing routes call them, and their behavior is unchanged. - `server/src/routes/openapi.ts`: entries for both routes. ## Verification - `cd server && npx vitest run src/__tests__/issue-chat-bootstrap-routes.test.ts` passes 17 tests. They run on embedded Postgres with privacy enforcement on. They cover: - Each field deep-equals its single endpoint's JSON, for an ordinary task and for a conversation task. - The single-task route equals the batch item, by UUID and by identifier. It is never answered with a 304. - Items keep the request order, with UUIDs and identifiers mixed. - Private, other-company, missing and unknown tasks are omitted, and none of their data appears. The single-task route returns 404 for them. - Malformed, extra or repeated parameters return 400. More than 10 ids also return 400. - A task-bridge key gets 403. - A 10-task request writes nothing: no inserts, updates or deletes, and no activity-log or comment changes. - The issue, agent, activity, live-run, cold-open, summaries, ETag and OpenAPI route suites pass: 56 files, 1,771 tests. - `cd server && pnpm typecheck` passes. - Query cost, local embedded Postgres, one 10-task request: about 49 SQL statements per task, 55–130 ms in total. A test fails if it goes above 60 per task. ## Risks - **Database load:** the stacked UI preloads up to 30 tasks, about 1,500 statements at idle time. This is the same work as opening those tasks, but it is done before the person clicks. The work runs in sequential requests of 10, with 2 tasks at a time. Most of the cost is existing run decoration. Live runs and the active run decorate the same running run, so that work runs twice. Sharing it is a follow-up. - **Stricter access:** live runs and the active run now apply the `issue:read` policy inside this endpoint. The existing routes keep their company-only check. - Hidden tasks are returned, as `GET /issues/:id` returns them. - No migration and no persisted-state change. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used in Claude Code with tool use (shell, browser automation, file editing, subagents). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [ ] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
653ab8b18f |
fix(chat): answer "my inbox" from the person's Mine list (#20334)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Agent Chat lets a person ask an agent about their own work, such as
"what in my inbox needs my attention?"
> - Agents find API routes through the agent API catalog, which is built
from the OpenAPI registry
> - The catalog described `GET /api/agents/me/inbox/mine` as the current
agent's assigned items and listed no `userId` parameter. The skill
reference for the route is keyed with `?userId={}`, so it never attached
to the catalog entry
> - A chat agent called the route without `userId`, got 400, and then
answered from an unscoped company task search. It named company work
that is not in the person's inbox and did not link the person's Mine
list
> - This pull request documents the route as the person's Mine list with
a required `userId`, and adds one chat guidance paragraph for inbox
questions
> - The benefit is that inbox answers use the person's own task scope
and link their Mine list
## Linked Issues or Issue Description
**What happened**
A person asked a CEO chat agent: "What in my inbox needs my attention
today?" The agent called `GET /api/agents/me/inbox/mine` without
`userId` and got 400. Then it listed every company task, including an
unrelated agent-created maintenance task. It said the person's inbox was
board-only and did not link the Mine list.
**Expected behavior**
The agent reads the person's Mine list with `GET
/api/agents/me/inbox/mine?userId=<person>`. It answers from those tasks
only, links each task it cites, and links the Mine list at
`/<prefix>/inbox/mine`. If the read fails, it says so and links Mine.
**Steps to reproduce**
1. As a board user, participate in some tasks. Let an agent create an
unrelated task in the same company.
2. In Agent Chat with the CEO, ask "What in my inbox needs my attention
today?"
3. The agent calls the Mine route without `userId` (400) and lists all
company tasks.
## What Changed
- `server/src/routes/openapi.ts`: `GET /api/agents/me/inbox/mine` now
has a summary and description that say it is a person's Mine list, not
the agent's own queue. It documents the required `userId` query
parameter and the optional `status` parameter, and the 400 response.
- `server/src/services/agent-conversations.ts`: one chat guidance
paragraph. For inbox questions, read the Mine route with the
`authorUserId` of the person's message, answer from those tasks only,
and link `[Mine](/<prefix>/inbox/mine)` (it opens the Mine view in every
layout). If the read fails, say so and link Mine.
- Regression tests in `openapi-routes.test.ts` and
`agent-conversations.test.ts`.
## Verification
- `npx vitest run src/__tests__/openapi-routes.test.ts
src/__tests__/agent-conversations.test.ts
src/__tests__/heartbeat-list.test.ts` (in `server/`): 107 passed. The
two new tests fail without the source change.
- `npx vitest run src/services/native-runtime/runner-api.test.ts`: 2075
passed.
- Live chat eval `inbox-search.show-my-work` on the eval candidate with
this commit: see the comment below for the run result.
## Risks
- Low risk. The change is catalog text, one query schema in the OpenAPI
document, and one chat guidance paragraph. The route behavior does not
change.
- The chat prompt grows by about 500 characters.
## Model Used
- Anthropic Claude Opus 5.5 (`claude-opus-5-5`), tool use and code
execution in Claude Code.
## Checklist
- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [ ] All Paperclip CI gates are green
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
|
||
|
DottaandClaude Opus 5.5
|
7f55323abb |
fix(recovery): end a remote workspace epoch whose writer died during preparation (#20346)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - A task with a remote workspace prepares that workspace before the
run selects its runtime, and preparation marks the workspace epoch dirty
with the run as its writer
> - If the server is killed during preparation, the run never selected a
runtime and never started provider work, but the dirty epoch remains
> - Failed-workspace preservation, the path that resolves a dead
writer's dirty epoch, applies to native runs only, so nothing owned this
epoch
> - The dirty epoch kept the dead run's sandbox lease as a pending
export and held the workspace against the task's retry, so the task
never recovered
> - This pull request lets the ownership reconciler end such an epoch at
the last committed save
> - The benefit is that a restart during workspace preparation no longer
wedges the task
## Linked Issues or Issue Description
**What happened?**
In a restart-chaos wave on Daytona (a remote workspace that clones a
real repository), the controller was killed while each run's first
workspace save was in flight. That save is now settled by #20326. Each
run ended `process_lost` with a bounded retry, and both runs were still
in `execution_stage: preparing` with `runtime_mode_resolved_at: null`.
The workspace authority stayed `dirty: true`, `snapshotId: null`, with
the dead run as writer. The dead run's environment lease was proven
owner-gone but was refused by its settle as `workspace_export_pending`,
and the reconciler reported `I2_settled` violations. The retries stayed
`queued` until the test harness deleted the environment 21 minutes
later. Then they failed `remote_workspace_execution_required`, because
the environment was gone.
**Expected behavior**
When a writer dies before runtime selection, its unsaved files are only
the restored source and preparation output, because no agent turn ran.
Recovery must end that epoch at the last committed save, so the lease
settles and the retry restores the workspace into a new directory. A
writer that reached runtime selection must keep its existing recovery.
**Steps to reproduce**
1. Run a task whose agent uses Paperclip Runner with a remote sandbox
workspace.
2. Kill the server while the run is still preparing the workspace (for
example during its first save).
3. After the restart, the run ends `process_lost` and its retry is
queued, but the workspace stays dirty and owned by the dead run, the
lease stays active, and the retry never starts.
**Paperclip version or commit**
`master` at
|
||
|
DottaandClaude Opus 5.5
|
16e9157577 |
fix(api): publish agent-grant access operations as board_or_agent in the API catalog (#20349)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Agents read the API catalog (`server/src/routes/openapi.ts`) to decide which operations they may call. The `x-paperclip-authorization` actor tells them who performs each operation. > - The invite, join-request and member-list routes admit agents that hold the matching company grant (`users:invite`, `joins:approve`, `users:manage_permissions`). > - The catalog still listed these operations as board-only. > - In a chat eval, a CEO agent with `users:invite` read `actor: "board"` on `POST /invites`, treated it as a final boundary, and told the person it could not invite anyone. > - This pull request publishes those operations as `board_or_agent` and names the grant each needs. It also names the Members page on member edits, which stay board-only. > - The benefit is that a granted agent uses the permission the board gave it, and agents without the grant still get a 403. ## Linked Issues or Issue Description **What happened?** A board granted the CEO agent `users:invite` with `PATCH /api/agents/:id/permission-grants`. The person asked in chat: "Invite alex@example.test to join this company." The agent searched the API catalog. It found `POST /api/companies/{companyId}/invites` with `x-paperclip-authorization: { actor: "board" }`. It concluded that invites are board-only and did not try the call. The route itself accepts agents with `users:invite`. **Expected behavior** The catalog shows `board_or_agent` for operations whose route accepts an agent with a company grant. The description names the grant. The agent creates the invite and returns the join link. **Steps to reproduce** 1. Grant an agent `users:invite`. 2. Read `GET /api/openapi.json` and find `POST /api/companies/{companyId}/invites`. 3. See `x-paperclip-authorization: { actor: "board" }`, although `assertCompanyPermission` admits the agent. **Paperclip version or commit** Master at the merge base of this branch. **Deployment mode** Any. Found in the runner full-stack chat eval `access.invite-teammate`. ## What Changed - Remove six operations from `BOARD_ONLY_OPERATIONS`: `POST`/`GET /companies/{companyId}/invites`, `GET /join-requests`, `POST /join-requests/{requestId}/approve` and `/reject`, and `GET /members`. Their routes use `assertCompanyPermission`, which admits agents with the grant. - Add descriptions that name the grant for each operation. The invite description also says it returns a shareable `inviteUrl` (no email field), and that membership starts only after acceptance. - Member edits (`PATCH /members/{memberId}`, `/role-and-grants`, `/permissions` and `POST /archive`) stay board-only. The route guard requires board access, and a test keeps the catalog in line with that guard. - Describe those member edits as board-only even for agents holding `users:manage_permissions`, and name the Members page (`/<prefix>/company/settings/members`) where a person makes the change. The member list description says the same. An agent then links the page instead of searching for an agent route. - Add a regression test in `openapi-routes.test.ts`. ## Verification - `cd server && node ../node_modules/vitest/vitest.mjs run --config vitest.config.ts src/__tests__/openapi-routes.test.ts` — 43 passed. The new test also checks that the Members page is named. With master's `openapi.ts`, the new test fails (`expected { actor: 'board' } to deeply equal { actor: 'board_or_agent' }`). - `src/services/native-runtime/runner-api.test.ts` — 2075 passed. ## Risks - Low risk. This changes catalog metadata only. Route guards do not change. An agent without the grant still gets 403. - Agents with these grants will now attempt these operations instead of stopping. That is the intended behavior of the grant. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), tool use and code execution. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandPaperclip
|
c67909cb0f |
fix: keep tool approvals valid through OAuth refresh and delayed review (#20340)
## Thinking Path > - Paperclip lets people manage agent work and approve app actions. > - An approval must bind the action that the person reviewed. > - OAuth access tokens can expire while that person is away. > - The old target snapshot treated each token refresh as a new authorization. It also imposed a one-hour review deadline. > - This pull request separates token renewal from authorization identity and keeps the human decision durable. > - Current access, the reviewed arguments, meaningful target changes, task closure, and execution receipts still control dispatch. > - People can approve later without repeating a decision after routine connection maintenance. ## Linked Issues or Issue Description **What happened?** A pending app action returned `approved_tool_target_changed` after its OAuth tokens refreshed. The tool and connection settings were unchanged. The same card also expired after one hour. A sign-in problem could discard an accepted decision before provider dispatch. **Expected behavior** The original card remains reviewable after routine OAuth refresh and a server restart. A later approval runs the exact reviewed action once. A connection repair preserves the decision while Paperclip verifies the same authorization. Meaningful changes require a new review. **Steps to reproduce** 1. Connect an OAuth MCP app and configure an action to require approval. 2. Request the action, then refresh its access and refresh tokens before approving. 3. Approve the original card. Before this change, it returns HTTP 409 even when the authorization is unchanged. 4. Wait more than one hour. Before this change, the card expires without a task-specific deadline. **Paperclip version or commit** Reproduced from master at `76254ead2229`. The final branch is rebased onto current master. ## What Changed - Add versioned approval target comparison. New snapshots bind credential identity epochs, consent, permissions, ownership, connection configuration, tool contracts, and credential references. Authenticated OAuth refresh preserves identity. Manual credential replacement does not. - Remove the implicit one-hour deadline from ordinary app action reviews. Keep explicit deadlines and terminal-task expiry. Synchronize the nullable interaction deadline in shared contracts and UI. - Keep accepted, undispatched actions when sign-in is required. Show a saved-approval repair state. Resume on committed connection changes after target and access revalidation, without an idle retry loop. - Preserve the existing execution claim, result receipt, and uncertain-outcome no-replay rules. - Explain meaningful target changes without inviting the person to retry the stale card. - Add a production task-thread Storybook example and document compatibility and recovery behavior. ## Verification - Red: production-refresher invalid-grant handling and formal-approval withdrawal during a sign-in wait also reproduced as failing regressions before their fixes. - Red: before implementation, the local OAuth HTTP fixture failed for both public and confidential clients with `approved_tool_target_changed` after token refresh. - Green: the extended fixture passes for public and confidential clients with personal and shared grants. It covers actual local HTTP token exchanges, multiple refreshes, a rebuilt gateway, an ended originating run, approval after 25 hours, duplicate approval, preserved sharing arguments, sign-in waiting, and event-driven repair. The gateways use the production OAuth refresher; an invalid refresh preserves the accepted decision before provider dispatch. Omitted-scope responses cover equivalent scope-reporting changes. The provider is an isolated local fixture, not a deployed Notebook service. - Negative controls refuse changed consent, scopes, tool definition, destination, and manual credential replacement before provider execution. - Existing gateway service, connected-tool acceptance, secret owner, secret service, unsigned-review queue, and connection-health suites passed. Explicit-deadline fixtures still prove expiry behavior, including deadlines on accepted sign-in waits. A withdrawn formal approval cancels a waiting action before connection recovery. - Approval card, task-chat, error-copy and target-comparison tests passed. Server and UI typechecks passed. Token gates and Storybook catalog checks passed. - Inspected the new saved-approval repair card in local Storybook using the production task thread. It shows a reconnect link and no second approval button or false running state. - `pnpm verify --explain` returned exit 2 and requested broad CI coverage. [Full CI](https://github.com/paperclipai/paperclip/actions/runs/42677855507) passed on `eb4f1c7caa9ee6e802c314ffc5eadec475a5de0c`, including build, typecheck/release registry, general and serialized tests, and all eight browser shards. - Greptile reviewed the same final head at 5/5. Both findings from the first review are fixed and their threads resolved; no open follow-ups remain. ## Risks - No database migration is needed. The new comparison reuses the vault identity epoch. Only authenticated OAuth refresh code can preserve that epoch; HTTP credential-write schemas cannot select that behavior. - Earlier signed requests keep their original exact-version checks and saved deadlines. Already-expired cards are not revived. A historical card whose credentials already changed needs a fresh review. - A new consent, account, permission set, destination, or manually replaced credential must not inherit the old authorization. Reconnection continues automatically only when revalidation proves the same saved authorization. - The interaction enrichment now permits a null deadline. Existing string deadlines remain valid. The web client and shared validator change together. - This PR does not deploy to staging or perform a live Notebook publication. ## Model Used OpenAI Codex, GPT-6 family, with repository tools, code execution, and browser inspection. The runtime does not expose the exact model ID, context-window limit, or reasoning-mode label; those values are not asserted. ## 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
|
a67b936885 |
fix(server): redact invitation bearer paths from HTTP logs (#20343)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - People join a company through an invitation link. The link carries a bearer token in its URL path. > - The HTTP logger removes query secrets and URL user information. It keeps the path. > - So invitation bearer tokens reach the HTTP log in the request URL, the log message, and the referrer header. > - PR #17848 fixed this, but it did not merge. Since then, #13742 changed the same redactor file. > - This pull request carries the #17848 commits onto current master. The original author is kept on both commits. > - The benefit is that invitation tokens do not stay in HTTP logs. The chat eval for membership management can then accept a real invitation safely. ## Linked Issues or Issue Description Supersedes #17848 (same commits, rebased onto current master after #13742). **What happened?** The HTTP logger writes invitation bearer tokens to logs. A synthetic POST to `/api/invites/<token>/accept` puts the token in `req.url`, in the log message, and in the `Referer`/`Referrer` header record. **Expected behavior** Logs keep the invitation route, the method, and the HTTP status. Logs do not keep the bearer token. **Steps to reproduce** 1. Check out master. 2. Run `server/src/__tests__/logger-invitation-redaction.test.ts` from this PR. 3. The synthetic token appears in the request URL, the message, and the referrer. 12 tests fail on master. **Paperclip version or commit** Master at the merge base of this branch. **Deployment mode** Source server. The tests use a disposable local HTTP listener. They use no database and no provider. ## What Changed - Remove the bearer segment from `/api/invites/<token>/...`, `/invites/<token>/...`, and `/invite/<token>` URLs, including mounted paths and absolute URLs. - Apply the same URL policy to `referer` and `referrer` request headers. - Keep record IDs on the non-token revoke route `/invites/<inviteId>/revoke` and on company-scoped invite routes. - Add regression tests through the real HTTP logger for 202, 400 and 500 responses, with both referrer header spellings. - Original work by @cryppadotta in #17848. This PR only rebases it onto master. ## Verification - From `server`: `node ../node_modules/vitest/vitest.mjs run --config vitest.config.ts src/__tests__/logger-invitation-redaction.test.ts src/__tests__/redact-sensitive.test.ts src/__tests__/http-log-policy.test.ts` — 46 passed. - With the master `logger.ts` and `redact-sensitive.ts`, the same new tests fail: 12 failed. - `http-log-aborted-response`, `cloud-principal-log` and `setup-token-session` tests pass locally. Some other HTTP-log suites could not load locally because workspace package builds were missing in this worktree. CI covers them. ## Risks - Low risk. Referrer query and fragment values are now removed from request logs, as they already are for request URLs. This removes some diagnostic detail to protect secrets. - A route that uses `/invites/<id>/...` for a non-bearer record ID, other than `revoke`, now shows `[REDACTED]` in logs for that segment. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), tool use and code execution, used to rebase and verify. The original change was authored in #17848. ## 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: Paperclip <noreply@paperclip.ing> |
||
|
DottaandClaude Opus 5.5
|
0f71ab796e |
fix(issues): name the existing order in dependency-cycle denials (#20342)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Tasks can wait for other tasks through blocking dependencies, and the server rejects a dependency that closes a cycle > - The rejection said only "Blocking relations cannot contain cycles" > - In chat, an agent asked to "make Website wait for Design too" got this 422 while Design already waited for Website > - The agent had no sign that the existing order must stay, so it guessed a swap: it removed Design's dependency on Website and added the reverse > - This pull request makes the 422 name the existing order, say that nothing changed, and say to ask the requester which order they want > - The benefit is that agents keep the existing prerequisite and ask the person, instead of silently reversing it ## Linked Issues or Issue Description **What happened** A chat agent was asked to make Website also wait for Design. Design already waited for Website. The server correctly returned 422 "Blocking relations cannot contain cycles". The agent then removed the existing Design-waits-for-Website dependency and added Website-waits-for-Design. The person lost the existing prerequisite without being asked. **Expected behavior** The rejection keeps both saved dependency sets as they were. The agent explains the conflict with the real task names and asks the person which order they want before it changes any dependency. **Steps to reproduce** 1. Create tasks Website and Design. Make Design blocked by Website. 2. PATCH Website with `blockedByIssueIds: [Design]`. 3. See that the 422 says only "Blocking relations cannot contain cycles", with no task names and no next step. ## What Changed - `server/src/services/issues.ts`: the cycle check now records the path it finds. The 422 message names the existing chain by task identifier (for example "COM-2 waits for COM-1, so COM-1 cannot also wait for COM-2"). It never includes titles, because the chain can pass through tasks the caller cannot read. Identifiers are the same fields that locked relation stubs already show. It says no dependencies changed and the existing ones are kept. It says not to remove or reverse an existing dependency, and to ask the requester which order they want. - The 422 `details` now carry `code: "blocking_cycle"`, the updated issue, the requested blocker, and the existing chain as ids and identifiers. - `server/src/__tests__/issues-service.test.ts`: a regression test checks the message and details for a direct cycle, the chain text for a two-step cycle through a task with a private title, that no title appears in the error, and that the existing dependency is unchanged. ## Verification - `cd server && pnpm exec vitest run src/__tests__/issues-service.test.ts` passes (152 tests). The new test fails on the old message. - Chat eval `task-coordination.reject-dependency-cycle` ran end to end with the first commit (run 42678089404, which named tasks by identifier and title). The agent kept Design waiting for Website and asked the person which order to keep with a question card. The saved dependency sets did not change. The second commit removes titles from the message; the rest of the text is the same. ## Risks - Low risk. The status code (422) and the first words of the message do not change. Clients that match the full old message exactly would no longer match. No code in this repository does that. - The check reads the identifiers of the issues in the chain only when it rejects a request, so the success path is unchanged. - The error can show the identifier of a task that the caller cannot read. Relation responses already show this identifier in locked stubs, so this shows no new data. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), tool use and code execution in Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
251c6d5c69 |
fix(recovery): settle a remote workspace save whose process died mid-save (#20326)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Native runs in remote sandboxes save the workspace as snapshot
operations (`workspace_operations`, phase `remote_workspace_snapshot`).
Each save invocation is listed in `saveWork.activeIds` while it runs
> - When a controller process is killed during a save, its invocation
stays listed and the operation stays `running`
> - Nothing recorded which process ran the invocation, so the shared
ownership reconciler could not prove the owner gone and never settled
the save
> - The task's bounded retry then waited behind that save forever
> - This pull request records the owning process for each save
invocation, and lets the reconciler settle a save whose owner is proven
gone
> - The benefit is that a restart during a save no longer wedges the
task
## Linked Issues or Issue Description
**What happened?**
In a restart-chaos wave on Daytona with a real cloned repository, two
tasks each started their first remote snapshot save (`boundary:
initial`, epoch 1). A SIGKILL hit the controller during the save, before
any upload. Each run ended `process_lost` and got a bounded retry. The
retries stayed `queued` forever, the snapshot operations stayed
`running` with `saveWork.activeIds: [<dead invocation>]` and
`quiescedAt: null`, and the sandbox leases stayed active. The ownership
reconciler reported the saves as owner-gone but had no way to settle
them.
**Expected behavior**
After a restart, recovery must settle a remote snapshot save whose
owning process is gone. It is marked failed with provenance, so the
task's retry can continue. A save that is still running must stay
untouched.
**Steps to reproduce**
1. Run a native task with a remote workspace (for example Daytona with a
`git_repo` project source).
2. Kill the controller (SIGKILL) while the first workspace save is in
flight, before its upload.
3. After the restart, the run ends `process_lost` and its retry is
promoted to `queued`, but the save operation stays `running` and the
retry never starts.
**Paperclip version or commit**
`master` at
|
||
|
DottaandClaude Opus 5.5
|
bce7503a82 |
perf(workspaces): time a restore's sandbox upload and extraction separately (#20331)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Remote (sandbox) workspaces are restored from a durable snapshot before a later turn, so restore time delays that turn's first token > - #20280 added run-log timing for the storage steps of a save and a restore > - A Daytona wave with a Paperclip-sized workspace (~750 MB) spent 39.9 s preparing the workspace of a restored run. The storage steps (download, decrypt, verify, publish) took 2.4 s, and `workspace.snapshot.sandbox_restore` took 35.6 s > - The archive helper unpacks the same archive locally in 3.6 s, so most of that time is elsewhere, probably copying 750 MB into the sandbox. One span cannot say which > - This pull request splits that span into the upload into the sandbox and the extraction in it > - The benefit is that the next measurement shows which part to fix ## Linked Issues or Issue Description Refs #20280 **What happened?** A restored run on Daytona spent 35.6 s in `workspace.snapshot.sandbox_restore` (wave `w-20261109T170632-s9921`, R1 workload, 754,838,327-byte snapshot). The run log records only that total, so it cannot show whether the copy into the sandbox or the unpack inside it is slow. **Steps to reproduce** 1. Run a remote workspace task with a large repository on Daytona and let a later run restore it. 2. Read the restored run's `run.performance.span` events. 3. Only `workspace.snapshot.sandbox_restore` covers the time after the storage download. **Expected behavior** The run log separates the upload into the sandbox from the extraction there. ## What Changed - `remote-workspace-transport.ts` (`restoreRemoteWorkspaceDirectory`): the archive copy into the sandbox runs inside `workspace.snapshot.sandbox_upload`, and the in-sandbox restore command inside `workspace.snapshot.sandbox_extract`. Both are children of the existing `workspace.snapshot.sandbox_restore`. - `execution-preparation-timing.ts`: the two new phase names. A traced `measureRunPhase` scope also carries its preparation ancestor's run (`recordRunId`). Without it, with an OTLP endpoint set, a step nested inside another step (here, upload and extract inside `sandbox_restore`) never reached the run log. Execution attempts still record nothing. - `doc/run-log-events.md`: the two rows in the durable storage timing table. - No behavior change. This is a run-log change: the rows stay in the instance database. ## Verification - New test `times the restore's sandbox upload and extraction as separate run-log steps` (`remote-workspace-transport.test.ts`). It restores a real archive through the real helper, inside the production `sandbox_restore` step with an OTLP endpoint set, with a 60 ms pause in each step, then reads the drained run phases. It fails without the spans or without the scope fix. - New test `records a step nested inside another step when an OTLP endpoint is set` (`execution-preparation-phases.test.ts`). It fails without the scope fix. - `npx vitest run` on `execution-preparation-phases`, `execution-preparation-timing`, `execution-timing-context`, `database-operation-attribution`, `native-workspace-finalization-work`, `remote-workspace-transport` and `remote-workspace-snapshot-storage`: 256 passed. ## Risks - Low risk. Two more measured steps on the restore path; spans under 50 ms are not recorded. ## Model Used Claude Opus 5.5 (`claude-opus-5-5`, Anthropic), 1M context window, tool use and code execution, via 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 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
4a349ea447 |
fix(chat): tell agents that instruction saves keep no history (#20328)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Chat agents can read and edit other agents' instruction files with dedicated tools > - #14420 changed instruction saves: Paperclip now keeps only the current file and records no revision history > - The agent-facing descriptions still describe a "canonical instruction revision" history > - In a live eval, a chat agent asked to undo an instruction change found an empty history, read it as a test artifact, guessed the earlier text from the current file's wording, wrote it, and reported a restore > - This pull request tells agents the real contract in chat guidance and in the tool and API descriptions > - The benefit is that an undo with no saved version leaves the instructions alone and asks the person, instead of inventing an earlier version ## Linked Issues or Issue Description Refs #14420 Refs #19515 (open: would list save records from the activity log. Those records hold no content, so they still cannot restore earlier text) **What happened** A person asked a chat agent: "Undo Morgan's last instruction change." Morgan's history was empty, because saves do not record revisions. The agent's reasoning said the history "is only populated with canonical revisions" and that there was "no revision tracking in this e2e harness". The agent then removed the last line of AGENTS.md and replied "Undone. Morgan's AGENTS.md is back to …". No saved source had the earlier text, so the agent guessed it. **Expected behavior** When there is no saved earlier version, the agent leaves the instructions unchanged. It says plainly that no earlier version was saved, and it asks the person for the text they want or which part to remove. **Steps to reproduce** 1. Create an agent with an AGENTS.md file. Edit the file once through `PUT /api/agents/{id}/instructions-bundle/file`. 2. `GET /api/agents/{id}/instructions-bundle/history?path=AGENTS.md` returns `{"revisions":[]}`. 3. Ask a chat agent to undo the agent's last instruction change. The agent rewrites the file from a guess. ## What Changed - `AGENT_CHAT_DIRECTIVE` now says three things: - Saves keep only the current instruction files. - An empty history means no earlier text is retained. - An undo with no listed revision must leave the file unchanged, say that no earlier version was saved, and ask for the wanted text. - The `get_agent_instruction_history` contract now says that it lists only pre-upgrade revisions, and that an empty list means Paperclip retains no earlier version. - The `restore_agent_instructions` contract now says that only listed pre-upgrade revisions can be restored. - The OpenAPI summary for `GET /api/agents/{id}/instructions-bundle/history` says the same. - The semantic tool contracts, the protocol manifest and the seeded eval catalog hash are regenerated. - Regression tests check the new guidance and both live tool contracts. ## Verification - `pnpm exec vitest run server/src/__tests__/agent-conversations.test.ts server/src/__tests__/board-access-denial.test.ts server/src/__tests__/codex-local-execute.test.ts server/src/__tests__/runner-api.test.ts` passes. - In `packages/paperclip-runner`, `pnpm exec vitest run src/semantic-tools/paperclip-semantic-tools.test.ts src/catalog src/evals/native-execution.test.ts` passes. - `node scripts/generate-semantic-contracts.mjs --check`, `generate-protocol-manifest.mjs --check` and `generate-replay-goldens.mjs --check` pass. - `tsc --noEmit` passes for `server` and `packages/paperclip-runner`. - The live result is the chat eval `agents.restore-instructions`, run with this change cherry-picked: run 42666621831 passed. The agent replied that Morgan's history is empty and no earlier version exists. It left AGENTS.md unchanged and asked for the exact text or the line to remove. Run 42660236086, without this change, failed: the agent guessed and rewrote the file. ## Risks - Low risk. Only agent-facing text changes, and no behavior changes. - The runner semantic catalog hash changes. This is the same kind of change as #19983. - If #19515 lands, history lists save records without content. The guidance wording must then say that listed saves without content cannot be restored. Whichever PR merges second must reconcile the two texts. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), with tool use and code execution in Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
f56b097fbd |
fix(recovery): move stranded legacy subtasks off a parent's held workspace (#20314)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Agents split work into subtasks. Since #19860, every subtask gets its own execution workspace from the project base, and a subtask that an older release bound to its parent's workspace moves to its own workspace on its next admission > - Some legacy subtasks never get a next admission. Another task held the shared workspace with unsaved files, so their runs failed setup before any provider work. Stranded-work recovery then blocked them and handed them to the board > - On staging, a parent and two of its children stayed blocked for four days this way. The children could have run in their own workspace at any time > - This pull request makes the scheduled recovery-action sweep move such a child to its own workspace and continue it, with no person > - The benefit is that a legacy subtask never stays blocked on a workspace that it should not share, and the parent keeps its own workspace ## Linked Issues or Issue Description Refs #19860 (subtasks get their own workspace; legacy children move at admission) Refs #20295 (the held workspace's exhausted save now settles automatically) Refs #20273 (stall wakes wait while a workspace hold refuses a run) **What happened?** A parent task and three children shared the parent's remote workspace, because an older release created the children before #19860. One child's ended run held the workspace with unsaved files. The other two children were woken, but each run failed setup with "The workspace still has an unfinished writer or save operation." (`setup_failed`, `executionRecovery.kind=bootstrap`, `providerWorkStarted=false`). Stranded-work recovery then set each child to Blocked with a board-owned escalation (`board_escalation_no_takeover_v1`) that asked a board operator to retry. The #19860 move happens only at the next admission, and a blocked child gets no admission. **Expected behavior** A subtask that failed only because of a legacy shared-workspace binding continues in its own workspace from the project base automatically. The parent and the shared workspace stay unchanged. **Steps to reproduce** 1. With a release before #19860, create a parent task and children that share its remote workspace. 2. Let one child's run end with unsaved files whose save is not settled. 3. Wake another child. Its run fails setup on the workspace hold, and stranded-work recovery blocks it with a board escalation. 4. See that the blocked child is never moved to its own workspace and never runs again. **Root cause** `server/src/services/execution-workspaces.ts` (`releaseAncestorWorkspaceBindingAtAdmission`) moves a legacy child only when a run of the child is admitted. `server/src/services/recovery/service.ts` (`reconcileStrandedAssignedIssues`) ignores Blocked tasks, so nothing admits a child that a setup failure on the shared hold blocked. ## What Changed - `server/src/services/execution-workspaces.ts`: extract the binding release into `releaseAncestorWorkspaceBinding(tx, …)`, which runs inside the caller's transaction. Admission uses it unchanged (same guards: no other running run of the task, no unsettled workspace recovery, only a relative's workspace). - New `server/src/services/recovery/legacy-workspace-rehome.ts` (`rehomeStrandedLegacyWorkspaceTasks`). It selects a board-owned stranded escalation whose latest run failed setup on the workspace writer hold before any provider work, for a Blocked task bound to a relative's workspace. In one transaction it releases the binding, resolves the escalation with the outcome `legacy_shared_workspace_rehomed`, sets the task to `todo`, records a pending wake (`legacyWorkspaceRehome.wake: "pending"`), and records `issue.legacy_workspace_rehomed` system activity. After commit it wakes the assignee once (`issue_recovery_action_restored`, with a fresh session) and marks the wake `delivered` only when a run is created. A wake lost to a crash, refused (`null`), or failed stays pending and backs off (5 minutes, doubling, at most an hour); the pass returns the earliest owed wake as its retry deadline, so the recovery worker schedules it. Candidates are paged by action id. - Guards: the Blocked status must come from that escalation (set after the failed run, and no status decision at or after the block), the shared dependency-readiness rule must allow work (cancelled blockers and done blockers that still save files keep it blocked), the failed run must belong to the current assignee, and the binding release must be allowed. Otherwise nothing changes. - `reconcileActiveRecoveryActions` (the company-scoped sweep that the scheduled issue-recovery worker runs for each company with an active recovery action) runs this pass first. `recoveryActionSweepCondition()` also selects a company whose moved task still owes its wake. - `doc/execution-semantics.md`: documents the move and what a moved child sees. ### What a moved child sees A fresh workspace from the project base, exactly like a new top-level task in the project. It does not get the parent's changes. It does not get its own earlier files from the shared workspace either, because nothing of its work ran there (its runs failed setup before any provider work). The child that still holds the shared workspace with unsaved files is not moved by this change: its save recovery owns the hold (#20295 settles it automatically), its files stay in the shared workspace and in that recovery's saved-work receipts, and it moves on its first admission after that recovery settles (the #19860 rule). ### Previous-release story (`rules/upgrade-safe-run-state.mdc`) - **Migrated, no schema change.** The change reads escalations and task bindings that older releases saved, and it changes only the task's binding row, the task status, and the escalation. It does not change saved run identity (native execution input, leases, checkpoints, sessions). The regression test seeds the exact staging shape (Nov 4–5 evidence fields, `retryReason: workspace_busy`, `latestRunErrorCode: setup_failed`). - **Mid-run deploy.** A child with a running run or an unsettled workspace recovery is not moved (the same guards as the admission release). ## Verification - Red (the move disabled), `npx vitest run src/__tests__/legacy-workspace-rehome.test.ts`: 1 failed, 2 passed. The staging-shape case failed because the stranded child stayed Blocked and bound to the parent's workspace. - Green: 8 of 8 pass. All cases run the scheduled path: the company is selected by `recoveryActionSweepCondition()` and the pass is `recoveryService(...).reconcileActiveRecoveryActions({ companyId })` with the real heartbeat wake. - Staging shape: a parent and 3 children share the parent's workspace. One child holds it with a dirty writer run. Two children are blocked by board escalations after setup failed on that hold. After the stranded-work sweep, both children are unbound from the shared workspace, their escalations are resolved by the system with text that asks no person to act, and each child is woken exactly once with a fresh session. Each child runs once through the real heartbeat path outside the shared workspace and completes. The parent and the writer child keep their bindings, the shared workspace row is unchanged, and nothing runs for them. A later pass changes nothing. - A move whose wake was lost to a crash: the scheduled worker still selects the company, delivers the wake once, and the child runs and completes. - A refused wake (`null`) or a thrown enqueue stays pending, backs off, does not hold up other wakes, is reported to the worker as the next deadline, and keeps the company selected after a restart; once accepted, it settles. - Thirty rejected escalations do not hide a later eligible one (paging). - A child with a cancelled blocker stays blocked with its board escalation. - A child whose last status decision came before the block moves; one decided after the block stays blocked. - A stranded child whose setup failed for another reason stays blocked with its board escalation. - A task bound to the workspace it created (the parent) is never moved. - Related suites pass: `subtask-workspace-independence`, `heartbeat-workspace-busy`, `heartbeat-remote-workspace-busy`, `heartbeat-held-comment-wake`, `recovery-deadline-acceptance`, `heartbeat-retry-scheduling`, `task-workspace-binding` (278 tests), and after the review changes `issue-blocker-stall-wakeups`, `issue-recovery-deadlines`, `issue-recovery-work-lifecycle`, `recovery-durable-wait-projection`, `owner-gone-attestation` (124 tests with the two workspace suites). - `pnpm verify`: exit 2 ("broader verification required", because `doc/execution-semantics.md` is a global CI input). Required CI supplies the broad coverage. - `npx tsc --noEmit -p server/tsconfig.json`: passes. ## Risks - The pass changes a Blocked task to `todo` only under narrow guards, and only for a task bound to a relative's workspace whose last run failed setup on that workspace's writer hold. A Blocked status from another decision, or an unresolved blocker, keeps the task blocked. - The moved child starts again from the project base. This is the product rule for subtasks. Nothing of its own work is dropped, because its runs failed before any provider work. - The pass runs inside the existing active recovery action sweep and selects at most 25 escalations and 25 pending wakes per pass. > 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, used through Claude Code with tool use (shell, file edits, browser read-only API checks) and extended reasoning. ## 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
|
34c1e6469c |
fix(storage): retry small uploads and resume broken downloads (#20313)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A task's files live in a sandbox. Paperclip saves them to object storage when a run ends and restores them when the next run starts > - Object storage has short blips: a single 500, a reset connection, a download cut off halfway. Each of these used to fail the whole save or restore once, with no second try > - This pull request makes a short blip invisible. Longer outages are handled by the storage-unavailable wait (#20299 and a follow-up) > - The benefit is that tasks keep moving through ordinary storage failures without anyone clicking Retry ## Linked Issues or Issue Description Refs #19831. Storage chaos waves `w-20261109T145628-s9902` (ST1) and `w-20261109T151711-s9903` (ST2). - A single-request upload (files up to the multipart threshold, which includes most workspace archives) sent once. A 500 or connection reset failed the save. - A download whose body broke mid-stream failed the restore, even though S3 can resume from the byte offset. Related: #20299 (storage outage is shown as an infrastructure wait instead of a failure). ## What Changed - `server/src/storage/s3-multipart.ts`: `sendFileRequestWithRetry` resends a single file request and records throttles and backoff in the save timing. It uses the same throttle backoff as multipart parts for 429/503/SlowDown and up to the same part attempt count for other failures. It honours the caller's abort signal. - `server/src/storage/s3-provider.ts`: - A file-backed `putObject` under the multipart threshold uses that retry and reopens the source file for each attempt, so a resend never reuses a half-read stream. The Content-MD5 is computed once from the file and still verified by S3 on each attempt. - `getObject` resumes a body that fails with a transient socket error (reset, timeout, premature close and similar). It continues with a ranged GET from the byte where the stream stopped, at most three times. The resumed request carries `If-Match` with the first response's ETag, so a changed object fails instead of mixing bytes. Body-failure diagnostics are still recorded on every response. Destroying the returned stream closes the S3 body it is reading, even mid-read, so read timeouts and client disconnects still release the connection. - Tests: - `server/src/__tests__/storage-s3-transient-retry.test.ts` (new) runs against a local HTTP server speaking S3. ## Verification Red/green (each new test fails against master and passes with this PR): - `storage-s3-transient-retry.test.ts`: - A small upload is resent after a 500 and after a connection reset, re-reading the file. The second PUT carries the full content. - A download cut off mid-body resumes with `Range: bytes=<offset>-` and `If-Match`, and returns the complete bytes. - A download whose object changed before the resume fails with a precondition error and does not return mixed bytes. - A stalled download that its reader destroys closes its connection. Regression: the storage and workspace-finalization suites pass locally, and `tsc --noEmit` passes for `server`. ## Risks - Resending an upload is safe because each attempt writes the full object with the same Content-MD5. Resuming a download is bounded to three resumes and refuses a changed object. - No schema migration and no API change. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edits). ## 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
|
a3e7e63330 |
Guard Native recovery with process-space proof and explicit containment (#19813)
## Thinking Path > - Paperclip manages agents and their work. > - Recovery must preserve execution ownership and accepted results. > - A saved PID proves nothing outside its recorded process namespace. > - Uncertain unfinished work must stay held until its original owners stop. > - Operators need a source-bound containment action and a separate ordinary Retry. > - Accepted results must use the existing finalizer without another provider turn. > - This PR keeps those paths safe, recoverable, and quiet between real obligations. ## Linked Issues or Issue Description Refs: #19713 Refs: #19591 Recovery could use a local PID to release or replace a controller from another process namespace. Connected-chat Retry also rejected successful containment on a task in `todo`. The new containment action used a broader permission than the existing owner-stop action. A later review found that result-less guards also rejected the existing accepted-result confirmation action. ## What Changed - Record and verify the controller's kernel/PID namespace. Keep unknown and foreign unfinished ownership held. Preserve independent durable stop evidence and the existing remote lease fences. - Use the shared accepted-result ownership reconciler. Bind settlement to the observed lease token, generation, boot, PID, namespace and process birth. Retain finalizer, cleanup, current-controller, live-owner and remote exclusions. - Share the remote-owner reader across recovery consumers. Scope runner leases to the same company, task and run. Do not make local PID probes for remote accepted-result ownership. - Gate original-host containment on `runtime:confirm_owner_stop`. Bind it to the current task, source, assignee, original expiry, lease and Stop state. Audit separate controller/provider containment and reviewed action outcomes. - Keep containment separate from ordinary Retry. Use one canonical committed receipt and the normal admission, budget, workspace and lineage checks. Preserve the recorded external-action outcomes. - Keep the controller-only confirmation actionable for both result-less work and accepted results. Result-less work gets its next source-bound attention in the same transaction. Accepted results keep their original result and use normal shared settlement/finalization. - Keep identity inspection quiet. Preserve the original foreign-owner expiry and use the existing scoped finalizer in accepted-result recovery controls. - Update the recovery card, validators, execution projection, docs and access maps. Reuse existing canonical columns; add no migration or ownership backfill. ## Verification Current publication head: `71ea76b568de09d6313d8992e803131d9d5c54f3`. **The current fix has a causal red/green check.** On the test-only revision, both foreign-local stopped/fenced controller cases failed because recovery probed saved runner, process-group and provider PIDs in this host's namespace. This measured the wrong probes; it did not run duplicate providers. After the fix, the same two cases passed every no-probe, no-claim, no-controller-stamp, no-stop-receipt and unchanged-authority assertion. Three existing remote preparation, grace and fenced-controller progress cases also passed. Total: **5 passed, 0 failed;170 filtered rows excluded**. All three normal affected App TypeScript programs passed on this head (660,681 and428 roots). The fix separates a controller wait from execution stop proof. Independent live/grace evidence still schedules recovery. A stopped/fenced foreign controller can pass this namespace guard only for remote execution, which keeps its later checkpoint and never-launched checks. Foreign local execution stays held before runner or provider PID inspection. Earlier source-scoped results remain evidence, not fresh executions on the current head: - On `c86dd741`, the complete failed-owner file passed **25/25**, with no skips or filters. Empty scans wrote no controller stamps. Foreign ownership kept its original expiry. Normal returned-claim settlement and attention at the exact deadline passed. - On `c86dd741`, **15 selected accepted-result/restart controls passed**. The81 filtered rows are excluded. Both completion cases used the existing scoped finalizer and kept their accepted result without another provider turn. - Trusted CI at synthetic merge `50d072a5` (actual master `1b7fe334` plus `c86dd741`) passed the complete **17-case maintenance-server file**. This includes the original90-real-second quiet observation and two qualified hourly windows. The quiet case took118,918ms. The four jobs that failed on the earlier publication all passed on that synthetic merge. - On `6243b32c`, the complete ownership-report file passed **29/29**. Native NULL namespace stayed unprovable and generic compatibility remained. Three selected recovery controls passed;170 filtered rows are excluded. - On `f81f2c3c`,356 business cases passed: whole ownership-route221, recoveryUI70, owner-proof38 and controller-confirmation8; selected accepted-result authority18; and one real connected-chat containment, separate authorized Retry, useful Native result and downstream reply case. The2,024 filtered rows are excluded. A separate earlier accepted-result causal case kept the original result and completed after restart without replay. Normal Shared exports, App programs, UI typecheck, token gates and map checks passed on their recorded revisions. Unchanged DB, adapter, Eval and Runner inputs/outputs have explicit carry proofs. The clean normal merge with fetched master `8fe524f7` preserves all31 exclusive owned paths and14,827 unrelated master entries. The nine overlapping paths were reviewed, including the upstream unidentified-run stop action. Fresh CI must validate that actual merge composition. PG cases used existing external PG17, fresh owned databases, pools at most4, one worker and no SQL observer during cases. Registered clients and recorded process groups closed. Independent final SQL audits found zero campaign sessions before normal database cleanup. Original failing fixtures remain retained. Original failures are preserved. One new test first failed compilation due to a missing getter import; an import-only correction passed its full owning programs before the causal red/green check. A local quiet attempt stopped in the normal prerequisite hook before any business case and its collector rejected two normal build markers. The later17-case CI pass establishes quiet recovery; it does not turn that failed local collector into a pass. An earlier useful case passed before its private cleanup collector failed on an owned read-only directory; normal cleanup completed after all owners closed. Earlier runner-shutdown cancellation and a separate private whole-suite failure remain unexplained. Current-head checks are complete:57 success,1 neutral and3 skipped, including the successful aggregate CI gate. The legacy status passed. Greptile is5/5 and Ponytail gives an explicit5/5 Ship verdict for this head. All seven review threads are resolved. Full recursive typecheck, the full test suite and release build were not repeated locally. No result here proves deployed suspension, a live provider or termination on another host. ## Risks - Unknown physical ownership can require an owner/admin action. A false containment statement can be unsafe. - New work, reassignment, Stop, renewed authority, accepted results or pending cleanup can invalidate an action. The operator must refresh. - Controller confirmation does not prove provider/tool containment. Completed external actions must not be replayed. - Generic legacy and child-process compatibility remains separate from the strict Native controller policy. - Existing saved state and canonical schema are preserved. Permanent safety refusals remain held with their normal named recovery path. ## Model Used OpenAI `gpt-6.1-sol` agents implemented and tested the changes with reasoning, repository tools and code execution. A GPT-6-based Codex managing agent reviewed and integrated them. The host does not expose the manager's exact build or the context-window sizes. ## 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 |
||
|
DottaandClaude Opus 5.5
|
365c771030 |
fix(chat): give project and workspace tool results their page links (#20324)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - People ask a chat agent to take them to a project or to the files of a task > - The agent finds the right project or workspace, but it cannot link the page > - The `list_projects` and `list_workspaces` tool results have no page route, and the chat link guidance names no project or workspace format > - The agent then writes a bare path or a shortened directory that the person cannot open > - This pull request adds the page `href` to these tool results and adds the project link format to the chat guidance > - The benefit is that chat replies give a clickable link to the real project or workspace page ## Linked Issues or Issue Description **What happened** In chat, a person asked "Take me to the Launch project and show its open tasks." The agent found the project. Its reasoning said "There's no project page route documented explicitly". The reply wrote the project as a bare path `/COM/projects/<id>`, which is not clickable. In a second chat, a person asked where the files of a task are. The agent named the correct workspace but gave a shortened directory and no link. **Expected behavior** The reply links the project page (`/<prefix>/projects/<id>`) and the task's workspace page (`/<prefix>/execution-workspaces/<id>`) as Markdown links, so the person can open them. **Steps to reproduce** 1. Create a company with a project and a task that has a saved execution workspace. Enable isolated workspaces. 2. In chat, ask the agent to take you to the project, or to say where the task's files are. 3. Look at the reply. It has no project or workspace link. Related: Refs #17985 (an earlier open PR that adds project and goal link wording for newly created projects only). This PR covers project discovery and workspaces and adds tool-result hrefs. ## What Changed - `list_projects` rows now carry `href: /<prefix>/projects/<id>`. Both the native runner tool authority and the MCP project-tools route pass the company prefix. - `get_workspace` and `list_workspaces` now carry `href: /<prefix>/execution-workspaces/<id>`, only while isolated workspaces are enabled. The workspace route redirects to the dashboard when the feature is off, so no href is offered then. - The chat link guidance (`AGENT_CHAT_DIRECTIVE`) adds the project link format. It tells the agent to link a returned `href`, and to name a workspace that has no `href` instead of linking it. - Regression tests: guidance text against the registered UI route, `callProjectTool` href, and an embedded-Postgres authority test for project hrefs and the workspace flag on/off. ## Verification - Red then green: the three new tests fail on `origin/master` source and pass with this change. - `npx vitest run` in `server/` for: `agent-conversations`, `task-workspace-api`, `project-tools-pagination`, `task-workspace-tools`, `chat-project-tools`, `dot-runner`, `heartbeat-list` (prompt-size limits), `task-markdown`, `muse-native-visibility`, `paperclip-runner-tool-authority`, `runner-api`, `codex-local-execute`, `board-access-denial`. All pass. - `npx tsc --noEmit -p server` passes. - Live chat eval: see the comment on this PR for the paid runner E2E run of `navigation-preferences.find-project-view` and `workspaces.locate-files` with this change. ## Risks - Low risk. The change adds one optional field to read-only tool results. The output schemas allow extra fields. - The guidance grows by two sentences. The prompt-size test in `heartbeat-list.test.ts` passes. - `list_projects` and the workspace tools now do one extra small read (company prefix, and the instance experimental settings for workspaces). ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), agentic coding with tool use and code execution. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
8fe524f796 |
fix(issues): let the board confirm an unidentified run stopped instead of an endless Retry wait (#20310)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Before a new attempt can start on a task, Paperclip must prove the previous run's agent stopped, so two agents never work the same task at once > - A local native run can be interrupted after it asked to start the runner but before it recorded the runner's process. The runner may have saved state and started a provider, and provider processes can outlive it, so the existing check (#19694) rightly refuses to assume the run stopped > - Today that case has no way out: every Retry is accepted and then waits forever as `process_identity_missing` (storage chaos ST1, t02/t03) > - This pull request makes the wait explicit and actionable. The task explains why it waits, and a board operator who has checked can confirm the run stopped; the parked Retry then starts at the click > - A second shape from the Daytona storage baseline (t06) parked a Retry with no reason at all: a follow-up was interrupted by a graceful shutdown while it was still restoring its workspace. No provider ran, but the restore hold kept the same agent held, so the Retry never started > - The benefit is that no Retry stays accepted-and-waiting with nothing anyone can do ## Linked Issues or Issue Description Refs #19831. Storage chaos wave `w-20261109T145628-s9902` (ST1), tasks t02 and t03. **What happened (CHA-3, run `3c3f6241`):** - The run requested a runner launch (`native.process_start_requested`), and a K2 restart hit before its process identity was recorded. - After the restart, the resumed attempt failed on the storage outage. The task was blocked. - The board's Retry (202) was parked as `executionWait.reason = process_identity_missing`: "The previous run has no verified stop record." - `recordUnidentifiedLocalRunnerStop` cannot prove the stop because this run's runner saved control-plane state. Once that exists, a provider may have started (#19694), and a provider is not guaranteed to carry `--run-id` or `PAPERCLIP_RUN_ID` (the Codex provider env omits it on purpose). The shared owner prover needs a PID, which the run never recorded. - Nothing could ever clear the wait. **Daytona storage baseline `w-20261109T154810-s9911`, task t06:** - The run succeeded. The follow-up was cancelled (`continuation_task_ownership_changed`), and the next run was interrupted by a graceful shutdown (`server_shutdown_interrupted`) 3 s into `preparing`. It was restoring the saved remote workspace and had launched no provider. - Restore recovery saved the files (`workspaceRecovery.status: recovered`, system recovery `complete`). But the `legacy_execution_requires_reconciliation` restore hold releases only for a different owner, or for a native Stop fenced before any provider turn, so it kept the same agent held. - The UI Retry returned 202 and parked with `executionWait.reason: execution_recovery`, the generic wait, so the task showed a Retry that never started. Automatic proof needs the runner to record its identity before saving state and to group provider processes under it. That is follow-up runner work; this PR removes the endless wait now. ## What Changed - `server/src/services/native-local-process-stop.ts`: new `confirmUnidentifiedLocalRunnerStopped`. It accepts only a finished native run of the task with no recorded process and whose latest launch evidence is an unconfirmed launch on this host in a local environment. It still refuses while a runner with `--run-id <run>` is running. It records `native.local_process_stopped` with `basis: "operator_attested"` and the attesting user. That is the receipt the existing continuation gate already accepts. - `server/src/routes/issues.ts`: `POST /issues/:id/recovery-actions/confirm-run-stopped` (`runId`, `stopEvidence` of 20 to 12,000 characters). It is board-only and requires issue read plus `runtime:confirm_owner_stop`, like the other stop attestations. It logs `issue.run_stop_confirmed` with the evidence, promotes deferred intents for the issue, and resumes each parked task Retry, so work starts at the click. Refusals return 409 with a code. - `packages/shared`: `confirmRunStoppedSchema`, and the presentation user action `confirm_run_stopped`. - `server/src/services/task-interruption-presentation.ts`: a Retry parked as `process_identity_missing` for a native run that recorded no process now presents as waiting for the user (`canRetry: false`, `userAction: "confirm_run_stopped"`), with a message explaining why and who can act. A run with a recorded process keeps the existing verification wait. - UI: - `ExecutionBlockerNotice` shows the explanation and, for users who can confirm a stop, "Confirm it stopped" with a short evidence form. - It reuses `PreviousServerStopConfirmation`, now with optional `action`, explicit `runId` and `formId`, and a custom button label. - The Inbox names the action. - `server/src/services/execution-blocker.ts`: `interruptedPreparationBeforeProviderWorkCondition`. The restore hold also releases for the same agent when the stopped run was a system-interrupted preparation (legacy runtime never resolved, still `preparing`) with no process identity, no runner or provider events, no finalization or result, and every lease released with a confirmed stop. It mirrors the existing interrupted-preparation proof and still requires the completed workspace-recovery receipt. Auto-replay is unchanged; this only lets the user's Retry or message start. - Review fixes: - `unidentifiedLocalLaunchRefusal` holds the endpoint's launch, host and lease checks, and the presentation offers `confirm_run_stopped` only when it passes. - The attested stop wakes cost accounting in the same transaction. - The confirmation form is keyed by task and run. - The route is registered in OpenAPI. ## Verification Red/green (each new test fails against master and passes with this PR): - `server/src/__tests__/issue-confirm-run-stopped-routes.test.ts`: - The parked Retry presents `confirm_run_stopped` with the explanation. - Confirming records the operator-attested stop and the activity entry, satisfies the stop check, and resumes the parked Retry: its receipt is coalesced into a newly started run. - It refuses with 409 while a runner for the run is running. - Agents and viewers get 403. - `ui/src/components/ExecutionBlockerNotice.confirm-run-stopped.test.tsx`: the notice offers "Confirm it stopped" (and no dead Retry) and posts the run and evidence. A viewer sees the explanation without the action. - `server/src/__tests__/interrupted-preparation-restore-retry.test.ts` (t06 shape): - The restore hold no longer blocks. - A Retry is accepted and resumed into a started run, and the audit hold keeps `replay: blocked`. - The hold stays when a provider turn started, when a runner event was recorded, when an operator stopped the run, or when the sandbox has no stop receipt. - `issue-confirm-run-stopped-routes.test.ts`: - A case per refusal code checks the presentation keeps the generic wait and the endpoint returns 409. - An `alreadyStopped` case. - An accounting-wake assertion. - The UI test switches targets with stale evidence. Regression and static checks: - `ExecutionBlockerNotice`, `WorkspaceExportRecovery` and its control tests: 74 passed. - `tsc --noEmit` passes for `packages/shared`, `server` and `ui/src`. `pnpm check:token-gates` is clean. ## Risks - A board operator can now attest that an unidentified run stopped. The attestation is permission-gated, requires written evidence, is activity-logged, and is refused while a runner for the run is visibly running. A wrong attestation could start a second agent beside an orphaned provider, the same trust model as the existing previous-server attestations. - The confirmation component now accepts a run without a recovery action. Existing callers pass the action as before. - No schema migration. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edits). ## 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
|
ff2d463eed |
feat(cli): manage agent grants and keep remote login on the public origin (#20323)
## Thinking Path
> - Paperclip lets people control their AI agents and permissions.
> - Remote operators need a browser-approved CLI login.
> - Proxies can send that login's approval link to an internal host.
> - Human member commands cannot grant an agent an explicit permission
such as commentary:view.
> - This change keeps approval on the canonical public origin and adds
Board-only single-grant commands.
> - The API checks company access and permission-management authority
and records each change.
> - An operator can sign in, grant one agent permission, and preserve
the agent's other grants.
## Linked Issues or Issue Description
**What happened?**
A proxied CLI login returned an approval URL on the upstream host. The
signed-in browser was on the public host. The CLI also had no command to
add an arbitrary explicit agent grant: member commands manage human
members, while agent permissions:update exposes fixed configuration
fields.
**Expected behavior**
Approval uses the configured canonical public origin. A Board operator
with users:manage_permissions can grant or revoke one explicit agent
permission without replacing other grants or activating an inactive
membership.
**Steps to reproduce**
Configure a public auth origin. Create a CLI challenge through a proxy
that replaces forwarded-host headers. Inspect the approval URL. Then try
to grant commentary:view to an agent through the CLI.
## What Changed
- Use the existing public-origin resolver for CLI approval URLs.
- Add agent permission:grant and permission:revoke commands.
- Add PATCH /api/agents/{id}/permission-grants with strict shared
validation and Board-only OpenAPI metadata.
- Serialize each mutation on the agent row. Preserve unrelated grants
and membership state. Commit the grant and its activity record together
and publish activity after commit.
- Document remote login and single-grant management.
## Verification
- The new approval-link regression failed before the fix and passes
after it.
- All 25 CLI auth route tests pass.
- All 72 affected CLI, login and OpenAPI tests pass.
- All 5 PostgreSQL grant tests pass, including actual commentary reads
(403 before grant, 200 after grant, 403 after revoke), preservation,
audit attribution, concurrent additions, invalid inputs, inactive or
missing membership, agent self-grants, cross-company denial, and
rollback on audit failure.
- Shared and CLI typechecks and server declaration-layer checks pass.
- pnpm verify requests broad CI coverage because doc/CLI.md is a global
verification input. Greptile reviewed final head
|
||
|
Dotta
|
1b7fe33432 |
fix(workspaces): preserve import verification across process restarts (#20311)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Online import preserves old local files before the web server can use a disposable filesystem. > - A parent import must authenticate saved child archives before it can omit their directories. > - The operation journal preserves import identity, but completed child checks existed only in process memory. > - A container restart therefore repeated downloads, decryption and restoration that had already succeeded. > - This pull request saves a versioned verification receipt on each immutable child snapshot. > - A replacement process can reuse those checks while it reacquires locks and verifies current local files. ## Linked Issues or Issue Description Refs #19854 for verification reuse within a live source fence. Refs #19873 for cohort import. Refs #20298 for the separate remote-hold admission fix. Searched related workspace verification and restart PRs; none provides durable child verification. **What happened?** Replacing a server container interrupts a long online import. The original operation and snapshot survive, but a later invocation downloads, decrypts and restores completed child archives again. The current cache ends with the source-lock owner. **Expected behavior** Completed immutable checks survive process and scratch loss. Resume keeps the same operation and snapshot, reuses only correctly bound verification, and publishes once. Current local files and Git state must still be checked under fresh source locks. **Steps to reproduce** 1. Import two nested child workspaces, then start the parent import. 2. Kill the importer after a child verification finishes. Remove its scratch directory. 3. Resume in a new process. Before this fix, the completed child is downloaded and restored again. **Paperclip version or commit** The regression fails on the previous migration verifier. The final branch includes master at `189fefb99183953be37f0d1084275fbb0f35d6e2`. **Deployment mode** Managed Linux server with PostgreSQL and explicit S3 storage configuration. ## What Changed - Add optional opaque storage namespace identity. S3 binds it to the explicit endpoint, region, bucket, normalized prefix and addressing style. Implicit endpoints retain full verification. - Save a versioned child-snapshot receipt only after canonical crypto, manifest and file restoration succeed and a second HEAD confirms the same strong ETag and size. - Bind reuse to the complete snapshot reference, key material, child lineage, source operation and storage namespace. Require a fresh strong HEAD and canonical validation that the current secret provider can unwrap the saved snapshot key. - Keep current directory/Git capture and source-owner checks on every invocation. Read-only verifier callers do not write checkpoints. - Merge checkpoint metadata under a short row lock without changing parent journal hashes, snapshot retention or update timestamps. Bound reads and writes. Lock timeouts, statement timeouts, deadlocks and serialization conflicts are safe cache misses. Other errors remain fatal. - Extend real PostgreSQL/Git/archive tests with process death, scratch removal, resume, changed bindings and single publication. ## Verification - Baseline RED: two import invocations read each child twice. With durable receipts, each child is read once across both invocations. - Real process regression passed on the final rebased head: SIGKILL the owned importer after one checkpoint, delete all scratch, and resume in a new Node process. It retains the same operation and snapshot, reads each child once across both processes, and records one publication. - 53 focused cases passed in 104.86 seconds on the final production bytes before rebase. They cover existing nested/cohort behavior, namespace identity, changed source/reference/key/lineage/ETag, unknown proof versions, weak validators and bounded row contention. - After clean rebase and stronger scratch/snapshot assertions, the real process case passed again in 18.54 seconds. Server TypeScript, module boundaries and diff checks passed on final head `9da68ee7ce4a778201ef85e21a11c49eeea7bc16`. Required CI supplies final-head broad coverage. - The initial restricted-network dependency setup failed; unchanged preflight passed with normal network access. Initial TypeScript failures in test pipe assertions were corrected. No failed attempt is counted as a pass. - Review found a missing current-master-key check. A real restart regression with all child checkpoints complete reproduced unsafe publication after switching to a different valid master key. Follow-up `7ea92c2abd9ffdb2e7dd81cecf62c8fc7a6e74a5` delegates to the canonical snapshot-key owner before both durable and in-memory cache hits. - The fixed process regressions passed 2/2. Ten key-authority and existing identity/source guard cases passed. The wrong-key process now refuses with zero additional child payload GETs, no publication, and unchanged original journal/snapshot rows. Server typecheck, module boundaries and diff checks passed on the follow-up. - Review also found that transient database failures could make the optional checkpoint cache stop a valid import. Read and save now fall back only for SQLSTATE `55P03`, `57014`, `40P01`, and `40001`. Connection loss, identity changes, crypto failures, programming errors and cancellation remain fatal. Reads use a read-only transaction with one-second lock and eight-second statement limits. - The checkpoint database-fault regression failed two cases before the fix. All four recoverable import cases then passed. The final selected run passed 21 cases, including fatal-error and cancellation controls, real row contention, both process-restart cases and key/identity guards. Server typecheck and boundaries passed. Fault injection is limited to the checkpoint database boundary; the importer, PostgreSQL, Git, archives, storage and publication remain real. - Latest master was merged without rewriting published history. Six real import/process-restart cases passed again on final head `8c8e4213a680ebb5c8d6864416bc3a4f271e1e76` in 38.03 seconds. [Required CI](https://github.com/paperclipai/paperclip/actions/runs/42658248222) finished with 54 successful checks and four intentional skips; Snyk passed. [Fresh review](https://github.com/paperclipai/paperclip/pull/20311#issuecomment-6745354381) is 5/5 with all four threads addressed and resolved. Hosted verification after normal release delivery remains pending. ## Risks - A stale receipt could skip authentication of a different object. Complete reference/lineage/namespace binding, fresh strong HEAD and row-locked save validation prevent that reuse. Missing or unsupported hints trigger full verification. - Mutable local files can change between attempts. Their source locks and fresh captures remain required; a checkpoint never certifies current local bytes. - Checkpoint persistence can contend with other snapshot owners. It has one-second lock and eight-second statement limits; the four classified transient SQL errors retain the normal fully verified path. Other database errors stop the import so lost source ownership cannot be hidden. - No schema, archive format, public API or saved-run identity change. Older snapshots without receipts remain readable and get full verification. Existing data and recovery obligations stay intact. ## Model Used OpenAI Codex, GPT-6.1 Sol (`gpt-6.1-sol`), with reasoning, repository tools and code execution. Context-window size is not exposed by the runtime. ## 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
|
0cf70c9d0c |
fix(chat): tell overviews to state when the picture is from (#20301)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - People ask the CEO chat agent for a quick picture of what the company is doing now > - Such a picture is only useful when the person knows how current it is > - The chat guidance asks for accurate status, but it does not ask for a time > - A live overview said "right now" and gave no time, so the person could not tell when it was observed > - This pull request adds one guidance sentence: state the picture's time as a UTC clock time, from the latest message timestamp > - The benefit is that overviews tell the person when their information is from ## Linked Issues or Issue Description Refs #19614 **What happened** A person asked the chat agent "Give me a quick picture of what the company is doing right now." The agent read agents, projects, tasks and approvals and gave a correct summary. The summary did not say when it was observed. The only time word was "right now". **Expected behavior** An overview of current work states when the picture is from, as a clock time. The native prompt already lists each new comment with its timestamp, so the agent has a time it can use. **Steps to reproduce** 1. Open a chat with the CEO agent in a company that has active, blocked and done tasks. 2. Ask "Give me a quick picture of what the company is doing right now." 3. The reply summarizes the work but gives no observation time. ## What Changed - `AGENT_CHAT_DIRECTIVE` status-update guidance: when the agent describes what the company, a project or agents are doing now, it states a UTC clock time, such as "As of 15:26 UTC". It uses the latest message's timestamp unless a record it read gives a later time. - A regression test checks that the directive contains this guidance. ## Verification - `cd server && npx vitest run src/__tests__/agent-conversations.test.ts src/__tests__/heartbeat-list.test.ts` passes (includes the prompt-size limits). - The chat-catalog eval `activity.company-overview` now grades a real observation time (stacked eval PR on `chat-eval/master-candidate`). This fix is cherry-picked onto that eval branch for the next live run. No live run has used it yet. ## Risks - Low risk. This is one guidance sentence. Overviews get one extra clause with a time. - The time is the message time, not a server clock read. Reads happen seconds after the message, so the difference is small. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), with tool use and code execution in Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandClaude Opus 5.5
|
12523fb677 |
fix(chat): send a task whose automatic recovery stopped to the person's Retry (#20306)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A person asks the chat agent (the controller) to act on tasks that other agents own > - When a task's automatic recovery stops, for example after three runner losses, the task is blocked and the person gets a Retry control on it. Retry rechecks the task's gates and keeps the failed attempt's evidence > - The blocker told every reader "Try again or send a new message to continue". A controller read that as its own instruction. It found that Retry is board-only, then resumed the task with a comment and reassigned the task to itself > - That skips the assignee's budget and hold gates and hides the failed-run Retry from the person > - This pull request names Retry as the person's control in the blocker text, and tells the controller to point the person to it > - The benefit is that "try that task again" leads to the supported, gated Retry instead of an agent takeover ## Linked Issues or Issue Description Refs #20270 (automatic recovery after a lost local runner) **What happened?** In the chat eval `chat-features.runner-opencode.local.recovery.retry-exhausted`, a person asked the CEO agent to "Try the website task again" after the website task's native recovery was exhausted. The agent read the task's `executionBlocker` (`canRetry: true`, nextAction "... Try again or send a new message to continue once the previous execution has stopped."). It found that `POST /api/issues/{id}/retry` is board-only. It then posted a `resume: true` comment, which moved the task from blocked to todo, and reassigned the task to itself to re-run it. The task lost its failed-run Retry control, and the original assignee's gates were not checked. **Expected behavior** The agent links the original task, says why it stopped, and tells the person to use Retry there. It takes the work over only when the person explicitly asks for that. **Steps to reproduce** 1. Let a native run exhaust its automatic recovery (for example, three runner losses). 2. In the controller chat, ask it to try that task again. 3. The agent comments to resume the task, or reassigns it, instead of pointing to Retry. ## What Changed - `execution-blocker.ts`: the text that is added when a person can retry is now `EXECUTION_BLOCKER_PERSON_RETRY_ACTION`: "A person can use Retry on this task, or send it a new message, to continue once the previous execution has stopped. Agents cannot retry it for them." - `agent-conversations.ts`: a new chat guidance paragraph. When another task's automatic recovery has stopped, the supported next step is the person's Retry on that task. The controller links the task and explains why it stopped. It does not reassign the task, change its status, comment on it to resume it, or create a replacement task, unless the person explicitly asks it to take the work over. ## Verification - `board-access-denial.test.ts`: the directive contains the new rule. - `execution-blocker-read.test.ts`: a retryable blocker's nextAction ends with the person-Retry text. - 54 tests pass locally for these two files. - Paid live run of `recovery.retry-exhausted` with this change and #20270 on the eval branch: the run ID will be added below. ## Risks - Low. Only text changes. Any UI that shows the blocker's nextAction shows the new sentence instead of "Try again or send a new message ...". - No schema or saved-state change. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, 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> |
||
|
DottaandClaude Opus 5.5
|
099adb2728 |
fix(recovery): settle a remote workspace save that stopped without finishing (#20291)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Native runs in remote sandboxes save the workspace as snapshots.
Each save is a `workspace_operations` row with phase
`remote_workspace_snapshot`
> - The ownership registry lists that kind as in-progress work, but it
had no settle step
> - When a restart cut off a failing final save, the row stayed
`running` after its invocations had all stopped and its run had ended
> - The retained-source fallback refuses while any workspace operation
is running, so a task whose saved files could not be verified could
never continue from its last save
> - This pull request lets the ownership reconciler settle that row as
failed
> - The benefit is that the automatic fallback can continue such a task
without any user action
## Linked Issues or Issue Description
**What happened?**
A native run in a remote sandbox finished its turn. Its final workspace
save failed because the sandbox disk was full ("No space left on device"
in the snapshot copy). A graceful restart then interrupted the retry.
The run ended as `native_workspace_sync_out_retry_exhausted`. Its
`remote_workspace_snapshot` row kept `status: running` for six days,
with `saveWork: { activeIds: [], unknown: false, quiescedAt: <time> }`
and `exportCleanup: { state: "pending" }`. The retained-source fallback
checked the task and refused with `workspace_operation_running`, so the
task stayed blocked.
**Expected behavior**
A save with no active or unknown invocation, whose run is terminal, is
finished work. The reconciler must mark it failed, so the fallback can
continue from the last committed save. A save that might still be
running must stay untouched.
**Steps to reproduce**
1. Run a native task in a remote sandbox. Make its final save fail, then
restart the server during the retry, so the save's work is quiesced and
the operation stays `running`.
2. Let the task's recovery refuse with an unverifiable-source failure
(for example `allocation_ownership_ambiguous`).
3. The retained-source fallback refuses with
`workspace_operation_running` on every check, and the task never
continues.
**Paperclip version or commit**
`master` at
|
||
|
DottaandClaude Opus 5.5
|
ef2b8d5324 |
fix(issues): make every Retry start work or say why (#20283)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - When a task's run fails, the board recovers it with a Retry button, so that button must either start work or say why it did not > - An audit of every Retry-type button found three gaps. The task Retry shown during a scheduled retry's countdown returned "accepted" but changed nothing. A stale Retry looked like success in the Inbox. "Retry now" skipped the task's read and change checks, so a member who cannot see a private task could start its retry > - This pull request makes the task Retry start a scheduled retry immediately, makes every Retry that did not start work say why, and gives "Retry now" the same authorization as the task Retry > - The benefit is that no Retry click silently does nothing ## Linked Issues or Issue Description Refs #19831. The board reported a dead Retry button. A read-only audit of every Retry button confirmed: - **Task Retry during a scheduled retry was a no-op.** `TaskInterruptionPresentation.canRetry` is true in `retry_scheduled`, so `ExecutionBlockerNotice` shows Retry. `requestTaskRetry` treats `scheduled_retry` as busy and inserts a `coalesced` wake pointing at that run. `resumeTaskRetry` reads only `deferred_issue_execution`, and dispatch uses coalesced user wakes only to carry comments. Nothing promoted the run: the API answered 202 `accepted: true` and the countdown did not change. - **A stale Retry looked like success.** `accepted: false, stale: true` comes back as HTTP 200. The Inbox ignored `accepted` and opened the task as if work had started. The task page showed no reason. - **"Retry now" checked less than the task Retry.** It ran only `assertBoard` and `getAccessibleResource`, skipping `assertIssueReadAllowed` and `issue:mutate`. A company member who cannot read a private task could promote that task's scheduled retry. - **"Retry now" said nothing for some outcomes.** It toasted only `promoted` and `gate_suppressed`. ## What Changed - `server/src/services/task-retry.ts`: when the Retry coalesces into the task's latest run and that run is `scheduled_retry` (with no other Retry already active), the receipt returns `scheduledRetryRunId`. - `server/src/routes/issues.ts`: - `POST /issues/:id/retry` then promotes that run through the existing `retryScheduledRetryNow`, the same path "Retry now" uses. It logs `issue.scheduled_retry_retry_now` with `source: "task_retry"` and returns `scheduledRetry: { outcome, message }`. - `POST /issues/:id/scheduled-retry/retry-now` now runs `assertIssueReadAllowed` and `decideIssueAccess("issue:mutate")`, matching `/retry`. A private task the member cannot read now answers 404. - `ui/src/hooks/useTaskRetry.ts`: new `taskRetryNotStartedReason`. When a Retry did not start work (stale, or a scheduled retry that is gate-suppressed, waiting, or gone), the hook returns that reason as its error. `ExecutionBlockerNotice` and `StalledDependencyNotice` already display it. - `ui/src/pages/Inbox.tsx`: a task Retry that did not start work shows the reason in the error toast and does not navigate. - `ui/src/hooks/useRetryNowMutation.ts` and `IssueScheduledRetryCard.tsx`: every "Retry now" outcome now shows its headline and message, including `no_scheduled_retry` on the card. - `ui/src/api/issues.ts`: `retryTask`'s response type includes `scheduledRetry`. ## Verification Red/green (each new test fails against master and passes with this PR): - `server/src/__tests__/issue-scheduled-retry-routes.test.ts`: - "starts a scheduled retry now when the task Retry is clicked during its countdown": 202 with `scheduledRetry.outcome: "promoted"`, the same run becomes `queued`, and no second retry run is created. - "refuses Retry now on a private task the member cannot see, like the task Retry": on master "Retry now" answers 200 `promoted`; now both routes answer 404 and the run stays `scheduled_retry`. - The seed now adds the board user's membership, which the stricter route needs. All 33 tests pass. - `ui/src/hooks/useTaskRetry.test.tsx`: a replaced attempt, a gate-held scheduled retry and a waiting scheduled retry each show their reason; a promoted one shows nothing extra. - `ui/src/pages/Inbox.test.tsx`: an Inbox Retry that did not start work (stale, or gate-held) toasts the reason and does not navigate. - `ui/src/components/IssueScheduledRetryCard.test.tsx`: "explains when there was no scheduled retry left to start". Other Retry buttons: the chat failed-run Retry already reports stopped and failed retries (`agents.test.ts`). Agent lifecycle "Retry now" and cost "Retry accounting" start no task work and already surface their API errors. Regression and static checks: - UI: 7 files, 168 passed. Server: task-retry and scheduled-retry suites, 149 passed. `issue-agent-mutation-ownership-routes.test.ts` passed. - `maintenance-server-routes.test.ts` "assembled maintenance server production wiring" fails identically on master on this Mac (`ensure-build-deps`). - `tsc --noEmit` passes for `server` and `ui/src`. `pnpm check:token-gates` is clean. ## Risks - A task Retry during a scheduled retry now starts the run immediately, through the existing "Retry now" promotion gates (pause, budget, cleanup waits). A gate that holds it is reported, not bypassed. - "Retry now" is stricter: a member without read or change access to the task gets 404 or 403, as on `/retry`. - No schema change. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edits). ## 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
|
d10c6e0e8b |
fix(chat): hold a chat message at the agent's daily limit instead of dropping it (#20292)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Agent Chat sends each person's message to the conversation agent through a durable outbox (`deliverConversationComments`). The outbox keys each wake receipt as `conversation-comment:<commentId>`. > - The outbox treats any receipt with that key as delivered. Queue admission writes a `skipped` receipt when the agent reached its daily run or cost limit. > - So a chat message sent after the agent used its daily allowance was consumed silently. No run started, the chat showed nothing, and nothing resumed it. > - A person's message must never be silently dropped. The daily limit is an operator control, so the message must wait honestly, not bypass the limit. > - This pull request makes the outbox hold the message at the daily limit, like it already does for a paused agent, and resume it when the limit resets. > - The benefit is that a chat message at the daily limit is saved, shown with its reason, and sent automatically at 00:00 UTC. ## Linked Issues or Issue Description No public issue exists. Inline bug report: **What happened** In a chat eval, the conversation agent (CEO) had `maxDailyRuns: 3`. After three chat turns and a separate failed-model phase on another agent, the person sent a fourth message. The comment was saved (POST 201), but no CEO run started. The conversation stayed `active` with no `executionRunId`. The ownership reconciler later reported `I7_visible_next_action`. The server log shows no wake error. Queue admission had written a `skipped` receipt (`heartbeat.daily_run_limit`) under the outbox key, so the outbox counted the message as delivered. **Expected behavior** A chat message that reaches the agent's daily limit is not consumed. It stays saved, the chat shows why it waits, and it is sent automatically when the limit resets. The daily limit still applies: the message does not start a run early. **Steps to reproduce** 1. Set a conversation agent's `runtimeConfig.heartbeat.maxDailyRuns` to N. 2. Send N chat messages and let each turn finish. 3. Send one more chat message. 4. See that no run starts, the chat shows no waiting state, and the message is never answered (a `skipped` wake receipt with reason `heartbeat.daily_run_limit` exists for `conversation-comment:<id>`). ## What Changed - `server/src/services/agent-conversations.ts`: add `conversationDeliveryHold`, which applies the same daily run/cost limit check as queue admission. The outbox returns `deferred` at the limit and writes no receipt, so the message stays owed. It reports the next UTC day start through a new optional `onDeferredDeadline` option. - `server/src/services/recovery/service.ts`: stranded-work recovery passes its `onDeferredDeadline` to the outbox, so the held message is retried when the limit resets. - `server/src/services/heartbeat/queue.ts`: if the limit is reached while an outbox wake is in admission (a race), admission throws a 409 with `reason: heartbeat.daily_*` for `conversation-comment:` keys instead of writing a `skipped` receipt. The outbox already maps a matching 409 to `deferred`. Other wakes keep the current skip behavior. - `server/src/routes/issues.ts` (`GET /issues/:id/queued-comments`): when a chat has no queue receipt but has messages held by the daily limit, return them as a `deferred` queue with `executionWait` (reason and message). Edit, send and discard stay off because the outbox owns delivery. - `ui/src/components/task-chat/TaskChatQueuedMessages.tsx`: show the `executionWait` message for such an outbox-held queue (no queue id and no target run). Receipt-backed queues keep the current behavior and do not duplicate the task status. - Tests: three real-database tests in `agent-conversation-delivery.test.ts` (hold and resume after the limit resets, recovery deadline, and a limit reached during real `heartbeatService.wakeup` admission) and one UI test. ## Verification - `cd server && pnpm exec vitest run src/__tests__/agent-conversation-delivery.test.ts`: 10 passed. Without the source change, the three new tests fail (`expected 'delivered' to be 'deferred'`; the real queue path writes the skipped receipt). - `cd server && pnpm exec vitest run src/__tests__/issue-recovery-conversations.test.ts src/__tests__/agent-conversations.test.ts src/__tests__/heartbeat-undispatched-execution-wait.test.ts src/services/heartbeat/queue.test.ts src/__tests__/agent-chat-messages.test.ts`: 215 passed. - `cd ui && pnpm exec vitest run src/components/task-chat/TaskChatQueuedMessages.test.tsx`: 36 passed. - `pnpm exec tsc --noEmit -p server/tsconfig.json`: clean. `node scripts/check-token-gates.mjs`: clean. - Manual: give a chat agent `maxDailyRuns: 1`, send two messages. The second message shows in the queued messages bar with "<agent> reached its daily run limit. Your message is saved and sends automatically when the limit resets at 00:00 UTC." ## Risks - Behavior change for chat only: a chat message at the daily limit now waits in the outbox instead of being consumed. It is sent after 00:00 UTC. If an operator raises the limit, the message is sent on the next recovery pass for that chat; the UI message does not promise an earlier send. - Saved state from the previous release: no persisted shape changes. Messages that an earlier release already consumed with a `skipped` receipt stay consumed. They are not replayed later, which avoids sending old messages without notice. - The queued-comments read adds one agent read and one daily-limit count for a chat with no queue receipt. This is the same query that admission runs. - The recovery deadline is in-process; instance-sleep scheduling does not get a new deadline from this change. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), agentic coding 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 - [ ] 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 LeachandPaperclip
|
052c638b51 |
fix(environments): honor stopped-task proof after warm preparation (#20289)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - A warm plugin Runner retains its task for later runs. > - A follow-up can find that the retained task has stopped. > - The resume path already asks settle to check status before it sends stop. > - A saved startup marker disabled this order, even after warm preparation had finished. > - This pull request applies the same stop-proof check to all observed-stop releases. > - The next run skips a repeated stop when status proves both execution and required preparation have stopped. ## Linked Issues or Issue Description Fixes: #20224 Refs #20203 ## What Changed - Remove the startup-marker exclusion from the existing status-first path. Keep the existing shared confirmation check. It still requires `preparationStopped: true` for startup work. - Keep stop followed by status when the first status lacks the required proof. Keep the startup drain, credential writeback, complete, and cleanup failure handling. - Add retained-task coverage with the startup metadata that warm preparation saves. Cover a startup cleanup whose first status lacks preparation proof. - Update the plugin task lifecycle contract. ## Verification - Local macOS, source base `3e8744fa91`: from `server`, `pnpm exec vitest run src/__tests__/environment-task-runtime.test.ts src/__tests__/plugin-warm-tasks.integration.test.ts src/__tests__/plugin-task-credential-writeback.test.ts` passed: 3 files, 82 tests. The integration tests use real embedded PostgreSQL and controlled plugin results. - After restoring the fix, the two affected files passed again: 73 tests. - Independent parent verification on commit `19f720ff68`: the two files filtered to the changed startup-proof and retained-task cases passed (4 tests). Independent source review found no actionable issue. - Baseline proof: with only the source condition restored to upstream, the new retained-task test with saved startup metadata fails because it sees an extra `stop`. The case without that metadata passes. The source fix was then restored. - The retained-task regression asserts exactly `environmentResumeLease`, `status`, `complete`, and `environmentAcquireLease`, with both task operations bound to the old task. The startup test also proves that missing preparation proof sends `stop` and a second `status`. - Test setup: `node scripts/agent-preflight.mjs --skip-rust-debug --explain` installed dependencies, then stopped because Cargo is unavailable on this Mac. Initial test collection failed because the plugin SDK output was absent. `pnpm --filter @paperclipai/plugin-sdk build` built that required dependency and the shared package; the targeted test run then passed. - Standard Greptile reviewed commit `19f720ff686058b4dd6b34f27cdfb9c8ff60e50a` at 5/5 with no review threads. [Review](https://github.com/paperclipai/paperclip/pull/20289#issuecomment-6743829614). - Full local checks were not run, as requested. [GitHub PR workflow 42644023868](https://github.com/paperclipai/paperclip/actions/runs/42644023868) passed on final commit `19f720ff686058b4dd6b34f27cdfb9c8ff60e50a`. It supplied broad typecheck, build, and test coverage. The separate Snyk status passed. Optional Storybook checks were skipped. - The change removes one provider stop call on the reported path. The issue's measured eight calls would become seven if other calls stay the same. The earlier five-call target is not claimed. No deployed latency measurement is claimed. ## Risks - A provider that cannot prove preparation stopped still gets stop and status. That fallback can cost one extra status call, as the existing status-first contract already allows. - A status failure keeps cleanup pending and blocks replacement. No termination proof or credential handoff is skipped. - No schema, API, dependency, or timer change. No added work on successful task reuse or ordinary task dispatch. - Live provider acceptance remains unrun. The local evidence checks orchestration and stop-proof handling. ## Model Used - OpenAI Codex, GPT-6 family, with reasoning, repository inspection, shell tools, and code execution. The exact serving model ID and context window are not exposed by this session. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
DottaandClaude Opus 5.5
|
dc9ad17515 |
perf(storage): time workspace snapshot save and restore steps and count storage retries (#20280)
## Thinking Path
- Cloud durable storage is a major source of failures and slowness on
staging. For example, a staging task failed when saving files after
automatic retries.
- Before measuring or fixing it, we need to see where storage time goes.
Today a saved-workspace restore before the turn appears only inside the
coarse `workspace_environment_realize` phase.
- After the turn, the save's encrypt, put, verify and multipart throttle
backoff have no timing at all. Only `captureMs`, `transferMs`,
`verifyMs`, `uploadMs` and `publishMs` exist, in `saveTiming`.
- Storage retries happen in the SDK and in the multipart uploader,
invisibly.
- This adds the measurement first, so the storage-mode chaos waves
(MinIO behind a fault proxy) can report save and restore p50/p95 and
point at the dominant wait.
## Linked Issues or Issue Description
**What happened?**
Durable-storage saves and restores were opaque in the run log and save
timing. A slow or repeatedly retried storage call could not be told
apart from slow capture, encryption or sandbox work.
**Steps to reproduce**
1. Run a remote task with S3-compatible durable storage, so it saves
after the turn and restores on the next run.
2. Inspect the run log and the `remote_workspace_snapshot` operation's
`saveTiming`. The restore's download, decrypt and verify, and the save's
put, verify and throttle backoff, have no timing.
**Expected behavior**
Each storage step has a duration, and storage request retries and
backoff are counted.
## What Changed
- `server/src/storage/request-stats.ts` (new): an async-local collector
of storage request statistics. Counts only:
- `requests`, `attempts` and `failedRequests`;
- `retryDelayMs` (SDK waits);
- `parts`, `throttled` and `backoffMs` (the multipart uploader's waits).
- `server/src/storage/s3-failure.ts`: `withS3FailureDiagnostics`, which
wraps every S3 send, records each request's SDK `$metadata.attempts` and
`totalRetryDelay`, on success and on failure.
- `server/src/storage/s3-multipart.ts`: records each uploaded part and
each resend wait, flagged throttled when the backend returned 503, 429
or SlowDown.
- `server/src/services/remote-workspace-snapshot-storage.ts`:
- Each step is a `storage.snapshot.*` run phase:
- save: `prior_attempts`, `encrypt`, `digest`, `put`, `verify`;
- restore: `download`, `decrypt`, `verify_plaintext`, `publish`.
- A caller can pass a `timing` record that receives the step durations,
the bytes and the request statistics.
- `server/src/services/remote-workspaces.ts`:
- The save passes a timing record, kept in `saveTiming.storage`.
- The sandbox unpack during a restore is its own phase,
`workspace.snapshot.sandbox_restore`.
- `execution-preparation-timing.ts`: the new phase names.
- `doc/run-log-events.md`: a new "Durable storage timing" section.
Because the restore runs inside run preparation, its steps of 50 ms or
more become `run.performance.span` rows with no extra plumbing. This is
run-log and instance-database data only; there's no Telemetry change.
## Verification
- `server`: `storage-s3-multipart-backoff`, `storage-s3-multipart`,
`storage-s3-multipart-throttle`, `storage-s3-failure-diagnostics`,
`remote-workspace-snapshot-storage` and `execution-preparation-timing`
tests: 86 passed. They include:
- collected stats for a throttled multipart upload: 12 requests, 7
failed, 7 throttled, 35.5 s backoff and 3 parts;
- a save and restore that fill every step duration and the byte count.
- `remote-workspaces.integration.test.ts`: 112 passed. Its `saveTiming`
assertion now includes `storage`.
- Server typecheck: no errors in the touched files.
## Risks
- **Overhead:** one async-local lookup per S3 request, plus timestamps
per snapshot step.
- **Collector scope:** the collector only exists inside a snapshot save
or restore. Other storage callers are unaffected; they record nothing.
- **`saveTiming`** gains a `storage` object. Readers that compare it
exactly (one test, updated) will see the new field.
## Model Used
Claude Opus 5.5 (`claude-opus-5-5`, Anthropic), 1M context window, tool
use and code execution, via Claude Code.
## Checklist
- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [ ] All Paperclip CI gates are green
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
|
||
|
DottaandClaude Opus 5.5
|
3ef45fdd92 |
fix(recovery): let a new task wait for its manually paused assignee (#20281)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - A person can pause an agent. Tasks assigned to a paused agent wait until the person resumes it. > - Stranded-task recovery looks for assigned work that cannot continue and moves it to `blocked` for the board. > - A task created for a manually paused agent has no run yet. Recovery saw an assignee that is not invokable and moved the new task to `blocked` a few seconds after creation. > - The person paused the agent on purpose. The blocked task and its "the board must choose the next action" comment fight that decision, and the person must unblock the task by hand after resume. > - This pull request lets new `todo` work with no run wait while a manual pause lasts. The existing assigned-todo dispatch starts it when the agent is resumed. > - The benefit is that work queued for a paused agent starts on resume, as the pause promises. ## Linked Issues or Issue Description **What happened** A chat evaluation pauses an agent (Morgan) and then runs `paperclipai issue create ... --assignee-agent-id <Morgan> --status todo`. The CLI returns the task with status `todo`. Within about five seconds the task changes to `blocked`, with the system comment "Paperclip cannot safely continue automatic recovery because the original assignee is not invokable. The source assignment is unchanged and the board must choose the next action." The server log shows only the expected 409 "Agent is not invokable" for the assignment wake. **Expected behavior** The task stays `todo` while the person keeps the agent paused. When the person resumes the agent, the task starts once. A comment and a `blocked` status are not added. **Steps to reproduce** 1. Pause an agent from its page. 2. Create a `todo` task assigned to that agent. 3. Wait for the stranded-task recovery sweep. See that the task is `blocked` with the comment above. ## What Changed - `server/src/services/recovery/service.ts`: in `reconcileStrandedAssignedIssues`, skip a `todo` task with no run when its assignee is paused with a `manual` lifecycle hold. The existing rule above it already skips work whose run the pause cancelled. Other pause causes, such as a budget hold, keep the existing escalation. - `server/src/__tests__/heartbeat-paused-assignee-new-task.test.ts`: a PostgreSQL test. The task stays `todo` with no comment and no run during two recovery sweeps. After resume it starts exactly once and finishes. A budget-held assignee still escalates. ## Verification - `cd server && npx vitest run src/__tests__/heartbeat-paused-assignee-new-task.test.ts`: 2 passed. Without the fix, the first test fails with `expected 'blocked' to be 'todo'`. - `npx vitest run src/__tests__/issue-recovery-actions.test.ts src/__tests__/heartbeat-refused-assignment-wake.test.ts`: passed. - `src/__tests__/issue-tree-pause-resume-run.test.ts` had timing failures on this low-memory machine both with and without this change (different tests each time). Those tests do not pause agents, so this change does not reach them. CI runs them on this branch. ## Risks - Low risk. The skip needs all of these: the agent is paused, it has a `manual` hold, the task is `todo`, and the task has no run. Work that already ran keeps the current rules. - No persisted state changes shape. State saved by the previous release carries forward: a task that the previous release already moved to `blocked` stays `blocked`. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), with tool use and code execution in Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Related: #20143 (a pause's stop never blocks the task behind an incident), #9998 (open: reject paused agents as assignment targets). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |