mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-15 18:56:05 +01:00
65ebd96bcf591295084e9ee3d7ee417cea66f4b4
3
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Pavel-GuseynovandClaude Opus 5.5
|
4d297b998a |
fix(server): give remote MCP tool calls a 30 second default deadline and record stream failure evidence (#19768)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The tool gateway calls remote MCP servers over Streamable HTTP, and a server can answer `tools/call` with a `text/event-stream` body > - Such a server can send progress notifications before the response, and a slow tool can take more than 10 seconds > - The gateway aborts a remote MCP `tools/call` that uses the default deadline after 10 seconds, the general tool default > - When the call fails, the call evidence does not show how far the stream got, or whether the cause was the deadline or a broken transport > - This pull request gives remote MCP calls a default deadline of 30 seconds, the default response timeout of the remote HTTP client, and records the failure cause and the stream progress > - The benefit is that slow remote tools complete, and an operator can tell a deadline from a peer reset in the call evidence ## Linked Issues or Issue Description Refs #10147. That issue reports that a named MCP gateway aborts remote tools after 10 seconds and ignores the connection timeoutMs. This pull request raises the default deadline of a remote MCP `tools/call` to 30 seconds. It does not make the named gateway read the connection timeoutMs. Related: #11910 (open) keeps a connection callable after one timed-out tools/call. On current master a timeout and a JSON-RPC response error already keep the connection healthy, and this pull request keeps that policy. I searched pull requests and issues for "event stream tools/call", "SSE MCP tool call hang", "remote MCP timeout stream", "mcp text/event-stream" and "tool_timeout remote MCP". The only related open pull request is #11910. ## What Changed - `server/src/services/remote-http-fetch.ts`: the default response timeout is exported as `DEFAULT_REMOTE_HTTP_RESPONSE_TIMEOUT_MS` (30,000 ms, unchanged value). - `server/src/services/tool-gateway.ts`: - A remote MCP `tools/call` that uses the default deadline now gets `DEFAULT_REMOTE_HTTP_RESPONSE_TIMEOUT_MS` instead of the general tool default of 10 seconds. The Railway `run-command` budget and an explicit `timeoutMs` keep their values. - The call evidence records the response status, content type and request id before the body is read. A body that fails partway records the bytes received so far as its size. - A failed call records `failureKind` (`invocation_timeout`, `protocol_failure` or `transport_failure`), the deadline and whether it was aborted, the stream progress, and the error with its cause chain. Each string in the chain is redacted with `redactSensitiveText`, and a cyclic cause chain is cut with a "[Circular cause]" marker. - A deadline abort is a 504 `tool_timeout` and starts a fresh MCP session on the next call, as before. The connection health policy of master does not change. - A deliberate stop of a call (`elicitation_required`, `provider_interaction_required`, `oauth_insufficient_scope`, `oauth_refreshed_retry_required`) keeps its own reason when the deadline passes while it is recorded, as on master. An error that is already a gateway error also gets a failure kind: `tool_timeout` is `invocation_timeout`; a malformed, invalid or oversized response is `protocol_failure`; and `remote_http_*` (for example a DNS failure) and `mcp_remote_fetch_failed` are `transport_failure`. - A non-success response body is read as a stream, so a body that fails partway records the bytes that arrived. The reader stops at the size limit and cancels the body, as the `tools/call` reader does. #19785 makes the same reader change for the context path. - `server/src/services/mcp-http.ts`: `readMcpHttpResponse` takes an `onProgress` callback. It reports bytes, chunks and events received, whether the matching response arrived, and whether the stream ended. As on master, the read returns as soon as the response with the matching id arrives. - Tests: - `server/src/__tests__/mcp-http.test.ts`: a partial event followed by a peer error keeps the progress that was consumed, and releases the reader. - `server/src/__tests__/tool-gateway.test.ts`: six scenarios run against a remote server that streams one progress event first: slow success after 12 seconds, the default deadline, an explicit 1,500 ms deadline, a peer reset, a cyclic error cause, and a primitive error value. - `server/src/__tests__/tool-gateway.test.ts`: three regression tests: a tool question sent after the deadline passed keeps `elicitation_required`; a DNS failure records `transport_failure`; and a 503 body that fails partway records the bytes that arrived. All three fail on |
||
|
Dotta
|
d387cc0ff0 |
feat(connections): add managed external MCP connectors (#12346)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Connection intents need secure provider implementations to complete setup. > - Some providers use managed OAuth or external credential brokers. > - Those tokens must stay out of durable Paperclip state and fail closed when refresh fails. > - This pull request adds managed connector backends and the required storage contract. > - The benefit is safer provider setup with governed credential lifecycles. ## Linked Issues or Issue Description Refs #11965 This is stack 8 of 11. It depends on stack 7 and replaces another reviewable part of #11965. ## What Changed - Add managed Google Workspace and external connector backends. - Add Vercel Connect support without storing provider bearer tokens. - Add replay-safe migration 0232 and its generated snapshot. - Fail closed and clear stale token bindings when organization OAuth refresh needs reauthorization. ## Verification - `pnpm --filter @paperclipai/server typecheck` - `pnpm --filter @paperclipai/server exec vitest run src/__tests__/tool-access-service.test.ts` - Result: 194 tests passed. - `pnpm --filter @paperclipai/db check:migrations` - `pnpm build` - `pnpm exec vitest run --project @paperclipai/server server/src/services/remote-url-credentials.test.ts` (5 passed, including URL userinfo vault extraction) ## Risks - Broker metadata errors can block provider setup. - OAuth refresh failure disables the shared organization connection until reauthorization. - Migration 0232 is generated, ordered after 0231, and safe to replay. > I checked `ROADMAP.md`. This stack continues the existing app connection work from #11965 and does not duplicate another planned item. ## Model Used OpenAI Codex, GPT-5. The runtime model ID and context window were not exposed. The model used reasoning, 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 linked the public source pull request with `Refs #` - [x] I have not referenced internal or instance-local Paperclip issues or links - [x] My branch name describes the change and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge |
||
|
DottaandPaperclip
|
931eec3fbf |
feat(mcp) [split 4/8]: wire gateway runtime and Smoke Lab (#9559)
## 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 4/8 and focuses on gateway runtime, Smoke Lab, plugins, and server wiring > - 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 policy core needs runtime execution, endpoint guards, route registration, heartbeat integration, and adapter MCP injection to become operational. - Proposed solution: Adds the remaining server routes/wiring/consumers, runtime tests, adapter-utils MCP contracts, and Claude/Codex injection implementations required by the server layer. - 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/03-server-tool-access`. - 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: SecurityEngineer for gateway, endpoint guard, token issuance, and runtime wiring; Greptile on every PR. ## What Changed - Adds the remaining server routes/wiring/consumers, runtime tests, adapter-utils MCP contracts, and Claude/Codex injection implementations required by the server layer. - 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` - Changed server test set — 26 files, 382 tests passed - Affected server adapter tests — 38 tests passed after concrete adapter boundary move - Adapter-utils and Codex focused tests — 76 tests passed ## Risks - Remote endpoint validation, token handling, and runtime supervision are security-sensitive and can fail closed or deny legitimate access if misconfigured. - 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> |