Commit Graph

198 Commits

Author SHA1 Message Date
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
roboomp e34f2a81e9 fix(tui): force-enabled MCP from tool-owned sources via enabledServers
Codex review on #3829: when an MCP server lives in a non-writable
source config such as opencode.json with enabled:false, the dashboard
re-enable had nowhere to write to — the writable mcp.json fallback
did not own the server, so setMcpServerEnabled fell through to the
denylist and the source's enabled:false kept the row disabled.

Added a parallel allowlist to the user-level mcp.json that overrides
a non-writable source's enabled:false flag without ever mutating the
foreign config:

- types + schema: new enabledServers array (mirrors disabledServers).
- config-writer: readEnabledServers + setServerForceEnabled helpers,
  and setMcpServerEnabled now writes to enabledServers on enable
  when no writable mcp.json owns the server, clears it whenever a
  writable source becomes the source of truth, and always clears the
  override on disable so a force-enabled server can be turned off.
- mcp/config (runtime loader) and state-manager (dashboard read):
  honor enabledServers as an override on enabled:false, while still
  letting disabledServers win.
- Added a regression test that walks the full lifecycle for an
  opencode.json server: enabled:false is surfaced as disabled, the
  dashboard re-enable force-enables via enabledServers without
  touching opencode.json, then disable clears the override and
  populates disabledServers.

Fixes #3827
2026-06-29 20:39:50 +00:00
roboomp 16ef3c54f4 fix(tui): re-enabled MCP alternate config sources
Codex review on #3829: the dashboard re-enable path still missed MCP
servers loaded from supported non-primary native config files such as
.omp/.mcp.json or user .mcp.json. Those rows carry enabled:false from
their source file, so falling back to the user disabledServers denylist
could not make the row active again.

- setMcpServerEnabled now accepts the loaded row's sourcePath and checks
  it before the primary project/user mcp.json paths.
- extension-dashboard passes the source path for writable MCP providers
  (native and mcp-json), avoiding accidental edits to third-party tool
  configs while still updating .omp/.mcp.json and standalone MCP JSON
  sources.
- Added a regression test for a server loaded from .omp/.mcp.json with
  enabled:false; re-enable flips that file to enabled:true and does not
  write the denylist.

Fixes #3827
2026-06-29 20:26:16 +00:00
roboomp 812b246e7e fix(tui): dashboard re-enable flips enabled:false in mcp.json
Codex review on #3829: when an MCP server's mcp.json entry carries
enabled:false, the dashboard toggle previously only removed the name
from the user-level disabledServers denylist. state-manager's new
`server.enabled === false` check (state-manager.ts:156) then still
marked the row disabled, leaving such servers impossible to re-enable
from /extensions.

Extracted setMcpServerEnabled() into mcp/config-writer.ts mirroring
/mcp enable | /mcp disable semantics:

- Server defined in project mcp.json -> update enabled on that entry.
- Else server defined in user mcp.json -> update enabled on that entry.
- Else (discovered third-party server) -> use the user-level disabledServers denylist.
- On re-enable, always clear any stale denylist entry.

extension-dashboard.ts routes mcp:* toggles through this helper. Added
four new regression tests covering: enabled:false re-enable, mixed
flag+denylist re-enable, disable on a config-resident server writing
enabled:false (not denylist), and discovered-server denylist round-trip.

Fixes #3827
2026-06-29 20:13:15 +00:00
can1357 ffa8519801 Merge remote-tracking branch 'origin/farm/3f9e6955/mcp-reauth-force-fresh-login' 2026-06-29 19:47:23 +02:00
roboomp 4ce9d659c9 fix(mcp/oauth): align prompt behavior with mcp sdk
The initial PR update changed the MCP OAuth default prompt to 'login consent'.
The reporter verified Cloudflare's flow actually matches the reference MCP SDK
when no prompt parameter is sent: Cloudflare then reuses the existing account
grant and opens the scope/permission picker first. Forcing any prompt keeps the
flow on Cloudflare's account/consent page instead.

Match the reference SDK behavior: omit prompt by default and send
'prompt=consent' only when the requested scope contains offline_access, where
OIDC Core requires re-consent for offline access. Explicit oauth.prompt values,
including the empty-string omit escape hatch, still take precedence.

Also rename the dynamically registered MCP OAuth client from Codex to oh-my-pi
so Cloudflare consent screens show the current product name.

Fixes #3817
2026-06-29 17:32:58 +00:00
roboomp a49c7027bf fix(mcp/oauth): force login screen before consent on /mcp reauth
The MCP OAuth flow defaulted the authorization-request prompt parameter
to 'consent'. Per OpenID Connect Core 1.0 §3.1.2.1 that asks the
authorization server to re-prompt for consent only while reusing the
existing browser authentication session. Cloudflare's MCP OAuth server
(and other strict OIDC providers) honor that literally, so /mcp reauth
landed on the consent screen attached to whichever account the browser
cookie was for, leaving no way to switch the signed-in account.

Default to 'login consent' instead so the provider first re-prompts for
authentication (the page Claude Code shows on its reauth flow) and then
re-confirms consent, preserving the original intent of always
re-displaying the authorize screen. RFC 6749 §3.1 requires providers to
ignore prompt values they do not support, so the two-value form is safe
for non-OIDC servers. Existing per-server overrides via mcp.json's
`oauth.prompt` (including the empty-string escape hatch) are unchanged.

Fixes #3817
2026-06-29 17:21:28 +00:00
can1357 6e166274cb fix(web-search,mcp): reused gemini oauth helper and formatted npx shim 2026-06-29 16:56:43 +02:00
can1357 45df90bdac fix(mcp): preserve non-npx cmd-shim direct launch 2026-06-29 16:45:46 +02:00
roboomp 3a9eed6194 style: bun run fix 2026-06-29 09:12:36 +00:00
roboomp e29792afea fix(mcp): preserved npx cmd shim launching
Kept PATH-resolved Windows npx.cmd shims on the cmd.exe wrapper path so npm owns subprocess stdio exactly like the reporter's working cmd /c configuration.

Fixes #3794
2026-06-29 09:12:16 +00:00
can1357 4534b8c80d fix(coding-agent/mcp): prevented unhandled promise rejections in sse transport
- Synchronously observed the response promise to prevent unhandled rejections during concurrent pending request failures.
2026-06-28 17:18:19 +02:00
roboomp fca640d159 fix(mcp): made legacy sse drops retriable
Changed legacy SSE pending requests to reject with a transport-closed error when the persistent stream ends, preserving MCP tool reconnect-and-retry behavior.

Fixes #3710
2026-06-28 07:59:02 +00:00
roboomp 49b8401413 fix(mcp): constrained legacy sse endpoints
Rejected legacy SSE endpoint events whose resolved URL uses a different origin than the configured SSE URL, preventing configured headers from being posted cross-origin.

Fixes #3710
2026-06-28 07:56:06 +00:00
roboomp 7bab084d78 fix(mcp): supported legacy sse transport
Added the MCP protocol 2024-11-05 HTTP+SSE transport so type:"sse" opens the endpoint stream, posts JSON-RPC to the announced endpoint, and correlates streamed responses.

Fixes #3710
2026-06-28 07:48:40 +00:00
can1357 00b9b236c8 refactor: consolidated error handling logic using AIError utilities
- Added `isTransientStatus` to determine retryable HTTP status codes.
- Replaced fragmented manual status/message checks with unified `AIError` classification and helper methods.
- Standardized auth failure detection across `coding-agent`, `mnemopi`, and provider clients.
2026-06-27 11:14:58 +02:00
roboomp bd51aee5fc fix(mcp): strip harness intent field at MCP tools/call boundary
OMP injects `INTENT_FIELD` (`i`) into every tool's wire schema. The direct

model tool-call path strips it via `extractIntent` in agent-loop, but the

eval `tool.*` bridge forwards args verbatim, so strict-schema MCP servers

(Linear, anything with `additionalProperties:false` / Zod `.strict()`)

rejected every call with `-32602 unrecognized_keys: ["i"]`. The eval

bridge surfaced the rejection as `hasError: true` instead of throwing, so

batch callers reading the value as success silently mutated nothing.

Move the strip to the MCP boundary so it owns the contract regardless of

caller: `MCPTool.execute` / `DeferredMCPTool.execute` route params through

a new `prepareOutboundArgs` that runs `stripHarnessIntent` before

`omitUnusedOptionalArgs`. `stripHarnessIntent` leaves `i` in place when

the server's own `inputSchema.properties` declares it, so a server that

legitimately uses `i` as a parameter is unaffected.

Fixes #3575
2026-06-26 16:17:58 +00:00
roboomp 52d1a04652 fix(mcp): reused attached windows console for stdio wrappers
Detected whether the OMP host already owns an inheritable Windows console before resolving stdio MCP spawn flags.

Skipped CREATE_NO_WINDOW for console-attached MCP wrapper chains so cmd.exe and PowerShell grandchildren reuse the existing terminal instead of allocating visible conhost windows.

Fixes #3567
2026-06-26 13:54:10 +00:00