mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-13 07:12:00 +01:00
e879a24293d751ff0d64161ffddccdede200e423
480
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
5a7378af56 |
fix(heartbeat): re-drive a queued run whose dispatch signal was lost after a restart (#20986)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The heartbeat queue starts queued runs. The run-dispatch worker re-runs the queue only when a write sends a "queued run dispatch" signal. > - A follow-up run can queue while its predecessor still holds its sandbox lease. The claim then declines the run without a log line, and the run stays `queued`. > - After a controller restart, nothing sent a new signal once that lease was settled. The run stayed `queued` forever. The issue had no Retry, and the reconciler counted the queued run as a valid next action. > - This pull request sends a dispatch signal from the native finalization writes that can clear a predecessor's cleanup gate, logs why a claim was declined, and bounds the wait. The dispatch worker re-drives an undispatched queued run after 60 s as a backstop, and the ownership reconciler reports and re-drives one still queued after 5 minutes. > - The benefit is that a follow-up starts within seconds of its predecessor settling, a lost signal delays it by about a minute instead of forever, and a run that still cannot start is visible. ## Linked Issues or Issue Description Refs #20909, #20935, #20953, #20970 **What happened?** Restart-chaos wave `w-20261111T223714-s9512` (master `6154b9485e` + #20907 + #20909, Daytona warm, Codex, W2, CR1 with a 360 s hold, `--ephemeral-server`, seed 9512), task t02. Run `0b95b1cd` crossed the outage with an accepted result. The next controller finalized it at 22:47:59Z. Its continuation run `1ad79418` was queued at 22:48:00Z (`retryOfRunId` = `0b95b1cd`, wake `d967700c`, source `native_status_decision`). The run stayed `queued` with `claimed_at = null` until the wave ended 19 minutes later. The agent stayed `running` with no running run. The task had no Retry. The grade was `R1:no_rest_within_budget` and U1 `no_owner_no_action ... queued 485s`. **Root cause** `claimQueuedRun` (`server/src/services/heartbeat/queue.ts`, `lockIssueExecutionClaim`) declines a queued run while `hasPendingRunCleanup` (`server/src/services/execution-blocker.ts`) is true for its predecessor. After the restart, the predecessor's warm `reuse_by_environment` lease was still `active` when the continuation queued, so the first claim was declined. The decline is correct, but nothing owned the retry. `registerRunDispatchWork` (`server/src/services/heartbeat/dispatch-work.ts`) has `next: async () => null`: the queue is only re-run by a later dispatch signal. At wave end the lease was `retained` with `cleanupStatus: success`, which does not block a claim. A dispatch pass in that state starts the run (verified locally). So no pass ran after the gate cleared. The ownership reconciler listed the run as `queued_run` with owner `run_dispatch` (always "alive"), and I7 treats any queued run as a next action, so nothing reported it. This is not the #20935 throttle. The wave base predates #20935, and a throttled intent keeps its wake intent `queued` with a `nextAllowedAt` deadline. Here the run row existed and its claim was never retried. On current master #20935 parks this specific `response_wake` self-wake, but any continuation, comment wake or Retry that queues behind a settling predecessor after a restart has the same gap. The red test uses a `same_agent` continuation, which master still enqueues. **Steps to reproduce** `server/src/__tests__/heartbeat-native-restart-suspension.test.ts`, "starts a continuation that queued behind its predecessor's sandbox release after a graceful restart". ## What Changed - Event-driven dispatch. A lease release already signals queued-run dispatch (`environmentsSvc.releaseLease`, `writeEnvironmentCleanup`, stop-and-retain settlement). I enumerated every `environment_leases` status writer: each one that moves a lease out of a blocking state already signals. The writers that do not signal only acquire a lease or set `pendingCleanupInFlight`, so they add a block and do not clear one. The native coordinator side had no signal, so: - `notifyNativeFinalizationWork` (the shared notifier of coordinator commits and terminal transitions) now also signals queued-run dispatch for the company. - The coordinator writes that clear `leaseOwner` or end the phase outside that notifier now signal in the same transaction: `releaseLapsedCoordinatorOwner`, native startup cleanup, observed-workspace preservation, `record_finalization_error` in the interaction bridge, the wake-queue terminal recovery, and the reconciler's ambiguous-state terminal failure. - Not changed: three non-transactional pre-dispatch closes in `run-completion.ts` and `agent-execution-stop.ts`. Their callers queue or settle the successor after the write, so the successor's own enqueue starts the queue. - `claimQueuedRun` now logs the reason when it declines a queued run at the issue execution claim (`predecessor_cleanup_pending`, `predecessor_still_executing`, `issue_execution_owned_by_another_run`, case handoff, task chat cleanup), when a predecessor is still settling in this process, and when a temporary budget hold keeps it queued. - New `server/src/services/queued-run-dispatch-liveness.ts`: `listUndispatchedQueuedRuns` reads queued runs in active companies whose ready, non-paused agent has no running run. It reads every page with a keyset cursor until the set is exhausted, so older held runs cannot hide a newer stranded run. `createQueuedRunRedriveSchedule` re-drives each such run 60 s after it was created, then every 2, 4, 8 minutes, capped at 15 minutes. Run ids that leave the set are dropped. - `server/src/services/heartbeat/dispatch-work.ts`: the queued-run dispatch worker's `next` now plans those re-drives. It re-reads only the companies the pass just dispatched, keeps the deadlines of every other company, and runs no query when the pass dispatched nothing. Only an instance-wide pass (startup or explicit wake) reads every company. It adds each due company to the dispatch scopes and returns the next due time. - `packages/db`: new partial index `heartbeat_runs_queued_created_idx (company_id, created_at, id) WHERE status = 'queued'` (migration `0470_heartbeat_runs_queued_created_index.sql`, `CREATE INDEX CONCURRENTLY IF NOT EXISTS`, registered as a top-level migration in `client.ts`). It returns `null` (no deadline, no SQL until a signal) when no run is undispatched or scheduling is suppressed. Every re-drive goes through the normal `startNextQueuedRunForAgent` and `claimQueuedRun` gates, so it never bypasses a hold, budget, pause or cleanup gate. - `server/src/services/heartbeat.ts`: wires `listUndispatchedQueuedRuns` into the worker (two lines). - `server/src/services/recovery/ownership-invariant-report.ts`: new check. A queued run still undispatched 5 minutes after creation is an I7 violation (`kind: queued_run`, `reason: queued_run_not_dispatched`), written to the activity log once. A younger one sets the report's next due time. - `server/src/services/native-runtime/native-finalization-reconciler.ts`: `applyOwnershipReportBackstops` sends a queued-run dispatch signal once, for each company with a newly found violation. Repeat passes do not signal again, so a held run does not wake the report every 30 s. - `tests/restart-chaos/README.md`: records the wave and the fix in the CR results table. The existing CR1 cell (#20909) and the enforced U1 check already cover this class in the harness. ## Verification - RED on master (`23909077ad`): - `heartbeat-native-restart-suspension.test.ts` "starts a continuation that queued behind its predecessor's sandbox release after a graceful restart": the continuation is `queued` behind the predecessor's `active` warm lease. The run-dispatch worker is registered, the lease then settles to `retained`/`success` with no dispatch signal, and the run stays queued: `AssertionError: expected null not to be null` after 75 s. - `ownership-reconciliation-report.test.ts` "reports and re-drives a queued run that no dispatch started while its agent had nothing running": `expected [] to deeply equal [ ObjectContaining{…} ]` (no violation for a run queued 19 minutes). - `ownership-reconciliation-report.test.ts` "signals queued-run dispatch when finalization work settles or a finished coordinator's lapsed owner is released": `expected [] to include 'settled'`. - GREEN on this branch: all pass. The restart test has two cases. When the lease is released through `releaseLease`, the continuation starts within 5 s. When the lease is settled by a write with no signal, the backstop starts it within 75 s. A new report test puts three older held runs ahead of a stranded run with a page size of 2, finds the stranded run, and checks that the backstop signals once. - `queued-run-dispatch-liveness.test.ts`: schedule bound, doubling, cap, and going quiet. - Focused suites pass (22 files, 495 tests): recovery-deadline-acceptance, live-recovery-work, heartbeat-run-dispatch-lifecycle, heartbeat-run-dispatch-work, deferred-execution-receipt-work, heartbeat-native-cleanup-admission, the ownership kinds suites, and others. `heartbeat-native-restart-suspension` and `ownership-reconciliation-report` pass in full. `tsc --noEmit` for the server passes. ## Risks - A queued run that is held on purpose (for example a temporary budget hold or a case handoff waiting for release) is now re-checked at most every 15 minutes after the first minute. Each re-check is one indexed query and the normal claim gates. Such a run is also reported once as an I7 violation. - An instance with an undispatched queued run keeps a dispatch deadline, so it does not idle while that run waits. An instance with no such run is unchanged. - Schema: one additive partial index, built concurrently. No data change. State saved by an older version is read as is. Nothing runs before `turn.start`. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edits, test 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> |
||
|
|
8818402e7a |
feat(inbox): route requests to people and record personal outcomes (#20959)
## Thinking Path > - Paperclip helps people manage AI agents and their work. > - The inbox collects requests that need a human decision. > - Teams need to know who owns each request and who may act on it. > - A shared close operation must differ from hiding a card for one person. > - This pull request adds personal routing and durable outcomes to the existing attention service. > - People can claim or transfer work and inspect what another person did for them. > - Domain permissions still decide who may resolve each request. ## Linked Issues or Issue Description **Subsystem affected** Server, shared contracts, and database. This is the routing foundation for a stacked inbox UI change. **Problem or motivation** A company-wide attention feed does not give teams a clear personal queue. Archive, withdrawal, transfer, and decisions by another person need distinct scopes and records. **Proposed solution** Resolve a request audience from its addressee, task participants, and configured owners. Provide personal, unassigned, company, all-assignee, and archived views. Add claim, transfer, personal archive and snooze, shared withdrawal, receipts, and personal history. **Alternatives considered** A single shared inbox leaves ownership and personal hiding ambiguous. The service keeps one canonical request and projects its audience instead of copying requests for each person. **Roadmap alignment** This extends the existing Multiple Human Users and Agent Reviews and Approvals capabilities in ROADMAP.md. **Additional context** Refs #19363 and #19520 for the current inbox navigation and confirmation replacement rules. Refs #20268 for related actionable-request work. That PR filters actionability; this stack adds audience routing and personal outcomes. This is part 1 of a two-PR stack. The inbox UI PR #20961 targets this branch. ## What Changed - Add nullable company and project decision owners and a company budget owner. - Add audience views and personal badge counts to the canonical attention projections. - Add claim, transfer, withdrawal, personal archive, snooze, and restore operations with permission checks and activity records. Lock addressed transfers to the current addressee or routing administrators, and validate the recipient against canonical resolution policies. - Add receipts and scoped history for work handled by another person. - Keep closed-task requests and replaced confirmations out of current cards. - Add an idempotent migration and a concurrent receipt-history index. - Update typed fixtures so the existing UI remains compatible with the new contracts. ## Verification - Server layered typechecking passes, including all three test-tree partitions. - Seven routing and interaction-resolution suites pass all 249 tests against a dedicated external PostgreSQL test database on `4caecc21d8`. Six new regressions failed before the repair; they cover takeover followed by approval, legitimate delegation, excluded creators/review requesters, and unauthorized claims. - Prior focused verification covers private approval history and receipts, transfer visibility, audience escalation, withdrawal governance, hidden-card counts, per-recipient confirmation replacement, and SDK source contracts. - A synthetic three-person company passed 37 routing and lifecycle acceptance checks. The UI stack adds browser acceptance for the shared controls. - The complete [GitHub Actions CI matrix](https://github.com/paperclipai/paperclip/actions/runs/43134997331) passes on `4caecc21d891677323a7335f23b1a015cce24c13` (Linux/Node 24: typecheck, build, general and serialized tests, Runner support, browser shards, and aggregate gates). Greptile is 5/5 on that exact head with no open findings. The fresh security scan passes after explicitly rechecking the addressee-transfer repair. - `pnpm verify` reports exit 2 because a CI-global script changed; full CI supplies that broader coverage. Embedded PostgreSQL could not start on this Mac because shared-memory slots were exhausted, so local database checks use a separate external test database. Linux CI covers the embedded setup. ## Risks - Personal badge counts change to the viewer's assigned requests. Accessible feed totals and task membership keep their existing scope. - The migration adds nullable owner fields and a dismissal fingerprint. Its index runs outside a transaction. The migration is classified and rerunnable. - Routing changes the addressee used by resolution authorization. The current addressee or a routing administrator must authorize an addressed transfer; the recipient must satisfy writer access, canonical resolver exclusions, and task review policy under row locks. - The shared validator export barrel crosses 1,000 lines because it exports the new attention schemas. Schema implementation stays in the dedicated attention validator module. ## Model Used OpenAI GPT-6 through Codex. Used repository tools, code execution, and browser checks. This session does not expose a more specific runtime model ID or its context-window size. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links - [x] My branch name describes the change and contains no internal Paperclip ticket id - [x] I have run focused tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes (OpenAPI contracts and docs/api/attention.md) - [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: Scott Tong <scott@scottsmbpm5max.lan> Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
|
e8e548e856 |
feat(db): classify migrations for application under the running release (#20913)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - `packages/db` owns the schema and its migrations. There are 467
migrations, and several ship each day
> - A zero-downtime deploy starts the new release while the previous
release still serves the same database
> - Core does not say whether a migration is safe to apply under the
previous release. A deployer must treat each pending migration as a
reason to pause, or a person must review it
> - On stacks that may not pause, releases wait for hours
> - Core owns the schema, so Core must own this knowledge and publish it
as data
> - This pull request adds a conservative classifier, an in-file
override, the published result, and a guard for new migrations
> - The benefit is that a deployer can apply routine compatible
migrations automatically, and incompatible changes become visible at
review time
## Linked Issues or Issue Description
**Subsystem affected**
Database migrations (`packages/db`), the cloud migrator artifact
(`scripts/cloud-migrator-artifacts.mjs`), and the pull-request policy
job.
**Problem or motivation**
Core has no machine-readable statement that a migration can be applied
while the previous release still reads and writes the database. A
deployer cannot tell an added nullable column from a dropped column. It
must pause for both, or refuse the release.
**Proposed solution**
Classify each migration as `online_compatible`, `requires_staging`, or
`unknown`. Publish the result in the migrator manifest. Fail a pull
request when a migration it adds is `unknown` and has no reviewed
override in the file.
**Alternatives considered**
- A hand-kept list in the deployer. It goes stale and it is not reviewed
with the SQL.
- A sibling JSON file in the artifact. It needs a workflow change, and
the attestation of `manifest.json` would not cover it.
**Roadmap alignment**
This is release infrastructure. It adds no product feature.
## What Changed
- `packages/db/src/migration-compatibility.ts`:
`classifyMigrationSql(sql)` returns `{ compatibility, classified,
reasons, override, overrideError }`. It parses statement by statement
(breakpoints, `;`, comments, string and dollar quoting, guarded `DO`
blocks). A statement that it does not recognize is `unknown`. A
migration takes the worst class of its statements.
- Lock and duration hazards have their own reason codes
(`lock_hazard_*`) and classify as `unknown`, not as compatible with a
warning.
- Override: `-- paperclip:migration-compat online_compatible:
<justification>` (or `requires_staging`) in the leading comment block of
the migration file. The justification is mandatory (10 characters or
more). A malformed, repeated, or misplaced directive is an error and
never improves the verdict.
- A table counts as new after a plain `CREATE TABLE`, or after `CREATE
TABLE IF NOT EXISTS` when no earlier migration created that table.
`classifyMigrationSequence` classifies migrations in apply order with
that knowledge.
- `packages/db/scripts/migration-compatibility.ts` (run with `pnpm exec
tsx` in `packages/db`): classify files, print the distribution, print
JSON, or run the guard (`--check`). No new package script.
- Published data: `manifest.json` of the cloud migrator artifact has a
new additive field `migrationCompatibility`. `version` stays `1`. No
existing field changes. The build reads the SQL from the packed
`db.tgz`, so the data describes what the migrator applies. `validate`
(publisher) and `verify` reject data that is malformed or does not match
the packed migrations. The workflow file does not change.
- Guard for new migrations:
`.github/scripts/check-pr-migration-order.mjs` (the existing "Validate
migration ordering against target branch" policy step) now also
classifies the migrations that the pull request adds. `check:migrations`
runs the same guard against the merge base with `origin/master`, so the
`@paperclipai/db` typecheck and build report it locally. `unknown`
without an override fails. `requires_staging` prints a notice and
passes. A new migration that uses `CONCURRENTLY` also fails when it is
not in `MIGRATIONS_WITH_TOP_LEVEL_STATEMENTS`
(`packages/db/src/client.ts`), because the migrator would run it inside
a transaction.
- `doc/DATABASE.md`: new section "Migration compatibility with the
running release" under "Schema change workflow".
Published shape:
```json
"migrationCompatibility": {
"version": 1,
"migrations": [
{
"idx": 468,
"tag": "0468_busy_wendell_rand",
"sqlSha256": "56a6e272…",
"compatibility": "online_compatible",
"classified": "online_compatible",
"override": false,
"reasons": [
{ "code": "add_column", "compatibility": "online_compatible", "message": "Adds a column that is nullable or has a constant default.", "statements": [1] }
]
}
]
}
```
`sqlSha256` is the hash that the migrator records in the migration
history. `overrideJustification` is present only when `override` is
`true`. Entries are in apply order (journal `idx`, then file name; the
shipped journal repeats `idx` 178).
### Distribution over the existing migrations
No existing migration is rewritten or annotated.
| Class | All 467 | Newest 100 |
| --- | --- | --- |
| `online_compatible` | 228 (48.8%) | 67 |
| `requires_staging` | 114 (24.4%) | 12 |
| `unknown` | 125 (26.8%) | 21 |
Migrations per blocking reason (one migration can have several):
| Migrations | Class | Reason |
| --- | --- | --- |
| 74 | `requires_staging` | `create_unique_index_existing_table` |
| 70 | `unknown` | `lock_hazard_create_index` |
| 66 | `unknown` | `lock_hazard_validated_foreign_key` |
| 56 | `unknown` | `lock_hazard_data_backfill` |
| 54 | `unknown` | `drop_constraint` |
| 41 | `requires_staging` | `add_validated_check` |
| 33 | `unknown` | `drop_index` |
| 16 | `unknown` | `do_block_procedural` |
| 15 | `unknown` | `replace_function` |
| 13 | `unknown` | `create_trigger_existing_table` |
| 12 | `unknown` | `data_change_existing_table` |
| 10 | `unknown` | `drop_object`, `lock_hazard_set_not_null`,
`set_default` (each) |
| 8 | `requires_staging` | `drop_column` |
## Verification
- `pnpm --filter @paperclipai/db exec vitest run
src/migration-compatibility.test.ts`: 113 tests pass. They cover a
positive and a negative case for each rule, each reason code, the
override (missing, short, misplaced, and repeated justification are
rejected), unknown by default, the guard, the shipped migrations, the
`CONCURRENTLY` list, and the command line.
- `node --test scripts/cloud-migrator-artifacts.test.mjs
.github/scripts/tests/check-pr-migration-order.test.mjs`: 21 tests pass.
- Real bundle at the first commit of this branch: `preview-artifacts.mjs
pack`, then `cloud-migrator-artifacts.mjs build`, `validate`, and
`verify-install` pass. The manifest lists 467 migrations and is 312 KB.
This run found the repeated journal `idx`, which the second commit
handles.
- `pnpm --filter @paperclipai/db run check:migrations` passes. With
`--check --base HEAD~150` the guard fails on three `unknown` migrations
and prints notices for two `requires_staging` migrations.
- `pnpm verify` exits 2 because `.github/scripts/**` is a global CI
input. Required CI supplies the broad coverage.
- Not run: the publish job (it needs the master-only AWS role), and no
deployer reads the new field yet.
## Risks
- The manifest grows from about 1 KB to about 312 KB. A consumer that
rejects unknown manifest fields would fail. The verifier in this
repository ignores unknown fields; consumers outside this repository
were not inspected.
- From this merge, an open pull request that adds an `unknown` migration
fails the policy step and the `@paperclipai/db` typecheck until the
author rewrites the migration or adds the override. `drizzle-kit` emits
validated foreign keys and plain indexes for existing tables, so this is
common. The error text and `doc/DATABASE.md` give the fix.
- `ADD CONSTRAINT ... NOT VALID` classifies as `online_compatible`.
PostgreSQL still enforces it on new writes, so the author must make sure
that the previous release does not write rows that violate it.
- The classifier reads SQL only. It cannot see that new code writes
values that the previous release cannot read (for example a new enum
value). The `requires_staging` override exists for that case.
- `master` cannot fail the guard: the guard only judges migrations that
are not on the merge base.
## Model Used
- Claude (Anthropic), model ID `claude-fable-5-2`, in Claude Code with
tool use and code execution.
## Checklist
- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [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 Fable 5.2 <noreply@anthropic.com>
|
||
|
|
c02de922af |
fix(chat): deliver the person's time zone to chat agents (#20898)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - People talk to agents in chat, and agents create routines and other scheduled work from that chat > - Paperclip does not record the person's time zone, so a chat agent cannot read "every weekday at nine" or "today" in the person's zone > - The agent then saves schedules in UTC, and the person gets the work at the wrong hour > - This pull request saves the browser time zone with the person's personal preferences and gives it to chat agents in the task prompt > - The benefit is that schedules and dates from chat match the person's local time ## Linked Issues or Issue Description **What happened** A person in America/Chicago wrote in chat: "Every weekday at nine in the morning, summarize Launch for me." The agent saved the routine schedule `0 9 * * 1-5` with the UTC time zone. It replied that it used UTC because the person's time zone is not recorded. **Expected behavior** The agent knows the person's time zone. It saves one enabled `0 9 * * 1-5` schedule with the time zone America/Chicago. Its reply names the zone. **Steps to reproduce** 1. Open the board in a browser with the time zone America/Chicago. 2. Open a chat with an agent. 3. Send "Every weekday at nine in the morning, summarize Launch for me." 4. Look at the routine trigger that the agent creates. Its time zone is UTC. ## What Changed - Database: add the nullable `time_zone` column to `user_workspace_preferences` (migration `0468`). This is an instance-wide personal preference by user, as AGENTS.md section 5 permits. - Shared: `WorkspacePreference` has `timeZone`. `updateWorkspacePreferenceSchema` accepts an optional IANA `timeZone` (max 64 characters, checked with `Intl.DateTimeFormat`). `coordinatorWorkspaceEnabled` is now optional. The request must contain at least one field. - Server: the preference service returns `timeZone` and updates only the fields in the request. The activity log entry stays the same. The OpenAPI entry uses the shared schema. - UI: a new `useBrowserTimeZoneSync` hook in `Layout` saves the browser time zone one time when it is different from the saved zone. It is silent and best-effort. It uses the selected company as the audit context. - Prompt: for a conversation (any adapter), the task context reads the chat owner's saved zone. Recovery keeps the zone that the first turn admitted. The chat block adds one line only when the zone is known: `The person's time zone is America/Chicago (from their browser). Read times and dates they give, such as "nine in the morning" or "today", in that zone, and save schedules such as routine triggers with that timezone; say the zone when you report a schedule.` ## Verification - `cd server && npx vitest run src/__tests__/workspace-preferences.test.ts src/__tests__/workspace-preferences-routes.test.ts src/services/heartbeat/task-markdown.test.ts src/__tests__/heartbeat-task-context.test.ts src/__tests__/openapi-routes.test.ts src/services/native-runtime/runner-api.test.ts` - `cd ui && npx vitest run src/hooks/useBrowserTimeZoneSync.test.tsx src/hooks/useCoordinatorWorkspaceEnabled.test.tsx src/components/Layout.test.tsx` - Red/green: without the task-context and task-markdown changes, the two new prompt tests fail. With the changes, they pass. - `pnpm check:token-gates` is clean. `tsc --noEmit` passes for `packages/shared` and `packages/db`. - Manual: open the board in a browser with a non-UTC zone. Then ask a chat agent to create a weekday routine at nine in the morning. The trigger uses the browser zone. ## Risks - Low risk. The migration adds one nullable column. - The PUT body is now more flexible: `coordinatorWorkspaceEnabled` is optional. An empty body still gets a 400 response. - The prompt grows by one line only for conversations with a known zone. - The saved zone changes when the person uses a browser in a different zone. The latest browser wins. > 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 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> |
||
|
|
cda23f08ac |
fix(db): preserve legacy project reads during rolling upgrades (#20860)
## Thinking Path > - Paperclip controls agent work while web servers change versions. > - The old and new servers can share a database during that change. > - Older servers still read and write the project environment column. > - Migration 0466 removes that column before those servers stop serving. > - A later repair alone leaves a committed missing-column window and loses values. > - This pull request applies the exact drop and repair in one transaction. > - Old reads and saved values survive while current placement behavior stays unchanged. ## Linked Issues or Issue Description Refs #20835 and #20852. **What happened?** Migration 0466 drops `projects.environment_id`. The previous server still selects that column and reads it during admission. Applying a separate repair afterward leaves a window in which those requests fail. The drop also removes saved values. **Expected behavior** A database that has not yet applied 0466 must keep the column and its values visible to concurrent old servers throughout the upgrade. Both published migration entries must be recorded truthfully. Current project responses must keep the new workspace placement contract. **Steps to reproduce** 1. Use a database before migration 0466 with nonempty project environment values. 2. Keep an old project writer transaction open and start the pending migrator. 3. Start an old project read, then commit the writer. 4. Without this repair, the saved values disappear. With this repair, the reader sees the preserved values and both migrations commit together. ## What Changed - Add migration 0467 to restore the legacy nullable UUID column and its original foreign key. Leave the published 0466 SQL unchanged. - Apply only the exact SHA-bound 0466/0467 pair in one transaction. Lock the project table, retain nonempty values in a temporary table, execute both real SQL files, restore values, and record both real journal entries before commit. - Apply the additive 0467 repair normally when 0466 was already committed. - Keep a deprecated database-only field and omit it from current project API responses. Current placement logic stays unchanged. - Add PostgreSQL regressions for concurrent old reads and writes, idempotence, rollback, and an already-applied drop. Document the overlap contract and retain the five newest generated snapshots. ## Verification - Node 24 cached agent preflight passed. - `pnpm exec vitest run packages/db/src/project-environment-overlap-migration.test.ts server/src/__tests__/project-environments.test.ts -t 'project environment rolling migration|saves storage, not'`: 4 passed in 32.57 seconds against real disposable PostgreSQL. - Before the owner fix, the regression had two failures: saved values disappeared and the restore-failure case committed. The fixed case proves that both DDL and journal entries roll back together. - `pnpm --filter @paperclipai/db typecheck` and `pnpm --filter @paperclipai/db check:migrations`: passed. - `git diff --check`: passed. Migration 0466 bytes are unchanged. The generated snapshot set is 0463 through 0467. - Review found a raw DELETE response that still exposed the database-only field. The delete projection now omits it; a real authorized HTTP DELETE passed with successful cleanup. The concurrency test now observes the old reader waiting inside PostgreSQL before releasing the old writer. All three migration cases passed with that barrier. Earlier HTTP fixture setup/cleanup failures remain recorded as failed attempts; they were not product-regression passes. - Candidate `047178538c0df3a9e382259a51635a6e46da5646` has fresh Greptile 5/5 with no open findings. [Required final-head CI](https://github.com/paperclipai/paperclip/actions/runs/43062354612) passed, including build, typecheck, migration checks, all test shards and the aggregate verification gate. The separate Superagent security scan ended inconclusive/neutral; it did not produce a passed scan. Deployed acceptance remains pending. No hosted database or app deployment was changed by this branch. ## Risks The pair takes a table lock for its transaction. Operators must retain their normal lock and statement timeouts. Both SQL files and their exact hashes must match before the pair runs. A failed restore rolls back the drop and both journal entries. If 0466 was already committed, 0467 restores the column and foreign key. It cannot recover values that were already deleted by the earlier release. This repair does not claim to recover those values. ## Model Used OpenAI Codex (GPT-6), with tool-assisted implementation and real PostgreSQL qualification. The exact serving model ID and context window 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 and contains no internal 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> |
||
|
|
369e238295 |
fix(projects): make workspace storage the project's only placement choice (#20835)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A repository project gets its working files from core staging or from a workspace storage plugin > - #20459 reported that an installed storage plugin attached every new repository project, so sandbox agents could not use those projects > - #20587 fixed creation with an **Available in** choice that selects storage > - #20651 then saved an environment on every new project and used it before the agent default to place runs. That solved a problem that did not exist: the project needs storage, not a run environment > - Creation saves the first environment of the chosen group, so a project silently sends every agent's runs to that environment. Project Configuration also shows two controls (Environment and Workspace storage) for one decision > - This pull request removes the project run environment and makes storage the only placement choice a project owns. Storage is fixed at creation > - The benefit is that runs follow the agent again, and project settings show one clear, read-only fact about storage ## Linked Issues or Issue Description Refs #20459. Follow-up to #20587 and #20651. Refs #11644. That issue asks for a project-level run environment override. This pull request removes the override that #20651 added for it, because the override is not necessary to fix #20459. #11644 stays a separate feature request. Related: #18176 (open) repairs placement through the legacy workspace-policy field. This pull request does not restore project placement, so it conflicts with the direction of #18176. ## What Changed - **Remove the project run environment.** Migration `0466` drops `projects.environment_id`. Run placement no longer reads the project. Runs use the agent default, then the instance default, then local execution. The low-trust task sandbox and the enforced sandbox policies keep their precedence. - **Keep the #20651 recovery and binding guards.** A recovered run still uses its admitted environment lease (source `run`). Company-binding checks before admission and during lease creation stay. - **Keep the creation choice.** **Available in** in New Project still selects storage. It still shows only when the environments need more than one storage owner. That can only occur when a storage plugin is installed. `POST /projects` with `environmentId` selects the storage of that environment and does not save the environment. - **Fix storage at creation.** `PATCH /projects/:id` rejects any storage change with `409 project_storage_locked`, from core to a plugin and from a plugin to core. Before, a core project could move to a plugin. - **Drop legacy writes.** Older clients can still send `executionWorkspacePolicy.environmentId`. The service removes it on create and update. - **Simplify Project Configuration.** Remove the Environment and Workspace storage selectors. Show an advisory **Available in** list of the environments that can use the project's storage. Show it when a storage plugin is installed or when the project uses one. When the project's plugin is not installed, show **Storage plugin unavailable**. Without a storage plugin, show nothing. - **Issue workspace card.** Remove the "project default" environment label. - **Shared contract.** Remove `Project.environmentId`, the `environmentId` project field, and `expectedStorageProviderPluginId` from the update schema. The create schema keeps `environmentId` and `expectedStorageProviderPluginId` from #20587. - **Docs and maps.** Update `doc/project-environment-compatibility.md`, `docs/api/goals-and-projects.md`, and the project feature and agent-access maps. - **SDK source pins.** Refresh `tests/runner-e2e/source-sdk-public-inputs.json` for the two shared files. The shared file blobs return to their values before #20651. The refresh also corrects the `pnpm-lock.yaml` pin and `lockSha256`: `master` pins an older lockfile blob than the one it contains. ## Verification - Targeted server suites pass (227 tests): `project-environments`, `project-workspace-provider.integration`, `heartbeat/environment-selection`, `execution-workspace-policy`, `environment-selection-route-guards`, `environment-run-orchestrator`, `openapi-routes`. - Related server suites pass (349 tests): `environment-service`, `environment-routes`, `plugin-workspaces.integration`, `issues-service`. - Targeted UI suites pass: `ProjectProperties.concurrency`, `IssueWorkspaceCard`, `NewProjectDialog`, `NewProjectDialog.managed-sandbox`, `ProjectDetail`. - New tests: - Creation saves storage, not an environment. Core and plugin projects both reject a storage change with `project_storage_locked`. - A legacy policy `environmentId` is dropped on create and update. - A new run follows the agent default for any project. A recovered run keeps its admitted lease after the agent default changes. - Project Configuration has no storage or environment selector. The note is hidden without a plugin. It lists the core or plugin environments for the project's storage. It shows **Storage plugin unavailable** for a removed plugin. - `pnpm --filter @paperclipai/server typecheck`, `pnpm --filter @paperclipai/ui typecheck`, and `pnpm --filter @paperclipai/shared typecheck` pass. - `pnpm --filter @paperclipai/db check:migrations`, `pnpm check:token-gates`, `node scripts/check-maps.mjs`, and `node tests/runner-e2e/source-sdk-public-inputs.mjs <HEAD> --check` pass. - Not run locally: the full `pnpm test:run`, `pnpm build`, and browser suites. CI supplies that coverage. - Manual check: open Project Configuration on an instance with a storage plugin. Confirm that there is no Environment or Workspace storage selector, and that **Available in** lists the environments. ## Risks - **Migration.** `0466` drops `projects.environment_id` and its foreign key. The migration deletes the saved project environments. Apply it before you serve the new revision. An old server revision that reads `projects` fails until it stops. This is the same pattern as earlier drop-column migrations (`0229`, `0289`). - **Behavior change for existing projects.** Projects created after #20651 shipped have a saved environment that overrides the agent. After this change, their runs follow the agent default again. This is the intended fix. Before #20651, a project environment selection was not applied to runs (#11644), so no older behavior is lost. - **No path from core to a plugin.** A core project, including one created before a plugin was installed, cannot move to the plugin. To use plugin storage, create a new project. - **API.** A `PATCH` that sends `environmentId` or `expectedStorageProviderPluginId` no longer has an effect. The schema strips these fields. > 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`) by Anthropic, in Claude Code, with tool use (shell, file edits, test runs). Extended thinking was available. ## 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> |
||
|
|
81c9143fe4 |
fix: upgrade Better Auth to 1.7.16 with account migration (#18906)
## Thinking Path > - Paperclip manages AI agents and their work. > - The board uses Better Auth for credential accounts and sessions. > - The server pins Better Auth 1.7.2. > - The November security release fixes 12 issues in Better Auth. > - The new version stops writing the old required account issuer field. > - This change updates the packages and the database contract together. > - New users can sign up, and existing users can still sign in. ## Linked Issues or Issue Description **What happened?** The server uses Better Auth 1.7.2. The [security release](https://better-auth.com/blog/security-release-2026-11-03) fixes 12 issues in 1.7.16. Earlier update PRs #17909 and #17911 target 1.7.7, which is below this security release. **Expected behavior** Use the patched stable release. Keep new and existing credential accounts usable. **Steps to reproduce** Read the server dependency pin. Upgrade it without a schema change. Better Auth then rejects the required account issuer column. See the [upgrade guide](https://better-auth.com/docs/guides/1-7-upgrade-guide). ## What Changed - Pin Better Auth to 1.7.16. The repository refresh bot owns the committed Core lockfile. - Make the old issuer column nullable. Keep existing issuer values. - Replace the issuer index with a unique provider/account index. - Generate the migration and retain the five newest schema snapshots. - Test new signup, new-account sign-in, legacy issuer sign-in, duplicate rejection, and separate provider identities with real PostgreSQL. - Update the database schema test. Test the historical issuer backfill separately. Verify upgrades over existing rows and rollback on duplicate provider/account keys. - Document the duplicate check and migration order. - Add the approved two-line dependency resolution step before the backup job’s frozen install. Leave the frozen check and backup tests intact. ## Verification - Core head: `af70282c7e24335b8688c8f3cc88fdf65045632c`; parent `0f1481e4462100357a7caa496b875cf0bc500d95` includes master `84fc67317`. - Nine changed files. The only workflow edit is the approved two-line backup dependency resolution step; no managed lockfile edits. - Migration `0465_better_auth_account_identity.sql` is three SQL statements; snapshot changes only `public.account`, five newest snapshots retained. - DB typecheck and 23 focused tests passed on the parent. Both real PostgreSQL migration/signup suites were rerun November 11: all 4 tests passed. Application source and tests are unchanged in this workflow-only commit. - The parent’s sandbox-provider lane passed all 205 file-sync tests but reported an unhandled EPIPE during the symlink rejection case; its aggregate verify therefore failed. No unrelated source change was made. The new head reruns the full applicable CI. - Reproduced the old backup install failure locally. Resolving manifests followed by a frozen install passed locally; restored the bot-owned committed lock afterward. Hosted backup qualification remains required. - Current-head CI and review are running; no final-head green claim or merge yet. ## Risks Apply the migration before the new server starts. Check for duplicate provider/account pairs first. Duplicate rows stop index creation. Keep separate users separate when resolving duplicates. The migration keeps issuer data. Do not restore its NOT NULL constraint while new rows contain null. No deployment is included. ## Model Used OpenAI GPT-6 Codex, with reasoning and tool use. The runtime did not expose a more specific model ID or context window. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] 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> |
||
|
|
f30ac226d6 |
Keep project environment mappings independent of shared defaults (#20651)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - A project supplies work context and repository storage to task runs. > - Creation selected a storage owner but did not retain the selected environment on the project. > - Configuration stored an environment inside checkout policy, but policy reads and run selection dropped it. > - Projects therefore continued to follow shared agent and instance defaults. > - This pull request saves placement on each project and uses it for new runs. > - The benefit is an independent Project to Environment association with the existing storage, trust, and recovery safeguards. ## Linked Issues or Issue Description Refs #20459 and #11644. Follow-up to #20587. The reported journey is Project > Configuration: changing one project's environment must not change another project's mapping or shared defaults. Related open PR #18176 repairs placement through the legacy workspace-policy field. This follow-up introduces a separate project association, saves creation defaults, and migrates saved legacy references. It overlaps that PR's placement, permission, and recovery safeguards. The changes should be reviewed together. #9588 concerns environment-variable response privacy and does not address placement. ROADMAP.md has no separate project-association work planned. ## What Changed - Add nullable `projects.environment_id` and expose `Project.environmentId`. Save the effective creation default once on the project. Keep environment placement separate from checkout/isolation policy. - Read and write Environment directly in Project Configuration. Keep the control available with isolated workspaces disabled. Each form reads its project's value, even when environment choices share a query cache. - Offer environments compatible with the current storage owner. Change the target and storage together when the existing storage selector attaches a core project to a plugin. Preserve retained-data restrictions and validate the observed storage owner again on save. - Resolve new task runs from their organizational project's target before agent and instance defaults. File sources from another project do not change placement. Explicit low-trust sandbox choices and enforced sandbox policies retain precedence. External provider placement remains separate. - Restore admitted native runs from their owned environment lease. Later project edits apply to new admissions. Require workspace-write permission for environment changes and clears. Check company bindings before runtime admission and during lease creation. - Migrate valid saved policy references into the project column. Normalize legacy API writes. Count and clear matching project references when an environment is deleted. Update API and behavior documentation. - Refresh the maintained SDK source registry for the two changed shared files. Runtime pins are unchanged. ## Verification - [Hosted CI](https://github.com/paperclipai/paperclip/actions/runs/42908045146) is green on `8b3b7d5b5aed51fc4c8e9c2e3506508bbaf9e00c`: **60 successful checks**, three skipped gates, no failures, and `ci / verify` passed. GitHub reports no merge conflicts. Standard Greptile scored the same commit **5/5** with both review threads resolved. - Source checkout on macOS arm64, Node 26.12.0, with fresh isolated PostgreSQL. No production data or live provider calls. - The two-project component test changes the first form with isolation disabled and a shared query cache. The second form keeps its target. Storage changes update only the selected project. A saved target remains selected while the list loads, and an explicit clear remains possible. - API integration creates two projects from the same default, patches one, reads both, and checks that agent and instance defaults remain unchanged. A later instance-default change does not rewrite either saved project target. Tests cover unavailable/incompatible choices, anonymous and cross-company denials, and storage preconditions. - The migration test runs the actual SQL against an existing project row and verifies its environment and disabled checkout policy survive. Deletion accounting and clearing affect only matching project references. - The final guard set passes **53 tests across four files**: `project-environments.test.ts`, `environment-selection-route-guards.test.ts`, `heartbeat/environment-selection.test.ts`, and `ProjectProperties.environment.test.tsx`. All **64 environment-run-orchestrator tests** pass, including direct lease acquisition without an extra caller argument. The broader affected set passed **109 tests across eight files** before the additional admission guards. These sets overlap. - UI typecheck, migration numbering/safety, token gates, module boundaries, query-error rendering, and diff checks pass. `pnpm verify --explain` returns exit 2 and requires broad CI because shared schema/runtime paths and behavior documentation changed. Broad coverage is delegated to current-head CI. - Early iterations failed due to a legacy parser that drops the environment field, a fixture without real company membership, and incorrect expectations for privacy-preserving 404 denials. The service now reads legacy references before normalization, the API fixture has real membership, and denial assertions match the existing privacy contract. The orchestrator mock also needed the reused company-binding guard in its service contract. The focused suites above pass after those corrections. - Local server typecheck remains blocked by installed ACPX, Slack, AI SDK, and Runner type mismatches in unchanged dependency-facing code. It reports no errors in the changed production paths. This is not a full local typecheck or suite pass. Hosted CI supplies full current-head coverage. - SDK source-proof and privacy qualification passes **56 tests across three files**, including the real built SDK. The maintained source registry matches the committed revision; runtime pins are unchanged. - Review reproduced two failures: an incoming policy without a storage owner could be validated before the existing owner was restored, and uppercase legacy UUIDs missed migration matching. Preserve the owner before validation and compare normalized UUID text. Both regressions now pass. The post-fix project/storage/orchestrator batch passes **107 tests**: `node node_modules/vitest/vitest.mjs run server/src/__tests__/project-environments.test.ts server/src/__tests__/project-workspace-provider.integration.test.ts server/src/__tests__/environment-run-orchestrator.test.ts --silent`. - The first hosted run failed build, typecheck, and canary at the same `run-completion.test.ts` call because lease acquisition required a new argument. The lease method now enforces binding internally. Its existing caller compiles without a new field, and the orchestrator tests prove the binding flag reaches the runtime. The full server check no longer reports that error; the local dependency errors above remain. ## Risks - Apply the normal database migration before serving the new revision. The migration adds a nullable foreign key and copies valid legacy project selections. It does not move repository data or resize provider storage. The five newest schema snapshots are retained. - New projects retain their creation target when shared defaults change. Existing projects without a saved selection continue to inherit defaults. A null target explicitly opts into inheritance. - Existing saved project selections now affect placement. Missing, archived, incompatible, and foreign-company targets fail validation; they do not silently choose another storage owner. A running admitted process keeps its original owned lease. - Local dependency limitations are listed above. No live-provider, deployed-browser, or production rollout qualification is claimed. ## Model Used OpenAI Codex, based on GPT-6, with reasoning, tool use, and code execution. The exact serving snapshot 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> |
||
|
|
353945c86d |
fix(tool-gateway): let agents create gateway tokens without receiving the value (#20613)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Tool gateways let external MCP clients use a company's tools with a
gateway client token
> - Since delegated connection management (#17737), an agent can create
and revoke gateway client tokens for its responsible person
> - #20581 stopped agents from getting the one-time token value, but it
did this by making token creation person-only
> - The product decision is that agents keep their delegated authority
to create and revoke tokens
> - This pull request restores agent token creation, but the agent never
receives the token value: the person reveals the value once on the
gateway's Tokens page
> - The benefit is that an agent can do the rotation work for the
person, and the secret still never enters chat, run events, logs or tool
output
## Linked Issues or Issue Description
Refs #17737
Refs #20581
**What happened**
After #20581, `POST /api/tool-gateway/gateways/{gatewayId}/tokens`
returns 403 `gateway_token_requires_person` to every agent caller.
Agents lost the delegated authority to create gateway client tokens that
#17737 gave them. Before #20581, an agent that created a token received
the plaintext value in its tool result and could repeat it in chat.
**Expected behavior**
An agent with delegated connection management can create and revoke
gateway client tokens. The agent never receives a token value. The
responsible person gets the value once, on the gateway's Tokens page.
Person callers keep today's behavior.
**Steps to reproduce**
1. Give an agent delegated connection management for a responsible
person with `tools:admin`.
2. As the agent, call `POST
/api/tool-gateway/gateways/{gatewayId}/tokens` (for example through
`call_api`).
3. See 403 `gateway_token_requires_person`.
## What Changed
- Agent create: `POST /api/tool-gateway/gateways/:gatewayId/tokens` from
an agent with delegated authority returns 201 with token metadata only
(`id`, `tokenPrefix`, `expiresAt`, `pendingReveal: true`), plus
`tokensPath`, a `reveal_credential` `requiredUserAction` for the
responsible person, and a `nextStep` instruction. The saved row has no
usable value: its stored hash is of a value that is generated and then
discarded. Gateway authentication also rejects `pendingReveal` rows.
- Person reveal: new person-only `POST
/api/tool-gateway/gateway-tokens/:tokenId/reveal`. It generates the
value, stores its hash, clears `pendingReveal` and returns the value
once. A conditional update makes a second or concurrent reveal return
409. Only the token's responsible person can reveal it (403
`gateway_token_reveal_requires_responsible_person`). Agent callers get
403 `gateway_token_requires_person`. A token revoked before reveal
cannot be revealed.
- Why a reveal that generates the value: no stored plaintext existed to
reveal, and storing one (even encrypted) for later display adds a secret
at rest. Generating the value at reveal time keeps the existing one-time
display and stores only a hash.
- Schema: `tool_mcp_gateway_tokens.pending_reveal boolean not null
default false` (migration `0460_gateway_token_pending_reveal`). Shared
`ToolMcpGatewayToken.pendingReveal`, and `ConnectionRequiredUserAction`
gets `reveal_credential` / `gateway_token`.
- UI: the Tokens page shows a "Waiting for you to reveal" section for
pending tokens with a "Reveal token" button. The value appears in the
existing "New token — copy now" one-time block. Pending tokens show the
status "Not revealed".
- Agent gateway create: the 201 for agent callers also includes
`gatewayPath`, `tokensPath` and a `nextStep` that tells the agent to
link the saved gateway.
- The agent create-token `nextStep` and the catalog say not to revoke
the old token in the same turn: its client keeps working on it until the
person installs the new value.
- The reveal update and its `tool_gateway.token_revealed` activity entry
commit in one transaction. The Tokens page offers Reveal only for tokens
that act for the signed-in person.
- API reference: new `docs/api/tool-gateway-tokens.md` (create, reveal,
revoke) in the docs nav.
- Revoke stays allowed for agents with delegated authority (unchanged
route).
- API catalog: create-gateway, create-token, reveal and revoke
descriptions now say who acts, that agents never receive values, the
page paths, and that the old token is revoked only after the client
works with the new one.
- Agent access map: create-token policy note and a new reveal entry.
- Evals `gateways-profiles.create-gateway` and
`gateways-profiles.rotate-token`:
- Remove the keyword grader (`human|operator|admin|manage`) and the
handoff text checks. Grade saved state, rendered links and secret
absence. A plain reply or a question card is accepted.
- Remove the setup probes that called the create routes as the agent.
Under delegated authority, those probes created a real gateway
("Forbidden agent gateway"), which failed create-gateway before the
agent ran.
- create-gateway: exactly one gateway for the intended profile and
company, created by the agent (or by the person after a handoff); the
person opens it from a rendered link; attribution matches the creator;
any agent-created token is still pending reveal; no token value in chat,
cards or run events.
- rotate-token: after the agent's turn, both setup tokens still work and
any replacement the agent created is pending the person's reveal; the
person follows the rendered Tokens page link and reveals (or issues) the
value in a private browser; the new value works before the old token is
revoked; the unrelated client keeps access; no token value appears
anywhere, including a regex scan for any full `pcgw_<id>.<secret>`
value.
- The changed scenario context and outcomes are recorded in
`chat-full-catalog-testops-manifest.json` `changedFields` and pinned in
the manifest test. The inventory digest, descriptors (`oracleVersion`
v2, `sourceFiles`) and SDK public input pins are updated. The unused
`external-gateway-handoff.ts` module is removed.
- Fixture realism: the create-gateway profile now allows a real read
tool (an empty profile made the agent ask which apps to add instead of
finishing), and the rotation tokens get a neutral owner note instead of
"Disposable rotation fixture" (the agent read that note as an eval
hint).
- Shared private-browser helpers (`external-private-browser.ts`,
`external-private-browser-worker.ts`) get one optional `pendingTokenId`
input and one `revealedPending` receipt field.
## Verification
- Red: on `master` route code, the new integration tests `lets agents
create and revoke gateway client tokens without ever receiving a token
value` and `reveals an agent-created gateway token value only to its
responsible person` fail (403 `gateway_token_requires_person`).
- Green: `cd server && npx vitest run
src/__tests__/connection-management.integration.test.ts -t gateway` (25
passed). The test checks that the agent `call_api` result has no `token`
field and no token-shaped value, that the agent cannot reveal, that the
person reveals once (second reveal 409) and the value authenticates,
that agent revoke works and stops the value, that a token revoked before
reveal never gets a value, and that person create still returns the
value.
- `npx vitest run src/__tests__/tool-gateway.test.ts
src/__tests__/openapi-routes.test.ts
src/services/native-runtime/runner-api.test.ts` (2247 passed).
- `cd ui && npx vitest run
src/pages/apps/gateways/panels/TokensPanel.test.tsx
src/pages/apps/gateways/gateway-helpers.test.ts` (16 passed); `pnpm
check:token-gates` clean.
- Eval graders and catalog: `pnpm exec vitest run --config
tests/runner-e2e/vitest.config.ts` on the gateway oracle tests,
`external.test.ts`, catalog consistency/contract, TestOps
export/retained, phase/clock callers and SDK input/type tests (all
pass); `node --test` on the TestOps manifest, adapter, contract,
streaming and reconcile tests (all pass); `node
tests/runner-e2e/source-sdk-public-inputs.mjs <HEAD> --check`
(unchanged).
- `node scripts/check-maps.mjs` passes. `tsc --noEmit` for `server`,
`ui` and `tests/runner-e2e`: no errors in changed files.
- Paid end-to-end run of both eval cases, run
[42872921890](https://github.com/paperclipai/paperclip/actions/runs/42872921890)
on
|
||
|
|
3bf7af6c77 |
perf(plugins): read the lifecycle inbox from per-resource cursors (#20576)
## 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. > - #20405 made the inbox page read linear in the company journal size. The journal has no retention, so each page still reads every event of the company to find the unacknowledged ones: about 1.7 s per 1M events. > - The delivery worker reads a page again after each acknowledgment. The idle-sleep check also reads the whole journal when all events are acknowledged. `acknowledge` checks every older event of the resource. > - On a hosted instance, the page read ran 181 times in one window at about 1 s each. > - This pull request adds a per-plugin, per-resource cursor. Reads start at the cursor, so they do not read acknowledged history again. > - The benefit is that page, idle-check and acknowledgment cost follow the number of resources and pending events, not the journal size. Delivery order and results stay the same. ## Linked Issues or Issue Description Refs #20405 **What happened?** The plugin lifecycle inbox page query (`select ... from "resource_lifecycle_events" ... DISTINCT ON ...`) ran 181 times at about 1 s each on a hosted database in one window. Each acknowledgment causes another full read of the company journal. The idle-sleep check (`hasPending`) reads the full journal when all events are acknowledged. `acknowledge` reads the full history of the resource. **Expected behavior** A page read, the idle-sleep check and an acknowledgment must not read acknowledged history again. Their cost must not grow with the journal size. **Steps to reproduce** 1. Give a company 400 agents with 2,500 `agent_runtime` events each (1M events). 2. Acknowledge the first 2,000 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 1.7 s. `hasPending` takes about 240 ms. **Paperclip version or commit** `8d2bf78323` (master). **Deployment mode** Hosted (PostgreSQL). Local repro uses embedded PostgreSQL 18. ## What Changed - `packages/db/src/schema/plugin_lifecycle_resource_cursors.ts`: new table `plugin_lifecycle_resource_cursors`. It has one row per plugin, company, resource type and resource id. `acknowledged_through_id` means: every event of that resource at or below this id is acknowledged by the plugin. Plugin and company deletion cascade. - Migration `0462_plugin_lifecycle_resource_cursors`: creates the table and carries existing acknowledgments forward (see Risks for the previous-release story). - `server/src/services/plugin-lifecycle-inbox.ts`: - `list` and `hasPending` use a backward loose index scan on `resource_lifecycle_events_resource_order_idx`. It visits each resource stream once and gets its latest event id. A stream whose latest event is at or below its cursor is settled and is not read again. For other streams, the read gets one head: the unacknowledged creation, else the first unacknowledged transition above the cursor. Then it applies `afterId`, id order and the limit, as before. - `acknowledge` reads only the events between the cursor and the acknowledged event. It still writes the per-event `plugin_lifecycle_acknowledgments` row. It moves the cursor only over a contiguous acknowledged prefix. A creation recorded after a pending transition does not move the cursor. - An event counts as acknowledged when it is at or below the cursor, or when it has an acknowledgment row. Thus rows written by an earlier release, and gaps in old history, keep their current behavior. - `server/src/__tests__/resource-lifecycle-events.test.ts`: new database tests for the migration carry-forward, gaps, late creations, cursor reads, the rollback rows and the plugin-delete cascade. All tests from #20405 still pass without change. - `scripts/benchmark-plugin-lifecycle-inbox.ts`: runs the migration backfill on the fixture and times it. Adds an acknowledgment case. Runs `VACUUM ANALYZE`, because hosted tables are autovacuumed. - `doc/DATABASE.md`: describes the cursor, its read cost and the migration. - The delivery worker (`plugin-lifecycle-delivery.ts`) is not changed. ## Verification Thesis: the cost is a read of the whole company journal (page, idle check) or the whole resource history (acknowledgment). If a per-resource cursor marks the acknowledged prefix, each read can skip that prefix. Then the cost follows the number of resource streams and pending events. The rows must stay equal. Local SQL timings: embedded PostgreSQL 18.1 on Apple Silicon, synthetic data, warm cache, one warmup, 7 samples, baseline and treatment order alternate per sample. The machine was shared with other jobs (load average about 35), so spread is wide. Values are median (min to max) in ms. Rows are equal (`assert.deepEqual`) for every list and probe case. Fixture A: 400 agents x 2,500 runtime events, 2,000 acknowledged per agent. 1,020,440 events, 800,420 acknowledgments. | Case | Before | After | Speedup | | --- | --- | --- | --- | | page of 100, all types | 1,700 (1,633-2,438) | 11.5 (9.5-12.2) | 148x | | page of 100, agent and project | 3.3 (2.5-5.4) | 4.2 (3.6-6.7) | 0.8x (slower) | | idle-sleep check | 238 (174-367) | 2.7 (2.1-3.4) | 88x | | acknowledge one head | 15.3 (9.4-30.1) | 20.3 (12.9-29.9) | neutral (noise) | | migration backfill | - | 9.3 s | - | Fixture B: 40 agents x 25,000 runtime events, 24,000 acknowledged per agent (long history). 1,020,080 events. | Case | Before | After | Speedup | | --- | --- | --- | --- | | page of 100, all types | 1,943 (1,642-4,979) | 4.4 (2.9-5.6) | 444x | | page of 100, agent and project | 3.5 (2.1-5.7) | 4.6 (2.8-12.0) | 0.8x (slower) | | idle-sleep check | 234 (220-243) | 2.8 (1.9-6.5) | 85x | | acknowledge one head | 64.6 (48.5-71.6) | 5.0 (2.6-7.1) | 13x | | migration backfill | - | 4.6 s | - | Fixture C: 5,000 agents x 200 runtime events, 190 acknowledged per agent (many resources). 1,025,040 events. | Case | Before | After | Speedup | | --- | --- | --- | --- | | page of 100, all types | 1,867 (1,649-1,975) | 76.9 (66.2-497) | 24x | | page of 100, agent and project | 10.3 (8.7-12.4) | 21.6 (20.2-23.2) | 0.5x (slower) | | idle-sleep check | 220 (201-298) | 2.8 (2.0-5.3) | 78x | | acknowledge one head | 4.9 (3.4-8.6) | 6.0 (5.2-7.6) | neutral | | migration backfill | - | 15.6 s | - | `EXPLAIN (ANALYZE, BUFFERS)` for the all-types page on fixture A: before 2,488 ms, 13,597 shared hits plus temp files; after 7.0 ms, 7,926 shared hits. Commands: ```sh pnpm --filter @paperclipai/server exec tsx ../scripts/benchmark-plugin-lifecycle-inbox.ts --baseline-ref=8d2bf78323 --agents=400 --per-agent=2500 --acked=2000 --samples=7 pnpm --filter @paperclipai/server exec tsx ../scripts/benchmark-plugin-lifecycle-inbox.ts --baseline-ref=8d2bf78323 --agents=40 --per-agent=25000 --acked=24000 --samples=7 pnpm --filter @paperclipai/server exec tsx ../scripts/benchmark-plugin-lifecycle-inbox.ts --baseline-ref=8d2bf78323 --agents=5000 --per-agent=200 --acked=190 --samples=7 cd server && npx vitest run src/__tests__/resource-lifecycle-events.test.ts src/__tests__/plugin-lifecycle-delivery.test.ts src/__tests__/project-instance-secrets.test.ts src/__tests__/agent-runtime-hooks.test.ts pnpm --filter @paperclipai/db check:migrations ``` - `resource-lifecycle-events.test.ts`, `plugin-lifecycle-delivery.test.ts` and `project-instance-secrets.test.ts`: 86 passed. `agent-runtime-hooks.test.ts`: 22 passed. - `tsc --noEmit -p server`: pass. `check:migrations`: pass. ## Risks - **Schema change.** Migration `0462` adds one table and two foreign keys to `plugins` and `companies`. It adds no index on an existing table. The backfill reads the acknowledgment table once. It took 4.6 s to 15.6 s on 1M-event fixtures with 0.8M to 1M acknowledgments. - **Migration order.** The number `0462` is reserved after open PRs that use `0459` to `0461` (#20571, #20568, #20573). Merge this PR after them. Before merge, rebase onto master and chain the snapshot after the latest migration. - **Previous-release story.** - Upgrade: the backfill stores, per plugin and resource, the end of the contiguous acknowledged prefix. A cursor never passes an unacknowledged event, so no event is lost. Acknowledged events above a gap keep their rows, so no event is delivered again. A test runs the migration statement on rows that only an earlier release wrote and checks that `list` returns the same rows before and after. - Rolling deploy: an earlier release that still runs writes only acknowledgment rows. The new reads honor those rows. - Rollback: the new code still writes every acknowledgment row, so an earlier release sees the same progress. - **Ordering assumption.** The cursor is safe because writers record a resource's events while they hold that resource's row lock (`paperclip_record_agent_runtime` locks the agent row; agent status and project writers document the lock). Thus a later commit for the same resource always has a higher id. A writer that breaks this contract could commit an event below the cursor, and that event would not be delivered. A lower id from a different resource is still delivered (existing test). - **Slower path.** A page that asks only for `agent` and `project` streams is 1 to 11 ms slower. These streams are short, so the old full read was cheap, and the new read visits each stream once. The cost is bounded by the number of resources. - **Many resources.** The page cost grows with the number of resource streams of the requested types. At 5,000 agents a page is about 77 ms, compared with about 1.9 s before. - The hosted planner can choose a different plan. The hosted improvement is not yet measured. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through 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> |
||
|
|
8e4c5e81b9 |
perf(issues): read created-from tasks by index and inbox review waits by page (#20573)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The task page shows "Created from this task". The inbox shows blocked-inbox attention for a page of tasks. > - On a hosted database, the created-from list ran about 4,000 times in one afternoon at about 499 ms each. The task page sends it each time a task opens. > - Its filter was an `OR` of three provenance paths over the list's own rows. PostgreSQL could not use an index for it. It read every task of the company and tested each one, with the list's other per-row conditions (such as the chat conversation state subquery). > - The blocked-inbox attention map also read external review waits for every in-review task of the company, not only for the tasks of the page. > - This pull request reads created-from tasks from three index probes, and reads review waits only for the in-review tasks that can change a finding of the page. > - The benefit is less database work on each task open and each inbox page. The responses stay the same. ## Linked Issues or Issue Description Refs #20400 (shares review-wait reads between the attention maps of one request). Refs #20551 (authorizes the review waits of a page with one read; this change narrows the waits that the page asks for, and adds a test that pins the batched read). Refs #13241 (adds the created-from list). **What happened** - `GET /api/companies/:id/issues?createdFromIssueId=…` used `createdFromIssueCondition`. Its filter was `origin_issue_id = source OR (origin_issue_id IS NULL AND origin_run_id IN source runs) OR (… AND EXISTS creation activity of a source run)`. The `OR` stops index use, so the statement read every task of the company. The list's other conditions, which include the chat conversation state subquery, ran for each task before the task was excluded. - `listIssueBlockedInboxAttentionMap` read review waits for every in-review task of the company graph. The caller uses the findings of the page rows only. **Expected behavior** The created-from list reads only the created tasks. The blocked-inbox map reads review waits only for in-review tasks that the liveness classifier can read for a page row. The rows, the order, and the attention values do not change. **Steps to reproduce** 1. Use a company with tens of thousands of tasks, runs, and activity rows. 2. Open a task page. Look at the created-from list statement: it reads the full task set of the company. 3. Open an inbox page with blocked-inbox attention. Look at the review-wait read: it gets the ID of every in-review task of the company. ## What Changed - `server/src/services/issue-creation-origin.ts`: `createdFromIssueCondition` is now `issues.id <> source AND issues.id IN (three branches)`. Each branch starts from an index: - recorded origin task: `issues_company_origin_issue_idx`; - legacy origin run: the source task's runs (`heartbeat_runs_company_origin_issue_idx`), then the new partial index on `issues (company_id, origin_run_id)`; - creation activity: `activity_log_run_id_idx` for the source runs, then the task by primary key. The precedence does not change: a recorded origin task wins, the run path applies only without one, and the activity path applies only without both. The activity path matches only an `entity_id` that is the canonical text of the task ID, as before (it also compares `id::text = entity_id`). - `packages/db`: add `issues_company_legacy_origin_run_idx` on `issues (company_id, origin_run_id) WHERE origin_issue_id IS NULL AND origin_run_id IS NOT NULL`. Migration `0461` creates it with `CREATE INDEX CONCURRENTLY IF NOT EXISTS` and is registered with the other concurrent migrations. `doc/DATABASE.md` documents the index and the benchmark command. - `server/src/services/issues.ts`: add `blockedInboxReviewWaitIssueIds`. It returns the in-review tasks in the blocker closure of the page rows. The blocked-inbox map reads review waits only for these tasks. The liveness classifier reads a wait only for a row or for a blocker that it reaches from a row, and the caller reads only the findings of the rows. The classifier does not change. - Tests: - `issue-created-from-routes.test.ts`: new cases for precedence in both directions, a source-run comment, an activity row whose task has another origin run, a non-canonical and a non-UUID activity entity ID, the source's own creation activity, and other-company rows. - `blocked-inbox-review-wait-scope.test.ts`: the narrowed set for chains, cycles, shared blockers and off-page tasks. The rows get the same findings with the narrowed waits as with the waits of every in-review task, and each narrowed wait is needed. - `issue-list-review-wait-authorization.integration.test.ts`: on real PostgreSQL with two authorized waits, `listExternalReviewWaits` and one issue-list request with blocked-inbox attention each send one bound-run message-link statement. With the per-wait authorization from before #20551, both tests fail (2 statements). - `scripts/benchmark-created-from-issues.ts`: local benchmark. It loads the baseline `issues.ts` and `issue-creation-origin.ts` from a pinned commit, asserts identical complete row payloads, and reports alternating timings and `EXPLAIN (ANALYZE, BUFFERS)`. ## Verification - `pnpm --filter @paperclipai/server exec vitest run src/__tests__/issue-created-from-routes.test.ts src/__tests__/blocked-inbox-review-wait-scope.test.ts src/__tests__/issue-list-review-wait-authorization.integration.test.ts src/__tests__/agent-task-hierarchy-depth.test.ts src/__tests__/issue-blocker-attention.test.ts src/__tests__/issue-list-assignee-filter-routes.test.ts src/__tests__/issues-parent-id-alias.test.ts src/__tests__/batched-chat-authorization.integration.test.ts src/__tests__/issue-liveness.test.ts` passes (126 tests). One timing test in `issue-list-assignee-filter-routes` failed one time under machine load and passed on rerun. - `pnpm --filter @paperclipai/server typecheck` and `pnpm --filter @paperclipai/db check:migrations` pass. - `pnpm verify` asks for broader verification because the change adds a file under `scripts/`. Required CI supplies that coverage. - Benchmark: `pnpm --filter @paperclipai/server exec tsx ../scripts/benchmark-created-from-issues.ts --baseline-ref=bd30ef8510` Fixture (synthetic, local PostgreSQL 18.1): one company with 30,000 tasks (10,628 with a recorded origin task, 10,445 with only a legacy origin run), 60,030 runs with wide context snapshots (145 MB), and 261,079 activity rows; a second company with 10,000 tasks and 20,000 runs. The busy source task has 63 created tasks through all three paths, plus decoys. The quiet source task has 3. The UI's page: `sortField=id`, `asc`, `limit=500`, routine executions included. Both versions return identical complete payloads. 9 alternating pairs after 2 warm-up pairs: | Case | List statement before | List statement after | List call before, median (min to max) | List call after, median (min to max) | | --- | --- | --- | --- | --- | | Busy source (63 tasks) | 234.9 ms, 47,437 buffers | 0.90 ms, 437 buffers | 156.0 ms (114.5 to 480.7) | 31.6 ms (19.9 to 62.7) | | Quiet source (3 tasks) | 90.7 ms, 47,413 buffers | 0.25 ms, 46 buffers | 126.1 ms (90.6 to 300.4) | 10.5 ms (7.2 to 26.1) | The list call time includes the enrichment reads (labels, runs, attention), which do not change. Inbox page (50 open tasks, blocked-inbox attention): the review-wait read goes from 2 statements with 1,215 parameters (every in-review task of the company) to 1 statement with 31 parameters. The page time does not change in this fixture (median 801 ms before, 873 ms after, both with a wide spread), because the fixture has no review waits and the other graph reads of the map are most of the time. With real waits, the server does an inherited GitHub review read for each wait it finds, so the gain grows with the number of off-page waits. Migration timing (local PostgreSQL 18.1): `0461` (measured as the same statement under its earlier number) on 500,000 tasks (846 MB, 20 companies, one third with a legacy origin run) takes 1.6 s. The index is 12 MB. ## Risks - The new index adds a small write cost to tasks with only an origin run. New tasks with a source task record `origin_issue_id`, so the index holds mostly older tasks. The migration does not lock writes. - The created-from rewrite must keep the same rows. The new tests and the benchmark payload check cover precedence, company scope, and ID text forms. Visibility and read filters are the list's other conditions and do not change. - The review-wait narrowing depends on the liveness classifier reading waits only for rows and their blocker chains. A unit test compares the findings of the rows with full and narrowed waits. A future classifier change that reads waits of other tasks must update `blockedInboxReviewWaitIssueIds`. The function comment says this. - The local timings are synthetic. Hosted acceptance needs a fresh query-statistics window after deployment. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, with tool use (shell, file edits, 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 - [ ] 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> |
||
|
|
1805a1d247 |
perf(workspaces): probe workspace run bindings through indexes (#20568)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The terminal workspace sweep checks each Git workspace for recent activity and for live consumers before it expires or moves the workspace > - These checks find the runs of a workspace with one SQL scope, `expirationScope`. The scope used one `OR` over all run bindings, and one branch decoded the context JSON of each run > - PostgreSQL could not use an index for that `OR`. Each check read every run of the company and its large context from TOAST. On a hosted database, each call took 2 to 5 seconds, about 5,200 s in total in one window > - This pull request splits the scope into one index probe per binding and adds one concurrent expression index for the legacy `executionWorkspaceId` context key > - The benefit is that each check reads a few index entries instead of all company runs. The cleanup decisions do not change ## Linked Issues or Issue Description **What existing behavior does this improve?** The workspace activity and cleanup-safety checks: `gitWorkspaceLastActivityAt`, `isGitWorkspaceExpirationEligible`, `workspaceMoveEligible` and the orphan-folder consumer check. All of them use `expirationScope` in `server/src/services/git-workspace-expiration.ts`. **Subsystem affected** Execution workspace cleanup in the server (`git-workspace-expiration.ts`, `isolated-repository-rehome.ts`, `isolated-task-directory-retention.ts` through the shared scope) and the `heartbeat_runs` schema in `packages/db`. **Current behavior** The `relevant_runs` CTE selects company runs where any of these is true: `issue_id` or `native_issue_id` is in the issue tree or the bound issues, the run is a bound issue's checkout or execution run, or the context JSON has a matching `issueId`, `taskId` or `executionWorkspaceId`. The planner does one sequential scan of the company runs and reads each context from TOAST. The cost grows with the run history of the company, not with the size of the workspace. **Proposed behavior** Each binding is a separate index probe. The `UNION` of run IDs goes to a primary-key probe. The SQL gives the same rows. **Reason and benefit** Local SQL measurements show a 34x to 169x decrease in time. The decrease grows with the run history (see Verification). ## What Changed - `expirationScope` builds `relevant_runs` from a `UNION` of index probes, one for each binding. The key sets go in as arrays (`= ANY(ARRAY(...))`). With `IN (subquery)`, the planner chose a hash join over a full scan, because it does not count the cost of reading TOASTed contexts. - The legacy context keys use `context_snapshot ->> 'key'`. This expression matches the existing indexes `heartbeat_runs_company_ctx_issue_created_idx` and `heartbeat_runs_company_ctx_task_created_idx`. For a non-object context, `->>` gives `NULL`, as the strict JSONPath decode did. Such a run can still match through a native binding. - New partial index `heartbeat_runs_company_ctx_execution_workspace_idx` on `(company_id, context_snapshot ->> 'executionWorkspaceId') WHERE context_snapshot ? 'executionWorkspaceId'`. The query repeats the predicate. For an array context, `?` can be true, but `->>` gives `NULL`, so the result does not change. The index is partial so that it serves only this probe. A full index that starts with `company_id` made the planner choose it for an unrelated company-scoped query. Migration 0460 builds it with `CREATE INDEX CONCURRENTLY IF NOT EXISTS`. The migration is in the list of migrations that run outside a transaction. - New tests: a legacy `issueId` or `executionWorkspaceId` binding on a run that is bound to a different issue still blocks cleanup. A plan test with `enable_seqscan = off` checks that no binding scans `heartbeat_runs` and that the three context indexes are used. - The benchmark script `scripts/benchmark-workspace-activity.ts` gets a `--runs-per-workspace` option. Unrelated fixture runs now have distinct context IDs, as real history does. - `doc/DATABASE.md` describes the probes and the new index. ## Verification - `pnpm --filter @paperclipai/server exec vitest run src/__tests__/git-workspace-activity.test.ts src/__tests__/native-harness-backup-expiration.test.ts`: 132 of 132 tests pass. The second file checks that the native session index stays the planner's choice for its query. These include the existing route matrix (root, child, bound, native, context issue, context task, context workspace, checkout, execution run), exactly-once counts, malformed and uppercase contexts, company isolation, controller leases and lease vetoes. - `pnpm --filter @paperclipai/server typecheck` and `pnpm --filter @paperclipai/db check:migrations` pass. - Benchmark (local SQL measurement, PostgreSQL 18.1, synthetic fixture, 8 workspaces per sample, before and after alternate in each iteration). The script compares the complete ordered caller results and the complete `relevant_runs` multisets of the baseline and treatment. They are equal in every run. `pnpm --filter @paperclipai/server exec tsx ../scripts/benchmark-workspace-activity.ts --baseline-ref=origin/master --runs-per-workspace=<N>` | Fixture | Check | Before median (min–max) | After median (min–max) | Speedup | | --- | --- | --- | --- | --- | | 4,800 runs, 330 MB | last activity | 523 ms (460–598) | 12 ms (8–15) | 44x | | 4,800 runs, 330 MB | eligibility | 452 ms (432–473) | 13 ms (11–15) | 34x | | 4,800 runs, 330 MB | idle eligibility | 465 ms (442–573) | 12 ms (10–13) | 40x | | 24,000 runs, 1.7 GB | last activity | 2,240 ms (2,214–2,454) | 13 ms (11–14) | 169x | | 24,000 runs, 1.7 GB | eligibility | 2,245 ms (2,097–2,377) | 17 ms (14–19) | 133x | | 24,000 runs, 1.7 GB | idle eligibility | 2,196 ms (2,179–2,381) | 15 ms (13–18) | 143x | Before: one `Seq Scan` on `heartbeat_runs`. After: `heartbeat_runs_company_issue_created_idx`, `heartbeat_runs_company_native_issue_contract_id_uq`, the three context indexes and `heartbeat_runs_pkey`. - Index build time: on a synthetic table with 200,000 runs and 8 KiB contexts (1.8 GB), the full form of this index took 3.7 s to build concurrently and used 15 MB. The partial index reads the same rows and holds only runs that have the key. ## Risks - Cleanup safety: a wrong "allowed" result could delete an active workspace. The scope keeps every binding of the old query. The benchmark checks exact equality of the run sets, and the existing tests check each binding route and each veto. - Migration: the index build reads every run context once. It runs concurrently and does not block writes. Until migration 0460 builds the index, the new query is still correct but the `executionWorkspaceId` probe scans. - Plan choice: the array form makes index probes the cheapest plan for a small tree. A very large issue tree gives a large array, but each element is still one index probe. - The sweep frequency does not change. This pull request reduces only the cost of each check. ## 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> |
||
|
|
d5c1f48180 |
perf(native): index the backup lease lookup by scope digest (#20571)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Native harness backup expiration removes old session backups. Before it removes a backup, it finds the environment leases that reference the backup root > - This lookup reads leases by run ID, by the `metadata.nativeHarnessBackup.sessionScopeSha256` digest, or by the identity's own lease ID, in one `OR` query > - The metadata arm has no index. So PostgreSQL reads every lease of the company, and the metadata of each lease, for each lookup > - On a hosted test database, this statement ran 21,736 times at a mean of 158 ms in one window. After other fixes, it still ran 2,364 times at a mean of 121 ms > - This pull request adds a small partial expression index on the digest > - The benefit is that each lookup reads a few index entries instead of all company leases. The query text and the result do not change ## Linked Issues or Issue Description **What existing behavior does this improve?** The lease lookup in `backupDependencies` in `server/src/services/native-runtime/native-harness-backup-expiration.ts`. `reconcileBackup` calls it for each backup root when the expiration worker loads or checks a root. **Subsystem affected** Native harness backup expiration and the `environment_leases` table schema. **Current behavior** The query is `company_id = $1 and (heartbeat_run_id in (...) or metadata->'nativeHarnessBackup'->>'sessionScopeSha256' = $n or id = $m)`. The run arm and the ID arm have indexes. The digest arm has no index. A `BitmapOr` plan needs an index for every arm, so the planner reads all company leases and evaluates the JSON expression on each row. Lease metadata holds sandbox details, so these reads are large. **Proposed behavior** The partial index `environment_leases_native_backup_scope_idx` keys leases by the digest. It holds only leases that have a backup stamp. Every arm of the `OR` now has an index, and the planner combines three index probes. The SQL and the returned lease IDs do not change. **Reason and benefit** This statement was in the top ten by total database time on a loaded hosted database. A local reproduction shows a query that is more than 6,000x faster and reads 9 buffers instead of about 436,000. **Breaking changes** None. The API, the lease-matching rules and the results do not change. ## What Changed - Add the partial expression index `environment_leases_native_backup_scope_idx` on `(metadata->'nativeHarnessBackup'->>'sessionScopeSha256')`, where that value is not null. - Add migration `0459`. It uses `CREATE INDEX CONCURRENTLY IF NOT EXISTS`. The migration runner runs it as a top-level statement, the same as migrations 0454–0458. - Add a regression test. It checks the lease-matching rules: run leases, a lease with only the backup stamp, and the identity's own lease match. Another digest, a non-string stamp, a top-level key with the same name, and the same digest in another company do not match. The test also checks that the custom plan and a forced generic plan of the generated SQL use `BitmapOr` with the new index and no sequential scan. - Add `scripts/benchmark-environment-lease-backup-lookup.ts`. It captures the SQL that the real service generates and measures it with and without the index. - Document the index in `doc/DATABASE.md`. ## Verification Cause and thesis. The cost comes from the one `OR` arm without an index, which forces a scan of all company leases and their metadata. Thesis: an index on the digest lets the planner use a `BitmapOr` of three index probes, so the cost no longer depends on the company's lease count. The results stay identical. The benchmark confirms the thesis. I did not change the SQL. With the index, the existing `OR` query already gets the index plan. A `UNION` rewrite would not help without the index, because the digest arm would still scan the company. Local SQL benchmark: ```sh pnpm --filter @paperclipai/server exec tsx ../scripts/benchmark-environment-lease-backup-lookup.ts ``` Fixture (synthetic): one company with 100,000 leases with 4 KiB of incompressible metadata each. 30% of the leases have a backup stamp for another session. A second company has 20,000 leases. The table is 678 MB. The backup root has 3 run leases, 1 lease with only the stamp, and 1 identity lease. The same digest is also in the other company. Embedded PostgreSQL 18 on an Apple laptop, which other work loaded heavily. The hosted database is Neon with different compute, so absolute times are local SQL measurements only. The script measures in the order: no index, index, no index, index. Each measurement has 2 warmups and 11 samples of `EXPLAIN ANALYZE` execution time and of the full `reconcileBackup` call. | Measurement | Without index | With index | | --- | --- | --- | | Lease query execution, run 1 | median 196 ms (138–562 ms) | median 0.010 ms (0.007–0.017 ms) | | Lease query execution, run 2 | median 185 ms (144–295 ms) | median 0.030 ms (0.020–0.045 ms) | | `reconcileBackup` wall time (includes file reads), runs 1 and 2 | median 188 ms and 216 ms | median 1.4 ms and 4.2 ms | | Plan | `Gather > Seq Scan` | `Bitmap Heap Scan > BitmapOr` of `heartbeat_run_idx`, the new index and `pkey` | | Buffers | 365,880 hit + 70,050 read | 9 hit | These numbers are from head `65bd9d7ac6`. The final head only renumbers the migration to 0459 after rebases; the SQL, the index and the service code are the same. Two earlier runs on the same fixture, SQL and index gave 236–425 ms without the index and 0.023–0.033 ms with the index. The machine had heavy load from other work, so the spread without the index is wide. Output check: in all four measurements, the service returned the same 5 lease IDs, and the complete query rows were equal. Index build (`CREATE INDEX CONCURRENTLY` from the migration file): 0.8–2.4 s for the 678 MB table over all runs. Index size: 3.4 MB with 30% of leases stamped. Tests: - `cd server && npx vitest run src/__tests__/native-harness-backup-expiration.test.ts`: 109 passed. The new test failed when I dropped the index before the plan check (confirmed red), and it passes with the index. - `pnpm --filter @paperclipai/db check:migrations`: passed. - `pnpm --filter @paperclipai/db typecheck` and `pnpm --filter @paperclipai/server typecheck`: passed. - `pnpm verify`: it asks for broader coverage (exit 2) because of the new script. See the CI results for this head. ## Risks - Migration time. A concurrent index build reads the full `environment_leases` table and does not block writes. Locally, the build took 0.8–2.4 s for 678 MB. The time grows linearly with the table size. I did not time it on a production-size clone. I expect a few seconds to tens of seconds on a large instance, 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 expression must stay textually equal to the query expression. If someone changes the JSON path, the planner cannot use the index. The new test catches this for custom and generic plans. - The index is partial, so ANALYZE does not collect statistics for the expression. The planner uses the default equality estimate. On the large fixture and in the small test, the planner still selects the index plan. A company with very few leases can still get a sequential scan, which is cheap. - Each lease write with a backup stamp writes one more small index entry. The write cost is small. - This change does not reduce how often the worker calls the lookup. It makes each call cheap. The other digest lookups in backup retention (`native-harness-backup-retention.ts`) and session retention checks can also use the index. - Previous-release story: no persisted state changes. The index is derived from existing lease metadata. - 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 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> |
||
|
|
765ccd8691 |
perf(chat): index chat-admission checks by company and payload issue id (#20562)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work.
> - Chat connections save each external message as a `chat_actions` row
before the server admits it as an agent wakeup.
> - The server checks if an issue holds chat input that it did not
admit. Recovery skips such an issue. Manual task start refuses it.
> - On a hosted instance, the recovery read with this check used the
most total database time: 488 calls at about 1 s each. The manual-start
check ran 155 times at about 0.9 s.
> - The check matches `chat_actions.payload->>'issueId'` against the
issue. No index covers this value, so each call reads the full
`chat_actions` table and its payloads.
> - This pull request adds one concurrent partial index for the check.
The check SQL does not change.
> - 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?**
The chat-admission check `unadmittedChatWakeupCondition`. Recovery uses
it in the per-issue candidate read. Manual task start uses it in the
held-input check.
**Subsystem affected**
Server chat admission, issue recovery, manual task start, and the
database schema.
**Current behavior**
The check finds `inbound_wakeup` actions by `company_id` and
`payload->>'issueId'`. Two nested checks find `failed_run_retry` actions
and later `inbound_wakeup` actions on the same company and payload issue
ID. `chat_actions` has no index on `company_id` or on the payload issue
ID. Each call reads the full table and reads each payload. On a hosted
instance, the two statements used about 630 s of database time in one
statistics window.
**Proposed behavior**
The check and its nested checks read a small set of rows from one index.
The admission result does not change.
**Reason and benefit**
The hosted database had many concurrent active queries. Request reads
waited behind background work such as this check.
**Breaking changes**
None. The migration adds an index only.
## What Changed
- Migration `0458_chat_admission_issue_index` builds
`chat_actions_company_issue_kind_idx` with `CREATE INDEX CONCURRENTLY IF
NOT EXISTS`. It runs through the existing top-level statement path,
which repairs an invalid index on retry.
- Keys: `(company_id, payload->>'issueId', kind, status)`.
- Partial: `kind IN ('inbound_wakeup', 'failed_run_retry')`.
- One index serves the outer input lookup, the failed-run retry lookup,
and the later-input lookup. All three match on the same company and
payload issue ID.
- The Drizzle schema declares the index. The snapshot chain moves to
`0458` and keeps five snapshots.
- New regression test
`server/src/__tests__/chat-admission-condition.test.ts` pins the
admission semantics on real PostgreSQL:
- Pending and failed input without a receipt holds the issue. Received,
processed and cancelled input does not. Other action kinds do not.
- A skipped receipt, a receipt for another agent, or a receipt in
another company does not admit input. A failed or cancelled run still
counts as admission.
- The issue ID is compared as company-scoped text. Uppercase, array,
null, other-issue and other-company values do not match.
- Only an admitted retry in the same conversation and issue that lists
the action releases skipped input.
- Only later admitted input in the same conversation, issue and session
generation releases a failed input. Pending input stays held.
- The check reads `chat_actions` only through the new index.
- `scripts/benchmark-chat-admission-index.ts` measures both caller
statements with and without the index and fails if the rows differ.
- `scripts/benchmark-chat-admission-index-migration.ts` times the
migration through the normal migration runner.
- `doc/DATABASE.md` documents the index and the benchmark commands.
## Verification
Local SQL measurements on synthetic data. PostgreSQL 18.1 (embedded),
default settings. The fixture has 100,995 `chat_actions` rows (662 MB):
60,000 GitHub actions with 6 KiB incompressible payloads, 2,000 issues
with 20 admitted inbound messages each (3 KiB payloads), and 1,000
failed-run retries. Each case has 2 warmups and 9 samples of `EXPLAIN
ANALYZE` execution time. Runs alternate: no index, index, no index,
index.
| Caller statement | Case | No index, median (range) | Index, median
(range) |
| --- | --- | --- | --- |
| Manual-start held check | quiet issue | 3,093 ms (2,150–3,519) / 3,586
ms (2,375–11,235) | 0.036 ms (0.034–0.052) / 0.040 ms (0.033–0.047) |
| Manual-start held check | admitted input | 3,555 ms (3,310–8,080) /
5,205 ms (3,353–9,525) | 0.049 ms / 0.052 ms |
| Manual-start held check | unadmitted input | 3,002 ms (2,864–3,221) /
4,179 ms (3,200–6,585) | 0.051 ms / 0.044 ms |
| Manual-start held check | failed, later admitted | 3,586 ms / 3,722 ms
| 0.066 ms / 0.054 ms |
| Manual-start held check | admitted retry | 2,733 ms / 4,953 ms | 0.049
ms / 0.053 ms |
| Recovery candidate read | quiet issue | 43 ms (41–48) / 96 ms (44–219)
| 0.047 ms / 0.055 ms |
| Recovery candidate read | admitted input | 203 ms (48–575) / 174 ms
(44–314) | 0.055 ms / 0.058 ms |
| Recovery candidate read | unadmitted input | 66 ms / 144 ms | 0.053 ms
/ 0.054 ms |
- Buffers per call fall from about 40,000 shared hits to 5–23.
- Locally, the recovery read without the index uses a hashed full scan
of `chat_actions`. The held check uses a full scan per call. On the
hosted instance both took about 1 s per call. The local fixture is
smaller than the hosted table.
- Correctness: complete positional rows match across all four runs for
all six cases (quiet, admitted, unadmitted, failed then admitted,
retried, other-company decoy). The script asserts the expected held
result for each case.
- Migration timing: 600,001 `chat_actions` rows (1,759 MB), 300,000 of
them in the index scope. First run through `applyPendingMigrations`: 4.2
s. Rerun after the history row is removed: 0.5 s. Concurrent build
alone: 3.4 s. Index size: 30 MB.
Commands:
```sh
pnpm --filter @paperclipai/server exec tsx ../scripts/benchmark-chat-admission-index.ts
OTHER_PAYLOAD_KIB=2 pnpm --filter @paperclipai/server exec tsx ../scripts/benchmark-chat-admission-index-migration.ts
cd server && npx vitest run src/__tests__/chat-admission-condition.test.ts # 6 passed
pnpm --filter @paperclipai/db check:migrations # passed
pnpm --filter @paperclipai/server typecheck # passed
```
## Risks
- The migration builds one index concurrently. It does not block writes.
On the 1.76 GB local fixture it took about 4 s. A larger hosted table
takes longer, but the build does not lock out reads or writes.
- Writes to `inbound_wakeup` and `failed_run_retry` actions maintain one
more index. Status updates on these rows were already not HOT updates,
because other partial indexes use `status`.
- The planner has no expression statistics for a partial index. The new
test pins that the check uses the index on an analyzed table with
unrelated rows.
- The check SQL does not change, so admission behavior does not change.
> For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and
discuss it in `#dev` before opening the PR. Feature PRs that overlap
with planned core work may need to be redirected — check the roadmap
first. See `CONTRIBUTING.md`.
## Model Used
- Claude Opus 5.5 (Anthropic), model ID `claude-opus-5-5`, used through
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
- [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>
|
||
|
|
56aa160a4e |
perf(native): index heartbeat runs by company and native session (#20560)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Native runs share a native session. Retention, recovery, resume and archive code reads all runs of one session from `heartbeat_runs`. > - On a hosted instance, the native retention lookup ran about 67,000 times in one day at about 22 ms, and about 10,000 times at about 11 ms in a later window. > - No index has `native_session_id`. Each read walks the agent's or the company's run history and filters the rows. > - This pull request adds one concurrent partial index on `(company_id, native_session_id)`. The SQL does not change. > - The benefit is less background database load. Request reads wait less, and the results stay the same. ## Linked Issues or Issue Description **What existing behavior does this improve?** The native session-run lookups. The largest caller is `sessionRetentionBlocker` in native harness backup and checkpoint retention. It selects `id, status, finished_at`, an incomplete-recovery flag and the issue ID for one company, agent and native session. **Subsystem affected** Server native runtime (retention, recovery, resume and archive reads) and the database schema. **Current behavior** `heartbeat_runs` has no index on `native_session_id`. PostgreSQL reads the whole run history of the company (or of the agent through `heartbeat_runs_company_agent_started_idx`) and filters by session. The cost grows with the run history, not with the size of the session. **Proposed behavior** The index finds the few runs of one session directly. Every caller already filters by `company_id` and `native_session_id`, so one index serves the agent-scoped and the issue-scoped reads. **Reason and benefit** This statement family used about 1,500 s of database time in one day on a busy hosted database. The local proof below shows 12.3 ms to 0.04 ms per call. ## What Changed - `packages/db`: add `heartbeat_runs_company_native_session_idx` on `(company_id, native_session_id) WHERE native_session_id IS NOT NULL`. Migration `0457_native_session_run_index` builds it with `CREATE INDEX CONCURRENTLY IF NOT EXISTS`. The migration is registered as a top-level-statement migration, the same way as 0454–0456. - Add a regression test in `native-harness-backup-expiration.test.ts`. It captures the real retention lookup and proves that its plan can use the index. It also pins the company and session scope: a live run of another company with the same session ID does not protect a checkpoint. A live run of the same agent in another session does not protect it. A live run of the same session does protect it. - Add `scripts/benchmark-native-session-run-index.ts`. It runs the real checkpoint retention pass (dry run), captures the exact SQL, and replays it with and without the index. It also times the migration through the normal migration runner. - `doc/DATABASE.md`: describe the index and the benchmark command. ## Verification Local SQL measurements, synthetic data. PostgreSQL 18.1 (embedded test server). Fixture: 100,000 runs (50,000 of the measured agent, 50,000 of a second agent), 4 runs per session, 4 KiB incompressible `runner_profile_json` per run, 638 MB table. Order: no index, index, no index, index. 15 `EXPLAIN (ANALYZE, BUFFERS)` samples after 3 warmups. | Pass | Plan | Median execution | Min / max | Shared buffers | | --- | --- | --- | --- | --- | | No index (1) | Seq Scan | 12.30 ms | 10.33 / 35.27 ms | 3,607 | | Index (2) | Index Scan using `heartbeat_runs_company_native_session_idx` | 0.036 ms | 0.021 / 0.115 ms | 38 | | No index (3) | Seq Scan | 12.38 ms | 10.24 / 26.14 ms | 3,607 | | Index (4) | Index Scan using `heartbeat_runs_company_native_session_idx` | 0.043 ms | 0.022 / 0.098 ms | 38 | - Correctness: the service SQL is the same in every pass. The returned rows (all columns) and the retention result are equal in every pass. The script fails on any difference. - Migration: the first run through the migration runner took 525 ms on the 638 MB fixture. The rerun took 273 ms and skipped the existing index. A direct concurrent build took 145–221 ms. Index size: 1.9 MB. - `pnpm --filter @paperclipai/server exec tsx ../scripts/benchmark-native-session-run-index.ts` - `npx vitest run src/__tests__/native-harness-backup-expiration.test.ts` (server): 108 passed. - `pnpm --filter @paperclipai/db check:migrations`: passed. Five snapshots remain. - `pnpm verify` exits 2 (a new script under `scripts/` is a global CI input). Required CI supplies the broad coverage. ## Risks - Low risk. The migration adds one index concurrently and does not lock writes. The SQL of every caller is unchanged. - The index adds a small write cost to `heartbeat_runs` inserts and to updates of `native_session_id`. Rows with a null session are not indexed. - Persisted run state: no saved run, session or checkpoint shape changes. State saved by the previous release carries forward unchanged. - Hosted latency is not measured yet. The numbers above are local SQL measurements. ## Model Used - Claude Opus 5.5 (Anthropic), model ID `claude-opus-5-5`, used through Claude Code with tool use (shell, file edits, local PostgreSQL). ## 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> |
||
|
|
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> |
||
|
|
0116e45958 |
perf(db): index pending budget Stops instead of scanning all run results (#20391)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Budget enforcement stops agent runs when a budget hard limit is reached. The stop intent is saved in each run's `result_json` until run control delivers it. > - The budget worker, run control, and the idle-sleep check find these pending stops with a predicate on `result_json`. No index supports that predicate. > - Each check therefore reads every heartbeat run and its large result JSON from TOAST. On a hosted database, this statement family was among the most expensive: about 8,000 calls at about 1 s each, about 9,800 s in total. > - Pending stops are rare. The predicate is constant, so a partial index can contain only those rows. > - This pull request adds that partial index. The SQL that the server sends does not change. > - The benefit is that each check reads a few index entries instead of the full table. The database has more capacity for request work. ## Linked Issues or Issue Description **What existing behavior does this improve?** The check for committed budget Stops that are not delivered yet. The budget worker runs it each time it looks for work (`hasPendingBudgetEnforcement`), and again for each delivery pass (`listPendingBudgetCancellationScopes`). Run control and the idle-sleep check use the same predicate. **Subsystem affected** Budget enforcement in the server (`server/src/services/heartbeat/budget-cancellation.ts`, `server/src/services/budgets.ts`, `server/src/services/heartbeat/run-control.ts`, `server/src/services/idle-sleep-safety.ts`) and the `heartbeat_runs` schema in `packages/db`. **Current behavior** The query groups all rows of `heartbeat_runs` and reads `result_json->'startupCancellation'->'budgetEnforcement'` for each row. A limit of 1 does not help, because the group and sort occur before the limit. Cost grows with the run history, not with the number of pending stops. **Proposed behavior** A partial index, `heartbeat_runs_pending_budget_cancellation_idx`, contains only runs with a pending budget Stop. The planner uses it for all four callers. The results are the same. **Reason and benefit** Local SQL measurements show a 300x to 1,000x decrease in time and buffer reads (see Verification). **Breaking changes** None. ## What Changed - Added the partial index `heartbeat_runs_pending_budget_cancellation_idx` on `heartbeat_runs (company_id)`. Its predicate is identical to `pendingBudgetRunCancellationPredicate`. - Added migration `0455_pending_budget_cancellation_index.sql`. It uses `CREATE INDEX CONCURRENTLY IF NOT EXISTS`, and it is in the list of migrations that run outside a transaction, as `0436` is. - Moved the scoped run-control read and the idle-sleep check into exported builders in `budget-cancellation.ts`, next to the scope list query. The SQL does not change. The callers and the test now share the same statements. - Added a real PostgreSQL regression test. It checks that delivered, malformed, and missing intents are not pending. It also EXPLAINs the statements that the callers send (scope list with limit 1 and 100, scoped read, `EXISTS` check) and requires the new index in each plan. If a person changes a query shape, the server predicate, or the index predicate, the test fails. - Added `scripts/benchmark-budget-cancellation-scan.ts`. It captures the SQL that the real service sends, then compares the plans, timings, and complete rows with and without the index. - Updated `doc/DATABASE.md`. ## Verification Red/green regression test: - Before the migration, the new test failed: the plan was `Seq Scan` with the predicate as a filter. - After the migration: `npx vitest run src/services/heartbeat/budget-cancellation.test.ts` passes (17 tests). - Also passed: `src/__tests__/budgets-service.test.ts`, `src/__tests__/idle-sleep-diagnostics-postgres.test.ts`, `src/services/budget-resume-continuation.test.ts`. - `pnpm --filter @paperclipai/db check:migrations`, and typecheck of `@paperclipai/db` and `@paperclipai/server` pass. Performance and result equality (local SQL measurements only): ```sh pnpm --filter @paperclipai/server exec tsx ../scripts/benchmark-budget-cancellation-scan.ts --runs=20000 --payload-kib=16 [--reverse] ``` - Fixture: embedded PostgreSQL 18.1. 20,241 runs in 2 companies, with incompressible result JSON of about 16 KiB (table 379 MB). 40 pending stops in 6 scopes, 200 delivered or malformed decoys, and 1 cross-company intent. - The SQL is the same in both treatments. The script toggles only the new index. 2 warmups and 7 samples per query. Rows are compared outside the timed part. Ordered queries must match in order. The scoped run-control read has no `ORDER BY`, so it is compared as a multiset. | Query (caller) | Baseline median | With index median | Shared buffers | | --- | --- | --- | --- | | `listPendingBudgetCancellationScopes(db, 1)` (worker `hasPending`) | 688 ms | 2.1 ms | 243,500 → 1,158 | | `listPendingBudgetCancellationScopes(db)` (delivery) | 799 ms | 1.4 ms | 243,500 → 1,158 | | Scoped pending runs (run control) | 357 ms | 1.1 ms | 121,835 → 322 | | `EXISTS` pending (idle-sleep check) | 440 ms | 0.37 ms | 240,560 → 2 | - Reverse order (index first) gave the same result: 524 ms → 1.6 ms, 464 ms → 1.4 ms, 306 ms → 1.0 ms, 306 ms → 0.42 ms. - Plans change from `Seq Scan` (or a broad bitmap scan) to `Index Scan` or `Index Only Scan` on the new index. - All result rows are equal in every run. Migration duration: - 20,241 runs (379 MB): index build 2.0 s to 3.9 s. - 200,241 runs with 8 KiB result JSON (2.1 GB): index build 5.6 s. With this table size, the four queries change from 1,148 / 879 / 514 / 4,392 ms to 1.1 / 1.2 / 0.85 / 8.3 ms. - The build is concurrent, so it does not block run writes. ## Risks - Migration: the build reads every run's result JSON one time. It is concurrent and resumable (`IF NOT EXISTS`). The executor repairs an invalid concurrent index on retry. - Write cost: run writes evaluate the index predicate. The index contains very few rows, so the extra cost is small. - Predicate drift: if a later change edits only one of the two predicates, the planner cannot use the index. The new regression test detects this. - The hosted timings are not measured yet. The local numbers come from a synthetic fixture. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude Code with tool use (shell, file edits, local PostgreSQL). ## 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> |
||
|
|
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> |
||
|
|
facd409073 |
perf(workspaces): index legacy workspace companion lookups (#20372)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Online workspace migration must retain prepared sources and recovery holds. > - Companion discovery reads saved workspace references from run profiles. > - The selector can scan large saved profiles even when it returns few rows. > - This pull request adds one company and workspace expression index. > - Companion discovery uses the same expression as existing migration lookups. > - The result preserves each source check and reduces history scan work. ## Linked Issues or Issue Description **What happened?** The companion selector used a JSON path expression with no matching index. A read-only plan on about 22,000 run rows showed a sequential scan over a table with about 1.3 GB of table and TOAST storage. A repeated inspection reached its statement timeout. Existing migration lookups use a different JSON operator form, so one expression index must match both callers. **Expected behavior** Use a company-scoped indexed lookup. Keep prepared and finalized references, the exact text comparison, native issue references, and run order. Keep all source and recovery validation. **Steps to reproduce** 1. Retain many run profiles with saved workspace references and wide JSON fields. 2. Inspect a workspace with few matching companion runs. 3. Explain the company and saved-workspace selector. Without the new index, it can scan run history and read unrelated JSON values. **Paperclip version or commit** Baseline: `0e3fdebea8257bdff11573081c76b306f12db230`. **Deployment mode** Server with PostgreSQL. The local proof uses disposable PostgreSQL 18.1. The observed server plan used PostgreSQL 17.11. Related: [batched companion discovery in #19735](https://github.com/paperclipai/paperclip/pull/19735). The existing batching remains in place. The duplicate search found no open workspace index change. This branch also carries the isolated API route-summary correction shared with [#20370](https://github.com/paperclipai/paperclip/pull/20370). ## What Changed - Add one concurrent index on company ID and the saved native workspace ID expression. The index has no state, runtime, or status predicate. - Use nested JSON operators in companion discovery. Existing migration lookups already use this expression. Keep both accepted states and the same ordering. - Register the migration on the existing top-level migration path. Retain its advisory lock, interrupted index repair, and completion journal behavior. - Add a real PostgreSQL migration and ordered-result test. Update the existing companion query counter for the equivalent SQL form. - Generate the schema snapshot and retain the five newest snapshots. - Describe the task case-list route as “Find the cases for a task.” This keeps the route on the first discovery search page. Search logic and test assertions are unchanged. ## Verification - `pnpm --filter @paperclipai/db exec vitest run src/heartbeat-workspace-companion-index.test.ts --maxWorkers=1`: **1 passed**. It builds the concurrent index through the canonical migrator, checks index validity, preserves prepared/finalized and cross-issue/null-issue results, excludes foreign and malformed references, and checks one journal entry after a second migration call. - `pnpm --filter @paperclipai/server exec vitest run src/services/remote-workspace-online-source.test.ts -t 'discovers all Git companions' --maxWorkers=1`: **1 passed; 86 not selected**. This uses the real source owner and retains finalized recovery holds, company isolation, and a fresh obligation between inspections. - `pnpm --filter @paperclipai/db typecheck`, migration numbering/safety checks, and `git diff --check`: **passed**. - One small local SQL comparison used the captured baseline selector, the changed selector, 3,012 synthetic rows, about 37.8 MB of table storage, and the new expression index. Ordered results were identical. One client timing changed from **8.70 ms to 0.86 ms**. This is one local warm-cache comparison, not a server latency claim. Hosted improvement is **unverified**. - Initial setup attempts failed before the affected test ran: registry access, local tsx IPC, and a missing SDK build. The canonical Node 24.22.2/pnpm 9.15.4 setup and SDK prerequisite resolved these setup failures. The local SQL timing client used Node 26.4.0; the focused application tests used Node 24.22.2. - [The first CI run](https://github.com/paperclipai/paperclip/actions/runs/42705129724/job/129460702073) failed the inherited case-history API first-page search assertion. The same failure was reproduced on the unchanged base: the route ranked sixth for “case for task.” The isolated route-summary correction from #20370 passed the same three assertions. The search algorithm and assertions were not changed. - No public command changes require a documentation update. The schema and migration record the added index. No hosted DDL or deployment was performed for this PR. - Final head `c50101dabf46ade42b545d2051d724d93f413ccd`: [full CI](https://github.com/paperclipai/paperclip/actions/runs/42707623920) passed with 60 successful checks and three configured skips. [Fresh Greptile review](https://github.com/paperclipai/paperclip/pull/20372#issuecomment-6752305627) is 5/5 on this head with no open threads. ## Risks - The index adds storage and write cost when a run profile changes. - A concurrent build reads the table and can wait for old transactions. The existing migration path runs it outside a transaction and can repair an interrupted invalid index. - Existing nested-operator lookups can use the index. The companion selector needs this code change to use it. Other JSON path queries are unchanged. - The small local result does not establish end-to-end import speed or PostgreSQL 17 hosted latency. Fresh deployed measurement remains necessary. ## Model Used OpenAI Codex, GPT-6 family. The exact runtime model ID and context window are not exposed in this session. Used reasoning, code editing, shell tools, and local PostgreSQL tests. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
|
d2fe85e57c |
fix(secrets): delete a secret that a binding proposal referenced (#20193)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Agents can propose a secret binding. A person approves or rejects it on a review card. > - Each binding proposal row refers to one source: a secret or a pending secret proposal. A check constraint enforces this. > - The secret reference used `ON DELETE SET NULL`. When a person deletes the secret, Postgres sets the reference to null. The check constraint then rejects the row. > - So a person cannot delete any secret that a binding proposal ever referenced. The API returns 500. > - This pull request deletes the proposal rows together with the secret, as the other source reference already does. Before the delete, it rejects pending proposals so their cards close with a reason. > - The benefit is that secret deletion works again and no review card points at a removed proposal. ## Linked Issues or Issue Description Refs #15644 (related: block secret deletion while bindings still exist). **What happened** `DELETE /api/secrets/:id` returned 500 for a secret that a binding proposal referenced. The server log showed `new row for relation "company_secret_proposals" violates check constraint "company_secret_proposals_shape_check"`. This happened for pending, rejected and approved proposals. It also happened after the person removed the agent's binding. **Expected behavior** The person can delete the secret. Proposals that referenced it go with it. A pending review card closes as rejected and tells why. **Steps to reproduce** 1. An agent proposes a binding for an existing company secret (`POST /api/agents/me/secret-proposals` with `secretId`). 2. Approve or reject the proposal, or leave it pending. 3. Delete the secret with `DELETE /api/secrets/:id`. 4. The response is 500 and the secret stays. ## What Changed - `packages/db`: `company_secret_proposals.secret_id` now uses `ON DELETE CASCADE`, the same as `secret_proposal_id`. Migration `0452_secret_proposal_secret_cascade.sql` replaces the foreign key (`DROP CONSTRAINT IF EXISTS`, then `ADD CONSTRAINT`). - `server/src/services/secrets.ts`: `remove` accepts an optional `beforeDelete` hook. The hook runs after the secret is marked deleted and before its row is removed. - `server/src/services/secret-proposals.ts`: new `rejectPendingBindingsForSecret`. It rejects pending binding proposals for a secret with a reason and returns them. This closes their review cards and writes the normal activity entries. `createBinding` now holds the secret row `FOR SHARE` in its insert transaction and refuses a deleted secret. So no new pending proposal can start after the delete marks the secret deleted. - `server/src/routes/secrets.ts`: the delete route rejects pending proposals in `beforeDelete`. For each one it sends the normal rejection notice and wake (`notifySecretProposalResolution`), so the proposing agent continues. - `server/src/__tests__/secret-proposals-routes.test.ts`: a regression test deletes a secret with a pending, rejected and approved binding proposal through the route. It checks the secret and proposal are gone, the card status is correct, and only the pending case sends one rejection notice and one wake. A second test checks that a binding proposal for a secret that the delete already marked deleted fails with 404. ## Verification - Red first: the new test failed on `origin/master` with the shape-check violation for all three cases. - `cd server && vitest run --config vitest.config.ts src/__tests__/secret-proposals-routes.test.ts`: 47 passed. - `vitest run src/__tests__/chat-run-milestone-governed-work.test.ts src/__tests__/agent-secrets-routes.test.ts src/__tests__/secrets-service.test.ts`: 112 passed. - `tsc --noEmit` for `server` and `packages/db`: pass. `pnpm --filter @paperclipai/db run check:migrations`: pass. - Found by the `secrets.secret-proposal` chat evaluation. Its cleanup deletes the fixture secret and failed on this error in runs 42563093640 and 42571127911. ## Risks - The migration only replaces one foreign key. It does not rewrite rows. Existing rows already have a non-null `secret_id` or `secret_proposal_id`, so nothing else changes. - Deleting a secret now also deletes its binding proposal rows. The activity log keeps the proposal history (`secret.proposal.*` and `secret.deleted`). Secret-kind proposal rows that created the secret keep `created_secret_id ON DELETE SET NULL`, so their history stays. - Other paths that delete secrets (company deletion, user secret cleanup) now cascade instead of failing on the same constraint. They do not reject pending cards first. ## Model Used Anthropic Claude Opus 5.5 (`claude-opus-5-5`), used with tool use and code execution in Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
5a51a7685d |
fix(budgets): raise_budget_and_resume continues the work its budget stopped, once (#20144)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A budget hard stop cancels the scope's running work; the board resolves it with Raise budget and resume > - In the restart-chaos H1 scenario, the cancelled tasks did not continue after resume and each needed a manual Retry > - Resume is the board's explicit decision to continue, so the work it stopped should continue once, as if the board clicked Retry > - This pull request files that Retry automatically, keyed by the cancelled run, once the run's stop has settled > - The benefit is that a budget raise restores the work without manual Retries, without repeating completed work, and without charging the run's attempt budget ## Linked Issues or Issue Description Refs #20143 (bug B; this PR builds on it), #20113 (bug A), #20101 (superseded: split into #20143 and this PR) **What happened?** Restart-chaos waves w-20261108T195944-s42 and w-20261108T210118-s9802 (H1). After `raise_budget_and_resume`, CHA-3's budget-cancelled run (enforcement stop) and CHA-1's run (agent lifecycle pause from the budget hold) stayed cancelled until someone clicked Retry. **Expected behavior** Resuming continues each task its budget stopped, exactly once, with no attempt charged, an activity entry, and no repeat of work that already continued. A run a person stopped stays stopped. **Steps to reproduce** Set a budget below spend while a task runs; let the stop cancel the run; resolve with Raise budget and resume. Before this change, no new run is queued. ## What Changed - New table `budget_resume_claims` (migration 0451). One row per cancelled run, outside the run's result JSON, so a Stop that settles during the resume cannot erase it. - `markBudgetCancelledWorkForResume` (called by `resolveIncident` before the incident closes): claims every task run its budget stopped since the incident opened. That covers runs budget enforcement fenced, including stops still in flight, and runs the agent lifecycle pause stopped for this budget (`pauseStop`, carried from the lifecycle operation's `pauseCause` so a resume before the Stop keeps the link). Runs a person stopped are excluded. Two indexed passes led by `(company_id, status)`. - `deliverBudgetResumeContinuations`: once a claimed run's stop settles, files the resolving board member's Retry (`requestTaskRetry`, idempotency key `budget-resume:<run>`, with the member's session source and the chat Retry callback). The Retry path checks, under the task lock, that the run is still the task's latest and the task is open, so continued or completed work is never repeated. Claim settlement and its activity entry commit together; retryable refusals and errors retry. - Delivery runs in the always-on budget-enforcement worker after each pass's Stop delivery. Its schedule includes deliverable claims and the earliest stale-claim expiry. The execution-continuation worker is a backstop. - `agent.paused` / `agent.resumed` logging is in #20113. `budget.cancelled_work_resume_requested` (incident) and `budget.cancelled_work_continued` / `_not_continued` (task) are recorded here. ## Verification - New `budget-resume-continuation.test.ts` (Postgres, real `resolveIncident` → claims → `requestTaskRetry` → real `resumeTaskRetry` into a queued run): - each task continues once, including a blocked task; repeat delivery or resolve saves nothing new; - a stop still in flight at resume, whose terminal write replaces the result JSON, is still continued; - a 30-task scope is fully continued; - a lifecycle-pause stop caused by the budget is continued, and one caused by a manual pause is not; - continued work, done tasks, a run that finished before its stop, and person-stopped runs (including ones enforcement also fenced) are not continued. - `lifecycle.test.ts`: the pause records its cause on the operation and on stopped runs, including when the budget is resumed before the pause's Stop. `budget-enforcement-worker.test.ts`: delivery after Stop delivery, and a wake at claim expiry. - 287 tests across the affected suites passed. Server typecheck (`typecheck-layers`, low-memory): no errors. - Not run: the H1 chaos rerun (Phase 3 runs it after merge). ## Risks - Resume now continues budget-stopped tasks automatically. A board that wants a task stopped can Stop it again; person-stopped runs are never continued. - A new table and one index (migration 0451). > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edits, test execution). Extended reasoning enabled. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] 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> |
||
|
|
c49ab6b9ee |
feat(issues): wake the parent once when a child or blocker stalls on the board (#20098)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A parent task delegates to children and waits on blockers. Its agent learns of progress only through wakes > - Today only completion (`issue_children_completed`) and resolution (`issue_blockers_resolved`) wake a waiting issue > - In restart chaos `w-20261108T181313-s9701` t05, a blocker stopped on a recovery action that needs the board, and its parent sat silently blocked for 50+ minutes > - This pull request wakes the waiting agent once per such stall, tells it what it can do, and shows the stall on the parent's page with the child's Retry > - The benefit is that a stuck child or blocker no longer silently strands its parent ## Linked Issues or Issue Description Refs #19831 (durable ownership plan; restart chaos campaign). Restart chaos wave `w-20261108T181313-s9701`, task t05 (M1, claude). **Problem** From the kept database: - CHA-8 `blocks` CHA-5. - CHA-8's board-owned recovery action (`native_session_retry_exhausted`) was created at 18:44:02.909 and resolved with outcome `blocked` at 18:44:08, when CHA-8 moved to `blocked` for the board. - CHA-5's only wake in the whole wave was its `issue_assigned` at 18:20:48. Its agent never heard about CHA-8. **Expected behavior** - When a child or blocker stalls on the board, the waiting agent is woken once. It can then re-delegate, escalate, or block with a reason. - The board sees the stall on the waiting task, with the child's Retry. ## What Changed - **Shared:** - `ISSUE_BLOCKER_STALLED_WAKE_REASON` (`issue_blocker_stalled`), `ISSUE_BLOCKER_STALL_RELATIONS`, and the types `IssueBlockerStallRelation` and `IssueBlockerStalledWakeContext`; - `Issue.stalledDependencies`. - **Server, `services/recovery/blocker-stall-wakeups.ts`:** - A stall is an issue that is not done or cancelled, whose latest recovery action needs the board. That means either open (`active`/`escalated`) and board-owned or escalated, or board-owned and resolved as `blocked` while the issue is still `blocked`. The second case is what t05 needed: recovery resolves its action that way when it hands the issue to the board. - Waiters are the parent, and every issue the stalled one `blocks`, when they are open, visible and agent-assigned. - Each waiter gets one wake per stall. The idempotency key `issue_blocker_stalled:{waiter}:{stalled}:{recoveryActionId}` is checked through the indexed `(company_id, payload->>'issueId')` path. A later stall has a new action id, so it wakes again. - Each wake writes activity `issue.blocker_stalled_wake`. - The pass runs from the recovery-action sweep (`reconcileActiveRecoveryActions`) and from the per-issue blocked sweep (`reconcileResolvedDependencyWakeBackstop`, which visits each blocked issue). No new worker. - A waiter's agent that cannot read the stalled issue (`issue:read`, which honors private visibility) is woken without its title, identifier or recovery details; the activity entry withholds them too. - **Database:** migration `0450` adds the partial unique index `agent_wakeup_requests_issue_blocker_stalled_uq` on `(company_id, idempotency_key)` for `issue_blocker_stalled:` keys in delivered or pending statuses, so two concurrent sweeps cannot both deliver a stall. It is built `CONCURRENTLY IF NOT EXISTS` outside a transaction (listed in `MIGRATIONS_WITH_TOP_LEVEL_STATEMENTS`). - **Queue:** `allowsDependencyBlockedWake` (in `run-dispatch/domain/wake-context.ts`) lets a stall wake run while the waiter's blockers are unresolved, at both dependency gates. Without it the wake was parked until the stalled blocker resolved, which is never. Pause holds are unchanged. - **Agent guidance:** the wake payload carries `blockerStall`, and `renderPaperclipWakePrompt` adds a "Stalled dependency" section. It names the task and its recovery action and the board's next action, then the three choices: re-delegate, escalate with a comment, or block with a reason naming the stalled task. - **Other reason lists:** `issue_blocker_stalled` joins the responsible-user wake reasons, the conversation execution-wake reasons (it never drives a chat turn) and the wake diagnostic's known reasons. - **Board surface:** - `GET /issues/:id` returns `stalledDependencies`, readable issues only (`readableIssueEdgeIds`). - `StalledDependencyNotice` shows on the issue's chat notices, next to `IssueBlockedNotice`: each stalled child or blocker, its board next action, a link, and the child's own Retry (`useTaskRetry`), offered only when the child's interruption allows it. ## Verification Red/green: - `server/src/__tests__/issue-blocker-stall-wakeups.test.ts` (14 tests). - On master: no `issue_blocker_stalled` wake is raised. - With this PR: the parent is woken once per stall, and re-woken after a new stall. A blocker's dependent is woken. - The t05 shape (an action resolved as `blocked`) wakes once, and nothing more after a Retry. - No wake for an agent-owned action, a done, unassigned or hidden waiter, or an action resolved another way. - `listStalledDependencies` lists children and blockers. - `heartbeat-dependency-scheduling.test.ts` "admits an issue_blocker_stalled wake while a blocker is unresolved". On master the wake was parked (`null`); once admitted, the run-start gate cancelled it. With this PR it runs with `dependencyBlockedInteraction`. The file: 14 passed. - `packages/adapter-utils` `server-utils.test.ts`: the new prompt test failed on master (no `blockerStall`). The file: 141 passed. - `ui/src/components/StalledDependencyNotice.test.tsx`: 3 passed. `IssueChatThread.test.tsx`: 109 passed. Regression and static checks: - Nearby server suites (recovery durable wait, dependency wake routes, recovery actions, run-preparation): 149 passed. - `tsc --noEmit` passes for `packages/shared`, `server` and `ui/src`. - `pnpm check:token-gates` is clean. Review round 1 (Greptile, Superagent), each with a regression: - A backlog waiter is never woken. The waiter's status and assignee are guarded at enqueue. - One stall per run: no coalescing, so a merged run cannot keep only the last stall's context. - Only a delivered or pending wake ends a stall. A refused one (wake-on-demand off) is retried after 10 minutes, and an agent that takes no wakes is skipped without writing a row. - Company-wide passes resume after the last stall a capped pass reached. The per-issue sweep also checks the issue's own children and blockers, which covers an unassigned stalled blocker. - The dependency gates no longer trust the reason string. `verifyBlockerStallWake` requires a system request, the stalled issue's current stall action, and a real child or blocker edge. An agent-requested wake with the same context is parked (tested). - The notice shows a refused Retry's reason, and a lost response as "Taking longer than usual…", with Retry still available (tested). Review round 2 (Greptile), each with a regression: - Race: two passes claiming one stall leave one delivered row (unique index; the loser's insert is skipped). - The company recovery sweep selects companies whose only stall is a resolved-`blocked` board action, so an unassigned stalled blocker with a todo waiter is reached. - Private stalled issue: the waiter's wake and activity carry no title, identifier or recovery details. - `GET /issues/:id` always returns `stalledDependencies`, so a cleared stall replaces the cached list. - The notice shows a failed child read with a Reload button (tested). - `pnpm --filter @paperclipai/db check:migrations` passes. Review round 3 (Greptile), each with a regression: - A stall wake that fails to enqueue is retried: the recovery-action pass reports `blockerStallWakeFailures` and the dependency pass counts it in `enqueueFailed`, so the worker throws and retries at its backoff (`issue-recovery-work-lifecycle.test.ts`). - The notice drops a stall once the child's refreshed status has moved on (`StalledDependencyNotice.test.tsx`, 7 tests). ## Risks - A deploy wakes the waiters of every issue that is stalled today, once each. That is intended (they are stranded now), and it's bounded at 200 stalls per pass. - A stall wake may run while blockers are unresolved, like a comment wake. It is bounded to one per stall, and pause holds still apply. - The agent skill (`skills/paperclip`, CODEOWNER-owned) does not yet list the new reason. The wake prompt carries the guidance. - One additive index, built concurrently; no persisted run state change. Existing rows cannot violate it before deploy, since no `issue_blocker_stalled:` keys exist yet. ## 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> |
||
|
|
e8a3f1f6f2 |
feat(recovery): prove a replaced container's records gone through a fenced controller epoch (#20054)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - After a restart, the control plane settles in-progress records (leases, claims, intents) only once it proves their owner process is gone > - The proof compares host and PID space, so a stamp from a replaced container or a rebooted machine stays unprovable forever, and its records never settle > - Self-hosted users on Docker, Kubernetes and Fly hit this on every redeploy; a Railway deployment with an exclusive volume is covered by `deployment_replaced` (#20026) > - This pull request adds a fenced controller epoch: each boot holds a database lease, and an expired boot is fenced so its records can be proven gone > - The benefit is that work stranded by a replaced container is recovered, without keeping an idle database awake ## Linked Issues or Issue Description Follow-up to #20026 and the Phase 2 durable-ownership data. Related: #19965 (owner stamps), #19883 (process space). No duplicate PR was found. **What happened?** A record stamped by a server process in another container reports `different_process_space`. A dead PID elsewhere is never proof, so the record stays `unprovable` and is never settled. Every redeploy of a self-hosted container leaves such records behind. **Expected behavior** A record owned by a server boot that was replaced (stopped cleanly, or lost its lease and was fenced) is proven gone and settled exactly once. A live boot's records stay with it. An idle server issues no recurring queries. **Steps to reproduce** 1. Run Paperclip in a container and start work, so an environment lease is stamped by that boot. 2. Replace the container. The new container has a new PID namespace. 3. The ownership reconciler reports `different_process_space` (unprovable) for the old lease, forever. **Paperclip version or commit** `master` before this PR. ## What Changed - New `controller_instances` table (migration 0449): boot id, epoch (bigserial), process space, pid, a lease on the database clock, `fenced_at` / `fenced_by_epoch`, `stopped_at`. - `controller-instance-lease.ts`, registered with the shared delivery-work coordinator (`controller-instance-lease` queue, runs during drain): - **Holds work → renews; idle → silent.** One upsert creates the row on first use and extends the lease only if the boot holds work: a stamp within the last 90 s, or a stamp whose ownership write may still be pending (held until the server's tracked work settles, capped at 10 minutes; a renewal that sees some other held record does not clear it, so a slow ownership write cannot be outlived), or an in-progress record held by its boot id. Those records are active environment leases, in-flight cleanup claims, sandbox creation intents, workspace finalize attempts, agent lifecycle steps, native finalizer and cleanup leases (`lease_holder` bound to the token), open native coordinator claims, and running legacy runs. Renewal runs every 30 s while the boot holds work. When it finds none, the lease lapses and the worker sleeps until the next stamp (`onControllerStamp`). - **No idle trailing statements.** When the server's last tracked work settles, the worker re-checks holdings at once, at most every 5 s (the rate limit also absorbs the worker's own run). So the last check happens with the work, not 30 s later. - **Quiesce, don't crash.** Each renewal has a 10 s bound. A failed or timed-out renewal applies an internal task drain, so no new work is dispatched. A recovered renewal resumes. The boot exits only after 60 s of failure (counted from the first failed attempt's start) while it still holds work, or at once when it finds itself fenced. - **Fence on demand, by a newer epoch.** For a stamp from another host or PID space, the prover reads that boot's row: - `stopped` or `fenced`: gone. - live lease: alive. - otherwise, the boot's last sign of life is the later of its lease expiry and the record's last write plus the TTL. Within 30 s of that, the owner stays `alive` (`controller_lease_grace`), so the settle watch keeps rechecking. After that, the prover fences the boot, but only with a lease row of its own (created on demand) and only an older epoch. - A graceful shutdown records `stopped_at`. - A reconciliation pass that settles nothing (`record: false` or `settle: false`) never fences and never creates the boot's row. - `owner-proof.ts`: these proofs apply only to server-held records. A stamp with a recorded process group stays unprovable, because agent processes can outlive their server. The reconciler passes each record's `updatedAt` to the proof. - `task-admission.ts`: - the quiesce drain never replaces an operator's drain, and has no idle owner, so HTTP stays open; - a `subscribeIdleWorkSettled` hook fires when the last tracked work finishes. - Off on a Railway deployment with an exclusive volume (it has its own replacement proof). `PAPERCLIP_CONTROLLER_INSTANCE_LEASE=on|off` overrides this. A stateless Railway service keeps the lease. A server without its own lease row never fences. - `maintenance-server-routes.test.ts`: in the post-run quiet window, the lease may check once more that the run's records were released. The test now excludes those statements, then asserts that the lease issues nothing in a further window. - Docs: `doc/execution-semantics.md`; the Phase 2 data doc records the two known gaps. ## Verification - `controller-instance-lease.test.ts` (15 cases): - a replaced container with all Railway variables empty is fenced and settled exactly once over two passes; - a live lease is alive; - an owner in grace, and work written after an idle lapse, stay alive and watched; - an older epoch never fences a newer one; - no fencing without its own lease row (off or unregistered), and an idle prover creates its row on demand; - a stopped boot is gone; - process-group stamps stay unprovable; - idle issues zero statements, and one upsert renews only while records are held; - **a 45 s outage:** quiesced, no exit, resumes; - **a renewal that never answers:** bounded, and the boot exits on time; - **a 70 s outage:** exits at 60–65 s, and the successor recovers the work exactly once; - fenced: stops; - the settle re-check; - graceful stop; off on a volume-exclusive Railway deployment, on for a stateless one, and the on/off override; - a running legacy run keeps the lease live. - Red, with the proof's instance check removed: the replaced-container, live, stopped and successor cases fail. - Wider: task-drain, idle-sleep, idle-local-work, owner-proof and ownership suites, plus the maintenance quiet-window tests. Server low-memory typecheck is clean. ## Risks - **Clock skew.** The proof compares a record's `updatedAt` (often the writer's clock) with the lease (database clock). The 30 s grace absorbs ordinary skew. - **A new exit path exists.** A server whose database is unreachable for more than 60 s while it holds work now exits, after first quiescing. This is the intended safety backstop. - **Bounded trailing activity.** A lease that holds work issues one statement every 30 s, plus at most one check when its work settles. An idle server issues none. - **Boots from before this release** have no row, and their records stay unprovable. The board "mark owner gone" action for those follows in a separate PR. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), via Claude Code with tool use. ## 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> |
||
|
|
96badeaa42 |
fix(lifecycle): never repeat a passed harness verification; settle a dead verifier's lease at once (#20030)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A new or reconfigured agent runs a harness verification (a hello probe in a test sandbox) before it is ready, and its runs wait until it passes > - In the final Daytona restart chaos wave, a Codex agent's runs waited about 417 s for their first sandbox, while boot took only about 32 s > - Each control-plane restart killed the verifier. The next process waited out the 120 s lifecycle lease and then booted a fresh sandbox. Nothing recorded a configuration that had already passed. > - This pull request records a passed verification by what makes it valid and reuses it, and settles a dead verifier's lease on the shared ownership path at once > - The benefit is that a restart never repeats a passed verification, and never leaves runs waiting out a dead process's lease ## Linked Issues or Issue Description **What happened?** In the final Daytona chaos wave, the Codex agent was created at 09:55:46 UTC and its three runs were queued. Verification ran four times, in four sandboxes, because faults K1, K4 and K2 each killed the verifying server. After K2 the server was ready at 10:00:20, but verification restarted only at 10:02:13: it waited out the 120 s lifecycle lease. The first run sandboxes were created at 10:02:43, 417 s after launch. **Steps to reproduce** 1. Create a Codex agent and assign it a task at once, so the task waits for harness verification. 2. Kill the control plane (SIGKILL) while the agent is verifying. 3. The restarted server waits out the dead verifier's 120 s lifecycle lease, then verifies again in a new test target. The task's first token arrives about 130 s after the fault. **Expected behavior** - A passed verification of the same effective configuration in the same runner image is never repeated. - A verification is never restarted from scratch while its holder process is alive. When the holder is proven gone, its lease is settled at once. - Runs gate only on a configuration that has never passed. **Root cause** `verify()` (`server/src/services/agent-harness-verification.ts`) leased a fresh sandbox on every attempt and recorded nothing on a pass. The lifecycle lease (`modules/agent-lifecycle/adapters/postgres.ts`) named no holder process, so a restart could only wait for it to expire. ## What Changed - Restart chaos TTFT driver: a `verify` phase, reported in its own rows, and a `--verify-fault K1|K2|K6` option. The fault fires 1 s after `verifying` is observed, so it interrupts a held lease. The seed keeps each agent's creation body, so the driver can hire an identical agent. - **Persisted pass:** new columns `agents.harness_verified_digest` and `harness_verified_at` (migration 0448). The digest is an HMAC, keyed with the instance's decision-signing secret, over: - the fully resolved effective configuration: every value the test used, including `cwd`, resolved credentials, and agent-over-environment precedence; - the selected AI account and its credential generation (connection and grant ids and `updatedAt`, never the credential); - the environment and the runner image. The key means the stored value cannot confirm a guessed credential offline. For a sandbox, the runner image is an immutable image digest or a qualified Daytona snapshot ID (`sandboxBinding`, now exported). For local runs it is the server version. A mutable image or snapshot name, or a local environment redirected to the managed sandbox, never reuses a pass. - **Server-only:** `normalizeAgentBaseRow` and `redactAgentRowForResponse` strip the digest and its timestamp from agent API responses. - `verify()` still runs every account, policy and secret check, then returns complete on a matching digest without booting a test target. It records the digest on a pass before the lifecycle step completes, so a restart in between reuses it. - **Lease holder:** the lifecycle `claim()` stamps the claiming controller process on the lease (`lifecycleOperation.leaseHolder`, supplied through the lifecycle effects port) and clears the stamp with the lease. The compare-and-swap release lives in the lifecycle module (`releaseAgentLifecycleLeaseOfHolderInTransaction`). - **Shared ownership path:** a new `agent_lifecycle_lease` kind with `settleAgentLifecycleLease`. - When the reconciler proves the holder gone, settle clears exactly that lease (compare-and-swap on the holder), notifies lifecycle work, and logs `agent.lifecycle_lease_reconciled`. - A live holder is never taken over. The lifecycle state, operation and progress are untouched. ## Verification - `restart-verification.test.ts` (embedded Postgres, real lifecycle store and worker, real reconciler pass): - A verification held by a dead process is claimed and completed at once after the startup pass, with the agent ready in under 10 s instead of waiting out the 120 s lease. It fails without the new kind. - A live holder is never taken over. - The claim stamps the holder, and `defer` clears it. - `agent-harness-verification-preflight.test.ts`: - A second `verify()` of the same configuration allocates no target. A revoked account still fails before reuse. A changed model, `cwd` or literal credential tests again. It fails without the reuse. - A failed verification records nothing. - Ownership report, all lifecycle module tests, remote, provider-API and unit harness verification tests, and crash-point call sites: 13 files, 220 tests pass. - Targeted local run: the TTFT driver's new `verify` phase (`--phases verify --verify-fault K1`, Codex, 2 samples) hires a new agent, assigns it a task at once, kills the control plane (K1) while it verifies, and times fault to first agent token: - Before (master server code): 129.8 s and 129.8 s. The restarted server waited out the dead verifier's 120 s lease. Run-log TTFT from run creation was only 4.1 s. - After (this branch): 15.8 s and 17.2 s. The startup reconciler pass settled `agent_lifecycle_lease` and verification re-ran at once. ## Risks - **A stale pass:** the digest covers every resolved value, the selected account generation, the environment and an immutable runner image, so any change tests again. Rotating the instance signing secret invalidates every digest, so each agent verifies once more. - **Migration:** two nullable columns, no backfill. Existing agents verify once more, then reuse. - **Settle:** it runs only on a `gone` proof, compare-and-swap on the exact holder. It never changes the lifecycle state. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file editing, test execution). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
576ba29fff |
feat(recovery): settle the legacy controller lease on the shared ownership path (#19953)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Legacy (non-native) runs hold a controller lease on `heartbeat_runs` while the server prepares and dispatches the adapter. The shared ownership reconciler now settles proven-gone owners (Phase 2 of the durable ownership plan) > - The lease stamped only a boot id. The reconciler could never prove its owner gone (`missing_identity` in the Phase 2 data), and recovery released it by age: lease expiry (60 s), then a 5-minute age guard on dispatched runs > - A hard-killed server's legacy run therefore stayed `running` for up to 6 minutes after the next boot > - This pull request records the controller process with the claim, registers a compare-and-swap settle, and makes live-run recovery use the shared prover instead of the age guard when the controller is proven gone > - The benefit is that a dead controller's run is settled at once, while a live, unprovable, or previous-release controller keeps today's behavior ## Linked Issues or Issue Description Refs #19933 (the settle hook, merged). Stacked on #19950 (migration 0446, registry) and targets its branch until it merges; then it retargets to master. **What happened?** After a hard kill, a dispatched legacy run stayed `running` until its controller lease expired and its last update was 5 minutes old. **What did you expect?** The next boot proves the old controller gone and fails the run as a lost process at once. **Steps to reproduce** 1. Start a legacy (non-native) run and let it dispatch the adapter. 2. Kill the server (SIGKILL). 3. Start the server. The run stays `running` for up to 6 minutes. ## What Changed - `heartbeat_runs.controller_process` (jsonb, migration 0447): `{bootId, pid, hostHash, processStartedAt}` of the claiming server process. It names the controller only while its `bootId` equals `controller_boot_id`. Revocation swaps the boot id, so a revoked lease never keeps it. - `legacyControllerClaim` writes it in the same UPDATE (from `currentControllerProcessStamp()`, no I/O). - `legacy-controller-lease.ts`: `legacyControllerOwnerStamp`, `legacyControllerProvenGone` (calls `proveOwnerGone`), and `settleLegacyControllerLease` (compare-and-swap expiry of the proven controller's outstanding lease after it wakes live-run recovery). - `ownership-registry.ts`: `legacy_controller_lease` registers `settle` and `listForRuns`. The owner is the bound process; other rows are boot-id fences. The listing covers outstanding leases only. - `heartbeat/recovery.ts` (3 small hunks): a controller of another boot that the shared prover proves gone has its live lease settled through `settleOwnableKind` and skips the age guard. The local child-process check after it is unchanged, so a live child still keeps the run running. - Migration 0447 builds the partial index with `CREATE INDEX CONCURRENTLY` (heartbeat_runs is a known-large table) and is added to `MIGRATIONS_WITH_TOP_LEVEL_STATEMENTS`. Bespoke code deleted: none. The kind had no liveness proof; the age guard stays for controllers that are not proven gone. ## Verification - `cd server && npx vitest run src/services/legacy-controller-lease.test.ts`: 33 passed. - `cd server && npx vitest run src/__tests__/ownership-reconciliation-report.test.ts`: 17 passed. - Red with `recovery.ts` and `ownership-registry.ts` from the base commit (tests, schema and lease module from this branch): 4 failed, 29 passed. Red: the crash-point reaper test, the expired-lease age-guard test, the reconciler settle test, the exact-controller CAS test. The tests were written after the code. - New tests: - the claim stamps this process, and a changed boot id unbinds it; - crash point: a controller dispatched the adapter and was killed with its lease live; the startup reaper (default 5-minute age guard) fails the run as `process_lost` at once; - a dead controller's expired lease skips the age guard; - counter-cases keep the run and lease through the reaper and a reconciler pass: a live earlier-boot PID, a dead PID in another PID space, and a previous-release row with no `controller_process`; an expired lease of a live controller stays behind the age guard; - a read-only pass (`record: false`) never settles; a recording pass expires the lease and leaves the run to live-run recovery; - settle refuses a controller that changed after the proof. - Not run locally: typecheck (CI runs it) and the wide suites. ## Risks - Previous-release story (rules/upgrade-safe-run-state.mdc): rows saved by the previous release have no `controller_process`. They stay boot-id fences with today's release (expiry, revocation, age guard). A rollback ignores the column. - Unprovable controllers (another host or PID space, unknown space, a live PID with an unreadable start time) never release early. Their handoff stays the lease expiry plus revocation, and the age guard. A different host is never proof. - Behavior change: a dispatched run whose controller is proven gone is failed as `process_lost` without the 5-minute wait. The retry decision is unchanged. - The settle expires the lease only. Live-run recovery still settles the run under its own checks. Accounting and workspace obligations are not touched. - The index build is concurrent. A cancelled build is retried by the migrator's invalid-index handling. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edits, local 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> |
||
|
|
bf3a067076 |
feat(recovery): settle the finalizer and cleanup leases on the shared ownership path (#19950)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Native runs keep their ownership in `native_run_finalizations`, and the shared ownership reconciler now settles proven-gone owners (Phase 2 of the durable ownership plan) > - The finalizer lease (`native-finalizer:*`, 5 min) and the retained-session cleanup lease (`native-cleanup:*`, 60 s) recorded only a random token. They named no holder process > - A server killed while it finalized or cleaned up kept the run waiting for the whole lease. The lease was released by age alone. A graceful shutdown also keeps an in-progress finalizer lease for the next boot to wait out > - This pull request records the holder process with each claim and moves both kinds onto the shared prover and a compare-and-swap settle > - The benefit is that a dead holder's run continues in the first pass after boot, and a live or unprovable holder is never released ## Linked Issues or Issue Description Refs #19933 (the settle hook and the first moved kind, merged). **What happened?** A finalizer or cleanup lease whose server process died stayed held until the lease expired (5 min or 60 s). The run stayed `running` and its follow-up work waited. **What did you expect?** The next boot proves the dead holder gone and frees the lease at once. A live holder, or one that cannot be proven gone, keeps its lease. **Steps to reproduce** 1. Accept a native result and let the finalizer claim it. 2. Kill the server (SIGKILL) before the finalizer commits. 3. Start the server. The run stays `running` for up to 5 minutes. ## What Changed - `native_run_finalizations.lease_holder` (jsonb, migration 0446): `{token, bootId, pid, hostHash, processStartedAt}` of the process that took a token-only lease. It names the holder only while `token` equals `lease_owner`, so renewals keep it and a later lease never inherits it. The executor's `controller_*` columns are not changed. - The finalizer claim (`native-run-finalizer.ts`) and the cleanup claim (`native-session-executor.ts`) write it in the same UPDATE as the lease, from `currentControllerProcessStamp()` (no I/O). - New `native-token-lease-settlement.ts`: compare-and-swap settle that expires exactly the proven-gone holder's outstanding lease, after it notifies the finalization or cleanup work that the expiry frees. - Both claims also wake the ownership worker queue in their transaction, so a quiet worker watches the new holder (Greptile round 1). - `ownership-registry.ts`: `native_finalizer_lease` and `native_cleanup_lease` register `settle`. Their owner is the holder process; rows without a holder keep the token fence. Listings cover outstanding leases only. - `heartbeat/recovery.ts`: restart recovery settles both kinds before it finalizes and schedules cleanup. A failure is logged and the reconciler pass retries it. - Two sparse partial indexes on `lease_expires_at` for the two listings, so the settle watch re-check stays indexed. Bespoke code deleted: none. These kinds had no liveness check; their only release was lease age (the takeover in the claim), which stays as the fence for unprovable holders. ## Verification - `cd server && npx vitest run src/__tests__/heartbeat-native-restart-suspension.test.ts -t "finalizer|cleanup|holder|exact lease"`: 12 passed (rerun after the rebase on master). The crash-point test also asserts the ownership wake for its run; it fails without the signal. - `cd server && npx vitest run src/__tests__/ownership-reconciliation-report.test.ts`: 17 passed. - Red on the base branch (server code at `bea3bfb20b`, tests and schema from this branch): 7 new tests fail, 5 pass (the 5 are existing tests). Red: the crash-point restart test, the two reconciler settle tests, the three live/unprovable counter-cases (owner was a token fence), and the exact-lease CAS test. The tests were written after the code. - New tests: - crash point: a finalizer claim was written and its holder killed before commit; the next restart finalizes the run, and this boot's claim stamps itself as holder; - a dead finalizer or cleanup holder is settled by a recording pass, never by a read-only pass (`record: false`); the accepted result and phase are untouched; - live counter-cases: a live earlier-boot PID (`pid_alive`), the current boot (`current_process`), a dead PID in another PID space (`different_process_space`, unprovable) keep the lease through a pass and a restart; - a holder written for another token, and a holderless row of the previous release, stay token fences, also with `PAPERCLIP_EPHEMERAL_SERVER=true`; - settle refuses a lease taken over after the proof (`owner_changed_since_proof`). - Not run locally: typecheck (CI runs it) and the wide suites. ## Risks - Previous-release story (rules/upgrade-safe-run-state.mdc): rows saved by the previous release have no `lease_holder`. They stay token fences and the existing claim takes them over at expiry. Neither lease outlives 5 min, so this covers at most 5 min after the upgrade. A rollback ignores the new column. - Unprovable holders (other host or PID space, unknown space): no operator action. The lease is short and self-expiring; at expiry the next claim takes it over and the old holder's next token compare-and-swap fails. That fence is the handoff. A different host is never proof. - The settle only expires a lease. Finalization and cleanup still run under their own checks (cleanup keeps its maintenance-history proofs). Settling ownership does not settle the accepted result, workspace preservation, or accounting. - Migration adds a nullable column and two partial indexes on a sparse predicate. Low risk. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edits, local 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> |
||
|
|
f5400e2594 |
feat(recovery): settle the accepted-result lease on the shared ownership path (#19933)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - A control-plane restart can kill the controller after an agent's result is accepted. The result then stays leased to a dead process. > - Today a private sweep (`native-dead-controller-result-lease.ts`, #19713) proves that controller dead with its own liveness rules. > - The durable-ownership plan (#19831) replaces private proofs with one prover (`proveOwnerGone`, #19853) and one registry (#19861). > - This pull request moves the accepted-result executor lease to that shared path and deletes the private sweep. It is Phase 2's first kind. > - The benefit is one tested rule for when a dead owner's result lease is released, and the reconciler now repairs a lease that restart recovery missed. ## Linked Issues or Issue Description Refs #19831 (plan), #19853 (prover), #19861 (registry), #19713 (old sweep), #19883 (legacy host fallback), #19927 (Phase 2 data). **What happened?** The accepted-result lease had its own liveness proof, separate from the shared prover. The reconciler could only report a dead owner, not settle it. When restart recovery missed a lease, it waited out the 20-minute lease. **What did you expect?** One prover decides, and both restart recovery and the periodic reconciler pass settle a lease whose controller is proven gone. **Steps to reproduce** 1. Accept a native run result. 2. Kill the controller (SIGKILL) and start a new server. 3. If the restart sweep does not see the row, the result stays leased until the lease expires. ## What Changed - `ownership-registry.ts`: a kind may register `settle` and `onUnprovable`. `native_accepted_result_lease` registers both. Other kinds stay report-only. - New `native-accepted-result-lease-settlement.ts`: a compare-and-swap settle that expires exactly the proven owner's lease. It keeps the old guards: the run must still be `running`, remote copyback stays fenced, and a lease taken over since the proof is left alone. - `ownership-invariant-report.ts`: the pass calls `settle` on `gone` and `onUnprovable` on `unprovable`. A read-only pass (`record: false`) never settles. New `settleOwnableKind()` for restart recovery. - `heartbeat/recovery.ts`: restart recovery calls `settleOwnableKind("native_accepted_result_lease")` instead of the old sweep. - Deleted `native-dead-controller-result-lease.ts`. - `owner-proof.ts`: accepts a board attestation (`operator_attested`). It also keeps the one-release local fallback for rows saved before their PID space was recorded, on single-host servers only, and flags each use so it is logged (`legacy_host_unknown_dead_pid_proof`, #19883). An ephemeral server returns `unprovable` (`process_space_unrecorded`) and the existing "confirm the previous server stopped" board action is raised. ## Verification - `cd server && npx vitest run src/__tests__/heartbeat-native-restart-suspension.test.ts`: 27 pass, 7 of them new. - `npx vitest run src/services/owner-proof.test.ts src/__tests__/ownership-reconciliation-report.test.ts`: pass. - Red/green (tests written after the code): 5 new behaviour tests fail on master and pass here. They cover the ephemeral board action, the reconciler-pass settle, the live-owner counter-case in that pass, the legacy marker, and operator attestation. The stamped-dead-controller and foreign-PID-space cases pass on both, as the old path behaved the same. - Server typecheck passes (`npx tsc --noEmit -p server/tsconfig.json`). - Not run locally: the full server suite (`pnpm verify` asks for broad coverage because a file was deleted). Required CI covers it. ## Risks - Previous-release story (rules/upgrade-safe-run-state.mdc): NRF rows saved before #19868 have no `controller_host_hash`. They keep the same one-release local proof as before on single-host servers, and each use is logged. On ephemeral servers they stay unprovable and ask the board, as before. Rows on another PID space keep their lease (the fence), as before. - Fail-open risk: settle is compare-and-swap on the lease token, boot, PID, host hash and start time of the proven owner. An `unprovable` owner is never released. Live-owner and foreign-space cases are tested. - No change adds work before `turn.start`. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, in 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 - [ ] 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> |
||
|
|
0363ebec70 | fix(connectors): stop GitHub access refreshes from advancing agent runtimes (#19902) | ||
|
|
fc6e1b0346 |
perf(native): keep local Codex and OpenCode runners warm by default (#19888)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - When someone messages an agent, the turn must start fast. Time to first token (TTFT) is a first-class metric. > - The Phase 0 baseline (#19846) found that every local turn launches a new runner and loads the provider session again, because `per_turn` is the default lifecycle. That made a follow-up message slower than a new task. > - Warm runners keep the runner and provider session alive between runs. #19874 fixed the defect that broke warm OpenCode follow-ups, and #19879 bounds an idle warm runner whose controller dies. > - This pull request makes warm the default for local Codex and OpenCode agents, and migrates existing agents that only carried the old creation default. > - The benefit: follow-up TTFT p50 drops from 12.0 s to 4.7 s (Codex) and from 21.7 s to 4.2 s (OpenCode), with no new restart-chaos failures. ## Linked Issues or Issue Description Refs #19846 (TTFT baseline), #19852 (startup sub-spans), #19874 (warm completion contract), #19879 (warm reconnect grace). **What existing behavior does this improve?** The default runner lifecycle for native agents, and how a follow-up message starts. **Current behavior** Agent creation writes `lifecycleMode: "per_turn"` into every native agent. Every follow-up launches a new runner and restores the provider session: 4.2 s (Codex) and 16.3 s (OpenCode) at p50 before the turn starts. **Proposed behavior** New native agents store no lifecycle. On a local target, an unset lifecycle resolves to `warm` with the default 5-minute idle window for Codex and OpenCode. Other harnesses, remote targets and managed providers without warm support stay `per_turn`. An explicit choice always wins. Migration 0443 clears the stored creation default from existing agents, except where an edit set "Turn by turn" explicitly. **Reason and benefit** Measured locally (`doc/plans/2026-11-07-warm-follow-up-ttft.md`). Warm follow-ups are 2.6–5× faster for Codex and OpenCode, and restart recovery latency is equal or lower. ## What Changed - `native-lifecycle.ts`: an unset `agentMode` resolves to warm (5-minute idle window) for local `codex_app_server` and `opencode_server` agents. `runtime-selection.ts` passes `localTarget`. Explicit `per_turn` or `warm` wins. An admitted session keeps its persisted policy. ACPX Claude stays `per_turn`: it relaunches its provider process on every follow-up by design, so warm did not shorten its follow-up (10.7 s warm vs 10.5 s per_turn). - Creation and configuration writes no longer store `lifecycleMode: "per_turn"` for Codex, Claude or OpenCode. This covers `normalizePaperclipRunnerAdapterConfig` (runs on every create and update), `paperclipRunnerTransitionConfig` (switching to the native runner), `agent-runner-selection.ts`, `codex-local` `build-config.ts`, and the simplified new-agent setup (`paperclipRunnerCreationLifecycle` in `@paperclipai/shared`). Grok, Pi and Cursor keep their explicit value. - Agent form: the Runner lifecycle field shows the unset state as "Automatic". "Turn by turn" and "Warm session" remain. - Migration `0443_native_warm_follow_up_default.sql`: clears `lifecycleMode = 'per_turn'` from native Codex, OpenCode and Claude agents unless `agent_config_revisions` shows an edit that set it to `per_turn`. Each migrated agent gets an `agent.updated` activity entry with the old value (`actor_id = migration:native-warm-follow-up-default`). Creation records no revision, so a "Turn by turn" pick made at creation cannot be told apart from the old default. The product owner approved migrating those agents. Claude agents follow the default, which is `per_turn` today. - Restart chaos harness and TTFT driver: `--lifecycle per_turn|warm|default` (saved in the wave plan). `default` removes the catalog's pinned lifecycle, so a wave exercises the real default path. - Docs: `doc/DEVELOPING.md` "Runner lifecycle default", and the results in `doc/plans/2026-11-07-warm-follow-up-ttft.md`, which also documents baseline finding 4 (Claude's resumed turn-start gap is ACPX's per-turn process generation, not the provider). ## Verification - `server`: `native-lifecycle.test.ts` (new default cases: local Codex and OpenCode warm, Claude, remote, unknown target, unmeasured harness, managed Claude, admitted per-turn session, explicit choices, non-native adapter), `agent-runner-selection.test.ts`, `runtime-selection.test.ts` (335 pass). - `packages/db`: `native-warm-follow-up-default-migration.test.ts` on embedded Postgres. Default agents are cleared, the explicit edit is kept, other harnesses and legacy adapters are untouched, audit rows are written once, and a rerun is a no-op. Without the explicit-edit filter the test fails. `check:migrations` passes. - `codex-local` `build-config.test.ts` (65), `ui` `config-fields.test.tsx` (40), restart-chaos unit tests (134), `tsc --noEmit` for `server`. - `adapter-utils` `paperclip-runner-lifecycle-default.test.ts`: configuration writes and adapter conversion store no lifecycle for Codex, OpenCode and Claude, keep `per_turn` for the others, and preserve explicit choices. It fails without the fix. - End-to-end default check on this branch (`ttft-baseline.ts --lifecycle default`, codex and opencode, 2 samples): the agents read back with no stored lifecycle, and every follow-up reused the runner and provider session (no `runner.process.launch`). Follow-up TTFT was 3.6–4.8 s. This check found the per-write `per_turn` injection fixed here. - Restart chaos (codex and opencode, A1/B1/B5, K1/K2/K6, seed 4242), 8/8 PASS each with no open signatures: forced warm, forced per_turn, and `--lifecycle default` on this branch (agents created without a lifecycle). - `pnpm verify` selects broader CI (harness config is a global input), so required CI supplies the broad coverage. ## Risks - Behavior change: local Codex and OpenCode agents keep a runner and provider process for up to 5 minutes after a run. Instance sleep waits for that idle window. The process is reaped by the idle timer, by graceful shutdown, or by the #19879 reconnect grace if its controller dies. - Migration: agents whose "Turn by turn" was chosen at creation become Automatic. The activity entry records the old value; choosing "Turn by turn" restores it. - Previous-release story (`rules/upgrade-safe-run-state.mdc`): a task session keeps the lifecycle policy it was admitted with, so sessions saved before this change stay `per_turn`. New sessions get the default. No run, checkpoint or launch state changes shape. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`) in Claude Code, with tool use (shell, file edit, test execution). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
5a20fca9f7 |
fix(recovery): judge an orphaned run's pid only in the PID space that recorded it (#19871)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The orphan reaper (`terminalizeOrphanedRunningRun`) ends a `running` run whose recorded process is gone > - It judged "gone" with `kill(pid, 0)` on the local host, wherever the process actually ran: another host, another PID namespace, or a sandbox > - So a run whose process is alive elsewhere could be terminalized while it still works > - #19868 fixed the same gap for native coordinator takeover and the runtime-control sweep. This pull request finishes finding 2 for the reaper > - The benefit is that a dead local pid never proves an owner on another host is gone ## Linked Issues or Issue Description **Bug description** The Phase 0 ownership inventory (finding 2) names the orphan reaper `terminalizeOrphanedRunningRun` (`recovery/service.ts`) as a proof that accepts "the pid is dead here" for a process anywhere. `heartbeat_runs` recorded `process_pid` with no PID space, and a sandbox pid could collide with a local one. **Steps to reproduce** 1. A run records `process_pid` on host A, or in a sandbox. 2. On host B, that pid number is not alive. 3. B's stale-lock sweep terminalizes the run as `orphaned_running_run`. **Expected behavior** The process-death authority applies only in the PID space that recorded the pid. Issue-terminal authority is unchanged. Refs #19846, follows #19868 ## What Changed - Migration `0442`: add nullable `heartbeat_runs.process_host_hash`. - `heartbeat/run-lifecycle.ts` `persistHeartbeatRunProcessMetadata`, the one writer of `process_pid`, stores `currentProcessSpaceHash()` for a local pid and `"remote"` for a sandbox pid. - `recovery/service.ts`: the process-death authority runs only when `process_host_hash` is null (rows saved before this column, for one release) or `sameProcessSpace` matches. - Tests: the reaper leaves a dead pid alone for another host, a sandbox, and an unreadable space, and terminalizes it in this space. The writer stamps local and remote pids. ## Verification - `npx vitest run src/__tests__/recovery-stale-issue-lock-sweep.test.ts`: 46 passed. - Red before the fix: with the old `recovery/service.ts`, the three cross-space cases fail (the run is terminalized). Green after. - `pnpm db:generate` produced `0442_low_salo.sql` (one `ADD COLUMN`), and the snapshot set was pruned to five. `pnpm --filter @paperclipai/db check:migrations` passed. Server `tsc --noEmit` shows no errors in the changed files. ## Risks - Previous-release story: runs saved with `process_host_hash = null` keep the previous local proof for this release. That matches #19868, because stranding in-flight runs at upgrade would be a restart failure. New rows are strict. - A run whose process really lived on another host now stays `running` until issue-terminal authority, its controller lease, or another owner settles it. - No new I/O before `turn.start`. The hash is computed at module load, and the stamp rides the existing process-metadata update. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used with tool use and code execution in Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
68db7ed9e3 |
fix(recovery): never accept a dead local pid as proof for an owner on another host (#19868)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - After a restart, recovery decides that an in-progress record's owner is gone and takes it over > - Two of those proofs use `kill(pid, 0)` on the local host. A pid that is dead here says nothing about a process on another host or in another PID namespace > - So a controller or runtime-control owner that is still alive elsewhere can be taken over while it still runs > - This pull request records the owner's PID space (the existing `controller-process-identity` host hash) and judges a pid only in that same space > - The benefit is that a different host is never proof. Those records follow their existing lease or TTL instead ## Linked Issues or Issue Description **Bug description** The Phase 0 ownership inventory (finding 2) found three proofs that accept "the pid is dead here" as proof for an owner on another host: `evaluateNativeControllerTakeover` (`native_run_finalizations` has no host column), the runtime-control sweep (`workspace-operations.ts`), and the orphan reaper `terminalizeOrphanedRunningRun`. Only `controllerProcessExited` checked the host. **Steps to reproduce** 1. Run two control-plane processes on different hosts, or in different PID namespaces, against one database. 2. Let host A own a native finalization lease or a runtime-control operation. 3. On host B, run restart recovery. If A's pid number is not alive on B, B takes over the live owner. **Expected behavior** An owner whose PID space differs from this process's, or cannot be read, is unprovable. Its record waits for its lease, fence, or TTL. Refs #19846 ## What Changed - Migration `0441`: add nullable `native_run_finalizations.controller_host_hash`. - `controller-process-identity.ts`: export `currentProcessSpaceHash()` and `sameProcessSpace()`. An unreadable space is stored as `"unknown"`, which never matches. - `native-restart-recovery.ts`: `currentNativeControllerIdentity` carries `hostHash`, which both coordinator claim writes (`native-restart-recovery.ts`, `native-session-executor.ts`) store. `evaluateNativeControllerTakeover` returns `controller_host_unprovable` before any pid probe when the recorded space is not this one. `native-dead-controller-result-lease.ts` selects the column. - `workspace-operations.ts`: runtime-control owner stamps record `hostHash`. `ownerProcessProvenGone` requires the same PID space (EPERM = alive), otherwise the existing staleness TTL applies. - Not in this PR: the orphan reaper `terminalizeOrphanedRunningRun`. `heartbeat_runs` has no host for `process_pid`, and its writers are spread across legacy and native paths. It follows as a separate PR with its own column. ## Verification - `npx vitest run src/services/native-runtime/native-restart-recovery.test.ts src/__tests__/workspace-runtime-control-recovery.test.ts`: 55 passed. New cases cover another host and an unreadable space (dead and recycled pid) for takeover, plus another host, unreadable space, and legacy stamp for the sweep. The sweep cases also confirm that the TTL still reclaims after the heartbeat lapses. - Red before the fix: the 5 new cases fail on the old code. Green after. - `native-controller-recovery.integration.test.ts`, `native-failed-run-owner-release.test.ts`, `native-startup-cleanup-recovery.test.ts`: 71 passed. - `pnpm db:generate` produced `0441_lucky_chameleon.sql` (one `ADD COLUMN`) and pruned to five snapshots. `pnpm --filter @paperclipai/db check:migrations` passed. Server `tsc --noEmit` shows no errors in the changed files. ## Risks - Previous-release story: `native_run_finalizations` rows saved before this column have `controller_host_hash = null`. They keep the previous local dead-pid proof for this release. Making them unprovable would leave every native run in flight at the upgrade in `awaiting_evidence` with no TTL, because local takeovers have no lease fallback. Rows written by this release onward are strict. Runtime-control stamps without `hostHash` are strict now, because that path has a staleness TTL. - Owners built from other receipts (workspace finalization owner, workspace stop proof) pass no host and keep the previous behavior. Track 2's shared `OwnerStamp` should absorb them. - A new row on a host that really moved (redeploy onto a new machine) waits for its lease or an operator instead of being taken over by pid. That is the intended "different host is never proof" rule. - No new I/O before `turn.start`. The hash is computed once at module load. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used with tool use and code execution in Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
e7936fb1ad |
fix(heartbeat): gate process-loss retries on the shared failure budget (#19856)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - When a server restart loses a run's process, the orphan reaper decides whether to retry the run > - `doc/execution-semantics.md` says process loss uses the shared durable failure retry counter, and `enqueueProcessLossRetry` already schedules against that budget > - The reaper still guards on `processLossRetryCount < 1`, but nothing has written that column since the move to the shared budget, so the guard does nothing > - So an exhausted chain still says "retrying once" and skips the interaction continuation path, and a legacy row with a non-zero counter is refused a retry it is owed > - This pull request replaces the dead guard with `failedRunRetryBudgetSpent`, the budget that the retry actually spends > - The benefit is one restart budget, read and written in one place, with honest run messages ## Linked Issues or Issue Description **Bug description** The Phase 0 ownership inventory (finding 5) found that `processLossRetryCount` has no writer and that its only reader is the reaper guard at `heartbeat/recovery.ts:2675`. The guard always passes for new rows. For rows written by earlier releases, it refuses a retry even when the shared failure budget has room. **Steps to reproduce** 1. Seed a legacy run that a restart stopped before adapter dispatch. Spend its whole failure retry budget (`executionRetryAccounting.failureRetries = max`). 2. Run `reapOrphanedRuns()`. 3. The run fails with "Process lost -- server may have restarted; retrying once", but no retry is scheduled. **Expected behavior** An exhausted budget gives no promise of a retry. A legacy counter value does not block a retry that the shared budget allows. Refs #19846 ## What Changed - `heartbeat/recovery.ts`: `shouldRetry` now checks `!failedRunRetryBudgetSpent(run)` instead of `processLossRetryCount < 1`. When the budget is spent, the reaper falls through to `scheduleInteractionContinuationInfrastructureRetryIfEligible`, which is what the old guard intended. - `packages/db/src/schema/heartbeat_runs.ts`: a comment marks `process_loss_retry_count` as legacy. There is no schema or migration change. - Tests: the three cases that used `processLossRetryCount: 1` to mean "retry already spent" now spend the real failure budget. Two new cases cover a legacy counter (a retry is still scheduled) and a spent budget (no "retrying once", `retryEligible: false`, no retry run). ## Verification - `npx vitest run src/__tests__/heartbeat-process-recovery.test.ts -t "process-loss|failure budget|bypass unknown process outcomes|immediate terminal-run cleanup|paused-tree work|restart stopped before adapter dispatch|reached adapter dispatch"`: 7 passed. - Red before the fix: with the old `recovery.ts`, both new cases fail. Green after. - Server `tsc --noEmit` shows no errors in the changed files. - The full `heartbeat-process-recovery.test.ts` file (419 cases) stalled locally at 0% CPU after about 15 minutes. That run was on a worktree whose package builds were stale at first. I ran the focused subset instead, and required CI runs the full file. ## Risks - Low risk. The retry decision now follows the same budget that `enqueueProcessLossRetry` already enforced. A run with budget left behaves as before. - Previous-release story: rows saved with `process_loss_retry_count > 0` are no longer refused for that reason alone. The shared failure budget, which earlier releases also wrote, bounds them. The column stays, so earlier-release readers (`queue.ts` startup resume) keep working. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used with tool use and code execution in Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
af0fe8fe81 |
Keep personal MCP connections functional while idle tenants sleep (#19427)
## Thinking Path > - Paperclip manages work for AI agents. > - Personal MCP connections should remain enabled while an idle tenant sleeps. > - Unfinished OAuth, transfer and delivery work must retain its owner and deadline. > - Cloud can receive bounded metadata and incoming work while the tenant remains the company-scoped authorizer. > - Current identity, revision and delivery acknowledgments establish when ownership has transferred. > - This change preserves useful wake, retries and revocation while making settled ownership inspectable without writes. ## Linked Issues or Issue Description Refs: #19377 and #19750. Hosted companion: [paperclip-cloud#1217](https://github.com/paperclipai/paperclip-cloud/pull/1217). **What happened?** Enabled personal MCP grants and subscriptions could keep an otherwise idle tenant awake. Ignoring those records would lose discovery, authorization, delivery and revocation obligations. **Expected behavior** Unconsumed authorization codes, device requests, file handoffs and delivery work retain their original deadlines. Settled connections remain enabled and become quiet after the current external owner acknowledges responsibility. Incoming useful work resumes normal authorization and delivery; revocation still takes effect for the current company. **Steps to reproduce** Complete normal PKCE/device consent and company selection, synchronize client/device and revocation state, retain the connection while idle, then exercise useful tools/events and revocation. Sleep eligibility requires current incoming/deadline ownership throughout. ## What Changed - Add bounded signed client/device metadata and revocation synchronization with exact revision, identity and ACK checks, startup recovery and finite legacy-device cutover. - Preserve company selection, authorization and operation identity. Register grant work before its INSERT in the same transaction; retain rollback, expiry and retry behavior. - Require current managed identity/resource before hosted publication. Receipt-cleared re-enable must obtain a fresh source ACK before device creation. - Reuse read-only settings and inspection budgets; connect MCP owners to normal startup, drain and shutdown. - Bound metadata pages and the actual signed request body while preserving the default limits of other operations. - Generate migration 0440 after the merged Legacy migration. Preserve all 438 existing executable migrations and the journal prefix; retain the normal five newest snapshots. - Preserve the merged transaction-owner forwarding and regression coverage from #19750 through normal ancestry integration. ## Verification - Current own Shared/SDK, database, eval-kernel and Runner builds passed, together with all seven normal application TypeScript programs. Missing generated package exports/workspace links were restored through normal setup; the original setup failures were retained. No source, compiler option, test assertion or timeout was weakened. - Normal migration generation changed only the five intended MCP tables; its SQL is byte-identical to the reviewed MCP delta. Fresh reference databases and both retained master/Muse histories upgraded through 0440, preserving historical hashes, row fields, schema and idempotence. Direct assertions cover the Legacy null default and MCP null/zero defaults. Two selected history cases passed; the other 21 database cases were unselected. - All six current MCP lifecycle cases passed with zero skips: normal PKCE approval intent, ACK recovery after outage, mutation during publication, disable/re-enable with original expiry, company-deletion rollback/commit and key rotation. - All 81 checks in five current whole files passed with zero skips: OpenAPI 34, read-only settings 2, diagnostic catalog 25, PostgreSQL diagnostics 7 and dependency scheduling 13. PostgreSQL fixtures ran normal migrations before test imports. - Earlier canonical proof at App `cd1c8c6` / Cloud `9f67531` includes real 60-second rate and three 15-second quiet windows, useful work and three revokes. Later paired proof at App `43af514` / Cloud `914eb0f` plus its reviewed candidate includes normal DCR, PostgreSQL metadata, a 144,017-byte signed publication, actual ACK, cap/default-413 controls and a real 15-second quiet window. The relevant producer/signing/wire inputs remain byte-identical in this integration; shared changes are qualified above. These remain source-scoped observations, not a fresh full canonical run on the integrated tree. - All known test owners, process groups and OS sockets closed; an independent final PostgreSQL audit found no remaining test sessions. One dependency-test trace recorded 46 opens and 45 close events. Its cause is unknown; this is not claimed as proof of graceful closure. - Fresh exact-head remote checks passed: 59 successful and 4 skipped contexts, with a complete current 5/5 Greptile review and all seven review threads resolved. Targeted local checks do not constitute a new whole-repository test run. ## Risks App and Cloud must agree on identity, revision, namespace, deadlines and revocation. Partial rollout, unavailable authority or missing ACK must retain awake debt. Shared startup/shutdown and settings changed since the earlier canonical profile, so historical observations carry only within their stated source scope. No deployed Railway/Neon suspension, Cloud alarm, savings or complete hosted Dot/Muse acceptance is claimed. ## Model Used OpenAI `gpt-6.1-sol` produced code and verification, with a Codex coordinator reviewing and integrating the changes. The coordinator's exact runtime model ID and context-window sizes were not exposed. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> Co-authored-by: Codex <noreply@openai.com> |
||
|
|
96bc021edb |
fix(server): certify settled legacy processes for future retries (#19377)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Bounded retries must preserve current authority, budgets and deadlines. > - The tenant can wait for supported future work after the source process and its cleanup stop. > - Retained process identity alone cannot prove that cleanup finished. > - This change records host-owned settlement and checks its exact identity before a retry can cover its wait. > - Uncertain process state, changed authority, live claims and cleanup debt keep the tenant awake. ## Linked Issues or Issue Description Refs #19001. Related dependencies: #19019 and #18988. Related #19340 covers explicit Native task Retry; this change covers future Legacy process retries. Current integrated source: `1645970a9322d1213ba25fc7b52366c285682977`, normally merged with actual master `131b566ca4fa866c4348739c75cfe48c805b3398`. The prerequisites above are merged. This PR targets `master`. **What happened?** A future issue, company or review retry can retain its failed source process identity. The deadline reader could not prove that host cleanup finished. Supported waits stayed blocked. Ordinary issue retries also needed an exact source-settlement check, and represented coalesced requests needed exact input coverage. **Expected behavior** A supported future retry covers only its existing wait, under current company and actor authority, after exact host settlement. Missing, malformed, stale, cancelled, Native or unfinished source state stays blocked. No retry gains new execution authority. **Steps to reproduce** Fail an ordinary process or fail environment preparation before adapter entry. Let normal cleanup schedule the existing retry. Inspect its settlement and deadline, stop and reconstruct the owner, and admit one useful successor at the saved deadline. Hold cleanup or change authority to verify that the reader refuses coverage. ## What Changed - Add nullable internal `legacyProcessSettlement` state through normally generated migration `0439_cultured_impossible_man`. No backfill or adapter result creates the record. - Write exact host settlement after stopped-process and cleanup checks. Compare the current run and reject live claims or cleanup debt. Optional persistence failure preserves normal cleanup and keeps sleep coverage blocked. - Add a separate preparation receipt for a claimed Legacy run that failed before any adapter or Native dispatch. Require the same boot, run times, preparing stage, released claim and completed cleanup. A null PID alone proves nothing. - Clear settlement on a new claim, process metadata or Native selection. Omit the internal record from public serialized data. - Check exact source lineage and current authority for ordinary issue, company and taskless retries. Cover completed coalesced requests only when the admitted retry represents their exact inputs. - Add the closed `current_review_retry` wait with current reviewer, stage and existing issue authority checks. Require a nonempty current stage ID. - Add actual producer, cold reconstruction, SQL negative, redaction and optional-persistence controls. - Repair CI fixture expectations for the additive column, startup mock, budget owner and popup return URL. Bind the controlled Native comment server explicitly to loopback. - Replace temporary Slack clock logging with an exact persisted-deadline assertion. Wait until that real deadline before the existing single default poll. Keep both forced polls, business assertions and timing limits. ## Verification - On final integrated source `1645970a`, normal Shared/SDK prerequisites, Runner TypeScript and the entire normal App layered typecheck pass. Tracked source and lockfile remain unchanged. - All 26 whole-file settlement, recovery-reader and bootstrap controls pass. Seven separate real PostgreSQL producer/negative cases pass at `cf16c6a`; each has one selected passing case and 18 filtered registrations. Filtered registrations are not acceptance. - Master then changed the preparation source-lock scope. Its three intersecting cases were rerun on final `1645970a`: an actual failed company process, an actual failed ordinary issue process, and held preparation cleanup. Each preserves its source identity and original deadline and admits one useful successor. Issue and preparation cases exercise normal automatic retry; the company case uses the normal explicit bounded-retry producer. - The unchanged 26 controls and four other PostgreSQL cases retain their `cf16c6a` proof after a scoped source comparison. Optional settlement/preparation persistence failures keep certification blocked while normal cleanup completes. An entered adapter without process identity stays uncertified. This is scoped carry, not a claim that every earlier case ran again on the final head. - All recorded test owners and controlled groups closed. Final read-only SQL audits returned no test sessions. One earlier postcase resource barrier observed a transient unattributed database backend; that database and original evidence remain preserved. Its later disappearance is recovery, not an explained cause. No assertion, deadline or test body was relaxed. - Migration0439 was generated normally after master history. All executable SQL and journal history are preserved; the five retained snapshots are0435–0439. Every Legacy-owned patch and unrelated actual-master blob/mode is preserved. - Published exact head `1645970a9322d1213ba25fc7b52366c285682977` passes all current required checks: 68 successful contexts and eight expected skips after exhaustive pagination and superseded-run accounting. The fresh complete Greptile review scores this exact head 5/5. All four inline review threads are resolved. The latest Ponytail turn stopped without a verdict; one normal retry was requested, with no active turn or new finding observed. No old-head score is used. The original old-base policy failure, its dependent aggregate failure, and the corrected description-gate failure remain recorded. No full local repository suite is claimed; remote typecheck, test and build gates pass. To repeat the real database coverage, set both `PAPERCLIP_RECOVERY_DEADLINES_TEST_DATABASE_URL` and `PAPERCLIP_RECOVERY_DEADLINES_TEST_ADMIN_DATABASE_URL` to the same fresh dedicated test database and run the selected `recovery-deadline-acceptance.test.ts` cases with one worker. An unset URL, skipped case or filtered registration is not acceptance. The fixtures use controlled local processes and real PostgreSQL. Reconstruction creates new service owners inside the same test process. They do not prove deployed Cloud alarms, physical host/database suspension, new-host recovery or Native capacity settlement. The original30-second retry remains below the Cloud270-second minimum wake lead. The merge projection also includes actual master `c3180398faca815fed7a62674bd4b0c4e88884ae`, whose common completion path now sends deferred/recovery notifications. Its unpublished normal merge tree is `16b028d57150f8caa576e913f38343a365b542dd`. Normal Shared/SDK, Runner TypeScript and the full App graph pass on that combined source. One unchanged ordinary-issue acceptance case was rerun there: an actual failed process retains the original automatic retry deadline, admits one useful successor, and passes the settlement, authority and no-duplicate assertions. It passed in 45.055 s; 18 filtered registrations are not acceptance. All known owners and groups closed, and the independent final database audit found zero remaining test sessions. The successful database was removed normally. Recorded worker socket peak is unavailable for this run; parent-process zero values are not used as worker measurements. All 29 Legacy-owned paths and every unrelated master blob and mode are preserved. Two outside-diff review observations concern existing waiting-chat compaction and cross-host Native result-lease recovery. Their files are byte-identical to current master. This focused Legacy change does not fix or qualify those separate paths. ## Risks Settlement proves exact local closure. It does not cover live provider work, Native ownership, cancellation, costs, claims, cleanup debt or changed authority. Independent checks remain. Ship normal migration0439 with the reader and writer. Preserve executable history and the newest five snapshots0435–0439. Generate the MCP child's next migration normally after this parent. The prerequisites are merged; the final remote checks must use this integrated exact head. This proof uses controlled local processes and external PostgreSQL. It does not establish deployed Cloud alarm acceptance, physical host or database suspension, cold host recovery, or R5 Native capacity settlement. The30-second retry is below the Cloud minimum wake lead. This focused repair follows the roadmap's bounded recovery and Cloud deployment work. It adds no new provider policy. ## Model Used OpenAI `gpt-6.1-sol` assisted with readers, fixtures, source review and verification through Codex. A coordinating GPT-6-family agent implemented and reviewed shared lifecycle wiring and integration. Tools included Git, native TypeScript checks and controlled tests. Exact served revisions and context limits were not exposed. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used and its available capability details - [x] I have checked ROADMAP.md and related work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have linked existing public work and described the issue - [x] I have not included internal issue IDs or private links - [x] My branch name describes the change - [x] Required focused checks pass at their explicitly recorded source/carry scopes above - [x] I have added or updated meaningful tests - [x] I have recorded the internal contract and migration requirements - [x] I have documented risks and evidence limits - [x] All required published exact-head Paperclip CI gates are green - [x] Published exact-head Greptile is 5/5 with no open findings in this focused diff; separate unchanged master observations are identified above - [x] I will address reviewer findings before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> Co-authored-by: Codex <noreply@openai.com> |
||
|
|
839d41aabb |
fix(server): retire owned plugin listeners before idle sleep (#19358)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Plugins receive durable agent lifecycle events through a registered host. > - The host uses a PostgreSQL LISTEN connection to wake delivery. > - An idle report could return `none` while that host-owned connection stayed open. > - This change gives that listener an exclusive owner and closes it during the current idle hold. > - Release or expiry restores listening before journal replay. > - Failed close and pending work continue to block sleep. ## Linked Issues or Issue Description Refs #18988, which is now merged. This focused ten-file change now targets master. Current integrated candidate: `0f83005122ef1bdb1f1e08f8f58424c9a840ebb1`, with actual master `644ee66f5d4ae8365fe8b9eca57df2be7405b5ab` preserved through normal merges. No history was rewritten. **What happened?** A normal local App with an installed lifecycle plugin could return an idle `none` report while its lifecycle LISTEN backend and socket stayed open. **Expected behavior** The host must close its own listener before it reports idle safety. It must retain other subscriptions, durable events, company scope, retry deadlines and useful reactivation. **Steps to reproduce** Start the normal server with a lifecycle plugin. Create an agent and wait for its durable acknowledgment. Stop the server, create another agent while it is stopped, and restart with the same files. Request an owned idle hold after ordinary replay. Check the exact listener backend and matched Node socket before shutdown. Let the valid hold expire naturally, then make an ordinary authorized agent update and check its durable plugin acknowledgment. Live executing or persistent preview services remain active. The separate hosted preview dependency [paperclip-cloud#478](https://github.com/paperclipai/paperclip-cloud/pull/478) retains its own App and operator prerequisites. This PR does not claim deployed preview or suspension acceptance. ## What Changed - Add a private, exclusive database listener owner that inherits the normal client policy. - Prepare plugin lifecycle retirement after native retirement under the same current idle hold. - Use one delivery-eligibility helper for dispatch and idle journal inspection. It preserves the same manifest, capability, company-disable query, and skip rules. - Close only the owned listener. Keep failed close ownership and inspection failures visible. - Resume listening on normal release or expiry, before journal replay. - Preserve disabled-feature retry history and the original deadline on re-enable. - Await lifecycle disposal during normal shutdown and consumer teardown. - Add owner regressions and document the lifecycle contract. ## Verification **Current integrated source:** `0f83005122ef1bdb1f1e08f8f58424c9a840ebb1`. Fresh normal Runner TypeScript passed in 0.785 seconds. The complete normal App layered semantic check passed in 28.284 seconds. All tracked source, the lockfile and the 18 separate private witness inputs stayed unchanged. Known compiler owners closed and the isolated temporary home was removed. **Current product qualification:** at measured integration `896cc9b62993e6ed10418d7a7417e1ddbf8dc89c`, all three whole files passed with no failures or skips: lifecycle delivery 40, runtime hooks 20, and runtime assignments 24. The actual physical LISTEN and natural-expiry controls passed. All owned clients and captured processes closed. Independent final session checks were zero before normal deletion of each successful test database. **Normal full-App physical retirement:** the original case passed in 47.574 seconds. It exercised real startup, readiness, SDK callbacks, durable ACK, foreign-company exclusion, physical stop, offline input and cold replay. The initial unknown hold and its ordinary release remain in the evidence. After the original 30.5-second cadence, a new valid hold returned quiescent. The exact current cold-App LISTEN backend and its matched Node socket closed before server stop. **Normal full-App natural expiry:** the separate original case passed in 77.391 seconds under its original 90-second body cap. The final valid 30-second hold expired naturally. A new listener backend and matched current App socket appeared before fresh input. An ordinary authorized agent-name PATCH produced journal event 20, the actual SDK callback and durable ACK. Foreign exclusion and readiness assertions passed. The final measured hold had no manual release, replacement hold or shortened TTL. The original preparatory unknown hold, ordinary release and 30.5-second cadence remain explicit. Both App cases used the normal current server helpers and actual SDK bundling/worker entry. A private compatibility change supplied the same owned HOME at the exact existing child-fork boundary. It did not change the product helpers, business assertions or timing limits. These separate acceptance fixtures remain outside the focused product diff. **Source carry to the current candidate:** all nine other owned files, listener mechanics, actual producer paths, SDK paths and measured hold behavior are unchanged. The sole owned intersection from newer master is an independent environment-delete cleanup callback in `app.ts`. The plugin witnesses do not call that route. Normal merges preserve every unrelated master blob and file mode. The fresh current semantic checks above qualify the integrated source. New master Native crash-recovery and saved-copyback Retry capabilities are not claimed by these plugin cases. **Actual master merge target:** newer master contains retained plugin Runner release/resume and other Native recovery fixes. The projected combined target passed fresh normal Runner TypeScript in 0.799 seconds and the full App semantic check in 29.715 seconds. All ten plugin-owned blobs and modes remain exact to the published head; every unrelated master change is preserved. The measured plugin cases create no Native provider turn or warm Native session. The unchanged native-first/plugin-second ordering still checks the native result before listener retirement. Later sparse-checkpoint and Native response-publication changes do not enter those paths. This is a scoped source carry, not new hosted Runner acceptance. **Failures retained:** an earlier reactivation fixture's real timer returned one millisecond before its original wall-clock expiry. Its private fixture now waits any positive remainder and retains the same expiry assertion and limits. The earlier partial-composition `unknown` result remains distinct from normal-startup proof. Current qualification also exposed missing normal workspace dependencies and a private guard that assumed loose Git refs. Normal owned dependency setup and an exact loose-or-packed reader fixed those setup failures. No business assertion, timeout, permission boundary or product sleep gate was relaxed. Repeat focused product coverage with a fresh external fixture database and the normal server Vitest command, one worker: ```sh pnpm --filter @paperclipai/server exec vitest run src/__tests__/plugin-lifecycle-delivery.test.ts --maxWorkers=1 --no-file-parallelism pnpm --filter @paperclipai/server exec vitest run src/__tests__/agent-runtime-hooks.test.ts --maxWorkers=1 --no-file-parallelism pnpm --filter @paperclipai/server exec vitest run src/__tests__/agent-runtime-assignments.test.ts --maxWorkers=1 --no-file-parallelism ``` Exact current head `0f83005122ef1bdb1f1e08f8f58424c9a840ebb1` has 60 successful checks, three skips and one neutral result, with no pending or failing contexts. Greptile completed its full review at 5/5 with no actionable issue. All root and nested review pages were read; the earlier eligibility finding is resolved. Ponytail's latest turn stopped before a verdict. A normal retry was requested; no current Ponytail verdict or score is claimed. Previous `3ca86ec` CI and scores are historical. A full local repository typecheck/test/build or browser run is not claimed. Controlled local App/SDK/PostgreSQL proof does not establish live provider, deployed Railway/Neon suspension, signed Cloud alarms or global exactly-once delivery. ## Risks - A listener close failure or uncertain inspection must continue to block sleep. - Release, expiry, configuration changes, and journal arrival can race with retirement. Epoch and current-hold checks retain conservative results. - Delivery remains at least once across uncertain ACK boundaries. This change does not add global exactly-once delivery. - The database helper closes only its exclusive listener client. Other owners and subscriptions keep their resources. ## Model Used OpenAI `gpt-6.1-sol`, reasoning and code/tool use. The managing Codex agent implemented shared database/composition seams and reviewed the change; its exact model ID was not exposed. The runtime does not expose a verified context-window size. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> Co-authored-by: Codex <noreply@openai.com> |
||
|
|
374a918d67 |
fix(runner): retain qualified OpenCode artifacts for cold recovery (#19622)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Native recovery must keep admitted provider state and useful workspace files. > - OpenCode binds its saved session to exact executable paths and hashes. > - A replacement image can remove those files even when session data is saved. > - Retaining a manifest alone cannot restore the missing launch files. > - This pull request saves the qualified files before provider work and restores the same owned paths. > - The benefit is cold continuity without relaxing the Runner guard or replaying a prior turn. ## Linked Issues or Issue Description Refs #19558. That fix retains the selected pack for ordinary continuation. This change adds durable file retention for a cold allocation. Related #19487 adjusts the admission budget; it does not retain launch files. A search of open and closed OpenCode cold/durable artifact PRs found no duplicate. **What happened?** A saved OpenCode session could outlive the image that supplied its Node executable, proxy, and native executable. A new image supplied different bytes. The Runner then correctly refused the changed launch profile. The saved harness did not contain the old files. **Expected behavior** Core must save qualified launch files before the first turn. It must restore those exact files under the same owned account and absolute paths. Unknown ownership, missing files, and changed hashes must still stop recovery. **Steps to reproduce** Start an admitted remote OpenCode session. Save its workspace, encrypted checkpoint, and harness data. Destroy the original allocation and controller files. Restore into a new allocation with an image whose qualified proxy has different bytes. Resume the same session. The new opt-in local qualification command automates this path with actual OpenCode and a local synthetic model. ## What Changed - Store company-bound, encrypted launch objects and canonical run/backup references. Generate migration 0438 through the normal database workflow. - Stage qualified Node, proxy, and native OpenCode files under a private content-addressed OS-account home. Record the account UID and exact launch paths. Preserve the Rust profile guard. - Gate first provider work on durable capture under the current native claim. Bound Stop/deadline waits while the actual late transfer keeps its worker and scratch ownership until settlement. Restore cold caches with verified staging and atomic rename. Keep live reattachment and historical image-rooted sessions on their existing paths. - Reuse bounded canonical expiration owners. Atomically fence old checkpoint authority with a strict company/run-scoped retirement marker before external deletion. Keep its indexed cleanup anchor across failed deletes and restart. Preserve earlier resume handles and reject later header or source-consumer writes. Add no idle scan or network call inside a transaction. - Add storage, race, executor, filesystem, and actual Rust resume regressions. Add the opt-in local Linux qualification command and operating limits in `doc/opencode-cold-recovery.md`. ## Verification - RED before repair: real PostgreSQL/AES cold restore lacked the launch files; normal executor preparation chose the new image pack. Separate regressions exposed lost upload receipts, missing cache repair, last-owner retirement, an account-UID mismatch, and a resume-handle race during external deletion. Each expected failure was captured before its repair. - Local qualification before the final master integration: 1,169 executor tests, 233 storage/archive/admission/expiration tests, all seven server typecheck layers, migration safety, and the actual Rust cold-resume test passed. - The maintained Linux command passed with two cached immutable Paperclip images whose proxy hashes differ. It removed the original account/controller trees, restored the production published cwd under a new lease, resumed the same actual OpenCode session, kept a useful file and prior model context, and made one new synthetic model request. No external model call or network was available in either container. - The object-store fixture uses real encrypted StorageService streams with an independent owned local directory. It is not hosted S3 acceptance or a complete task HTTP/actor workflow. Historical tasks still require normal effects reconciliation and recovery after deployment. - Review regressions reproduced partial-cache publication, resume-versus-deletion, fresh-writer and source-consumer resurrection, and unbounded Stop/deadline waits before each repair. They now pass through real filesystem, encrypted storage, PostgreSQL, and SDK boundaries. File validation is split from database ownership (384/831 source lines). - Clean runtime head `aadab7acc69ff959678030db46ffe2dcfbefe28f`: the maintained actual OpenCode cold qualification passed with `dirty:false`. It preserved the provider session and recorded launch paths, restored useful workspace content and prior model context, and made exactly one resumed synthetic model request. The 314,560,000-byte launch capture took 23.118 seconds; the later harness was 833,536 bytes. Subsequent repairs change backup-reference retirement and governed continuity-cache classification; capture, restore, launch, and published-workspace inputs are unchanged, so that runtime receipt remains scoped to those paths. The continuity-break consumer is verified separately. - Retention repair `c14d386cecf7123257918ba6a8d47307df2da397`: 68/68 affected real PostgreSQL/AES storage tests and 15/15 focused executor authority/admission cases pass. All seven server typecheck layers pass (source layers and unchanged test shards, with the final 590-file test layer repeated after fixture edits). All review threads have evidence-backed resolutions. Its subsequent CI caught the separate lifecycle-classification gap described below. The refreshed Paperclip review reported paused work, not a completed new review. - Final head `4d1cd3e3309e328b11abc85adc6d78c6d0c7f55d`: all 19 focused root-entry/executor cases and all three source declaration layers pass. [Greptile reviewed this exact head at 5/5](https://github.com/paperclipai/paperclip/pull/19622#issuecomment-6695984874). Final-head [CI verification](https://github.com/paperclipai/paperclip/actions/runs/42308908167/job/128214100339) passes: 60 successful checks, five conditional skips, and the explicitly neutral paused Paperclip review. No check is pending or failing. - CI also caught the launch cache missing from the session-root lifecycle contract. Both the writer scan and a real governed-replacement consumer failed locally before repair: old launch bytes remained in the active root. The cache now archives with the prior session state; the consumer verifies exact bytes are preserved in the archive and absent from the active root. - The latest review found that backup retirement could delete an independent run pin while a selected session retained the checkpoint. A real PostgreSQL/AES regression failed before the repair. Backup-only ownership now uses a NULL-safe exact backup predicate; the run pin belongs to fenced checkpoint expiration. The regression verifies checkpoint decryption and launch-file restoration after pending-backup cleanup, followed by deletion only after removing the final resume handle. - An executor first-session case checks new-session durable capture through the real capture helper and receipt propagation into the actual harness manifest at the backup boundary. Real storage cases verify the run-profile receipt and canonical backup/run pins. - Sequential closure reuse and simultaneous first admission are different scopes. A deterministic two-uploader test measures both independent uploads, one verified ready winner, loser-only deletion, and usable restoration for both runs. Each capture is bounded to 512 MiB and five minutes; up to three pending receipts per run are retained. There is no claim of global single-flight capture or a company-wide egress bound. - Earlier optional warm-ownership verification encountered missing local binaries. After exact-source setup, all nine ownership assertions passed; two other suites encountered local PostgreSQL setup/teardown contention, so those interrupted attempts are not passes. Required current-head CI supplies the broad final coverage. No deployment or hosted historical recovery is claimed. ## Risks - This adds two tables and a forward migration. The file object is bounded to 512 MiB. Capture adds one initial transfer; later harness archives exclude those large files. Company-scoped ready objects are shared and retained until the last canonical owner retires. Concurrent initial admissions can duplicate bounded transfers; each keeps independent ownership and cancellation rather than waiting on another controller. A continuity break retains the controller cache with its prior state until normal session-root cleanup, adding up to one bounded launch closure per retired state. - Cold continuity requires the same qualified Linux platform, supported runtime dependencies, owned account UID/home, and published absolute cwd. A mismatch fails closed. No alias or Rust guard change is included. - Existing image-rooted profiles have no retained file object. This change cannot backfill missing historical binaries. A fresh governed session must reconcile prior effects first. - Live reattachment does not read artifact objects. Storage or provenance failures before a new turn hold admission. They do not grant new execution, model, tool, or replay authority. ## Model Used OpenAI Codex, GPT-6 family, with reasoning, repository tools, code execution, and local browser tools. The exact served model ID and context window are not exposed in this session. No claim is made for unavailable model metadata. ## 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> |
||
|
|
21a1e78852 |
feat(environments): add managed plugin environment rows (#19490)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A managed instance gets its instance-level environments from the `PAPERCLIP_MANAGED_CONFIG` document > - That document cannot declare a plugin environment yet, so a managed instance cannot get a plugin-provided environment without manual setup > - The full change is split into three pull requests. This first one adds the plugin environment rows and their write floor, and nothing calls them yet > - The second pull request lets the managed config declare plugin entries and provisions them at boot. The third sets the instance default once and advertises the new managed-config capability > - The benefit is a smaller review for each step, and no instance changes behavior until the last step lands ## Linked Issues or Issue Description **Subsystem affected** Server environments (managed-config environment provisioning, environment routes) and the board UI's adapter Test target. **Problem or motivation** A managed instance cannot get a plugin environment from its managed config. Company settings cannot create one either. So each managed instance that needs one requires manual setup. **Proposed solution** Over three pull requests, let an `environments` entry in the managed config declare `driver: "plugin"`. Boot then provisions one locked row for each plugin entry, and makes one of them the instance default once on new instances. This pull request adds only the row layer: create or adopt a marked row, archive it, find it, and keep tenants from editing it. **Alternatives considered** Manual setup on each instance, and a new control-plane endpoint. Both need a step outside the deploy. The managed config already delivers the sandbox entry. **Roadmap alignment** This extends the existing managed-config `environments` section, which already provisions the managed sandbox. It adds no new product surface. ## What Changed - New `server/src/services/managed-plugin-environments.ts`, used through new `environmentService` methods (`ensureManagedPluginEnvironment`, `archiveManagedPluginEnvironment`, `findManagedPluginEnvironment`): - The ensure validates the driver config through `normalizeEnvironmentConfig` and `validatePluginEnvironmentDriverConfig`, the same path an operator-created plugin environment takes. - It marks the row `managedByPaperclip: true` and `managedPluginEnvironment: "<pluginKey>:<driverKey>"`. - It adopts an existing unmarked row for the same plugin and driver instead of adding a second one, so a hand-made row keeps its agents. - A row that already matches gets no write. Changes write activity log entries. - Archive sets an active marked row to `archived`. The next ensure reactivates it. - Migration `0437`: unique index `environments_managed_plugin_environment_idx` on `metadata ->> 'managedPluginEnvironment'`. This is the only schema change of the three pull requests. Snapshot `0432` is pruned to keep the five newest. - `server/src/routes/environments.ts`: `isPlatformProvisionedEnvironment` also covers the new marker (write floor and redaction). `managedPluginEnvironment` is a reserved marker key, so a client cannot stamp it on its own row. Nothing provisions marked rows yet, so the marker-clear escape hatch stays open for them. The next pull request refuses it while the managed config declares the row's entry. - `ui/src/lib/adapter-test-environment.ts`: `resolveManagedSandboxEnvironmentId` requires `driver === "sandbox"`. The plugin row also carries `managedByPaperclip`, so the old "not local" check could take it for the managed sandbox. ## Verification - `cd server && npx vitest run src/__tests__/managed-plugin-environments.test.ts` (embedded Postgres): - The ensure creates the marked row through the plugin validator. A second ensure writes nothing and logs nothing. - A hand-made row for the same plugin and driver is adopted. It keeps its id and its agents. - Archive, then ensure, reactivates the row. - The ensure refuses while the plugin has no running worker, and refuses a name that another environment holds. - The unique index allows one row per marker. - `npx vitest run src/__tests__/environment-routes.test.ts`: on a cloud-managed instance, edit, env-var edit, status change and delete of a marked plugin row are refused, including for instance admins. The marker alone keeps the floor. A client cannot stamp the marker. The marker clear is allowed. - `npx vitest run src/__tests__/environment-service.test.ts`: passes. - `cd ui && npx vitest run src/lib/adapter-test-environment.test.ts`: the plugin row is never taken for the managed sandbox. Under managed-sandbox-only, an agent on the sandbox tests in the sandbox, and an agent on the plugin environment tests there. - `pnpm --filter @paperclipai/server typecheck`, `pnpm --filter @paperclipai/ui typecheck` and `pnpm --filter @paperclipai/db check:migrations` pass. - I did not run the full `pnpm test:run`, `pnpm -r typecheck` or `pnpm build`. CI covers them. `pnpm verify` asks for broader verification because of the migration. ## Risks - Migration: one new partial unique index on a small table. No row carries the marker before this change, so the index cannot fail on existing data. - No caller uses the new ensure yet, so no instance changes behavior. - The UI change affects only lists that hold a non-sandbox row with `managedByPaperclip`. Today that is only the local row, which the old check already excluded. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, through Claude Code with tool use (file edits, shell, test runs). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] 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> |
||
|
|
7e3caa7bac |
fix(tree-control): resume work a pause gate refused after release (#19421)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work.
> - The board can pause an issue tree. A pause stops work. Resume or
release must continue that work once.
> - PR #19394 made release re-dispatch a run that a pause Stop
cancelled.
> - That rule covers only a run that an operator Stop ended and that the
hold snapshot named.
> - A scheduled retry that comes due during a pause is not stopped. The
dispatch gate cancels it with `issue_paused`. The release did not see it
as owed work, so the task stalled with no run.
> - This pull request makes every gate that refuses work for a pause
record the covering hold. Release then re-dispatches that work exactly
once.
> - The benefit is that a paused task with a parked retry or a queued
run continues after release, with no manual wake.
## Linked Issues or Issue Description
Refs #19394
**What happened?**
A child task had a `workspace_busy` scheduled retry. The board paused
the parent tree. The retry came due during the pause. The
scheduled-retry gate cancelled it with `issue_paused` ("Scheduled retry
suppressed because the issue is held by an active subtree pause hold").
The board released the hold. Four minutes later the child was still
`in_progress` with no run, no execution blocker, and no new activity.
**Expected behavior**
Release (or `mode: "resume"`) re-dispatches the refused work exactly
once. No reconciliation and no user message are necessary.
Stranded-issue recovery does not add a second run.
**Steps to reproduce**
1. Give an assigned child task a parked `workspace_busy` scheduled
retry.
2. Pause the parent tree.
3. Let the retry come due while the pause is active. The gate cancels it
with `issue_paused`.
4. Release the pause. No run starts for the child.
**Paperclip version or commit**
master at
|
||
|
|
5c185db77c |
fix(tree-control): resume work a subtree pause interrupted (#19394)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Task tree control lets the board pause a task tree and then resume
it.
> - A pause stops the runs that are active in the tree. The release must
then continue the work that was pending.
> - A child task whose run was active at pause time did not start again
after release. The child became blocked and asked for a user message.
> - A pause is an operator hold, not a failure. It must not strand work,
and it must not require reconciliation for a run that never started.
> - This pull request re-dispatches interrupted work one time on
release, and it settles a legacy Stop that never entered the adapter.
> - The benefit is that pause and resume keep a task tree moving without
manual recovery, and real safety gates stay in force.
## Linked Issues or Issue Description
**What happened?**
A parent task delegated two children. One child's run was active (in
host workspace preparation, before the adapter started) when the board
paused the parent's tree. The pause cancelled that run. After the board
released the hold, no new run started for that child. The child became
`blocked` with the execution blocker cause
`legacy_execution_requires_reconciliation`. Later the next action became
"Send a message to continue." A sibling child that had no active run at
pause time resumed normally.
**Expected behavior**
A run that a subtree pause cancels is an operator hold. On release, the
pending work starts again one time, automatically. It does not go
through execution reconciliation when the run never entered its adapter.
It does not wait for a user message.
**Steps to reproduce**
1. Create a parent task and a child task that is assigned to an agent.
2. Wake the child. While its run is in host preparation (or queued),
call `POST /api/issues/:parentId/tree-holds` with `{ "mode": "pause" }`.
3. Call `POST /api/issues/:parentId/tree-holds/:holdId/release` without
`wakeAgents`.
4. Before this change, no new run starts, and `getExecutionBlocker`
reports `legacy_execution_requires_reconciliation` for the child.
**Root cause**
- (a) The pause records each interrupted run in the hold member snapshot
(`activeRunId`). Nothing used that record on release.
- (b) Release woke tasks only when `metadata.wakeAgents === true`.
Without it, release relied on stranded-issue recovery. That recovery
stands down when the latest run was cancelled by an operator
(`isOperatorCancelledRun`). So the interrupted child had no owner.
- (c) The pause Stop of a legacy run in preparation is written directly
as `cancelled`. Legacy terminalization classifies a cancelled run with
no bootstrap evidence as an unknown outcome. It records
`legacy_execution_requires_reconciliation`, although the gated dispatch
never entered the adapter.
- Native runs: the verified native Stop does not create the
reconciliation hold, but gap (b) applies to native runs too. The release
change is runtime-independent.
## What Changed
- `issueTreeControlService.findOwedPauseResume` decides if a task is
owed one resume. The task's latest run must be an operator Stop that a
pause hold member snapshot names, recorded while that hold was active.
The task must still be executable by the same agent, and no pause may
still cover it. The obligation belongs to the run, so overlapping pauses
keep it until the last covering release.
- New `services/issue-tree-resume.ts` re-dispatches owed work with wake
reason `issue_tree_resumed` (`holdId` and `interruptedRunId` in the
context). A compare-and-set claim (`treeHoldResume`) on the interrupted
run makes the resume happen at most once. A failed wake returns the
claim.
- Pause release and `mode: "resume"` call it for every member of the
released hold, also without `wakeAgents`. When `wakeAgents` is true, the
existing loop wakes every executable member.
- A task that still has an execution blocker is not replayed. The
release writes `issue.tree_resume_wake_skipped`, the blocker stays
visible, and the obligation stays unclaimed.
- `releaseHold` updates only an `active` hold. A repeated release gets
`409`.
- `settleCancelledLegacyPreparation`: when the executor that owned a
cancelled legacy run proves that dispatch never entered the adapter, it
records bootstrap evidence and resolves that run's
`legacy-execution:<runId>` hold as `false_positive`. It also corrects an
automatic no-replay disposition and restores a `blocked` status only
when that disposition made the change. Then it re-dispatches owed work,
for a release that happened while the Stop was unwinding. It requires a
requested Stop with no existing recovery receipt, and no process,
session, native finalization, unreleased lease, or execution-start
event.
- `settleUnrecoverableExecutions` waits while a legacy executor still
holds its controller lease, and records a `legacyCancellationBlock`
receipt when it blocks a task for a cancelled legacy run.
- Docs: `doc/execution-semantics.md` and `doc/composer-stop.md` describe
the resume rule.
## Verification
- New regression
`server/src/__tests__/issue-tree-pause-resume-run.test.ts` uses embedded
Postgres, the real heartbeat, and the real tree-control routes:
- A run paused during preparation: nothing runs during the hold, there
is no blocker, and release creates exactly one `issue_tree_resumed` run
that succeeds. A second release returns `409` and creates no run.
- Release before the stopped preparation finishes unwinding (with the
periodic disposition run in between): exactly one resumed run after the
executor settles.
- An automatic no-replay disposition that ran first: the blocker clears,
the status returns to `in_progress`, and release resumes once.
- Overlapping child and parent pauses: no resume while the parent pause
remains, then exactly one resume after it is released.
- A queued run cancelled by the pause: release creates exactly one
resumed run.
- A task reassigned to a user during the hold: no resume.
- A mid-turn legacy Stop with unknown outcome: no replay, the
reconciliation blocker remains, and `issue.tree_resume_wake_skipped` is
logged.
- Red before each fix: the first 4 cases failed on master; the 3 review
cases failed on the first PR commit. All 7 pass now.
- Also passed locally: `issue-tree-control-routes`,
`issue-tree-control-service`, `heartbeat-workspace-busy`,
`heartbeat-undispatched-execution-wait`, `execution-control-work`,
`legacy-workspace-restore-recovery`, `legacy-continuation-authority`,
`task-retry`, `legacy-execution-recovery`, `agent-conversations`,
`heartbeat-retry-scheduling`, `satisfied-connection-intents`.
- Not verified here: a live staging pause/resume. Re-check on staging
after deploy.
## Risks
- Behavior change: release now starts the work that the pause
interrupted, also without `wakeAgents`. Before, that work stayed idle.
Other idle tasks still wake only with `wakeAgents`.
- The settlement trusts the executor's adapter-entry flag. The flag is
set only inside the atomic dispatch gate, which refuses a cancelled run.
The settlement also requires no process, session, lease, or execution
event.
- A native run whose Stop has not finalized at release time is not a
target. Stranded recovery and explicit wake still apply.
## Model Used
- Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude
Code with tool use (shell, file edit, test execution).
## Checklist
- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [ ] All Paperclip CI gates are green
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
|
||
|
|
958f9aa9f3 |
feat(issues): limit agent-created task trees to parent and child (#19312)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Agents split work into tasks. A task can have a parent task. > - Today agents can nest tasks to any depth: parent, child, grandchild, and deeper. > - Deep agent-made trees are hard to follow, and an agent that works on a child task makes grandchildren by default. > - This pull request limits agent-created trees to two levels: a top-level task and its children. > - A request for a grandchild does not fail. The server puts the new task beside the requester and tells the agent. > - Board users keep full control. They can still nest to any depth. Existing deep trees stay valid. > - The pull request also records which task created each new task, so the audit trail stays complete when the parent changes. ## Linked Issues or Issue Description No public issue exists. This is the feature description. Related: #16853 (budgets that include descendant work) also reasons about task trees; this change does not alter its traversal. **Subsystem affected** Issues (tasks): create, child create and move in the issue service and routes. Agent tools, chat suggestions and creation provenance use these paths. **Problem or motivation** Agents create subtasks as deep as they want. An agent that runs on a child task gets that child as the default parent, so its follow-ups become grandchildren. The trees become deep and hard to read. The parent link is also the only record of which task created a follow-up, so that record is lost when the parent changes. **Proposed solution** Limit agent-created trees to a parent and its children. When an agent asks for a grandchild, put the new task under the requester's parent and return `parentAdjusted`. Do not reject the request. Keep board users unrestricted. Record the originating task in a new server-set `origin_issue_id` column. Keep `origin_run_id` as the originating run. **Alternatives considered** Reject grandchild requests with an error. We did not do this, because agents then fail mid-task. Add a database constraint. We did not do this, because existing deep trees must stay valid and nesting can return later. ## What Changed - Add `MAX_AGENT_TASK_HIERARCHY_DEPTH` (2) and the `IssueParentAdjustment` type in `packages/shared`. - Add `resolveNewTaskParent` and `planIssueParentChange` in `server/src/services/issues.ts`. `create`, `createChild` and `update` use the agent limit by default (`parentDepthPolicy: "flatten"`). - The issue create, `/children` and PATCH routes pass `allow_nested` for board users. For agents, the routes check authorization against the parent that the server actually writes. - The requester keeps its behavior when the server moves a task beside it: `blockParentUntilDone` blocks the requester, and the new task inherits the requester's workspace and privacy. - The server never moves a task above a low-trust boundary root that contains the requester. - These subsystems keep any depth, because they find their work by parent: watchdog reviews, monitor recovery, email-channel placement, and routines that a person configured. - An agent move of a task that has children skips only the parent change and returns `reason: "has_children"`. - Add `issues.origin_issue_id` (nullable, `on delete set null`, indexed) in migration `0434`. Only the server sets it. The `createdFrom` detail field, the `createdFromIssueId` filter and task-creation provenance read it first. - Chat prompts now let a chat agent create one task, a task with children, or separate tasks. The conversation is never a parent. - The runner `create_task` tool accepts an optional `parentTaskId`. A chat agent can create a top-level task and then put subtasks under it. The run must be able to read and change the parent. A conversation is never a parent. The depth limit still applies. Project tools map the field to `parentId` through the issue route. The protocol manifest, semantic contracts and eval fixture are regenerated. - Watchdog runs that have an active watchdog keep their requested parent on all create paths, because their access is limited to the watched subtree. - Fix a bug that the test drive found: the chat (MCP) `create_task` ignored the requested `status`, so a `backlog` task became `todo` and started to run. It now uses the same rule as the native tool. - Update `skills/paperclip`, `doc/SPEC-implementation.md`, and add the plan `doc/plans/2026-11-05-two-level-task-hierarchy.md`. ## Verification - `cd server && npx vitest run src/__tests__/agent-task-hierarchy-depth.test.ts src/__tests__/agent-task-hierarchy-depth-routes.test.ts`: 19 new tests for service and route behavior, all pass. - Related suites (64 files: issues service, thread interactions, privacy, watchdogs, provenance, runner tool authority, decisions, routines, project tools): 1208 tests pass. Two tests changed because of the intended behavior and pass now. Two `email-channels.integration` tests fail locally because the test cannot find a bundled skill file from this worktree path. This change does not touch that code. - `packages/db`: migration safety, snapshot drift and journal tests pass. - Typecheck passes for `server`, `packages/shared`, `packages/db`, `cli` and `ui` (local `ui` errors come only from generated Storybook catalog files that are absent). - Local test drive on an isolated instance with real Claude agents: 29 of 29 use cases pass, including an agent on a child task (follow-ups placed beside it, with the notice) and a chat that creates a top-level task with three subtasks. - I did not run the full `pnpm test:run` or `pnpm build` locally. CI runs them. ## Risks - Behavior change for agents: some follow-ups that were grandchildren are now siblings. The parent of the requester is woken when all its children finish, which now includes these tasks. - The runner `create_task` input contract adds one optional field, so the protocol manifest and the pack contract change. The tool output does not change: it shows the actual `parentId`, and the activity log records `parentAdjusted`. - Migration `0434` adds a nullable column, a foreign key and an index. It does not backfill rows. - `IssueCreationSource.run` is now nullable. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`) in Claude Code, with tool use (shell, file edits, local 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 - [ ] 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> |
||
|
|
f2593b7905 |
feat: make Coordinator and Agent Chat standard and speed up navigation (#19360)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - People enter that work through the workspace, agent conversations, and tasks. > - Coordinator and Agent Chat still require experimental enrollment. > - Task link prefetches do not warm the compact queries used by the default task view. Chat also waits for a request when its navigation list already holds the conversation. > - This pull request makes Coordinator and Agent Chat standard and reuses the reads needed for navigation. > - People can choose Classic workspace in Experimental settings. Cached work can appear while fresh data loads. ## Linked Issues or Issue Description **What existing behavior does this improve?** Workspace selection, Agent Chat availability, and navigation between Chat, Tasks, and task details. **Subsystem affected** Cross-cutting: UI, workspace preferences, instance settings, and the database migration. **Current behavior** People must enable Coordinator and Agent Chat in Experimental settings. Opening a task can repeat requests because prefetch warms the full detail and comment queries, while the default view reads compact task data and a recent comment window. Opening Chat waits for a detail request even when the current person's conversation list contains it. **Proposed behavior** Coordinator is the standard workspace for everyone. Experimental settings offer Classic workspace, off by default. Old Coordinator experiment values do not select Classic. Agent Chat is available to everyone, including instances with an old saved or managed off value. Task link intent warms the queries used by the mounted view. Chat can use the current person's company-scoped conversation list during refresh. **Reason and benefit** People can use the standard workspace without setup. Repeat navigation can show cached content while the server refreshes it. **Breaking changes** Existing Coordinator experiment and Agent Chat off values no longer disable these surfaces. Classic requires an explicit new opt-in. The workspace API keeps its existing response shape and audit checks. The migration retains legacy values and adds a separate Classic preference. Execution permissions, company access, and chat privacy checks still apply. **Additional context** Roadmap check: this graduates existing surfaces and improves their navigation. It does not add a new agent execution capability. Duplicate searches found no open Coordinator workspace PR. Related server cache work, Refs: #17828, addresses compact issue-list invalidation; this PR addresses client navigation queries. ## What Changed - Add a default-off Classic workspace preference. Derive the existing workspace API response from that choice. Keep personal persistence, account isolation, and audited writes in the workspace preference service. - Replace the Coordinator experiment control with Classic workspace. Use Coordinator while preferences load. Update the existing settings examples and navigation recipe. - Retire the Agent Chat experiment control. Ignore old stored and managed off values. Retain the compatibility key and the existing execution and access checks. - Reuse account- and company-scoped chat list data with its original freshness timestamp. - Share compact task and recent-comment query options between prefetch and the task view. Keep full-history prefetch for Classic tasks and expanded history. - Add upgrade, replay, cache, account isolation, and browser switching regressions. Update product, database, development, and roadmap documentation. ## Verification - Targeted preference, settings, migration, cache, and UI regression tests passed during implementation. The cache tests hold refresh requests pending and check that content still renders. They also check account isolation and stale refresh behavior. - A real browser test verified fresh Coordinator defaults, Classic opt-in persistence, and return to Coordinator. It used an isolated source checkout with fresh data on macOS. - Token, migration numbering and safety, map, and Storybook catalog checks pass after the rebase. - Repository typecheck and build pass. Typecheck and token gates also pass after review repairs. The broad local run passed all six chat integration shards (1,931 tests). It then found the host-CLI-dependent recovery fixture described below. Its remaining duplicate work was stopped after the full current-head CI matrix passed; this is not a claim that the broad local command completed green. - Review repairs passed 419 UI/migration fixture tests, 19 Coordinator navigation tests, and all 214 tests in the five affected server suites. The 24 Chat/workspace browser tests pass, including standard Coordinator switching and explicit Classic persistence. - The broader browser repair run passed 85 tests. It retained two failed assertions and skipped five later serial cases. The affected files then passed all 11 tests after the fixes: reopen the desktop panel after reload and open More for Audit and Connectors. This includes the five serial cases that had not run. - The broad local suite exposed a recovery fixture that executed the host Codex CLI during a simulated remote probe. On a host with Codex 0.154.0, model fallback broke its fixed-model assertion. The fake sandbox now reports Codex 0.160.0. All 20 selected recovery/provider compatibility cases pass after the repair. - A CLI company import/export suite timed out during server startup in the second CI run. The port fallback is intentional in that fixture. No root cause was established. That suite passes on the final commit in CI. This records recovery, not a claimed product fix for the earlier timeout. - The first CI run exposed remaining assumptions about the retired flags and Classic UI. The repair commit updates those fixtures and navigation steps. It also restores the missing enabled Cases destination in Coordinator. Earlier local recovery attempts used the wrong fixture notification and advanced fake time during live execution; the corrected tests use the subscribed recovery queue and wall time during execution. - Greptile reviewed commit `98972b030d4505d838427a2fca6b196b8cad6d47` at **5/5** with no new comments. The original finding is resolved. - Current-head CI is green: **56 successful checks**, with three checks skipped by workflow filters. This includes typecheck, build, the complete Vitest matrix, all eight browser shards, Runner verification, release checks, and security checks. [Final CI run](https://github.com/paperclipai/paperclip/actions/runs/42132651414). - Typecheck caught missing `pid` and `startedAt` fields in the simulated command result. The fixture now returns the complete process contract, and the server test-layer typecheck passes. - Storybook catalog alignment was checked. Storybook play tests were not run. Hosted latency and cold bundle loading were not measured. ## Risks - The new default changes the workspace for people who previously left the Coordinator experiment off. They can opt into Classic in Experimental settings. - The additive migration must run before the new server code. It is safe to replay and preserves explicit Classic choices. - Cached content can be stale during background refresh. Existing query invalidation and source timestamps still control freshness. - These changes reduce avoidable loading waits. They do not establish a measured improvement in hosted request latency or initial bundle download time. ## Model Used OpenAI Codex, based on GPT-6, with reasoning, repository tools, code execution, and browser automation. The exact serving revision 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: Scott Tong <scott@scottsmbpm5max.lan> Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
|
bf10bc695a |
feat(task): show the agent's context window in task properties (#19273)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Agents run through a harness (Codex app-server, or Claude through ACPX), and the runner turns harness events into Paperclip events > - Both harnesses report how full the model's context window is. Codex sends `modelContextWindow` with its token usage. Claude sends `used` and `size` in ACP `usage_update` > - Paperclip dropped both values. The task view could not show how close an agent is to compaction, and a compaction left no visible mark in the task chat > - This pull request carries the context-window occupancy through the runner, stores it with the existing usage events, and shows it as a "Context" row in the task Properties panel > - It also shows a "Context compacted · 230k → 38k tokens" divider in the task chat when the agent compacts its context > - The benefit is that operators can see when a long task is near its context limit and when the agent lost detail to a compaction ## Linked Issues or Issue Description Refs #19252 (same pattern: a canonical harness event turned into task-chat UI). Refs #19267 (status notices; it turns Claude's compaction banner into a notice, and this PR reads that notice too). **What existing behavior does this improve?** The task view shows no context-window information, and a compaction is invisible in the task chat. **Subsystem affected** Runner usage normalization (Rust and TypeScript transport), the PRP usage schema, a read-only issue API, the task Properties panel and the task-chat transcript. **Current behavior** Codex `modelContextWindow` and ACP `used`/`size` are dropped in the runner. `context.compacted` always has null sizes, Claude compactions emit no `context.compacted` event at all, and the live runner turn hides every marker except "interrupted". **Proposed behavior** `usage.reported` carries an optional `context {usedTokens, windowTokens}`. The task Properties panel shows a Context row ("10% of 258k") with a hover breakdown (tokens used, input and cached input, output, cost, compactions). The transcript shows a compaction divider, with sizes from the usage before and after the compaction. **Reason and benefit** Operators can see when a long task is close to its context limit, and they can see where the agent's earlier context was summarized. **Breaking changes** None. The new schema field is optional, and the new endpoint is read-only. ## What Changed - PRP `usage.schema.json`: add an optional `context {usedTokens, windowTokens}` object. Regenerate the schema bundle and the protocol manifest. - Rust `provider_events.rs`: add `codex_context_usage` (last request `totalTokens` and `modelContextWindow`) and `acpx_context_usage` (ACP `used`/`size`). ACPX `usage_update` now sets `context`. - Rust `provider_backend.rs`: Codex `thread/tokenUsage/updated` sets `context` on live (non-historical) usage. - Rust `provider_events.rs`: emit `context.compacted` for the completed `context_compaction` provider notice that #19267 produces from Claude's compaction banner. Only this path marks a Claude compaction, so the server counts each compaction once. - `runnerd-codex-transport.ts`: `rehydrateRunnerdUsageNotification` keeps a valid `context` on `tokenUsage`, so it reaches the stored `item.completed` usage item. - Server: new `GET /api/issues/:id/context-usage` (issue read access is required) and `issueContextUsage` service. It reads only the assignee's current session (the session of its newest usage report), and returns that session's latest context reading, non-zero totals, cost, and the count of compactions across all of the session's runs. Added to OpenAPI. Shared type `IssueContextUsage`. - DB: migration 0432 adds two partial indexes on `heartbeat_run_events`, built concurrently. One indexes usage items by run and time. The other indexes `context.compacted` events by company and session. `EXPLAIN` shows that both lookups use them. - UI: new `ContextUsageMeter` (bar, percentage, hover card) and `IssueContextUsageRow` in the Execution section of Properties. The row refreshes every 8 s while a run is active, and once more when the run ends. - UI transcript: `native-run-events.ts` maps `context.compacted` to a `context` activity entry with pre/post sizes from neighbouring usage items. `transcript-adapter.ts` turns it into a new `context_compacted` marker variant. The live runner turn keeps that marker as its own timeline row. - Storybook: `task-context-window.stories.tsx` (Properties panel in a task, all states, hover breakdowns, divider). ## Verification - `cargo test -p paperclip-runner-core --lib provider_events` (new tests: ACPX context, Codex context, banner compaction, notice compaction) - `pnpm --filter @paperclipai/paperclip-runner exec vitest run src/live/runnerd-codex-transport.test.ts` - `cd ui && npx vitest run src/components/ContextUsageMeter.test.tsx src/components/transcript src/components/task-chat src/components/issue-properties` - `cd server && npx vitest run src/__tests__/issue-context-usage.test.ts` (embedded Postgres) - `node scripts/typecheck-low-memory.mjs`, `pnpm check:token-gates`, `pnpm --filter @paperclipai/paperclip-runner run check:protocol-types` - Live check on a local worktree instance through the runnerd path: - A Codex `gpt-5.5` task showed "10% of 258k" (27k of 258k, input 77k with 49k cached, output 1.1k). - A Claude Sonnet 4.6 task showed "17% of 200k" (34k of 200k). - Storybook: Tasks and conversations / … / Task chat / Context window. - Not verified live: a real compaction. The divider is covered by unit tests through the same projector path the task chat uses. ## Risks - The schema field is optional, and older runners omit it. The UI renders nothing until a harness reports a window. - Migration 0432 builds two partial indexes concurrently on `heartbeat_run_events`, which is a large table. The indexes cover only usage items and compaction events, so they stay small. They add a little write overhead to those events. - Claude in-progress usage rows still record zero tokens with `requests: 1`. The final turn usage is correct. This PR does not change accounting. - A "Compact now" action is out of scope. It needs a compaction-only run that does not produce a run result. It will come in a follow-up. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude Code with tool use (shell, file edits, in-app browser for Storybook and the live instance). ## 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> |
||
|
|
0eaabe5c0b |
refactor(accounting): remove company serialization from paid settlement (#18917)
Settle paid operations through operation-scoped transactions. Preserve exact ledger evidence and replay while moving aggregate budget decisions to durable deferred work. Co-Authored-By: Paperclip <noreply@paperclip.ing> |
||
|
|
612d9931a6 |
Support per-turn Runner sessions on ephemeral web servers (#18932)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The web server controls native Runner sessions in remote sandboxes. > - An ephemeral web server must recover without its previous local filesystem. > - The existing admission rule accepts warm sessions. Managed Claude, Grok, and OpenCode sessions resolve to per-turn execution. > - Per-turn execution also needs durable provider-session bytes after the sandbox is retired. > - This pull request adds encrypted backup publication and removes the warm-only admission rule. > - Both lifecycle modes can use the existing native ownership and checkpoint contracts during a web deployment. ## Linked Issues or Issue Description Refs: #18518, #18529, #18826, #18835, #18841, #18864, #18887. **What existing behavior does this improve?** Native Runner execution on web servers with `PAPERCLIP_EPHEMERAL_SERVER=true`. **Current behavior** The admission rule requires a warm Runner lifecycle. Managed provider credentials can resolve an agent to per-turn execution, so enabling the ephemeral deployment profile blocks those turns. A database controller checkpoint does not preserve provider files after the old web home and per-turn sandbox are removed. **Proposed behavior** Admit native execution in either resolved lifecycle. Keep provider and credential choices unchanged. Publish encrypted provider state to shared storage before sandbox cleanup. Restore it from a fresh web home for the next turn. Use the existing controller lease and generation checks for active takeover. **Reason and benefit** Operators can remove the web server volume without excluding per-turn providers or forcing managed accounts into warm sessions. New native Daytona profiles automatically enter both rolling-deployment qualification lanes. **Breaking changes** The additive migration creates a backup publication ledger. Existing volume-backed local-disk installations retain their local backup path. S3 installations publish encrypted backups and require a stable secrets master key. Existing-volume enrollment must import surviving session data before detaching the volume. ## What Changed - Admit native Runner execution independently of its warm or per-turn lifecycle. Keep host-owned legacy execution excluded from ephemeral deployments. - Store private provider state as encrypted objects. Journal uploads before I/O, read them back, and fence publication against the current native owner. Keep the previous published backup when storage or ownership checks fail. - Restore exact provider bytes into a private cache. Require an exact durable backup before sandbox cleanup. Extend the existing bounded retention pass to retire stored objects without adding idle polling. - Import selected legacy controller and provider state in the existing offline maintenance process. Preserve queued work, task pointers, pending recovery, and original files. Prefer canonical database checkpoints. - Preserve already-failed legacy controllers with missing original artifact proof as encrypted archives. These archives do not become execution authority. Successful retained sessions still require proof. Report archival and executable imports separately. - Verify unmanaged Grok API keys through the existing xAI account validator. The live test exposed a readiness gap that kept remote Grok agents paused after platform checks. Preserve failed credentials and the separate runtime warning. - Ingest Grok whole-prompt usage at the owning ACP boundary. Bind the receipt to the exact outbound request/session; reject replay, conflicts, and incomplete authority. Separate turn cost from cumulative session cost in the sidecar schema and Rust normalization. Preserve managed billing identity and identify direct xAI API credentials. - Preserve published workspace directory names across cold allocations, including destroyed-allocation cleanup. Keep unpublished epoch/lease directories isolated and refuse to overwrite an occupied preparation. This fixes the cold-session identity regression exposed by #18922. - Increase bounded checkpoint-header inspection to 1 MiB. Retain the aggregate inspection limit. - Register all nine native Daytona profiles in per-turn and warm rolling-deployment suites. Delete the entire departing server home, exercise ordinary reads and writes, require active ownership transfer, then check a later turn on another fresh server. Permit only exact notification replay after reconnect. Ownership stays with the existing modules: Runner validates its protocol state; Core validates run scope and controller ownership; storage owns object I/O; the maintenance process owns offline conversion; the E2E wrapper owns process replacement and probes. No provider protocol is added to the shared admission rule. Grok receipt parsing stays in its provider module. The shared sidecar gains an explicit cost basis because its existing price field means session-cumulative spend, while Grok reports prompt totals. Existing consumers retain cumulative behavior when the new field is absent. ## Verification - Focused native admission, archive validation, publication fencing, cleanup, readiness, legacy import, and E2E observer tests pass. Real PostgreSQL cases verify that queued intent and historical recovery rows stay unchanged. - Full typecheck and build pass on the rebased branch. The review follow-up passes server typecheck, 29 focused PostgreSQL/archive/readiness tests, and module-boundary checks. Revision fb8d39a7a1 reached 55 successful CI checks and Greptile 5/5 with no unresolved threads. The Grok follow-up passes Runner/server typechecks, 238 ACP runtime/projection/usage tests, 16 sidecar-schema tests, the Rust cost regression, and Core billing regression. The |
||
|
|
ce332ecec0 |
fix(execution-admission): keep admitted project sets after settlement (#18871)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - #18746 added execution-admission storage: one admission row per attempt and one charge row per admitted project. > - Settlement deletes every charge row. The admission row keeps only the primary project and an opaque context revision. > - Thus, after settlement, the complete admitted project set is lost. > - A retained provider session still carries the references of its previous admission. Example: a task mentions A and B, the admission settles, and then a user removes B from the task text. The next admission must still count B, or start a fresh context. The primary project or the context token cannot supply B. > - This pull request keeps the complete admitted set on the admission row after settlement. Charge rows stay the only active capacity. > - The benefit is that later retained-session resolution can read the previous set. Settled history does not hold capacity or block project deletion. ## Linked Issues or Issue Description Refs #18746 (corrects its storage). Related: #2831, #14333. **Subsystem affected** Server execution admission (`server/src/modules/execution-admission/`) and the `packages/db` schema. **Problem or motivation** `settleAdmission` deletes all charges of an admission. No other storage records the complete project set. After settlement, the set cannot be recovered. Current task text is not a replacement, because it can change after admission. **Proposed solution** Add `execution_admissions.admitted_project_ids uuid[]`. It holds the sorted, distinct set. It is written once at admission and a trigger blocks changes. It has no project foreign key. `{}` is a known empty set. Null means unknown. Only settled rows can have null. Migration `0428` fills the set of unsettled `0426` rows from their charges, because those charges are still complete. Settled `0426` rows stay unknown. **Alternatives considered** - Keep charge rows after settlement with a `released_at` column and a partial active index. The project foreign key would then block project deletion permanently, or the key must be removed from active charges. One table would also hold two different facts. - A separate history table with one row per project. This is more rows, joins and constraints for data that is only read as a whole set by admission ID. - Encode the set in `context_revision`. That token is opaque and owned by the context owner. The database cannot validate it. - Use `heartbeat_runs.context_snapshot`. It is mutable and is per run, not per admission attempt. ## What Changed - Schema and migration `0428_retained_admitted_project_sets`: add `admitted_project_ids`, a check (null only when settled, no null elements, at most 100 entries) and an immutability trigger. The backfill fills unsettled rows from their charges and leaves settled rows null. The migration is safe to run again. Migration `0426` is not changed. - `recordAdmission` writes the set. A replay compares against the stored set. `settleAdmission` still deletes the whole charge set in one transaction and does not change the stored set. - Add `readAdmittedProjects` (company-scoped, by admission ID). It returns `known` with the set, or `unknown`. - Add a pure `retainedSessionProjects` rule: a retained session uses previous ∪ current. An unknown previous set returns `fresh_context_required`. No consumer uses it yet; C1b integrates retained sessions. - Update `doc/DATABASE.md` and `server/src/modules/README.md`. Not changed: no public cap writes, no claim or dispatch integration, no session integration, no provider activation, no `serialize` → cap 1 migration. Active counts still use only the indexed charge rows. ## Verification Red, on unchanged `master` code, with the new tests: - `server`: `npx vitest run src/modules/execution-admission/adapters/postgres.test.ts`. The new tests fail. With a temporary reader that uses the only storage `master` has (charges), the settled A+B admission reads `[]` instead of `[A, B]`. - `packages/db`: `npx vitest run src/execution-admission-migration.test.ts` fails, because no `0428` migration exists. Green, with this change: - `server`: `npx vitest run src/modules/execution-admission`: 27 passed. The new PostgreSQL test covers: A+B admission → settlement → set still readable, charges and counts zero, settlement replay keeps the set, a second attempt with a different set, primaryless and empty sets, company isolation, rollback, immutability, the check constraint, and deletion of a project that has only settled history (its ID stays in the set). It also checks that a project with an active charge still cannot be deleted. - `packages/db`: `npx vitest run src/execution-admission-migration.test.ts`: 2 passed. The upgrade test starts from the shipped `0426` schema with an unsettled A+B row, an unsettled empty row and a settled row. After the upgrade the sets are `[A, B]`, `{}` and null. A second run of `0428` changes nothing. - `pnpm --filter @paperclipai/db typecheck` and `check:migrations` passed. `node scripts/check-module-boundaries.mjs` passed. - Not run locally: the server layered typecheck. The local process was killed for memory (a 5.8 GiB limit shared with other jobs). CI runs it. This PR does not prove C2 claim races or C3 lifecycle authority. ## Risks - Migration: one `ADD COLUMN`, an `UPDATE` that only touches unsettled rows (none in production, because nothing writes admissions yet), a check and a trigger. It does not delete or rewrite data. - Settled rows created before `0428` stay unknown. A future consumer must start a fresh context for them. It must not treat them as empty or primary-only. - The stored set has no project foreign key. Company scope is checked at admission time, under the same project locks and the charge foreign keys. Size: about 70 production lines (schema 8, migration SQL 22, adapter 28, domain 15), 105 test lines and 24 doc lines. Generated metadata (`0428_snapshot.json`, `_journal.json`, removal of `0422_snapshot.json` by the standard pruning) is separate. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`) in Claude Code, with tool use (shell, file edits, embedded PostgreSQL tests). Default 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 - [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: nickyleach <331803+nickyleach@users.noreply.github.com> Co-authored-by: Paperclip <noreply@paperclip.ing> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
2a16dcf629 |
fix(workspaces): preserve shared directory trees during enrollment (#18835)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work.
> - A volume-free web server needs existing task files in durable object
storage.
> - Historical workspace records can share a physical folder or contain
other task folders.
> - A task-level migration hold cannot fence every alias, and capturing
a parent recursively duplicates nested task data.
> - Offline enrollment imports children first and verifies their durable
coverage before omitting them from a parent archive.
> - Snapshot retention preserves those child versions for as long as a
covering parent depends on them.
> - Existing files, task bindings, and archived states remain intact
through the storage migration.
## Linked Issues or Issue Description
Builds on merged #18826.
**What happened?**
The per-workspace importer could capture a shared project folder while
another workspace record still owned the same path. It also recursively
included runtime-created nested task workspaces in each parent import.
Packaged enrollment rejected existing workspace data instead of invoking
its migration owner.
**Expected behavior**
Require offline maintenance for overlapping paths. Import nested tasks
before their parents, verify every excluded directory against its saved
snapshot, preserve dependencies through retention, and resume
interrupted imports without replacing existing remote authority.
**Steps to reproduce**
Create a project workspace and another task workspace below its
`.paperclip/worktrees/` directory. Try importing either while ordinary
writers are running, then run offline enrollment. Interrupt a parent
import after upload and resume it.
## What Changed
- Add physical-overlap admission checks and an offline entry-point
guard, including global running and queued executions.
- Import pending records in leaf-first pages through the existing
migration owner.
- Verify nested snapshots by downloading, restoring, and comparing the
original source before and after parent upload; check transitive
dependencies.
- Publish parent dependency receipts under child snapshot row locks.
Retention pins those children and revisits them when the parent is
deleted.
- Index imported-child dependency lookups with a partial GIN index; a
PostgreSQL query-plan regression covers 2,500 unrelated snapshots.
- Propagate cancellation through capture and object storage operations
and preserve existing retry journals.
- Wire workspace migration into packaged maintenance and document its
preservation contract.
## Verification
- The original overlap and duplicate-parent regressions failed before
implementation.
- 73 migration and retention tests pass against real PostgreSQL and
files, including interrupted uploads, resumed batches, equal-path
aliases, changed or missing child data, nested descendants, and active
or queued executions.
- Six S3-compatible storage integration tests pass, including migration
through the maintenance owner and restoration of both parent and child
after deleting the original filesystem.
- The standalone process case was initially skipped because the worktree
had no bundle. After rebuilding the server and runtime bundle, that test
passed against PostgreSQL and S3: authenticated ready/export receipts,
preserved decision secret, blocked application routes, and clean SIGTERM
shutdown.
- Server typecheck, server build, runtime bundle build, and
module-boundary checks passed. Full repository CI passed on
|
||
|
|
fef86e32e9 |
feat(execution-admission): add project concurrency policy and admission storage (#18746)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Projects group tasks. Agents run tasks as heartbeat runs. > - Today, workspace mode and shared-workspace flags decide whether tasks run at the same time. A project cannot set its own limit on concurrent task executions. > - A correct project limit must count every project that a task uses, including @-mentioned projects. It must also keep the count when a project is archived or deleted, and when a run is admitted more than one time. > - This change cannot go into one pull request with claims and settlement. Thus this pull request adds only the policy semantics and the durable accounting storage that later claims will use. > - This pull request adds a new `execution-admission` module, three tables and the deletion guards. No route writes a cap and no claim path enforces it. > - The benefit is a tested storage contract (constraints, idempotency, rollback, indexed counts, lifecycle preservation) before any behavior changes. ## Linked Issues or Issue Description Refs #2831, Refs #14333 (related agent-level and adapter-level limits; not duplicates). **Subsystem affected** Server execution admission (new module `server/src/modules/execution-admission/`) and the `packages/db` schema. **Problem or motivation** A project cannot limit how many of its task executions are active. The current shared-workspace flags mix resource safety with capacity. A future project limit needs storage that records each admission attempt and every project that the attempt charges. Run IDs cannot be the identity, because a run can be released before dispatch and admitted again. **Proposed solution** Add a separate policy table that keeps "never configured" apart from "explicit no limit". Add admission epochs with one charge row per distinct project. Settlement deletes the charge set in one transaction. Later pull requests integrate claims, settlement, waits and the public control. **Alternatives considered** Columns on `projects` would leak an unenforced cap through every project reader. A project column or a `uuid[]` on `heartbeat_runs` cannot enforce per-project uniqueness or company scope. `budget_reservations` has no project set. A counter would be a second source of truth. ## What Changed - Added `project_task_execution_policies`. No row means never configured. A null maximum means an explicit "no limit". A check accepts only positive maximums. A composite foreign key keeps the project and company together. The row is deleted with its project. - Added `execution_admissions`. Each row is one admission attempt of a run: `(run_id, attempt)` is unique, and a partial unique index allows one unsettled attempt per run. It keeps the context revision, the nullable primary project (provenance, no foreign key) and the settlement identity. A check sets `settlement_id` and `settled_at` together. - Added `execution_admission_project_charges`. It has one row per distinct admitted project, unlimited projects included. Composite foreign keys keep the admission and the project in one company. The project foreign key has no cascade, so a charge cannot disappear with its project. `(company_id, project_id)` is indexed for active counts. - Added migration `0426_absent_guardian.sql` (renumbered from `0423`, then `0425`, as master added `0423`–`0425`). The generated SQL was reordered so the unique indexes exist before the composite foreign keys that need them. Snapshots `0419` and `0420` were pruned to keep five snapshots. - Added the `execution-admission` module (domain → application → adapters): - Domain: policy patch parsing and application (omitted keeps, null clears, partial updates keep other fields, malformed values are rejected), canonical project-set preparation (primary ∪ references, deduplicated, ascending, bounded with an explicit `too_many_projects` result), and the settlement replay decision. - Application: internal `readTaskExecutionPolicy` and `updateTaskExecutionPolicy` commands. No route, tool, CLI or import calls them. - Adapter: the project-set lock (`FOR NO KEY UPDATE`, ascending, one statement), the policy store, and the storage transitions `recordAdmission`, `settleAdmission` and `countActiveProjectCharges`. These transitions are not exported from the module entry point. Claims (later) must add the capacity check and the lifecycle evidence. - Project deletion returns `409 project_execution_admission_pending` while the project has a charge. Agent deletion returns `409 agent_execution_admission_pending` while its runs have an unsettled admission, and removes settled history. Company deletion calls a new module participant before it deletes runs and projects. - Documented the tables, the lifecycle rules and the proposed global lock order in `doc/DATABASE.md`. Added module ownership to `server/src/modules/README.md`. ## Verification - `cd server && npx vitest run src/modules/execution-admission` → 2 files, 26 tests pass (the Greptile finding on an empty primary ID added an assertion to an existing test). The adapter tests use embedded PostgreSQL and cover: - policy semantics, the maximum check and the company foreign key; - charges for every distinct project, a primaryless set and an empty set (no rows); - replay of one admission ID, conflict on a changed record, refusal of a second live attempt and the partial unique index; - settlement of the whole set once, replay, a different settlement, then attempt 2 for the same run; - company isolation of projects, runs and admission IDs, the duplicate-charge key, and rollback of an aborted transaction; - active counts per project, and an `EXPLAIN` that uses `execution_admission_project_charges_project_idx`; - archive keeps charges, `projectService.remove` returns 409, a direct project delete fails on the foreign key, and delete succeeds after settlement; - `deleteAgentDependencies` returns 409 while unsettled, and `deleteCompany` removes only that company's admissions. - `cd packages/db && npx vitest run src/execution-admission-migration.test.ts` → pass. It removes the new tables and receipt, seeds a legacy project and run, applies pending migrations, and compares the existing rows. - `pnpm --filter @paperclipai/db check:migrations` → pass. Five snapshots remain (`0421`, `0422`, `0424`, `0425`, `0426`; `0423` is SQL-only on master). - `node scripts/check-module-boundaries.mjs` → pass. - `packages/db` `tsc --noEmit` → pass. A scoped `tsc` of the module entry points and domain tests → pass. - Existing tests for the changed deletion services → 6 files, 161 tests pass (`resource-lifecycle-events`, `prompt-injection-prevention`, `agent-lifecycle/lifecycle`, `case-deletion`, `project-workspace-root-provenance`, `companies-service`). - Not run locally: the full `server` typecheck. `tsc` and `tsgo` were stopped by the out-of-memory killer on the 7 GB machine (exit 137, also single-threaded). The full `pnpm -r typecheck`, `pnpm test:run` and `pnpm build` were not run locally; CI ran them on head `520b69d553`: all gates pass, including Typecheck, Build and the 12 server test shards. After merging master (`1c9ca5829f`) into head `c088729361`, the migration check, migration upgrade test, module tests, module-boundary check and `packages/db` typecheck were rerun locally and pass; CI is rerunning the full suite. ## Risks - Migration risk is low. It creates three empty tables and indexes. It does not change existing tables or rows. - Behavior changes only when admission rows exist. Nothing writes them yet, so project, agent and company deletion behave as before. - Later claim work must take the project-set lock in the documented order. A claim that takes project locks after issue locks could deadlock with project deletion. - Settlement deletes charge rows, so a settled admission keeps its primary project and context revision but not its full project set. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`) in Claude Code, with tool use (shell, file edits, embedded PostgreSQL tests). Default 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 - [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: nickyleach <331803+nickyleach@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
|
ccc63ea458 |
fix(server): prove completed history before idle sleep (#18646)
Reviewed and verified at
|
||
|
|
5f1b59ba2c |
fix(db): preserve binary data in checkpoint backups (#18650)
Reviewed and verified at
|
||
|
|
b8f8b904fa |
feat(routines): preserve automation ancestry through webhooks (#18772)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Routines let external services start agent tasks through a normal webhook. > - A repair routine must recognize failures from work that it created. > - The receiver added in #18714 put Sentry payload parsing and repair policy in core routine intake. > - This pull request replaces that receiver with verified creation provenance on normal routine calls. > - Sentry can filter the resulting tags before it sends a normal routine webhook. > - The benefit is reusable provenance and loop prevention without a provider-specific route. ## Linked Issues or Issue Description **Problem or motivation** A Sentry repair alert can start another repair when its own work fails. The dedicated receiver in #18714 solved this through a Sentry-specific payload contract. Ordinary routine webhooks also need a trusted way to retain the automation that created their tasks. **Proposed solution** Use the existing public `/fire` route and its current authentication. An optional source run credential proves a live Paperclip caller. Store the caller separately from webhook JSON. Follow recorded task creation links and export bounded ancestry tags on run-failure events. Let Sentry alert filters exclude repair-origin work. **Alternatives considered** Keep the dedicated Sentry receiver, accept sender-supplied origin IDs, or filter only the immediate routine. The first couples core intake to one provider. The second accepts unverified claims. The third misses child tasks, standalone follow-ups, and calls through another routine. **Roadmap alignment** This improves the shipped Scheduled Routines and Activity log capabilities. It does not add a new scheduler or incident system. Related work: #18714 is the implementation replaced here; #9727 adds source-task navigation and UI attribution; #10206 proposes exception-led routine dispatch. This PR uses existing task creation records and does not implement either of those broader proposals. ## What Changed - Remove the Sentry-only route, parser, authentication exception, and payload-specific tests and documentation from #18714. - Add optional `X-Paperclip-Source-Run-Token` support to normal routine webhooks. Validate the standard run JWT, company, instance, agent, live run, and cancellation state. Authenticated routine API calls also retain their validated caller. - Add nullable `routine_runs.created_by_run_id`. Keep the reference after source-run deletion so missing history becomes unknown. Separate callers during coalescing, including legacy open tasks. - Follow company-scoped routine receipts, creator runs, task parents, and legacy creation activity. Mark missing, foreign, cyclic, private, or excessive ancestry for safe alert filtering. - Add event-local `automation_origin_status`, `automation_routine_ids`, and `task_shareable` tags through the existing Sentry SDK hook. Cap ancestry at 32 reads and share a 750 ms deadline across metadata and ancestry lookup. Keep the SDK one-second wait and eight-lookup concurrency bound. Redact the source credential from HTTP logs and Sentry request headers. - Preserve unexpected database failures as server errors without rejection bookkeeping in an aborted transaction. Document 409, 415, and 422 responses for the normal webhook. - Keep `routineScopedRunId` as generic opt-in `X-Paperclip-Idempotency-Scope: routine` support. Default trigger receipts and timestamped HMAC replay protection retain their current scope. - Add [the Sentry repair tutorial](https://github.com/paperclipai/paperclip/blob/codex/routine-creation-provenance/docs/guides/board-operator/sentry-run-failure-repairs.md), API documentation, OpenAPI headers, and capability-map updates. ## Verification At commit `0f4a0051528edd1367a921a76a8050c3fd087872`: - `pnpm -r typecheck` and `pnpm build` passed for the full workspace. Migration numbering and safety checks passed. - The focused service, route, Sentry, redaction, and OpenAPI suites passed 228 tests in five files. The database Sentry resolver and OpenAPI invocation passed 33 tests in two files. These are separate invocations with some overlap. - The full routine HTTP suite passed all 15 tests. The authenticated API case failed before the caller-wiring fix and passed after it. The fixture loads the real route-owned services. No test deadline was raised. - The real `@sentry/node@10.71.0` suite passed all 12 tests. It uses an in-memory transport and sends no events to Sentry. It covers terminal failures, saved-message interruptions, event isolation, and source-token redaction. - Regression tests also cover forged, expired, foreign, and cancelled caller credentials; payload spoofing; inherited routine ancestry; legacy coalescing; deleted history; private work; cycles; query and time limits; database error causes; and routine-wide receipts. The new error and budget cases failed before their fixes. - `node scripts/check-maps.mjs` and `git diff --check` passed. The map check covers 35 areas and 192 capabilities. - The full local Vitest command was stopped for a required master merge after four chat shards passed 1,278 tests. No full local pass is claimed. An earlier combined HTTP run hit an existing malformed-ID timeout whose original cause is not established; the final full HTTP suite passes. An extra local run of the unchanged heartbeat scheduling file passed 12 of 13 tests and failed a readiness-status assertion. Its logs also reported a missing fixture user membership. The same 13-test file now passes in current-head CI. - All GitHub Actions checks pass on this commit. Three jobs initially lost their self-hosted runners. One server shard initially failed while deleting a company in an unchanged heartbeat test because of a `company_skills` foreign key. One retry passed all affected jobs without code changes. That server shard passed all 499 tests, and all eight browser shards passed. - Fresh Greptile review is 5/5 on this commit. All six review threads are resolved. The separate Paperclip Review check remains queued. Its bot reports that work is paused. A maintainer must resume the assigned review work to complete that check. No full green check state is claimed while this external gate is pending. - Live Sentry configuration and deployment are outside this PR's acceptance evidence. The tutorial describes setup, a controlled failure, and a repair-origin exclusion check for operators to run after deployment. ## Risks - Normal app webhooks retain the sender's full JSON in company-readable routine history and the execution task. Alert filters and task visibility are not content scrubbers. Operators must scrub sensitive event data in Sentry before forwarding it and keep connection tests in setup mode. - Existing users of `/sentry-run-failures` must move to `/fire` and add the documented Sentry filters before enabling repair intake. The old route is removed. Stored tasks and receipts remain. - The additive migration uses `ADD COLUMN IF NOT EXISTS`. It does not rewrite historical tasks or infer missing provenance. - Unknown or unshareable ancestry is excluded by the tutorial's alert rule. Missing history, an enrichment timeout, or more than five routine IDs can therefore suppress a repair webhook while retaining the Sentry error. - An external webhook without a verified source credential starts a new receiving-routine origin. Its JSON cannot establish upstream Paperclip ancestry. Sentry must not store a Paperclip run credential. - Routine-wide receipt reuse needs a stable sender key. A static Sentry custom header is not a per-event key. The tutorial keeps delivery deduplication separate from loop prevention. > 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 OpenAI Codex, GPT-6 model family, with reasoning, repository access, code editing, and test execution. The session does not expose the exact model variant or context window size. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] 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> |