fix: bound tool response sizes; make list_applications search actually filter - #147
fix: bound tool response sizes; make list_applications search actually filter#147schahal wants to merge 1 commit into
Conversation
…y filter An unbounded list_applications on an instance with hundreds of applications returns ~3M chars (~771k tokens) — beyond most context windows outright. Three compounding causes, all fixed here: list_applications: - `search` was forwarded as a query param ArgoCD's ApplicationQuery does not have; the gRPC gateway silently drops it and returns the whole fleet while the caller believes the response is bounded. It is now a client-side case-insensitive partial match on name, applied before pagination, with metadata.totalItems reflecting the filtered count. - Items carried the full inline Helm values twice (spec.source.helm and again inside status.sync.comparedTo). Sources are now reduced to their identifying fields and status.sync to its verdict. - No default limit: an unqualified call now returns at most 50 apps (metadata.hasMore/totalItems support paging). - New server-side filters ArgoCD actually supports: projects (repeated), selector, repo; and a detail:"name" level for fleet-wide sweeps. get_application: drops metadata.managedFields, status.history, the full operationState (collapsed to phase/message/timestamps), and status.sync.comparedTo by default; includeHistory/includeOperationState opt back in. spec is untouched. ~116k chars -> ~20k on a real app. list_clusters: drops connection config and info.apiVersions (95% of a real ~227k-char payload) keeping name/server/connection state/app count/ server version. All tools: responses larger than MCP_MAX_RESPONSE_CHARS (default 100,000 chars, 0 disables) are replaced with an error naming the tool's narrowing parameters, so no tool can flood the client context. Ref: argoproj-labs#59 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Satbir Chahal <schahal@nextdoor.com>
|
Token usage for a simple prompt w/ v0.8.0 docker image
w/ this branch docker image
|
|
@leoluz looking at this repo history, if I'm not mistaken you may be only active maintainer? Possible to take a review/approve/merge? I have my current ArgoCD-MCP server image built off this change and is working like a charm. |
| const text = JSON.stringify(result); | ||
| if (this.maxResponseChars > 0 && text.length > this.maxResponseChars) { | ||
| return { | ||
| isError: true, |
There was a problem hiding this comment.
is this response guard called on every tool call?
For write operations like Sync, run_resource_action ect..
In the case they somehow exceed the limit, it would get the isError = true even though the operation succeeded. This may make the LLM behave incorrectly if it tries to retry the tool call.
Maybe we we only do this for read operation?
Fixes #59
Problem
Validated against a large real-world ArgoCD instance (hundreds of applications, dozens of projects, ArgoCD v3.x) running v0.8.0 of this server:
list_applications()returns ~3M chars (~770k tokens) — it exceeds most model context windows outright.searchis silently ignored: it is forwarded as a query param that ArgoCD'sApplicationQuerydoes not define, so the gRPC gateway drops it and the whole fleet comes back while the caller believes the response is bounded.search:"<exact-app-name>"with nolimitstill returned every application. This is worse than not offering the parameter, since callers cannot tell their filter did nothing.spec.source.helm.values+status.sync.comparedTo.source.helm.values) — evenlimit=1cost ~11.4k chars.get_application≈ 117k chars, dominated bystatus.history(54k) +status.operationState(29k).list_clusters≈ 227k chars, of whichinfo.apiVersionsis ~95%.Changes
list_applicationssearchis now a real filter: client-side case-insensitive partial match on name (ArgoCD has no server-side equivalent), applied before pagination;metadata.totalItemsreflects the filtered count. The bogus query param is no longer sent.repoURL/path/chart/targetRevision/ref/name— inline Helm values dropped),status.syncreduced tostatus/revision(s)(droppingcomparedTo, the second Helm-values copy). Multi-source apps (spec.sources) are now represented too.limitof 50 — an unqualified call can no longer return the fleet.metadata.{totalItems,hasMore}support paging, andlimit=1+totalItemsremains the cheap way to count.projects(repeated),selector(labels),repo.detail: "name"level (name/namespace/project/sync/health only) for fleet-wide sweeps in a single call.get_application— dropsmetadata.managedFields,status.history, fullstatus.operationState(collapsed to phase/message/timestamps/retryCount), andstatus.sync.comparedToby default;includeHistory/includeOperationStateopt back in.specis untouched.list_clusters— drops connectionconfigandinfo.apiVersions; keeps name, server, labels/annotations, namespaces, connection state, app count, server version.All tools — a response-size guard in
addJsonOutputTool: responses overMCP_MAX_RESPONSE_CHARS(default 100,000 chars ≈ 25k tokens;0disables) are replaced with an error naming the tool's narrowing parameters (e.g.get_resources→ pass specificresourceRefs). No tool, present or future, can flood the client context. Documented in the README.HttpClient— array query-param support (projects=a&projects=b, matching gRPC-gateway repeated-field encoding).Measured impact
list_applications()barelist_applications(search:"x")no limitlist_applications(detail:"name"), full fleet via pagingget_applicationlist_clustersEnd to end: an AI-agent session that exercised these tools against the same instance dropped from ~615k tokens of context to ~35k (~94%) after switching to an image built from this branch.
Behavior changes to note
list_applicationsnow returns at most 50 items (previously: everything).metadata.totalItems/hasMoresignal the rest.metadata.totalItemscounts applications after filtering — for a fleet count, call unfiltered withlimit=1.get_applicationno longer returns history/full operation state unless asked (opt back in viaincludeHistory/includeOperationState).MCP_MAX_RESPONSE_CHARS=0for the old pass-through behavior.Test plan
projects), default limit, Helm-values/comparedTo/managedFieldsstripping,detail:"name"shape,get_applicationstrip + opt-ins,list_clustersstrip, size guard (trip + hint +0disables)pnpm lint,pnpm build,pnpm test— 33/33 pass🤖 Generated with Claude Code