Skip to content

execute tool silently drops the query argument (and asCsv) for every accepted path #108

Description

@IanKemp

Describe the bug
The execute tool accepts a query object but never sends it. The values are handed to Client.resolveUrl(path, args), which performs URI template expansion rather than appending a query string, so unless the path contains a {?...} expression they are discarded without any warning. Because validateExecutePath rejects any path containing ?, a caller cannot supply such a template, which makes query inert for every path the tool will accept.

Net effect: nothing reached through execute can be filtered or paginated, and only the first default page of any collection is reachable. asCsv is inert for the same reason.

Steps to reproduce
Any paginated endpoint. All toolsets enabled, --read-only:

{ "method": "GET", "path": "/api/Spaces-1/projects", "query": { "take": "1" } }

Expected: one item. Actual: the server's default page of 30, and the response's own echoed link shows the parameter never arrived:

"Links": { "Self": "/api/Spaces-1/projects?skip=0&take=30" }

Filters behave the same way:

{ "method": "GET", "path": "/api/Spaces-1/artifacts", "query": { "regarding": "ServerTasks-1234" } }

returns every artefact in the space, with Self echoed as ?skip=0&take=30&order=desc and no regarding. Since skip and take are also query parameters, the result set cannot be paged either, so rows beyond the first 30 are unreachable through execute.

Expected behavior
Arguments are forwarded to the server, so that it responds correctly.

Environment and versions

  • MCP Client: Claude Code 2.1.234
  • Model & Version: Opus 5
  • Octopus Server Version: 2025.1.9954
  • Octopus MCP Server Version: 2.3.2 (current main)

Root cause

src/tools/execute.ts:131

const url = dispatchable.resolveUrl(path, query);

In @octopusdeploy/api-client (^3.11.3):

resolveUrl(path: string, args?: RouteArgs): string { return this.resolve(path, args); }
resolve = (path: string, uriTemplateParameters?: RouteArgs) => this.resolver.resolve(path, uriTemplateParameters);

The second argument is a set of URI template parameters, not a query string. Every curated tool in this repo passes a template accordingly, for example src/tools/findInterruptions.ts:274:

~/api/{spaceId}/interruptions{?skip,take,pendingOnly,regarding}

execute passes a literal path with no {?...} expression, so there is no expansion point and the values are dropped silently. There is no caller-side workaround, because src/helpers/validateExecutePath.ts rejects the only syntax that would work:

if (raw.includes("?")) {
  return { ok: false, reason: "Path must not contain a query string. Pass query parameters via the `query` argument instead." };
}

The validator directs the caller to query, which is the field that does not function.

Also affected: asCsv

src/tools/execute.ts:311

const requestQuery = asCsv ? { ...(query ?? {}), format: "csv" } : query;

format: "csv" is added to the same object that is then discarded, so asCsv: true never requests CSV.

Tests do not catch it

src/tools/__tests__/execute.test.ts:4

const resolveUrl = vi.fn((path: string) => `https://octopus.example${path}`);

The mock ignores its second argument, mirroring what the real client effectively does for a non-template path, and no test asserts that query values reach the resolved URL. The defect is therefore invisible to the unit suite.

Suggested fix

Synthesise the template from the supplied keys so the client's own expansion and encoding are reused:

async function dispatchExecute(
  client: Client,
  method: HttpMethod,
  path: string,
  query: Record<string, string> | undefined,
  body: unknown,
): Promise<unknown> {
  const dispatchable = client as unknown as DispatchableClient;
  const keys = query ? Object.keys(query) : [];
  const template = keys.length > 0 ? `${path}{?${keys.join(",")}}` : path;
  const url = dispatchable.resolveUrl(template, query);
  return dispatchable.dispatchRequest(method, url, body ?? null);
}

Worth pairing with a test whose resolveUrl mock actually expands, asserting the parameters appear in the URL handed to dispatchRequest.

If appending is preferred over templating, note that the values still need URL encoding, and that path is already validated as containing no ?, so the first separator is unconditionally ?.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions