mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-21 16:27:18 +02:00
4d92589799ab795bfbc317d9175e9eb58575aff5
5
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Nicky LeachandPaperclip
|
190080642e |
perf(tools): publish catalog refreshes in bounded chunks (#16602)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - MCP discovery publishes tools that setup and access profiles use. > - Large catalogs currently write one row at a time. > - Repeated page entries can produce duplicate results, and profile writes can partly commit. > - This pull request publishes tools in bounded chunks and makes profile synchronization atomic. > - The result supports large catalogs before connector runtime hooks are enabled. ## Linked Issues or Issue Description **What happened?** Catalog refresh performs one database write per tool. Repeated tool names across discovery pages appear more than once in the result. Default-profile synchronization uses separate statements, so failure can leave a profile without its binding. OAuth callbacks also acquire connection/application locks in the reverse order from publication and setup, allowing a concurrent refresh to deadlock. **Expected behavior** Publish at most 500 tools per catalog statement. Keep one result per tool name, using its last descriptor. Preserve existing IDs, first-seen timestamps, disabled actions, and unchanged review decisions. Commit or roll back default-profile changes together. OAuth persistence must acquire application then connection locks before secret or connection writes. **Steps to reproduce** 1. Refresh a synthetic MCP catalog with 2,500 tools. 2. Return the same tool on two discovery pages with different descriptions. 3. Fail the profile-binding insert during default-profile creation. 4. Overlap personal, organization, or managed OAuth persistence with setup finalization or discovery of a new tool. **Additional context** This is the first feature extracted from #16503. The separate test-only maintenance PRs #16601 and #16610 are merged. This PR now targets master and is the active review. Standing assignments #16603, tool definitions #16604, and access/credential hooks #16605 remain stacked drafts without review requests. ## What Changed - Deduplicate discovery results by tool name and keep the last descriptor. - Upsert catalog rows in transactions of at most 500 tools. - Preserve row identity and operator-disabled status on conflict. Determine default-access candidates from the rows read under each publication lock, so an overlapping refresh cannot restore an action turned Off after another refresh finishes. - Lock the application before the connection during publication, setup finalization, removal, and all three OAuth persistence paths. The OAuth ordering fix is included here so this PR is independently safe; it does not depend on #16605. - Synchronize the managed default profile, its entries, bindings, and managed policies in one transaction. - Add PostgreSQL publication tests and document partial-failure recovery. - Target the existing callback failure injection at profile creation instead of a transaction ordinal. ## Verification - `pnpm exec vitest run server/src/__tests__/tool-catalog-publication.test.ts server/src/__tests__/tool-access-service.test.ts`: 436 tests passed with the OAuth and overlapping-refresh corrections on top of the merged #16610 fix. - The new tests exercise real PostgreSQL with 250 and 2,500 tools, repeated discovery pages, preserved choices, lock contention, and failures between chunks and during profile creation. - Four selected regression cases fail with the production source restored to the parent commit. The implementation was then restored and rechecked. - Six additional real-PostgreSQL regressions cover personal, organization, and managed OAuth persistence against both setup finalization and new-tool discovery. All six reproduce PostgreSQL deadlock `40P01` with the OAuth ordering fix removed; all six pass with it restored. Discovery uses synthetic credential headers to isolate publication locking from credential rotation. - Apex initially returned 3/5 on `0d6013a9649388041975406fe4b0247cd0e695aa`, identifying an overlapping-refresh permission race. Two new PostgreSQL regressions cover both absent and quarantined tools; both prove an agent goes from deny back to allow without the correction, and remains denied with it. The test pauses one refresh after its initial catalog snapshot, completes another refresh, turns the action Off through the service, and resumes the stale refresh. - `pnpm --filter @paperclipai/server typecheck`: passed. - `node scripts/check-maps.mjs`: passed (structural validation only). - `git diff --check`: passed. - Reviewed the complete diff for secrets and personal data before push. - Earlier hosted run 38975428064 failed the maximum-size newline skill-audit watchdog; other interrupted jobs reported runner shutdown/cancellation. The original watchdog failure did not identify whether startup or auditing timed out, so its exact cause remains unknown. The separate merged #16610 isolates the audit deadline from module startup and preserves the audit hang guard. This branch is rebased onto that fix. - Full local build and repository-wide tests were not run. At the previous head `0d6013a9649388041975406fe4b0247cd0e695aa`, all 55 hosted checks succeeded (two optional Storybook checks skipped), and standard Greptile returned 5/5 before Apex identified the permission race above. At corrected head `dc120d2bc879c54c14347db30e74c753bcff61e6`, all 55 hosted checks succeeded (two optional Storybook checks skipped), and a fresh standard review returned 5/5 with no open findings. The second Apex review also returned 5/5 on that exact head (successful Apex check completed 2026-10-16T23:36:44Z). Both earlier findings are addressed, there are no open review threads, and the branch is mergeable against master. ## Risks Each successful catalog chunk remains committed if a later operation fails. A retry completes publication. Provider discovery occurs outside database locks. Default-profile changes commit separately from the catalog chunks and roll back together on failure. Tests use a controlled MCP response; they do not qualify a live provider. There is no schema migration or new plugin API in this PR. ## Model Used OpenAI Codex, GPT-6, with reasoning, repository inspection, code execution, and testing. The exact deployment 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 (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> |
||
|
|
3935e300dd |
fix(acpx): bound Claude MCP tool calls when runtime MCP servers attach (#16360)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Hosted `claude_local` runs use the ACPX engine. Paperclip gives each
run its runtime MCP servers (for example the `paperclip-assigned` tool
gateway) as ACP `mcpServers` entries.
> - Claude's default MCP tool timeout is far longer than a run. ACP
`mcpServers` entries have no per-server timeout field. So the agent
waits on an MCP call until the call answers.
> - Hosted error reports show runs that end with "Run exceeded the
adapter execution timeout (sandbox)". The stop-time diagnostics from
#15962 show the problem: the agent went silent about 50 seconds into the
run, three tool calls of ACP kind `other` stayed pending, and nothing
ended them until the 4h sandbox backstop.
> - The gateway bounds the remote provider call (10s default, 60s max).
But other steps on the gateway `tools/call` path have no deadline: DB
queries, row locks and pool acquisition, the credential steps before the
provider timer starts, and the stdio child exit wait. One stuck step
holds the agent turn for hours.
> - This pull request gives the agent's MCP client a deadline:
`MCP_TOOL_TIMEOUT` for ACP Claude runs that receive runtime MCP servers.
> - The benefit is that a stalled gateway call becomes an MCP error that
the model can see. The turn continues and does not spend the full 4h
execution budget.
## Linked Issues or Issue Description
- Refs #15962 (diagnostics that identified the silent turn with pending
tools)
- Refs #15448, #15456, #15850 (related gateway timeout and
connection-health work; this PR does not overlap with them)
**What happened?**
A hosted `claude_local` run sent parallel MCP tool calls. No call
answered. The run stayed silent for about 4 hours. Then the sandbox
execution timeout ended it with "Run exceeded the adapter execution
timeout (sandbox)".
**Expected behavior**
A stalled MCP call ends with an error after a bounded time. The agent
sees the error and continues.
**Steps to reproduce**
1. Start an ACP `claude_local` run that has runtime MCP servers.
2. Make the gateway `tools/call` request stall (for example, hold a lock
that the request needs).
3. See that the run stays silent until the adapter execution timeout.
**Paperclip version or commit**
`master` at
|
||
|
DottaandPaperclip
|
c46e41e81c |
fix(heartbeat): validate native MCP gateway ownership (#14914)
## Thinking Path > - Paperclip lets people manage agents and govern their tool access. > - Native runs receive an immutable MCP tool assignment for one agent. > - The gateway must enforce that owner when it authenticates a run token. > - Older gateway rows stored the owner only in metadata. > - This change validates the gateway and profile, binds new rows, and repairs valid older rows on reuse. > - It also delivers each native assignment once. > - Other agents cannot use the assignment, and explicit shared gateways keep their configured scope. ## Linked Issues or Issue Description Builds on #14012 by @busla (Jón Levy). That PR adds agent binding and seven regressions. This PR carries that fix onto current master and adds legacy authentication, profile validation, and duplicate-delivery coverage. Related: #14864 improves discovery memory use. **What happened?** Native gateway creation stored an owner in metadata but left `agentId` null. Authentication could therefore accept another agent's run token. Managed discovery could also deliver historical native assignments again. **Expected behavior** A native assignment accepts only its owner's run token. The gateway and profile must refer to the same immutable assignment. The current assignment enters the run configuration once. **Steps to reproduce** 1. Run the database fixtures in `heartbeat-runtime-mcp-servers.test.ts` on the baseline. 2. Create a native assignment and inspect its stored gateway owner. 3. Authenticate with another agent's run token, then inspect legacy reuse and managed delivery. 4. The baseline fails six ownership and delivery cases. The fix passes all twelve cases. **Paperclip version or commit** The red baseline is `f2e0f1963`. This PR is based on `cad26c6bf`, which includes the merged discovery fix. **Deployment mode** Native Paperclip Runner execution and managed Codex MCP delivery. Reproduction uses isolated database and HTTP fixtures. ## What Changed - Store the agent owner and agent context on new native gateways. - Validate profile and gateway assignment metadata before reuse or token creation. - Bind valid legacy rows with a company-scoped, null-owner update and validate the result. - Reject mismatched run tokens before legacy repair. - Identify native assignments by gateway metadata, the reserved profile key, or profile source. Reject missing or malformed provenance, including JSON null. - Exclude historical native assignments from managed gateway delivery. Keep their rows for existing runs. - Add twelve database and HTTP regressions and document the runtime contract. ## Verification - Red baseline: six regressions fail and four controls pass before the initial fix. Two additional regressions reproduce metadata-loss admission and a JSON-null TypeError before the review fix. - All twelve ownership regressions pass on the final code, including owner admission, cross-agent rejection, metadata loss, JSON-null HTTP 401, and explicit shared-gateway admission. Policy, listing-memory, and discovery HTTP coverage also passes. - Full workspace typecheck and build pass locally. Server typecheck and compilation pass again after the review fix. The final ownership and grant patches pass 42 combined database and HTTP regressions. - [Full CI](https://github.com/paperclipai/paperclip/actions/runs/37011383657) passes for `626a08ae66361cf586105877e24d806b1a7a9c20`: all 54 checks succeed; two optional Storybook checks skip. This includes full typecheck, build, all test shards, all eight E2E shards, runner verification, and the canary dry run. - Greptile scores that exact head 5/5. No review threads remain unresolved. ## Risks Invalid historical native gateway or profile metadata now rejects authentication. Valid unbound rows are repaired only when their owner reuses the assignment. Conflicting owners are never overwritten. Historical rows are retained for existing runs. Explicit shared gateways use ordinary profiles and keep their configured scopes. The reserved native profile namespace remains agent-owned even when gateway metadata is cleared. No schema or dependency changes are included. ## Model Used Original fix and seven regressions in #14012: Anthropic Claude Opus 5.5, `claude-opus-5-5`, 1M context, as reported by @busla. Extensions and verification: OpenAI Codex (GPT-6), with reasoning, repository inspection, code execution, and tests. This session does not expose the exact serving model identifier 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 - [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> |
||
|
Dotta
|
1ab159d3a7 |
feat(apps): consolidate connector management (#12684)
Completes the post-managed-OAuth connector lifecycle, Paperclip Cloud provisioning defaults, governed test flows, and consolidated Apps UI.\n\nCo-Authored-By: Paperclip <noreply@paperclip.ing> |
||
|
DottaandPaperclip
|
3db2e6bdd2 |
feat(mcp) [split 8/8]: add e2e coverage and operator docs (#9563)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Governed MCP access spans contracts, runtime enforcement, adapters, UI surfaces, and operator verification > - The parity reference PR #9534 is too large for effective automated or human review > - The feature therefore needs a linear stack whose individual diffs stay below the 100-file review limit > - This pull request is split 8/8 and focuses on end-to-end coverage, operator docs, evals, and release notes > - The benefit is a standalone, testable review boundary while preserving byte-for-byte parity at the top of the stack ## Linked Issues or Issue Description - Related parity reference: #9534 - Problem: The complete stack needs discoverable browser scenarios, operator guidance, threat modeling, eval coverage, and a parity proof before merge. - Proposed solution: Adds MCP user-story and Smoke Lab e2e suites, docs/evals/release notes, the skill update, and the root e2e driver script registration. - Alternatives considered: keeping #9534 as one 403-file review, or rewriting the feature to manufacture seams; both were rejected in favor of path extraction plus compile-driven boundary moves. - Roadmap alignment: this advances the existing governed MCP/tool-access work already represented by #9534; it does not introduce a separate roadmap initiative. - Stack position: base branch is `pap10341-split/07-ui-apps-activation`. - Merge policy: merge bottom-up, in order, only after the complete eight-PR stack has been reviewed and the top-of-stack parity gate remains empty. - Requested review: QA for flag audit and e2e/browser acceptance; Greptile on every PR. ## What Changed - Adds MCP user-story and Smoke Lab e2e suites, docs/evals/release notes, the skill update, and the root e2e driver script registration. - Keeps this PR below 100 changed files and independently typecheckable. - Preserves the final tree from #9534 when combined with the other seven stack levels. ## Verification - `pnpm typecheck` - `node --check scripts/e2e-mcp-user-stories.mjs` - `pnpm exec playwright test --config tests/e2e/playwright.config.ts --list` — 43 tests discovered - `git diff pap10341-split/08-e2e-docs 6b40e3876d9297105d4ec306e47e46d351c86172` — empty (0 bytes) ## Risks - Browser suites depend on runtime services and environment setup; this PR validates discovery locally while QA owns full flag-on/flag-off execution. - Stack risk: merging out of order can expose incomplete layers; mitigate by following the documented bottom-up merge policy. - Parity risk: later edits to an intermediate branch can drift from #9534; mitigate by re-running the empty top-of-stack diff before merge. > 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, exact model ID `gpt-5.4`; runtime-managed context window; medium reasoning with repository, shell, Git, GitHub CLI, and code-execution tools 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] Internal references are omitted except the execution-plan link explicitly required for this coordinated split stack - [x] My branch name describes the change and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] 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 ## Stack Coordination - Internal execution plan: [PAP-13874](/PAP/issues/PAP-13874#document-plan) - Parity reference: #9534 - Stack: #9556 → #9557 → #9558 → #9559 → #9560 → #9561 → #9562 → #9563 - Merge bottom-up only after full-stack review and an empty parity diff at #9563. --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |