Skip to content

Fix MCP send_http_request ignoring environmentId - #654

Merged
gschier merged 1 commit into
mountain-loop:mainfrom
ricardoltt:fix/mcp-environment-id
Sep 15, 2026
Merged

gschier merged 1 commit into
mountain-loop:mainfrom
ricardoltt:fix/mcp-environment-id

Conversation

@ricardoltt

Copy link
Copy Markdown
Contributor

Summary

The MCP send_http_request tool advertises environmentId but discards it, so an explicit environment does not take precedence over the active selection. Forward the ID through the plugin API to the desktop and CLI hosts, validate it before sending, and preserve the current fallback when omitted. The override applies only to that send and does not change the active selection.

Submission

  • This PR is a bug fix.
  • If this PR is not a bug fix, I linked the feedback item where @gschier explicitly gave me permission to work on it.
  • I have read and followed CONTRIBUTING.md.
  • I tested this change locally.
  • I added or updated tests, or tests are not reasonable for this change.
  • I added screenshots or recordings, or this change does not affect the UI.

Explicit permission feedback item (required if not a bug fix): Not applicable — fixes an already-advertised parameter.

No UI is changed; screenshots are not required for this change.

Validation

  • Real MCP SDK with in-memory transport: the baseline drops ev_staging; the fixed handler forwards it. Both preserve an omitted ID. Regression tests also cover concurrent overrides, a missing request and host errors.
  • Plugin runtime test: IDs survive event serialization without changing the plugin context.
  • Model and plugin Rust suites: 254 tests pass, covering the optional wire field, base/sub-environments, variable inheritance and rejection of missing, empty, foreign-workspace and folder IDs.
  • CLI host integration: three GETs to a temporary loopback server return HTTP 200. With A active, explicit B sends X-Environment: b, omission sends a, and the base environment sends global. Invalid IDs produce no additional request or response history.
  • cargo check -p yaak-cli -p yaak-app-client --features yaak-app-client/wry, the API/runtime/MCP builds, MCP typecheck and repository lint pass. Six focused JS tests pass; the earlier full JS run passed 626 tests.

The desktop interaction scenario and complete Rust workspace suite have not been run. An extra standalone runtime tsc check reports errors in unchanged files/configuration; its production build and executable regression test pass.

Release coordination: older hosts ignore this new plugin API field. Align the MCP plugin's minimum supported Yaak version with the host release that includes the fix; version numbers are unchanged here.

Related

No issue linked. This fixes the existing per-request argument; environment listing and changing the window's selected environment are separate features.

@github-actions github-actions Bot added the contribution: in scope Community PR appears to be in scope for maintainer review. label Sep 14, 2026
@github-actions

Copy link
Copy Markdown

Thanks for the PR. This appears to match Yaak's contribution policy and is awaiting review by @gschier.

This only means the PR is in scope for review. It does not mean the change has been reviewed or accepted for merge.

@github-actions
github-actions Bot requested a review from gschier September 14, 2026 18:31
@greptile-apps

greptile-apps Bot commented Sep 14, 2026

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

The PR appears safe to merge; the environment override is consistently forwarded, workspace-scoped, backward compatible, and covered across the affected host and runtime boundaries.

Summary

  • Adds an optional, backward-compatible environment override to the plugin request payload.
  • Restricts overrides to base or sub-environments belonging to the request workspace.
  • Preserves active-environment fallback behavior when the field is omitted.
  • Adds model, runtime, MCP, and CLI regression coverage for forwarding, inheritance, invalid IDs, concurrent sends, and unchanged active selection.
  • Documents per-send environment selection and compatibility with older hosts.

Diagram

sequenceDiagram
  participant Client as MCP Client
  participant MCP as MCP Plugin
  participant Runtime as Plugin Runtime
  participant Host as Desktop or CLI Host
  participant DB as Workspace Database
  participant HTTP as HTTP Send Pipeline

  Client->>MCP: send_http_request(id, environmentId?)
  MCP->>Runtime: httpRequest.send(request, environmentId?)
  Runtime->>Host: send_http_request_request
  alt Explicit environment supplied
    Host->>DB: Load environment by ID
    DB-->>Host: Environment record
    Host->>Host: Validate workspace and environment scope
  else Environment omitted
    Host->>Host: Use active environment
  end
  Host->>HTTP: Send with resolved environment ID
  HTTP-->>Host: HTTP response
  Host-->>Runtime: send_http_request_response
  Runtime-->>MCP: Response
  MCP-->>Client: Tool result
Loading

Reviews (1) · Last reviewed commit: "fix(mcp): forward environmentId when sen..."

@gschier gschier left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thanks for catching this!

@gschier
gschier merged commit ab616fd into mountain-loop:main Sep 15, 2026
3 checks passed
gschier added a commit that referenced this pull request Sep 15, 2026
Fresh databases stopped getting a default workspace with the home screen
(#660), so the fixtures from #654 that took the first workspace panicked
on an empty list. They now create one.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contribution: in scope Community PR appears to be in scope for maintainer review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants