mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-01 15:35:12 +01:00
refactor(tools): isolate connection access and removal (#17167)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Connection grants and installations control which identities and agents can reach tools. > - Removal must close those access paths and stop runtimes before cleaning up owned secrets. > - These responsibilities still shared the setup coordinator after earlier extractions. > - This pull request gives grants, installations, credential validation and removal explicit owners. > - The benefit is a smaller coordinator and focused access-lifecycle modules with existing security ordering intact. ## Linked Issues or Issue Description Depends on #17165. Earlier stack: #17144 → #17146 → #17147 → #17155 → #17157 → #17159 → #17161 → #17162 → #17165 → this PR. **What existing behavior does this improve?** Maintenance of connection audiences, delegation, installation and removal. **Current behavior** The 6,419-line service still owns grant mutation, installation reach and resumable credential cleanup alongside app setup. **Proposed behavior** A grant owner manages audiences and delegation. An installation owner manages reach and its profile bindings. A removal owner keeps access revocation, runtime shutdown, secret classification and cleanup together. The coordinator delegates through typed APIs. **Reason and benefit** The entry point falls to 4,821 lines / 198,065 bytes, meeting the requested under-5,000-line target. Across the full stack it falls from 21,525 lines / 829,519 bytes. App setup remains together in the coordinator. ## What Changed - Extract credential validation (75 lines), grants (603), installations (350) and removal (658). - Preserve company boundaries, audience advisory locks, membership locks, assignment locks and audit order. - Keep the existing runtime supervisor and secret client. Retain live HTTP options for provider shutdown. - Keep Slack credential-mutation leasing and resumable owned-secret cleanup with removal. ## Verification - Removal, grant-resolution and the stronger real HTTP OAuth fixture pass before extraction (13 cases); the existing service and publication baselines also pass. - The original combined targeted run passed 525 cases across seven suites. After replaying onto master, 601 cases pass across nine suites, including Braintrust lifecycle and connector-policy coverage, with one database worker. - Full repository typecheck and build passed before merge integration. After integration, server typecheck, module-boundary and whitespace gates pass; fresh CI covers the full repository gates. - All 94 pre-existing function/method bodies match by normalized syntax comparison. A full-stack TypeScript symbol audit preserves all 18 exports and 89 service methods. - The earlier local full-suite run was interrupted after 79 minutes and remains unverified. All current-head CI gates pass on `d5307fcea2365e51a75d49db82deeed236d19951`, including the full sharded test matrix, browser suites, typecheck, build and canary dry run. Greptile is 5/5 with no open review threads after merge integration. - GitHub canceled duplicate CI run [39716841632](https://github.com/paperclipai/paperclip/actions/runs/39716841632) during the atomic stack update. Replacement run [39716843243](https://github.com/paperclipai/paperclip/actions/runs/39716843243) passed on the same head; no failing test was retried. ## Risks - Audience replacement must not erase an inactive member marker and widen authority. Its advisory and membership locks remain in the grant owner. - Uninstall must remove only its own profile bindings. Existing operator-authored bindings keep their current behavior. - Removal must remain fail-closed and retryable after provider failures. Its order, shared-secret retention, supervisor and cleanup references are unchanged and covered by removal tests. - No schema, public API, policy or scheduling change is intended. The first eight PRs have landed; this PR follows the updated catalog extraction. ## Model Used OpenAI Codex, GPT-6 family, with repository tools and code execution. The exact serving model ID and context-window size are not exposed in this session. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [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>
This commit is contained in:
1 parent
f7210cd31f
commit
4dfcb4c9b8
6 files changed
+1722
-1620
No files matched your search
@@ -208,3 +208,17 @@ boundaries pass. No new supervisor, background loop or runtime cache is created.
|
||||
- Separate guarded remote HTTP, descriptor risk policy, discovery, publication, default access, cached reads, health, attention and aggregator account synchronization.
|
||||
- `tool-access.ts`: 8,997 → 6,418 lines. New owners range from 39 to 492 lines. Remote options remain live; catalog flights and aggregator sync jobs retain per-service lifetime. Publication locks and transaction-scoped default access remain unchanged.
|
||||
- Before/after: 470 public service/HTTP integration cases plus 44 catalog-publication, HTTP rebinding and aggregator-sync cases pass. Server typecheck and module-boundary checks pass. All 157 previous function/method bodies match by normalized syntax comparison.
|
||||
|
||||
### Connection access and removal; final target
|
||||
|
||||
- Extract credential-reference validation, grant audiences/delegations, installations and removal. Each owner keeps its locks, audit writes and cleanup order. Removal uses the existing supervisor and secret client, retains the Slack mutation lease, and keeps access revocation with resumable secret cleanup.
|
||||
- `tool-access.ts`: 6,418 → **4,820 lines / 198,002 bytes**. The full stack started at 21,525 lines / 829,519 bytes. The four new modules have 75, 603, 350 and 658 lines.
|
||||
- Removal, grant resolution and the strengthened real HTTP OAuth journey passed before extraction (13 cases). The final combined public-path run passes 525 cases across seven suites. Full repository typecheck and build pass. Module-boundary and whitespace checks pass; all 94 previous function/method bodies match by normalized syntax comparison.
|
||||
- The earlier local full-suite attempt remains interrupted and unverified. PR descriptions retain that limitation and original CI failures alongside current-head CI outcomes.
|
||||
- The requested size target is met. App setup, finish/reconnect orchestration, local template definitions and examples remain in the coordinator; further extraction is outside this batch.
|
||||
|
||||
### Merge integration
|
||||
|
||||
- The first eight PRs landed on master. The catalog extraction was replayed after the Braintrust connector policy change in #17141. Risk classification and publication retain that policy in their extracted owners; setup retains its policy call.
|
||||
- The final entry point is 4,821 lines / 198,065 bytes. All 157 catalog and 94 connection-lifecycle method bodies match their updated bases by normalized syntax comparison.
|
||||
- The integrated stack passes 601 targeted cases across nine suites, including Braintrust lifecycle and connector-policy coverage. Server typecheck, module-boundary and whitespace checks pass. Fresh CI and reviews are required for both updated PR heads before merge.
|
||||
Reference in new issue
Block a user