mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-04 19:28:46 +01:00
ce332ecec0
## 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>