Commit Graph
215 Commits
Author SHA1 Message Date
can1357 e8502806ff fix(mcp): retry smithery poll timeouts until the authorization deadline
A single hung/slow poll now aborts with TimeoutError after 30s; without
this, that one timeout escaped #waitForSmitheryCliApiKey and dropped the
browser login to the manual API-key fallback instead of retrying until
the 5-minute deadline. Catch isTimeoutError in the poll loop and
continue; export SmitheryCliPollResponse to type the retried response.
2026-07-23 22:15:21 +02:00
can1357 db2601eaff Merge PR #6425: fix(mcp): bound oauth discovery and smithery poll fetches with abort timeouts (@roboomp) 2026-07-23 22:15:21 +02:00
roboomp af3883c052 fix(mcp): bound oauth discovery and smithery poll fetches with abort timeouts
MCP OAuth endpoint discovery ran metadata, well-known, and recursive
authorization-server fetches with no AbortSignal, and the Smithery
browser-login poll received a never-aborting signal. An endpoint that
accepts the TCP connection but never responds stalled /mcp add,
/mcp reauth, the add wizard, or /mcp smithery-login indefinitely; the
5-minute login/poll deadlines run only after discovery resolves or
between polls, so a hung fetch never reached them.

- discoverOAuthEndpoints and fetchResourceMetadataScopes gain an optional
  signal and wrap every fetch in withTimeoutSignal(DISCOVERY_FETCH_TIMEOUT_MS,
  opts?.signal), threaded through the recursive authorization_servers call.
- pollSmitheryCliAuthSession wraps its fetch in
  withTimeoutSignal(SMITHERY_POLL_TIMEOUT_MS, signal) so a hung poll aborts
  and the loop reaches its 5-minute deadline.

Fixes #4103
2026-07-23 19:45:41 +00:00
roboomp 15577406f8 fix(mcp): serialize mcp.json config writes and use unique temp path
Every exported read-modify-write on mcp.json (add/update/remove server, disabled/force-enabled lists) now runs under a per-file withFileLock, so overlapping in-process or cross-process mutations no longer lose updates. writeMCPConfigFile writes to a pid+uuid temp file instead of a shared ${filePath}.tmp, so concurrent writers cannot rename each other's temp out from under them (ENOENT / clobber).

Fixes #4104
2026-07-23 19:40:13 +00:00
can1357 80ec1cb6ae Merge PR #6347: feat: optionally render MCP results as Markdown (@zeroknots) 2026-07-23 11:53:05 +02:00
zeroknots dd510f2ccc feat(coding-agent): render MCP Markdown results 2026-07-23 13:26:00 +07:00
slee1996 2145ab8f8e fix(coding-agent): retry MCP tool auth challenges 2026-07-22 23:44:38 -06:00
roboomp 04179bf0b8 fix(mcp): route task proxies through source tool and mark tools non-strict
MCP-backed tools never declared an explicit strict value, so OpenAI-family
serializers (post-#4336/#4340) had no false to preserve and models over-filled
mutually exclusive optional fields. Task/subagent proxies also rebuilt a raw
tools/call instead of executing through the source MCPTool, bypassing intent
stripping, placeholder pruning, local-URL resolution, reconnect, abort, and
result metadata; strict servers rejected proxied calls with
unrecognized_keys ["i"].

- MCPTool/DeferredMCPTool now declare `readonly strict = false as const`.
- createMCPProxyTools delegates to the current source tool, re-resolved by raw
  MCP server/tool metadata so reconnect replacements are honored, and keeps the
  Task 60s timeout by combining its abort signal with the caller's.
- Regression coverage: strict flags, proxy parity for i/placeholder shaping,
  declared-i passthrough, and reconnect re-resolution.

Fixes #6208
2026-07-21 22:37:12 +00:00
Mathews-Tom f0d31bcf56 chore: merge upstream/main (mechanical, merge-tree verified clean) 2026-07-18 04:29:23 +05:30
can1357 357d683007 Merge PR #5894: fix(mcp): process-group kill and SIGKILL escalate on stdio transport close (@Mathews-Tom) 2026-07-17 21:22:06 +02:00
Mathews-Tom 22f3701d92 test(mcp): distinguish zombie grandchildren from live ones in kill checks
kill(pid, 0) succeeds for a zombie too: a grandchild whose parent (the
killed leader) is gone sits as <defunct> until whatever reaps orphans
gets around to it, which can lag on some hosts. processExists() now
reads the process's ps state and treats a zombie as already reaped
instead of still alive, so the group-kill regression tests assert what
they actually claim to test.
2026-07-18 00:47:14 +05:30
Mathews-Tom 224796b13a fix(mcp): sweep group SIGKILL after a cooperative detached leader exit
terminateStdioProcess() treated a detached leader's cooperative SIGTERM
exit as proof the whole process group was gone, so close() skipped the
group SIGKILL and left a SIGTERM-trapping/ignoring grandchild running as
an orphan — exactly the process tree this change set out to reap. A
detached transport now always sweeps the group SIGKILL after SIGTERM,
even when the leader itself already exited.

waitForProcessExit() also left its losing Bun.sleep() timer running
after Promise.race settled from the other side, holding the event loop
open for up to the full grace window on every close(). It now uses a
cancellable setTimeout cleared in a finally block.

Adds a regression test spawning a non-trapping leader with a
SIGTERM-trapping grandchild to cover the gap the first fix closes.
2026-07-18 00:02:21 +05:30
Mathews-Tom c30c3ab0fb fix(mcp): process-group kill and SIGKILL escalate on stdio transport close
Before: StdioTransport.close() did a bare `this.#process.kill()` — a single
direct SIGTERM to the immediate child, with no wait and no escalation. On
Linux (and other non-Windows/non-macOS POSIX hosts), the MCP server is
spawned detached (setsid, its own session leader) so terminal job-control
signals can't stop it. A detached server that traps/ignores SIGTERM — or a
grandchild it spawns inside that session — survived omp process exit and
was orphaned, re-parented to PID 1.

After: close() runs a bounded, idempotent teardown:
  1. End stdin first (cooperative EOF) so a well-behaved server can exit on
     its own before any signal is sent.
  2. Send SIGTERM: to the whole process group (negative-pid `process.kill`)
     when this transport actually spawned detached on a POSIX host, else to
     the direct child only. A negative-pid signal is never attempted for a
     non-detached transport, since it could hit an unrelated group. ESRCH
     from the group signal means the group is already gone (treated as
     success); any other group-signal failure falls back to a direct-child
     signal.
  3. Wait up to ~1s for the direct child to exit; if it hasn't, escalate to
     SIGKILL (group-or-direct, same rule as step 2) and wait a further
     bounded ~0.5s before returning. Total worst case (~1.5s) stays well
     inside the ~3s MCP disconnect-all budget in agent-session.ts dispose().

`#process` is captured into a local and nulled before the first `await`, so
repeat/concurrent close() calls see it already cleared and skip re-signaling
— idempotent per the existing contract documented above close().

Extracted the signal/escalate logic into an exported `terminateStdioProcess`
(plus a `KillableSubprocess` structural type, decoupled from the stdio pipe
generics) so tests can drive group-signal escalation with an explicit
`detached` flag — `StdioTransport.connect()` ties `detached` to the host's
real `process.platform` via `resolveStdioSpawnCommand()`, so a POSIX
detached session can't be reproduced end-to-end through `connect()` on a
non-Linux dev/CI host, but a real detached process group can still be
spawned directly on any POSIX host to exercise it.

Tests added to stdio.test.ts: detached child trapping SIGTERM escalates to
SIGKILL; a detached parent's SIGTERM-trapping grandchild is only reached by
the group SIGKILL (proves group, not direct-child-only, signaling); a
well-behaved child closes promptly without escalating; a non-detached
transport never attempts a group signal. Extended
test/mcp-stdio-transport.test.ts's existing close() idempotency coverage
with a case where the first close() had to run the full escalation path.

Fixes #5578.
2026-07-17 23:22:06 +05:30
roboomp 189ac462cc fix(mcp): narrowed failed DCR block to unapproved clients
Only a 403 response that identifies unapproved_client now blocks generateAuthUrl before its clientless probe. Invalid metadata, invalid redirect, generic forbidden, retryable, and server failures preserve the probe path.

Consolidated fallback coverage across 400, 403, 429, and 503 responses.

Fixes #5852
2026-07-17 14:56:56 +00:00
roboomp 490686602c fix(mcp): excluded retryable 4xx DCR statuses from reauth block
Added #isDefinitiveRegistrationRejection so only non-retryable 4xx DCR errors block generateAuthUrl; 408/425/429 fall through to the clientless authorization probe alongside transport and 5xx failures.

Fixes #5852
2026-07-17 14:49:51 +00:00
roboomp 7faa6eb2dd fix(mcp): scoped reauth block to definitive DCR rejections
Only a 4xx DCR client error blocks generateAuthUrl; transport (status 0) and 5xx failures fall through to the clientless authorization probe so providers accepting client-less authorization keep working.

Fixes #5852
2026-07-17 14:45:56 +00:00
roboomp 33f2f00d58 fix(mcp): blocked reauth after failed client registration
Surfaced rejected dynamic client registration from generateAuthUrl before probing or returning an authorization URL without client_id.

Added coverage for a Cropwise-style 403 unapproved_client response.

Fixes #5852
2026-07-17 14:39:15 +00:00
can1357 3a4220a16a merge PR #5699 via eval/pr-5699: fix(mcp): escape Windows cmd shim command path too 2026-07-17 04:39:14 +02:00
roboomp e509fc3cbc fix(mcp): escape Windows cmd shim command path too
Applied the same cmd.exe percent/quote neutralization to the resolved command token so a % in the shim path is not expanded before launch.

Fixes #5696
2026-07-16 13:49:14 +00:00
roboomp 0788933110 fix(mcp): escape Windows cmd shim args against injection
Building the cmd.exe /c command line and spawning with windowsVerbatimArguments so cmd.exe expansion cannot eat or inject on %VAR%, quote, and metacharacter args (BatBadBut / CVE-2024-24576).

Fixes #5696
2026-07-16 13:39:42 +00:00
roboomp 967befdf42 fix(mcp): corrected Windows cmd shim spawning
Passed batch commands and arguments separately to cmd.exe /c so cmd.exe no longer strips a wrapper quote into the command token.

Fixes #5696
2026-07-16 12:58:05 +00:00
Kormákur 4a510a912f fix(mcp): match tool ownership by server name, not tool-name prefix
MCPManager evicted a server's tools by matching the raw mcp__<name>_
prefix against sanitized tool names. One server's sanitized name can
prefix another's (atlassian vs imported atlassian:atlassian), so every
reconnect of the shorter-named server dropped the sibling's tools and
re-announced them moments later, spamming paired xd:// unmount/mount
notices on each transport flap. Names containing sanitized characters
never prefix-matched at all, leaving stale tools registered after
disconnect. Replacement and removal now match mcpServerName.
2026-07-16 10:47:56 +00:00
can1357 ebe79d6f53 fix(mcp): retain DCR metadata fallback 2026-07-14 22:58:47 +02:00
can1357 80f9329e31 Merge PR #5456: fix(mcp): preserve discovered registration endpoint (@roboomp) 2026-07-14 22:58:47 +02:00
roboomp fb98465923 fix(mcp): preserved discovered registration endpoint
Threaded the authorization server's advertised registration endpoint through OAuth discovery, add, reauth, and the client flow instead of deriving metadata from the authorization endpoint.

Added pathful-issuer discovery and end-to-end DCR regression coverage.

Fixes #5267
2026-07-14 17:46:24 +00:00
can1357 4b5c32a092 Merge PR #4950: fix(mcp): resolve local image paths for tool calls (@roboomp) 2026-07-14 18:45:22 +02:00
can1357 4c3df0ee39 Merge remote-tracking branch 'origin/farm/903c642e/fix-stdio-tcc-spawn' 2026-07-11 07:38:17 +02:00
can1357 1758a6c749 Merge remote-tracking branch 'origin/farm/6d5f8413/fix-mcp-oauth-refresh-race' 2026-07-11 00:10:43 +02:00
roboomp b60dc669ea fix(auth): fenced oauth refresh writes
- Fenced final OAuth refresh update and terminal-disable CAS statements by row id, serialized credential data, active lease owner, and unexpired lease time.
- Passed an AbortSignal through MCP OAuth token refresh and bounded owned refresh operations below the lease TTL while awaiting the aborted fetch to settle.
- Added regressions for stolen-lease update/disable attempts and timed-out MCP token fetch abort behavior.

Fixes #5081
2026-07-10 21:49:18 +00:00
roboomp cf021ad393 fix(auth): serialized mcp oauth refreshes
- Added durable SQLite refresh ownership for stored OAuth rows, with canonical re-read before refresh and compare-and-set persistence.
- Routed MCP proactive and forced OAuth refresh through the shared owner so waiters reuse the winner's rotated credential.
- Added MCP regression tests for shared SQLite refresh ownership and stale invalid_grant losers.

Fixes #5081
2026-07-10 20:25:49 +00:00
Victor Araújo bce6aa89a4 fix(mcp): included OAuth scopes in dynamic client registration
Clerk and similar providers bind DCR clients to only the scopes declared at
registration. Authorize then requests scopes_supported (including openid),
which rejects with "client is not allowed to request scope 'openid'". Match
Claude Code by sending config.scopes as RFC 7591 scope on the DCR body.
2026-07-10 15:08:28 -03:00
roboomp a0a6949a4a fix(mcp): matched stdio spawn overload for tcc
Switched stdio MCP server launches to the argv-first Bun.spawn overload used by JS eval so macOS TCC Apple Events prompts reach children like xcrun mcpbridge.

Added regression coverage for the spawn call shape.

Fixes #5085
2026-07-10 15:04:57 +00:00
roboomp bf64806474 fix(mcp): kept macos stdio servers attached
Left Darwin stdio MCP server launches in the inherited session so macOS TCC can prompt for Apple Events permissions used by xcrun mcpbridge.

Added resolver coverage for Darwin while preserving Linux detach and Windows console behavior.

Fixes #4987
2026-07-09 21:39:42 +00:00
roboomp b097019fef fix(mcp): resolved local image paths for tool calls
- Resolved session '/data/workspaces/can1357__oh-my-pi__4946/.omp-session/2026-07-09T16-06-43-993Z_019f47a1-a619-7000-9062-5f5d863afa45/local' file arguments before forwarding MCP tools/call requests to external servers.
- Threaded local protocol options into custom MCP tool context and added focused coverage for image_path attachments.

Fixes #4946
2026-07-09 16:38:24 +00:00
roboomp 7049966def fix(mcp): hydrate JSON-body OAuth scopes from resource metadata
When the error body already advertises OAuth endpoints, `/mcp add` and `/mcp reauth` use `authResult.oauth` directly and skip `discoverOAuthEndpoints`, so scopes advertised only in the RFC 9728 protected-resource metadata document never reach the grant.

Add exported `fetchResourceMetadataScopes(url, opts?)` that fetches the metadata doc and returns `scopes_supported` / `scopes` / `scope`. Hoist the shared `readMetadataScopes` reader out of `discoverOAuthEndpoints`. At all three call sites (wizard, `/mcp add`, `/mcp reauth`), when `oauth` is populated from the JSON body but `oauth.scopes` is empty and `authResult.resourceMetadataUrl` was advertised, fetch the metadata and merge scopes onto `oauth`.

Regression tests cover the resource-metadata fetch and its failure/empty-doc paths.

Refs #4467
2026-07-03 23:27:18 +00:00
roboomp bef1f5d773 fix(mcp): merge challenge scopes into JSON-body oauth endpoints
`/mcp reauth` and `/mcp add` consume `authResult.oauth` directly when the JSON error body carries endpoints, skipping `discoverOAuthEndpoints`. Leaving `oauth.scopes` empty there meant a challenge-only `scope="…"` still yielded a scope-less grant even though `AuthDetectionResult.scopes` recorded it.

Merge `challengeScopes` into the returned `OAuthEndpoints` inside `analyzeAuthError` so every consumer sees the same scope. Added a regression test where the JSON body advertises endpoints without `scopes` and the challenge is the only source.

Refs #4467
2026-07-03 16:15:01 +00:00
roboomp debce0757b fix(mcp): carry OAuth scopes from challenge and resource metadata
MCP servers such as JIT gateways advertise required scopes via the RFC 6750 `WWW-Authenticate` challenge (`scope="..."`) and via RFC 9728 protected-resource metadata (`scopes_supported` / `scopes` / `scope`), then reject follow-up requests with `insufficient_scope` when a bearer token was issued without them. OMP's discovery only picked up `scopes_supported` from the auth-server metadata document, so `/mcp reauth`, `/mcp add`, and the MCP add wizard silently minted scope-less tokens.

- Extract `scope`/`scopes` from the WWW-Authenticate challenge into a new `AuthDetectionResult.scopes` field via `extractOAuthChallengeScopes`.

- Thread a `protectedScopes` option through `discoverOAuthEndpoints` and its recursion; capture `scopes_supported`/`scopes`/`scope` off resource-metadata documents; use those scopes when the auth-server metadata omits them.

- Pass `authResult.scopes` from `analyzeAuthError` into every discovery call site (`/mcp reauth`, `/mcp add`, MCP add wizard).

- Add regression tests for insufficient_scope + resource_metadata, resource-metadata `scopes_supported` passthrough, and challenge-scope threading.

Fixes #4467
2026-07-03 16:10:21 +00:00
can1357 3315ef0007 fix(network): bounded smithery and startup fetches 2026-07-02 23:51:18 +02:00
can1357 8b3d0a7190 Merge remote-tracking branch 'origin/farm/1f41837c/timeout-bare-fetches' 2026-07-02 23:43:11 +02:00
roboomp 7d1ab692a4 fix(mcp): explain missing client_id when DCR was rejected
MCPOAuthFlow silently swallowed a rejected dynamic client-registration
response and then threw the opaque "OAuth provider requires client_id"
error from the fallback authorize probe. Users adding Figma's MCP server
hit this because Figma's DCR endpoint 403s every unlisted client (per
the Figma MCP catalog gate) and the surfaced string gave them nothing
actionable to do.

Capture the DCR endpoint + HTTP status (or the thrown transport error)
on the flow instance and, when the authorize probe confirms client_id
is required, throw an error that names the endpoint, the outcome, and
the manual oauth.clientId/oauth.clientSecret workaround. The no-DCR-
endpoint branch gets its own message so users know the server never
advertised registration in the first place.

Fixes #4307
2026-07-02 11:58:03 +00:00
roboomp 76c480646e fix(cli): added fetch timeouts
Added timeout-backed AbortSignals to update, Hindsight, and Smithery fetch calls so stalled endpoints abort instead of hanging indefinitely.

Added regression coverage for the timeout signals on the exposed command/client paths.

Fixes #4229
2026-07-02 08:39:21 +00:00
can1357 99346570ba Merge remote-tracking branch 'origin/farm/4b964ee9/mcp-http-body-timeout' 2026-07-01 04:46:20 +02:00
roboomp 5d2f9ae5a3 fix(mcp): kept http timeouts active through body reads
- Moved Streamable HTTP request and notify timeout cleanup after response body consumption.\n- Added regression coverage for stalled request JSON bodies and stalled notify error bodies.\n\nFixes #3974
2026-07-01 02:43:00 +00:00
roboomp 822314dea5 fix(mcp): attach stdin.write rejection handler before flush()
In the request() send path, stdin.write() could return a pending thenable while stdin.flush() threw synchronously. Control jumped to the outer catch before wrote.then(undefined, failFromSend) was ever installed, so a later async EPIPE on the write would fire as an unhandled rejection — reintroducing the very orphan path the parent fix removes.

Attach the rejection handler on the write result immediately after dispatching it, before calling flush(). If flush() then throws synchronously, wrote is already covered; if write() itself throws, flush() was never called and there is no flushed thenable to orphan.
2026-07-01 01:16:19 +00:00
roboomp 797bbe667e style: bun run fix 2026-07-01 01:10:12 +00:00
roboomp 0e90d57be6 fix(mcp): route stdio write/flush failures without parking request()
StdioTransport.request() awaited stdin.write() and stdin.flush() before returning the internal deferred promise. When the child stopped draining stdin (wedged process, or full OS pipe buffer with no reader), Bun's FileSink returned a pending Promise that never settled — the async function got stuck above 'return promise', past the timeout timer and the abort handler. cleanup() + reject() still ran on the inner deferred, but the outer async-function promise never adopted it, so the caller's await hung forever and the deferred rejection surfaced as an unhandled promise rejection.

Send the frame without awaiting: sync EPIPE throws (Windows) still reject the request immediately; async EPIPE rejections (POSIX processTicksAndRejections) are wired to the same reject() via a guarded failFromSend handler that no-ops after cleanup(). The returned promise now settles from the response, the timer, the abort signal, or the read loop's transport-close broadcast.

Regression test spawns 'sleep 60' (POSIX only), sends a 1MB tools/call payload past the pipe buffer, and asserts the deferred rejects with the timeout error before the outer window elapses and produces no orphaned unhandled rejections.

Fixes #3945
2026-07-01 01:08:56 +00:00
can1357 224000ea1a Merge remote-tracking branch 'origin/farm/bfaba2dc/fix-mcp-oauth-escape-cancel' 2026-06-30 16:18:39 +02:00
roboomp 3106a15f7d fix(mcp): raced oauth login against cancellation
Esc and wizard abort signals now race the MCP OAuth login promise directly, so cancellation wins even before OAuthCallbackFlow reaches its callback wait and registers an abort listener. OAuthCallbackFlow also checks pre-aborted signals before opening/waiting on the callback server and its wait path handles already-aborted signals.

Threaded the abort signal into MCP OAuth fetches so dynamic client registration, metadata discovery, authorization probes, and token exchange unblock promptly when the user cancels.

Added a regression test where MCPOAuthFlow.login never observes ctrl.signal, matching the pre-wait race called out in review.

Fixes #3888
2026-06-30 10:21:26 +00:00
roboomp 6f65772093 fix(mcp/oauth): preserved random-port fallback for fresh dynamic-client-registration flows
PR review pointed out that the unconditional opt-out blocks the safe case: when no static `client_id` is set, `MCPOAuthFlow.#tryRegisterClient` does DCR with whichever loopback URI we actually bound, so the provider issues a `client_id` tied to the fallback port and the authorize request is accepted. First-install users whose default port 3000 is busy could no longer authenticate.

Gate `allowPortFallback` on `staticClientIdFromConfig(config) === undefined`: pinned client ids (config-supplied or embedded in the authorization URL) keep the strict-port behavior that fixes #3887; unresolved client ids fall back as before. Factored the static client-id resolution into a module-level helper so `MCPOAuthFlow.#resolveClientId` and `resolveCallbackOptions` share the same logic.

Added an oauth-flow.test.ts case that wires a mock registration endpoint + occupies the preferred port and asserts the fallback URI is what DCR registers and what the authorize request advertises. Tightened the existing strict-port test's title to call out the static-clientId trigger.
2026-06-30 10:13:38 +00:00
roboomp d2c767507b fix(mcp/oauth): failed fast when callback port is busy instead of advertising a random one
When the MCP OAuth callback server's preferred port (default 3000) was unavailable, `OAuthCallbackFlow.#startCallbackServer` silently bound a random port and forwarded the mismatched `redirect_uri` to the authorization server. Providers that validate redirect URIs against a registered callback (e.g. Atlassian) returned an opaque HTTP 500, leaving the local flow waiting for a callback that never arrived until the 5-minute timeout fired.

Added `OAuthCallbackFlowOptions.allowPortFallback` (default `true`, preserving every existing AI-provider flow) and threaded `allowPortFallback: false` through `MCPOAuthFlow`'s `resolveCallbackOptions`. With fallback disabled, login now throws a `ConfigurationError` that names the busy port and the remediation (free the port, or set `oauth.callbackPort`/`oauth.redirectUri` in `mcp.json`) before opening the browser. The existing `oauth.redirectUri`-strict path is reworded along the same lines so callers see one consistent message family.

Fixes #3887
2026-06-30 10:05:24 +00:00